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.