From 5046b3fc897ef787cd9cf4d9c54c8d2507dd333e Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Tue, 8 Sep 2026 13:10:44 -0400 Subject: [PATCH] [Refactor] (Espryt): sync the driver framebuffer from the pushed record and answer the read buffer from the resolved read surface --- MobileGL/MG_Backend/DirectGLES/Managers.cpp | 168 ++++++++++++++++++++ MobileGL/MG_Backend/DirectGLES/Managers.h | 39 +++++ 2 files changed, 207 insertions(+) diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.cpp b/MobileGL/MG_Backend/DirectGLES/Managers.cpp index 95cbb065..b91ad9fe 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.cpp +++ b/MobileGL/MG_Backend/DirectGLES/Managers.cpp @@ -8212,11 +8212,105 @@ namespace MobileGL::MG_Backend::DirectGLES { return false; } +#if MOBILEGL_PIPE_PUSH + const MG_Pipe::MGPFramebufferState* PushedFramebufferRecord(FramebufferTarget asTarget, + MG_Pipe::MGPipeHandle fbo) { + if (MG_Pipe::MGPipeHandleIsNull(fbo)) return nullptr; + const auto& applier = MG_Pipe::MGPipeApplier(); + const MG_Pipe::MGPFramebufferState& record = + asTarget == FramebufferTarget::Read ? applier.ReadFramebuffer : applier.DrawFramebuffer; + // A record the client has never sent is all zeroes, whose Fbo is the null handle and + // therefore matches no twin. That is the same test as "is this record mine", so + // there is no second emptiness check to get out of step with it. + if (!(record.Fbo == fbo)) return nullptr; + return &record; + } + + // The record's DrawBuffers[] is an ATTACHMENT INDEX with -1 for None (D-C1), which is + // what the frontend array holds for every framebuffer that has a twin. It deliberately + // cannot spell FrontLeft / FrontRight / BackLeft / BackRight: those are the DEFAULT + // framebuffer's own tokens, the default framebuffer has no BackendFramebufferObject at + // all (it is pDefaultFramebufferInfo->defaultFBO, which is why MGPipeHandles.h reserves + // {0,1} for it), and the legacy arm's branch for them says so in as many words - "shouldn't + // remap". So the conversion below is total for every object that reaches this twin, and + // the record's IsDefault is the thing a later phase would test if that ever changed. + static void DecodePushedDrawBuffers(const MG_Pipe::MGPFramebufferState& record, + FramebufferAttachmentType* out) { + for (Uint i = 0; i < MG_State::GLState::FramebufferObject::MAX_DRAW_BUFFERS; ++i) { + const Int8 index = record.DrawBuffers[i]; + out[i] = index < 0 ? FramebufferAttachmentType::None + : static_cast( + static_cast(FramebufferAttachmentType::Color0) + index); + } + } +#endif + void BackendFramebufferObject::SyncReadBufferToBackend( const SharedPtr& stateFBOObject) { if (!stateFBOObject) { return; } +#if MOBILEGL_PIPE_PUSH + // P4a (D-C2): the READ buffer is answered from the RESOLVED read surface of the READ + // framebuffer's own record, which is what structurally closes the read-buffer + // shared-FBO defect class. The comment two lines below - "when this is reached from + // SyncCurrentFBO's 'same FBO as draw' skip path" - describes a hazard that becomes + // unrepresentable: the record says which target it is, and ReadSurface was resolved + // from the read framebuffer's own read buffer before it ever crossed. + // + // The record carries the resolved SURFACE and not an index, so the attachment POINT + // is recovered by matching it against Color[] - the one place both spellings sit + // side by side. An empty surface (Res == the null handle, which D-C1 makes the + // spelling of "no attachment" independently of Kind's numbering) is + // FramebufferAttachmentType::None, i.e. GL_NONE. A surface that matches no colour + // point cannot be a legal read buffer and is refused rather than guessed. + if (FramebufferSubsystemEnabled()) { + const MG_Pipe::MGPipeHandle fbo = g_backendFramebufferObjects.HandleOf(stateFBOObject.get()); + const auto* record = PushedFramebufferRecord(FramebufferTarget::Read, fbo); + if (record == nullptr) { + MGLOG_E_ONCE("MGPipe: framebuffer %u has no read-target applier record on the " + "handle arm, so its read buffer cannot be pushed (handle {%u, %u})", + stateFBOObject->GetExternalIndex(), fbo.Slot, fbo.Gen); + return; + } + FramebufferAttachmentType pushedReadBuf = FramebufferAttachmentType::None; + if (!MG_Pipe::MGPipeHandleIsNull(record->ReadSurface.Res)) { + Bool matched = false; + for (Uint i = 0; i < MG_Pipe::kMGPipeMaxColorAttachments; ++i) { + const auto& color = record->Color[i]; + if (color.Res == record->ReadSurface.Res && color.Level == record->ReadSurface.Level && + color.Layer == record->ReadSurface.Layer) { + pushedReadBuf = static_cast( + static_cast(FramebufferAttachmentType::Color0) + static_cast(i)); + matched = true; + break; + } + } + if (!matched) { + MGLOG_E_ONCE("MGPipe: framebuffer %u's resolved read surface {%u, %u} matches no " + "colour attachment of its own record - refusing to guess a read " + "buffer", + stateFBOObject->GetExternalIndex(), record->ReadSurface.Res.Slot, + record->ReadSurface.Res.Gen); + return; + } + } + if (pushedReadBuf == m_frontendReadBuffer) { + return; + } + m_frontendReadBuffer = pushedReadBuf; + + const GLenum glPushedReadBuffer = GetBackendAttachmentType(pushedReadBuf); + if (m_backendReadBuffer != glPushedReadBuffer) { + m_backendReadBuffer = glPushedReadBuffer; + // Still bound as READ first, for the reason the legacy arm gives below: + // glReadBuffer targets whatever FBO is bound to GL_READ_FRAMEBUFFER. + Bind(FramebufferTarget::Read); + g_GLESFuncs.glReadBuffer(glPushedReadBuffer); + } + return; + } +#endif auto frontendReadBuf = stateFBOObject->GetReadBuffer(); if (frontendReadBuf == m_frontendReadBuffer) { return; @@ -8337,9 +8431,58 @@ namespace MobileGL::MG_Backend::DirectGLES { GLenum glFBOTarget = MG_Util::ConvertFramebufferTargetToGLEnum(asTarget); Bind(asTarget); +#if MOBILEGL_PIPE_PUSH + // P4a (D-C2): the record for THIS bound target. The client emits one per bound + // target that moved, or one with Target = Both when the two bindings name the same + // object, so a record that reaches this twin always describes the target it is + // being synced as - and the "same FBO as draw" skip whose one surviving job was + // SyncReadBufferToBackend stops being a hazard, because the record says which + // target it is instead of the call site having to remember. + const MG_Pipe::MGPFramebufferState* pushedRecord = nullptr; + FramebufferObject::FramebufferAttachmentArray pushedDrawBuffers{}; + if (FramebufferSubsystemEnabled()) { + // MONOLITH GLUE, named as such: the Framebuffer handle of an object this backend + // still arrives holding. A framebuffer has a handle but NO wire lifetime (D-I2) - + // there is no framebuffer create or destroy in the catalogue and none is invented + // - so the handle exists purely to key set_framebuffer_state, which is exactly + // what it is used for here. + const MG_Pipe::MGPipeHandle fbo = g_backendFramebufferObjects.HandleOf(stateFBOObject.get()); + pushedRecord = PushedFramebufferRecord(asTarget, fbo); + if (pushedRecord == nullptr) { + MGLOG_E_ONCE("MGPipe: framebuffer %u has no %s-target applier record on the handle " + "arm, so it cannot be synced from the pushed record (handle {%u, %u})", + stateFBOObject->GetExternalIndex(), + asTarget == FramebufferTarget::Draw ? "draw" : "read", fbo.Slot, fbo.Gen); + return; + } + // The record's own Target has to agree with the binding it is being applied to. + // Both is legal for either; anything else means a record was written into the + // wrong applier slot, which is the mistake D-C2 exists to make visible rather + // than a call-site discipline nobody can check. + const Uint8 expected = asTarget == FramebufferTarget::Read + ? static_cast(MG_Pipe::MGPipeFramebufferTarget::Read) + : static_cast(MG_Pipe::MGPipeFramebufferTarget::Draw); + if (pushedRecord->Target != expected && + pushedRecord->Target != static_cast(MG_Pipe::MGPipeFramebufferTarget::Both)) { + MGLOG_E_ONCE("MGPipe: framebuffer %u's record says Target=%u but it is being synced " + "as the %s target - refusing rather than applying it to the wrong " + "binding", + stateFBOObject->GetExternalIndex(), pushedRecord->Target, + asTarget == FramebufferTarget::Draw ? "draw" : "read"); + return; + } + DecodePushedDrawBuffers(*pushedRecord, pushedDrawBuffers.data()); + } +#endif + // -------------------- Connect attachments (set buffers) ----------------------- // 1. Remap draw buffers +#if MOBILEGL_PIPE_PUSH + const FramebufferObject::FramebufferAttachmentArray& stateDrawBuffers = + pushedRecord != nullptr ? pushedDrawBuffers : stateFBOObject->GetDrawBuffers(); +#else auto& stateDrawBuffers = stateFBOObject->GetDrawBuffers(); +#endif Bool drawBufferClean = false; if (memcmp(m_frontendDrawBuffers, stateDrawBuffers.data(), FramebufferObject::MAX_DRAW_BUFFERS * sizeof(FramebufferAttachmentType)) == 0) { @@ -8439,6 +8582,31 @@ namespace MobileGL::MG_Backend::DirectGLES { static_cast(~0u)); m_syncedBackendIdGeneration = g_attachmentBackendIdGeneration; } +#if MOBILEGL_PIPE_PUSH + // P4a (D-C4): the record's ContentHash is what says this target's resolved state + // moved, and it REPLACES the frontend attachment versions AS A KEY. It covers every + // field the record carries, so an attachment set that moved, a draw-buffer array + // that moved, an extent that moved and a recycled Fbo whose successor happens to + // carry an identical attachment set are all one compare - and it moves in ONE place + // rather than in an array of 41 the twin had to walk. + // + // The re-arm is expressed through m_syncedFrontendAttachmentVersions rather than + // around it, so the attachment loop below is unchanged on both arms: a moved record + // arms every point and the loop then does exactly what it does today, including the + // empty-colour-point detach that keeps m_backendColorSlots a permutation of the + // PHYSICAL layout. + // + // OVER-FIRING IS FREE AND UNDER-FIRING IS FATAL, so the per-attachment versions stay + // as a second, narrower gate underneath: a frontend attachment that moved without + // the record moving still re-attaches. That direction is the safe one and it is the + // reason the array is re-armed rather than retired here; the array itself retires + // with the frontend attachment objects, which is E's SyncAttachmentObject work. + if (pushedRecord != nullptr && m_syncedRecordHashes[SizeT(asTarget)] != pushedRecord->ContentHash) { + std::fill(m_syncedFrontendAttachmentVersions.begin(), m_syncedFrontendAttachmentVersions.end(), + static_cast(~0u)); + m_syncedRecordHashes[SizeT(asTarget)] = pushedRecord->ContentHash; + } +#endif const auto& attachments = stateFBOObject->GetAllAttachmentObjects(); const auto& attachmentVersions = stateFBOObject->GetAllFramebufferAttachmentVersions(); for (SizeT i = 0; i < attachments.size(); ++i) { diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.h b/MobileGL/MG_Backend/DirectGLES/Managers.h index 822c1f15..67a1148e 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.h +++ b/MobileGL/MG_Backend/DirectGLES/Managers.h @@ -1812,11 +1812,50 @@ namespace MobileGL::MG_Backend::DirectGLES { // mismatch means some backend texture id was re-minted since, and any of this // twin's attachment points may still hold the dead id even though the frontend // attachment versions match - so the walk re-attaches everything first. + // + // SERVER-OWNED AND IT SURVIVES P4a (D-B3). It answers "did *I* re-mint a driver + // texture id", which no client-side version can answer; dropping it would + // 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 + // BOUND TARGET, 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). + // + // The hash covers every field the record carries - Fbo included, so a recycled + // framebuffer handle whose successor happens to carry an identical attachment set + // can never be suppressed against its predecessor, and DrawBuffers[8] included, so + // 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, because Draw and Read sync different things off two + // different records; 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}; +#endif }; extern TwinRegistry g_backendFramebufferObjects; + +#if MOBILEGL_PIPE_PUSH + // P4a (D-C2): the applier's record FOR THIS BOUND TARGET, or null. + // + // The applier holds set_framebuffer_state as WORKING STATE - two records, Draw and Read, + // written by whichever emission named that target (Target = Both writes both). This twin + // is per FRAMEBUFFER OBJECT, so the two have to be matched: the record is this twin's + // only if its Fbo names this twin's handle. A mismatch means the object being synced is + // not the one currently bound to that target, which is a real sequence (a scratch FBO + // synced while another is bound) and is answered with null rather than with the other + // framebuffer's attachments. + // + // `fbo` is the handle the caller resolved for this twin; passing it in rather than + // resolving it here keeps the monolith-glue lookup at one site per sync. + const MG_Pipe::MGPFramebufferState* PushedFramebufferRecord(FramebufferTarget asTarget, + MG_Pipe::MGPipeHandle fbo); +#endif // True when the read buffer names a fixed-point (norm/snorm) attachment that the // backend actually stores in a floating-point format. GL clamps a read from a // fixed-point colour buffer to [0,1] (GL_CLAMP_READ_COLOR defaults to