[Fix] (Espryt, P8-MF): the push arm's flush ladder keeps a partial range-invalidating map behind an upload-ring copy still queued on the store, as dev f973008c's pull arm does - FlushPendingRangesFrom's ring tier stamps ringCopyRetireSerial and InvalidateFlushAccessFor refuses tier 1 for a partial range while that copy is unretired (the whole-buffer orphan-map stays exempt); Adreno 830 loses the queued copy and create-instancing on the monolith arm drew terrain sections empty

This commit is contained in:
2026-09-30 05:43:55 -04:00
parent 3717424229
commit ac9ab078cc
3 changed files with 47 additions and 13 deletions
+7 -1
View File
@@ -1490,9 +1490,13 @@ namespace MobileGL::MG_Backend::DirectGLES {
hostBaseFrom, hostBaseTo, start, end);
}
#endif
// Read per range, like the pull arm: a ring copy staged for an earlier range of
// this same drain puts every later partial range of the store behind it
// (InvalidateFlushAccessFor's queued-copy rule, dev f973008c).
const Bool ringCopyInFlight = resource.ringCopyRetireSerial > DirectGLES::CompletedFrameSerial();
const GLbitfield access =
mapUsable ? InvalidateFlushAccessFor(start, end, coveredFrom, coveredTo, limit,
resource.storageSize)
resource.storageSize, ringCopyInFlight)
: 0u;
if (access != 0) {
BindBufferId(TempBufferTarget, resource.id);
@@ -1512,6 +1516,8 @@ namespace MobileGL::MG_Backend::DirectGLES {
BindBufferId(GL_COPY_WRITE_BUFFER, resource.id);
g_GLESFuncs.glCopyBufferSubData(GL_COPY_READ_BUFFER, GL_COPY_WRITE_BUFFER,
(GLintptr)ringOffset, (GLintptr)start, (GLsizeiptr)size);
// Commands recorded now retire with the fence the next Present inserts.
resource.ringCopyRetireSerial = DirectGLES::CurrentFrameSerial() + 1;
} else {
UploadRangeFrom(resource, hostBase, start, end);
}
+18 -5
View File
@@ -1005,10 +1005,12 @@ namespace MobileGL::MG_Backend::DirectGLES {
// matches, exactly like the per-VAO memo stamps.
Uint64 drawCleanEpoch = 0;
// Frame serial whose completion retires the last glCopyBufferSubData the upload
// ring staged INTO this store (FlushPendingRangesNow's ring tier); 0 = none. Until
// it retires, FlushPendingRangesNow keeps every partial range off the
// range-invalidating map: on Adreno 830 that map, on a store with a queued copy,
// loses the copy (see FlushPendingRangesNow). Draw thread only, like the flush.
// ring staged INTO this store (the flush ladder's ring tier: FlushPendingRangesNow
// on the pull arm, FlushPendingRangesFrom on the push arm); 0 = none. Until it
// retires, both keep every partial range off the range-invalidating map: on
// Adreno 830 that map, on a store with a queued copy, loses the copy (see
// FlushPendingRangesNow and InvalidateFlushAccessFor). Draw thread only, like the
// flush.
Uint64 ringCopyRetireSerial = 0;
// Zero-copy coherent persistent map (EXT_buffer_storage): the GL store is
// immutable, persistently+coherently mapped, and persistentPtr is what the app
@@ -1192,10 +1194,14 @@ namespace MobileGL::MG_Backend::DirectGLES {
// ("do not take tier 1"), which drops the range onto the staging ring and costs a copy
// rather than a corruption.
//
// `ringCopyInFlight`: the store still has an upload-ring glCopyBufferSubData queued that the
// GPU may not have run (GLESBufferResource::ringCopyRetireSerial not yet retired).
//
// 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) {
SizeT mapEnd, SizeT limit, SizeT storageSize,
Bool ringCopyInFlight) {
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
@@ -1207,6 +1213,13 @@ namespace MobileGL::MG_Backend::DirectGLES {
// 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;
// A partial range-invalidating map must not overtake a ring copy into this store the GPU
// has not run yet. Adreno 830 answers such a map on a store with a copy still queued by
// losing the copy (dev f973008c, the pull arm's FlushPendingRangesNow: create-instancing
// drew whole Embeddium terrain sections empty, SSIM 0.870). The range goes to the ring
// instead, GPU-ordered behind the copies. The whole-buffer orphan-map is exempt: it
// rewrites every byte from the shadow the queued copies were taken from.
if (!wholeBuffer && ringCopyInFlight) return 0u;
return GL_MAP_WRITE_BIT |
(wholeBuffer ? GL_MAP_INVALIDATE_BUFFER_BIT : GL_MAP_INVALIDATE_RANGE_BIT);
}
+22 -7
View File
@@ -819,19 +819,19 @@ namespace {
} // namespace
TEST(EsprytFlushLadder, AWholeBufferRangeOrphansTheStore) {
EXPECT_EQ(InvalidateFlushAccessFor(0, kStore, 0, kStore, kStore, kStore),
EXPECT_EQ(InvalidateFlushAccessFor(0, kStore, 0, kStore, kStore, kStore, false),
static_cast<GLbitfield>(GL_MAP_WRITE_BIT | GL_MAP_INVALIDATE_BUFFER_BIT));
}
TEST(EsprytFlushLadder, ALargePartialRangeInvalidatesExactlyThatRange) {
const SizeT start = 4096;
const SizeT end = start + kEsprytInvalidateRangeMinBytes;
EXPECT_EQ(InvalidateFlushAccessFor(start, end, start, end, kStore, kStore),
EXPECT_EQ(InvalidateFlushAccessFor(start, end, start, end, kStore, kStore, false),
static_cast<GLbitfield>(GL_MAP_WRITE_BIT | GL_MAP_INVALIDATE_RANGE_BIT));
}
TEST(EsprytFlushLadder, ASmallPartialRangeFallsThroughToTheStagingRing) {
EXPECT_EQ(InvalidateFlushAccessFor(4096, 4096 + 64, 4096, 4096 + 64, kStore, kStore), 0u)
EXPECT_EQ(InvalidateFlushAccessFor(4096, 4096 + 64, 4096, 4096 + 64, kStore, kStore, false), 0u)
<< "below the threshold the map WAITS out the WAR hazard on the CPU instead of "
"substituting pages, which is the whole reason tier 2 exists";
}
@@ -846,19 +846,33 @@ TEST(EsprytFlushLadder, AMapWiderThanTheQueuedRangeRefusesTierOne) {
const SizeT queuedStart = 4096;
const SizeT queuedEnd = queuedStart + kEsprytInvalidateRangeMinBytes;
// Page-aligned outward, the exact widening the in-tree note records.
EXPECT_EQ(InvalidateFlushAccessFor(queuedStart, queuedEnd, 0, queuedEnd + 4096, kStore, kStore), 0u)
EXPECT_EQ(InvalidateFlushAccessFor(queuedStart, queuedEnd, 0, queuedEnd + 4096, kStore, kStore, false), 0u)
<< "a widened INVALIDATE_RANGE declares bytes dead that the shadow is not about to "
"rewrite, and overwrites whatever the GPU put there";
// And narrower, which is the same corruption read the other way round: bytes left
// unwritten inside a range that has just been declared dead.
EXPECT_EQ(InvalidateFlushAccessFor(queuedStart, queuedEnd, queuedStart, queuedEnd - 8, kStore, kStore), 0u);
EXPECT_EQ(InvalidateFlushAccessFor(queuedStart, queuedEnd, queuedStart, queuedEnd - 8, kStore, kStore, false),
0u);
}
TEST(EsprytFlushLadder, AnEmptyRangeIsNeverTierOne) {
EXPECT_EQ(InvalidateFlushAccessFor(4096, 4096, 4096, 4096, kStore, kStore), 0u);
EXPECT_EQ(InvalidateFlushAccessFor(4096, 4096, 4096, 4096, kStore, kStore, false), 0u);
}
// dev f973008c's rule on the push arm (P8-MF): Adreno 830 loses an upload-ring copy still queued
// on a store when a partial range-invalidating map of that store overtakes it, so a partial range
// waits behind the copy on the ring. The whole-buffer orphan-map is exempt.
TEST(EsprytFlushLadder, APartialRangeDoesNotOvertakeAQueuedRingCopy) {
const SizeT start = 4096;
const SizeT end = start + kEsprytInvalidateRangeMinBytes;
EXPECT_EQ(InvalidateFlushAccessFor(start, end, start, end, kStore, kStore, true), 0u)
<< "a range-invalidating map of a store with a queued ring copy loses the copy on Adreno 830";
EXPECT_EQ(InvalidateFlushAccessFor(0, kStore, 0, kStore, kStore, kStore, true),
static_cast<GLbitfield>(GL_MAP_WRITE_BIT | GL_MAP_INVALIDATE_BUFFER_BIT))
<< "the whole-buffer orphan-map rewrites every byte from the shadow the copies were taken from";
}
#else
// The same five names in a pull build, for the G2/G14 reason above: the ladder's push arm
// The same six names in a pull build, for the G2/G14 reason above: the ladder's push arm
// (FlushPendingRangesFrom) is the only one that carries this decision as a function - the pull
// arm's FlushPendingRangesNow is byte-frozen against 5cb826b0 (ID-15) and may not grow one.
#define MGL_PUSH_ONLY_OR_SKIP() \
@@ -869,4 +883,5 @@ TEST(EsprytFlushLadder, ALargePartialRangeInvalidatesExactlyThatRange) { MGL_PUS
TEST(EsprytFlushLadder, ASmallPartialRangeFallsThroughToTheStagingRing) { MGL_PUSH_ONLY_OR_SKIP(); }
TEST(EsprytFlushLadder, AMapWiderThanTheQueuedRangeRefusesTierOne) { MGL_PUSH_ONLY_OR_SKIP(); }
TEST(EsprytFlushLadder, AnEmptyRangeIsNeverTierOne) { MGL_PUSH_ONLY_OR_SKIP(); }
TEST(EsprytFlushLadder, APartialRangeDoesNotOvertakeAQueuedRingCopy) { MGL_PUSH_ONLY_OR_SKIP(); }
#endif // MOBILEGL_PIPE_PUSH