swung0x48/MobileGL
1
0
Fork 0
mirror of https://github.com/MobileGL-Dev/MobileGL synced 2026-09-09 12:48:32 +09:00
Code Issues Packages Projects Releases Wiki Activity

[Fix] (Espryt): resolve the framebuffer record by the handle it names and refuse before binding, so a DSA blit or clear configures the framebuffer it is about to write instead of one that never got its attachments

Browse Source
This commit is contained in:
swung0x48
2026-09-08 16:57:36 -04:00
committed by rereview
parent e021b4f62b
commit 2bd8f3c82d
2 changed files with 104 additions and 57 deletions
Download Patch File Download Diff File Expand all files Collapse all files
MobileGL/MG_Backend/DirectGLES/Managers.cpp
+55 -43
View File
@@ -7919,6 +7919,14 @@ namespace MobileGL::MG_Backend::DirectGLES {
g_fboSyncedSlotVersions = {0};
g_fboSyncedObjectVersions = {0};
g_fboSyncedObjects = {};
#if MOBILEGL_PIPE_PUSH && MOBILEGL_ESPRYT_FBO_HANDLE_ARM_MEMOS_LINKED
// E's MAJOR-4 / A9: the handle arm's own memos say the same thing about the same
// targets, so they are invalidated HERE rather than at each of the nine call sites -
// six in DirectGLES.cpp and three in SanityTest.cpp, and it was the three that were
// missed. See the declaration in Managers.h for why the call is gated and which line
// each package flips.
InvalidateFramebufferHandleArmMemos();
#endif
}
void BackendFramebufferObject::InvalidateSyncedState() {
@@ -8213,17 +8221,20 @@ namespace MobileGL::MG_Backend::DirectGLES {
}
#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;
const MG_Pipe::MGPFramebufferState* PushedFramebufferRecord(MG_Pipe::MGPipeHandle fbo) {
// FramebufferRecordFor answers null for the null handle and for a slot nothing has
// described (both silent - that is every framebuffer before its first emission), and
// refuses a stale generation loudly while counting it in
// StaleFramebufferRecordLookups. So there is no second emptiness test here to get out
// of step with the applier's, and none of the three cases is a binding question.
return MG_Pipe::MGPipeApplier().FramebufferRecordFor(fbo);
}
Bool PushedFramebufferIsBoundTo(FramebufferTarget target, MG_Pipe::MGPipeHandle fbo) {
if (MG_Pipe::MGPipeHandleIsNull(fbo)) return false;
const auto binding = target == FramebufferTarget::Read ? MG_Pipe::MGPipeFramebufferTarget::Read
: MG_Pipe::MGPipeFramebufferTarget::Draw;
return MG_Pipe::MGPipeApplier().BoundFramebuffer[static_cast<SizeT>(binding)] == fbo;
}
// The record's DrawBuffers[] is an ATTACHMENT INDEX with -1 for None (D-C1), which is
@@ -8266,10 +8277,15 @@ namespace MobileGL::MG_Backend::DirectGLES {
// 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);
// ID-19: the OBJECT's record, not "the record of whatever is bound to READ". A
// framebuffer whose read buffer is being pushed need not be the read binding at
// all - glNamedFramebufferReadBuffer and the DSA clears reach here by name - and
// ReadSurface is resolved from THIS framebuffer's own read buffer under every
// Target, Named included (c0e's MGPFramebufferState comment).
const auto* record = PushedFramebufferRecord(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})",
MGLOG_E_ONCE("MGPipe: framebuffer %u has no applier record on the handle arm, so "
"its read buffer cannot be pushed (handle {%u, %u})",
stateFBOObject->GetExternalIndex(), fbo.Slot, fbo.Gen);
return;
}
@@ -8429,15 +8445,26 @@ namespace MobileGL::MG_Backend::DirectGLES {
MGLOG_D("Syncing FBO with backend ID %u to backend for state ID %u, as %s FBO", m_backendFBOId,
stateFBOObject->GetExternalIndex(), (asTarget == FramebufferTarget::Draw ? "DRAW" : "READ"));
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.
// P4a (D-C2 as corrected by ID-19): THE RECORD OF THIS FRAMEBUFFER OBJECT, resolved
// BEFORE the driver framebuffer is bound.
//
// The order is the whole of C-1's fix. v1 bound first and refused afterwards, so a
// glClearNamedFramebufferfv / glBlitNamedFramebuffer on a framebuffer that is bound
// to neither target left this twin's freshly minted driver FBO bound with NO
// ATTACHMENTS and the caller then issued the clear or the blit against it -
// GL_INVALID_FRAMEBUFFER_OPERATION and nothing cleared, where the legacy arm cleared
// correctly. A refusal now happens before anything is bound, so the driver's binding
// is exactly where the caller left it and the failure is one named log line rather
// than a wrong picture.
//
// The record is per FRAMEBUFFER OBJECT and is found by handle, so it describes this
// object whether it is bound to Draw, to Read, to both or to neither, and its own
// Target is NOT compared against `asTarget`: a Named record legitimately names no
// binding at all. What is still gated on `asTarget` is what reaches the DRIVER's
// bound target - glDrawBuffers and the four cross-object masks below, which stay
// Draw-only for the reason the OIT comment gives.
const MG_Pipe::MGPFramebufferState* pushedRecord = nullptr;
FramebufferObject::FramebufferAttachmentArray pushedDrawBuffers{};
if (FramebufferSubsystemEnabled()) {
@@ -8447,33 +8474,18 @@ namespace MobileGL::MG_Backend::DirectGLES {
// - 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);
pushedRecord = PushedFramebufferRecord(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<Uint8>(MG_Pipe::MGPipeFramebufferTarget::Read)
: static_cast<Uint8>(MG_Pipe::MGPipeFramebufferTarget::Draw);
if (pushedRecord->Target != expected &&
pushedRecord->Target != static_cast<Uint8>(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");
MGLOG_E_ONCE("MGPipe: framebuffer %u has no applier record on the handle arm, so it "
"cannot be synced from the pushed record and is left unbound rather "
"than bound-and-unconfigured (handle {%u, %u})",
stateFBOObject->GetExternalIndex(), fbo.Slot, fbo.Gen);
return;
}
DecodePushedDrawBuffers(*pushedRecord, pushedDrawBuffers.data());
}
#endif
Bind(asTarget);
// -------------------- Connect attachments (set buffers) -----------------------
// 1. Remap draw buffers
@@ -8583,7 +8595,7 @@ namespace MobileGL::MG_Backend::DirectGLES {
m_syncedBackendIdGeneration = g_attachmentBackendIdGeneration;
}
#if MOBILEGL_PIPE_PUSH
// P4a (D-C4): the record's ContentHash is what says this target's resolved state
// P4a (D-C4): the record's ContentHash is what says this framebuffer'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
MobileGL/MG_Backend/DirectGLES/Managers.h
+49 -14
View File
@@ -1819,7 +1819,7 @@ namespace MobileGL::MG_Backend::DirectGLES {
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
// 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).
@@ -1830,9 +1830,13 @@ 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, 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.
// 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.
Array<Uint64, SizeT(FramebufferTarget::FramebufferTargetCount)> m_syncedRecordHashes = {0};
#endif
};
@@ -1841,20 +1845,30 @@ namespace MobileGL::MG_Backend::DirectGLES {
g_backendFramebufferObjects;
#if MOBILEGL_PIPE_PUSH
// P4a (D-C2): the applier's record FOR THIS BOUND TARGET, or null.
// P4a (D-C2 as corrected by ID-19): the applier's record for THE FRAMEBUFFER OBJECT this
// handle names, 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.
// v1 asked the applier for its two BOUND-target working records and answered null unless
// one of them happened to name this twin - which meant every DSA entry point
// (BlitNamedFramebuffer, the four ClearNamedFramebuffer*) drove a framebuffer that is
// bound to neither target, found no record, declined, and then had the clear or blit
// issued against a driver FBO that never got its attachments. The record is now keyed by
// the framebuffer HANDLE (MGPipeApplierState::FramebufferRecords, wire v3), so a record
// that comes back is this framebuffer's by construction and it comes back whether the
// object is bound to Draw, to Read, to both or to neither. A null here means "no emission
// has ever described this framebuffer, or the handle's generation is stale" - both of
// them seam defects on an integrated tree, never a binding question.
//
// `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);
const MG_Pipe::MGPFramebufferState* PushedFramebufferRecord(MG_Pipe::MGPipeHandle fbo);
// THE BINDING QUESTION, WHICH IS NOW A DIFFERENT QUESTION FROM THE DESCRIPTION (ID-19(d)):
// "is the framebuffer this handle names the one bound to `target`". One array compare
// against MGPipeApplierState::BoundFramebuffer, never a record lookup - a Named record
// describes an object without claiming any binding for it, so asking the record would
// give the wrong answer by construction.
Bool PushedFramebufferIsBoundTo(FramebufferTarget target, 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
@@ -1963,6 +1977,27 @@ namespace MobileGL::MG_Backend::DirectGLES {
// GL_FRAMEBUFFER binds both targets.
void BindFramebufferId(GLenum fbTarget, Uint id);
Uint CurrentFramebufferBinding(FramebufferTarget target);
#if MOBILEGL_PIPE_PUSH
// THE HANDLE ARM'S OWN FRAMEBUFFER MEMOS, AND THEY ARE PACKAGE E's STORAGE
// (DirectGLES.cpp: g_fboSyncedSerials, g_fboRecordsTrusted). E's review MAJOR-4 handed
// this to D because InvalidateFramebufferBindingCache is in THIS file and has three
// callers E cannot reach - MG_Test/SanityTest.cpp's ScopedStateGuardMocks::ResetShadows
// and ScopedBackendTwinMocks' constructor and destructor - which clear the pre-handle
// trio and would leave the handle-arm memos claiming a target is synced across a GLES
// function-table swap. Calling it from INSIDE InvalidateFramebufferBindingCache is what
// makes forgetting impossible, and that call is written below.
//
// IT IS GATED, AND HERE IS THE HANDSHAKE, because the definition is `static` in E's file
// on the tree this package was built against (esprytdraw v2, DirectGLES.cpp:2789) and an
// internal-linkage function cannot be called from Managers.cpp. E's verification round
// drops that one keyword; D's verification round flips this constant to 1, in this file,
// one line. Neither side can do it silently: the flip has no other reader and the
// declaration below has no other definition.
#define MOBILEGL_ESPRYT_FBO_HANDLE_ARM_MEMOS_LINKED 0
#if MOBILEGL_ESPRYT_FBO_HANDLE_ARM_MEMOS_LINKED
void InvalidateFramebufferHandleArmMemos();
#endif
#endif
void InvalidateFramebufferBindingCache();
// A driver framebuffer id is about to be deleted: ES reverts every target that
// currently binds it to 0, so the binding shadow has to follow or the next