From e004269d962a3f83b69ca72d5182ca52f46f1742 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Sun, 20 Sep 2026 19:36:16 -0400 Subject: [PATCH] [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. --- MobileGL/MG_Backend/DirectGLES/Managers.cpp | 26 ++++++++++--- MobileGL/MG_Backend/DirectGLES/Managers.h | 43 +++++++++++++++------ 2 files changed, 52 insertions(+), 17 deletions(-) diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.cpp b/MobileGL/MG_Backend/DirectGLES/Managers.cpp index 89aa1ab8..697a8140 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.cpp +++ b/MobileGL/MG_Backend/DirectGLES/Managers.cpp @@ -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); } diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.h b/MobileGL/MG_Backend/DirectGLES/Managers.h index 9211b292..c3b2bd7f 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.h +++ b/MobileGL/MG_Backend/DirectGLES/Managers.h @@ -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 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.