[MG_Backend] key the handle arm's attachment walk on the object, not the binding

SyncToBackendByHandle gated its record-driven attachment walk (the eleven
applyPoint calls plus NoteFramebufferTextureAttachments) on
m_syncedRecordHashes[asTarget], a PER-TARGET slot, while the work inside is
object-level: glFramebufferTexture2D/glFramebufferRenderbuffer write the
driver FBO behind whichever binding is current, so a Draw walk and a Read walk
write the same points. Two slots can therefore hold two individually true,
jointly false accounts of one physical state.

improved-transparency flips a composite framebuffer between two attachment
states and syncs one of them as DRAW in a depth pre-pass. That walk detaches
the colour point, stamps the Draw slot; the end-of-frame READ-side sync of the
other record then found its own slot already stamped by an earlier identical
sync, walked nothing, left the point detached, and the driver refused the
following glBlitFramebuffer with INVALID_OPERATION - the default framebuffer
froze from frame 835. The object form is immune because a per-point version
array gates it underneath; the handle arm has no frontend array, so this memo
is its only gate.

Replace it there with m_syncedAttachmentRecordHash, one slot for the object.
The per-target array stays for the object arm, where it is a re-arm trigger
over that version array rather than a claim. InvalidateSyncedState now zeroes
both memos: it invalidates the version array, which is not the handle arm's
gate, so a memo left standing would let a re-sync of the same record skip the
re-attach that call exists to force.
This commit is contained in:
2026-09-20 19:36:16 -04:00
parent 7059869848
commit e004269d96
2 changed files with 52 additions and 17 deletions
+21 -5
View File
@@ -9872,8 +9872,10 @@ namespace MobileGL::MG_Backend::DirectGLES {
std::fill(std::begin(m_backendDrawBuffers), std::end(m_backendDrawBuffers), GL_NONE);
// NOTE: this does NOT empty the backend ES framebuffer - m_backendFBOId keeps every
// attachment it had, possibly under a non-identity permutation. Declaring the table
// identity here is safe only because every attachment version below is invalidated too,
// so the next sync re-attaches all non-empty attachments at their identity points AND
// identity here is safe only because every gate that decides which points get
// re-issued is invalidated too - the attachment versions below, and under
// MOBILEGL_PIPE_PUSH the record memos stamped at the end of this function - so the
// next sync re-attaches all non-empty attachments at their identity points AND
// (see SyncToBackend's attachment loop) detaches any colour point whose frontend owner
// is empty. Without that detach a stale image would survive under a point the table now
// claims for a different, empty attachment.
@@ -9887,6 +9889,14 @@ namespace MobileGL::MG_Backend::DirectGLES {
// Every attachment version is invalidated above, so the next walk re-attaches
// everything regardless; stamp the generation so it does not re-arm twice.
m_syncedBackendIdGeneration = g_attachmentBackendIdGeneration;
#if MOBILEGL_PIPE_PUSH
// The record memos are the gate the PIPE_PUSH arms' attachment walks actually read -
// the handle arm has nothing narrower under its key - so leaving them standing here
// would let a re-sync of the same record skip the re-attach this function exists to
// force. 0 is never a live hash, so a zeroed memo is a guaranteed miss.
m_syncedRecordHashes = {0};
m_syncedAttachmentRecordHash = 0;
#endif
}
static Bool SyncAttachmentObject(GLenum glFBOTarget,
@@ -11345,13 +11355,19 @@ namespace MobileGL::MG_Backend::DirectGLES {
ApplyReadBufferFromRecord(record, 0);
}
// 3. The attachments. Two gates, both server-owned.
// 3. The attachments. Three gates, and the key is the object's rather than the
// binding's: idGenerationMoved ("I re-minted a driver id, so a point may hold a
// dead one"), attachmentPointsMoved ("the colour permutation moved, so a point
// whose record did not change still has to be re-issued") and the record hash.
// NOT m_syncedRecordHashes: the per-target array is the object arm's re-arm
// trigger, and the points written below belong to the driver FBO rather than to
// the binding they were walked for - see m_syncedAttachmentRecordHash.
const Bool idGenerationMoved = m_syncedBackendIdGeneration != g_attachmentBackendIdGeneration;
if (idGenerationMoved) {
m_syncedBackendIdGeneration = g_attachmentBackendIdGeneration;
}
if (idGenerationMoved || attachmentPointsMoved ||
m_syncedRecordHashes[SizeT(asTarget)] != record.ContentHash) {
m_syncedAttachmentRecordHash != record.ContentHash) {
const auto applyPoint = [&](FramebufferAttachmentType point,
const MG_Pipe::MGPSurface& surface) {
const Bool isColorPoint = point >= FramebufferAttachmentType::Color0 &&
@@ -11388,7 +11404,7 @@ namespace MobileGL::MG_Backend::DirectGLES {
}
applyPoint(FramebufferAttachmentType::Depth, record.Depth);
applyPoint(FramebufferAttachmentType::Stencil, record.Stencil);
m_syncedRecordHashes[SizeT(asTarget)] = record.ContentHash;
m_syncedAttachmentRecordHash = record.ContentHash;
NoteFramebufferTextureAttachments(fbo, record);
}
+31 -12
View File
@@ -2276,11 +2276,12 @@ namespace MobileGL::MG_Backend::DirectGLES {
// reintroduce exactly the class of bug commit d7655247 fixed on the buffer side.
Uint64 m_syncedBackendIdGeneration = 0;
#if MOBILEGL_PIPE_PUSH
// P4a (D-C4): MGPFramebufferState::ContentHash as of this twin's last sync, PER
// TARGET IT WAS SYNCED AS, and it is the second of the hash's two jobs - "the server's
// render-pass memo key, and the CLIENT's emission suppressor". It replaces
// m_syncedFrontendAttachmentVersions AS A KEY (the array stays: it is what the
// legacy arm compares, and it is the mechanism the handle arm re-arms through).
// P4a (D-C4): MGPFramebufferState::ContentHash as of this twin's last sync on the
// OBJECT arm (SyncToBackend), PER TARGET IT WAS SYNCED AS, and it is the second of
// the hash's two jobs - "the server's render-pass memo key, and the CLIENT's emission
// suppressor". It replaces m_syncedFrontendAttachmentVersions AS A KEY (the array
// stays: it is what the legacy arm compares, and re-arming it is what a miss here
// does). The handle arm reads neither slot of this one - it has its own key below.
//
// The hash covers every field the record carries - Fbo included, so a recycled
// framebuffer handle whose successor happens to carry an identical attachment set
@@ -2288,14 +2289,32 @@ namespace MobileGL::MG_Backend::DirectGLES {
// a suppressed record provably means the draw-buffer array did not move, which
// provably means the fragColor broadcast count did not move.
//
// PER TARGET rather than one, and it stays that way under ID-19's per-OBJECT record:
// there is now ONE record for this framebuffer, but syncing it as Draw and syncing it
// as Read do different work (glDrawBuffers and the four cross-object masks are
// Draw-only, glReadBuffer is Read-only), so "I have already applied this record" is a
// per-target claim and one memo would let the second target skip work the first never
// did. 0 is never a live hash (a computed 0 is remapped to 1 by the client's
// suppressor), so a zeroed memo is a guaranteed miss.
// PER TARGET rather than one, and it stays that way under ID-19's per-OBJECT record,
// but ONLY because the arm that reads it uses this memo as a re-arm TRIGGER and not as
// a claim: a miss re-arms every point, and a hit still leaves the per-point versions
// underneath to decide what the walk touches. The split there says how narrowly one
// target re-arms; it never means "the driver's attachments are this record". 0 is
// never a live hash (a computed 0 is remapped to 1 by the client's suppressor), so a
// zeroed memo is a guaranteed miss.
Array<Uint64, SizeT(FramebufferTarget::FramebufferTargetCount)> m_syncedRecordHashes = {0};
// P5e (fb): the HANDLE arm's key, and the ContentHash of the record whose attachments
// this twin last APPLIED through SyncToBackendByHandle. Unlike the array above, this
// one IS a claim: nothing narrower sits under it, because that arm walks the record
// and has no frontend attachment array to compare against.
//
// ONE FOR THE OBJECT, NOT ONE PER TARGET, and that is the difference that matters.
// Attachments belong to the driver FBO OBJECT: glFramebufferTexture2D and
// glFramebufferRenderbuffer write the object behind whichever binding happens to be
// current, so a walk run for Draw and a walk run for Read write the SAME eleven
// points. Per-target slots let the two keep separate, individually true and jointly
// false accounts of one physical state: a composite framebuffer synced as DRAW in a
// depth pre-pass detaches the points its other attachment state attaches, and the
// end-of-frame sync of that same record on the READ side then found its own slot
// already stamped by an earlier identical sync, walked nothing, left the colour point
// detached, and the driver refused the following glBlitFramebuffer with
// INVALID_OPERATION. Keying on the object is what makes "this hash is applied" mean
// "these points are on the driver now".
Uint64 m_syncedAttachmentRecordHash = 0;
// P5e (fb): the read-buffer decision, taken from the record alone. Both
// SyncReadBufferToBackend overloads funnel through it, so the rule lives in one
// place and the object form is visibly the half that only finds the handle.