[Fix] (DirectGLES): retire the probe's last frontend read under split and refuse tier 1 of the flush ladder when the map and the queued range disagree in either direction

MobileGL/MG_Backend/DirectGLES/Managers.cpp

@@ -1043,6 +1043,14 @@ namespace MobileGL::MG_Backend::DirectGLES {
             // sizeable (page-coverable) range to engage, and below it the driver
             // falls back to waiting out the WAR hazard on the CPU.
             constexpr SizeT kInvalidateRangeMinBytes = 128u * 1024u;
+#if MOBILEGL_PIPE_PUSH
+            // The push arm's copy of this threshold lives in Managers.h, where a unit case can
+            // reach it (InvalidateFlushAccessFor). Two constants, one value, and the compiler
+            // is what keeps them one: the pull arm's FlushPendingRangesNow is byte-frozen
+            // against 5cb826b0 (ID-15), so the constant it reads may not move to a header.
+            static_assert(kInvalidateRangeMinBytes == kEsprytInvalidateRangeMinBytes,
+                          "the two arms of the three-tier ladder must use the same tier-1 threshold");
+#endif
 
             // Push every queued range of `resource` from the shadow into the backend
             // store, without ever letting a driver resolve the WAR hazard against
@@ -1126,12 +1134,11 @@ namespace MobileGL::MG_Backend::DirectGLES {
                     // shadow's to rewrite. Widening to page bounds looked free and was
                     // not - the widened bytes clobbered GPU-written data (an SSBO
                     // counter beside the app's SubData) with the stale shadow.
-                    const Bool wholeBuffer = start == 0 && end == limit && limit == resource.storageSize;
+                    const GLbitfield access = mapUsable ? InvalidateFlushAccessFor(start, end, start, end,
-                    if (mapUsable && (wholeBuffer || size >= kInvalidateRangeMinBytes)) {
+                                                                                  limit, resource.storageSize)
+                                                        : 0u;
+                    if (access != 0) {
                         BindBufferId(TempBufferTarget, resource.id);
-                        const GLbitfield access =
-                            GL_MAP_WRITE_BIT |
-                            (wholeBuffer ? GL_MAP_INVALIDATE_BUFFER_BIT : GL_MAP_INVALIDATE_RANGE_BIT);
                         void* dst = g_GLESFuncs.glMapBufferRange(TempBufferTarget, (GLintptr)start,
                                                                  (GLsizeiptr)size, access);
                         if (dst) {
@@ -2667,16 +2674,33 @@ namespace MobileGL::MG_Backend::DirectGLES {
             }
             const auto* record = ResourceRecordOf(res);
             if (record == nullptr) return false;
-#if MOBILEGL_PIPE_VERIFY
+#if MOBILEGL_PIPE_VERIFY && !MOBILEGL_BUILD_DISAGGREGATED
             MOBILEGL_ASSERT(!record->HasLiveHostWrites,
-                            "MGPipeResourceRecord::HasLiveHostWrites is set, but P3a has no producer for it");
+                            "MGPipeResourceRecord::HasLiveHostWrites is set, but this build has no producer "
+                            "for it - P5 b1's producer is MGPSubData::HasLiveHostWrites and is split-only");
 #endif
             if (record->HasLiveHostWrites) return false;
-            // The same question the legacy arm asks at this exact point in the order, for the
+            // THE LAST FRONTEND READ IN THIS FUNCTION, AND P5 b1 RETIRES IT - under split
-            // reason written at the top of this function. A null object is the "no frontend to
+            // only, because that is the only build where it is both wrong and replaceable.
-            // ask" case (nothing reaches this probe without one today) and is treated as "not
+            //
-            // mapped", which is what the record already says.
+            // It asked the object whether it was mapped because HasLiveHostWrites was pinned
-            if (frontend != nullptr && frontend->IsMapped()) return false;
+            // false and an emulated persistent map mutates the shadow with no call, no serial
+            // and no epoch; answering from the record alone made such a buffer read
+            // draw-CLEAN forever, SyncPersistentMappedRange was never reached again, and the
+            // frame drew the last uploaded bytes with no diagnostic
+            // (MG_Test/SanityTest.cpp's DirectGLESBufferDrawProbe is exactly that case).
+            //
+            // TWO THINGS REPLACE IT AND BOTH HAD TO LAND FIRST: the record now carries the map
+            // state (the line above, from MGPSubData::HasLiveHostWrites), and the client
+            // pushes the mapped span by block at every validate point, so the serial moves for
+            // a write the application made with no API call. Under a spawn there is no object
+            // on this side to ask, which is why this was never going to stay a choice.
+            //
+            // A null object is the "no frontend to ask" case and is treated as "not mapped",
+            // which is what the record already says.
+            const Bool askTheObjectWhetherItIsMapped =
+                MG_Config::Transport == MG_Config::TransportMode::Monolith;
+            if (askTheObjectWhetherItIsMapped && frontend != nullptr && frontend->IsMapped()) return false;
             if (resource->pendingRespecify || !resource->storageInitialized) return false;
             if (!resource->pendingRanges.empty()) return false;
             if (resource->storageSize != static_cast<SizeT>(record->Desc.Width)) return false;

MobileGL/MG_Backend/DirectGLES/Managers.h

@@ -901,6 +901,44 @@ namespace MobileGL::MG_Backend::DirectGLES {
         // has to receive the handle in a payload instead.
         MG_Pipe::MGPipeHandle HandleOfBuffer(const MG_State::GLState::BufferObject* bufferObject);
 
+        // TIER 1 OF THE THREE-TIER FLUSH LADDER, AS A PURE FUNCTION (P5 b1).
+        //
+        // `GL_MAP_INVALIDATE_RANGE_BIT` is not a hint, it is an ASSERTION THAT THE OLD BYTES
+        // ARE DEAD - and it is only true of the bytes this call is about to rewrite from the
+        // authoritative shadow. Managers.cpp:1125-1128 records what happens when it is not:
+        // widening the map to page bounds "looked free and was not - the widened bytes
+        // clobbered GPU-written data (an SSBO counter beside the app's SubData) with the stale
+        // shadow". It fails SILENTLY, unlike tier 3, which only stalls.
+        //
+        // WHY IT IS A FUNCTION NOW, AND WHY IT TAKES THE MAP RANGE SEPARATELY FROM THE QUEUED
+        // ONE. Under split the bytes are not re-read at every use any more: the server may not
+        // hold a pointer into the client's shadow at all (R-11), so `hostBase` becomes a
+        // SNAPSHOT taken into SEG_STAGE at emission, and the window between the snapshot and
+        // the apply is new. A snapshot that covers less than the map does is exactly the
+        // widening that drew blood, with a thread boundary instead of a page alignment as the
+        // cause - so the two extents are separate parameters and a disagreement returns 0
+        // ("do not take tier 1"), which drops the range onto the staging ring and costs a copy
+        // rather than a corruption.
+        //
+        // Returns the glMapBufferRange access bits, or 0 when tier 1 must not be taken.
+        inline constexpr SizeT kEsprytInvalidateRangeMinBytes = 128u * 1024u;
+        constexpr GLbitfield InvalidateFlushAccessFor(SizeT queuedStart, SizeT queuedEnd, SizeT mapStart,
+                                                      SizeT mapEnd, SizeT limit, SizeT storageSize) {
+            if (mapEnd <= mapStart) return 0u;
+            // THE WIDENING REFUSAL. Not >=, not "covers": exactly, in both directions. A map
+            // narrower than the queued range leaves bytes unwritten inside a range it has just
+            // declared dead, which is the same corruption read the other way round.
+            if (mapStart != queuedStart || mapEnd != queuedEnd) return 0u;
+            const SizeT size = mapEnd - mapStart;
+            const Bool wholeBuffer = mapStart == 0 && mapEnd == limit && limit == storageSize;
+            // A partial range below the threshold goes to the ring instead: the map's
+            // page-substitution fast path needs a page-coverable range to engage, and below it
+            // the driver falls back to waiting out the WAR hazard on the CPU.
+            if (!wholeBuffer && size < kEsprytInvalidateRangeMinBytes) return 0u;
+            return GL_MAP_WRITE_BIT |
+                   (wholeBuffer ? GL_MAP_INVALIDATE_BUFFER_BIT : GL_MAP_INVALIDATE_RANGE_BIT);
+        }
+
         // The handle arms of the two draw-path entry points below. IsBufferDrawCleanByHandle
         // asks the applier the same five questions IsBufferDrawClean asks the frontend object,
         // with identical semantics (D-A4); EnsureBufferResourceForHandle is the ensure path