diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.cpp b/MobileGL/MG_Backend/DirectGLES/Managers.cpp index ca8b9ce1..970f3369 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.cpp +++ b/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; - if (mapUsable && (wholeBuffer || size >= kInvalidateRangeMinBytes)) { + const GLbitfield access = mapUsable ? InvalidateFlushAccessFor(start, end, start, end, + 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 - // reason written at the top of this function. A null object is the "no frontend to - // ask" case (nothing reaches this probe without one today) and is treated as "not - // mapped", which is what the record already says. - if (frontend != nullptr && frontend->IsMapped()) return false; + // THE LAST FRONTEND READ IN THIS FUNCTION, AND P5 b1 RETIRES IT - under split + // only, because that is the only build where it is both wrong and replaceable. + // + // It asked the object whether it was mapped because HasLiveHostWrites was pinned + // 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(record->Desc.Width)) return false; diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.h b/MobileGL/MG_Backend/DirectGLES/Managers.h index 9a59d9a2..38359e4d 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.h +++ b/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