From 711d5c61ba7b30f54f23ca1a3706a1f67aaffa61 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Tue, 8 Sep 2026 14:32:28 -0400 Subject: [PATCH] [Feat] (Pipe): key the framebuffer record by the framebuffer handle and resolve the two bindings through it, so a framebuffer that is named but bound to neither can still be described --- MobileGL/MG_Pipe/PipeApply.cpp | 155 ++++++++++++---- MobileGL/MG_Pipe/PipeApply.h | 172 ++++++++++++++++-- MobileGL/MG_Test/Pipe/FramebufferEmitTest.cpp | 96 ++++++---- 3 files changed, 341 insertions(+), 82 deletions(-) diff --git a/MobileGL/MG_Pipe/PipeApply.cpp b/MobileGL/MG_Pipe/PipeApply.cpp index 6cf0358d..11bb40bb 100644 --- a/MobileGL/MG_Pipe/PipeApply.cpp +++ b/MobileGL/MG_Pipe/PipeApply.cpp @@ -1110,11 +1110,18 @@ namespace MobileGL::MG_Pipe { // ---- P4a's working state, cleared for the same reason and with the same serial rule // (D-J4). The OBJECT records - texture and renderbuffer resources, sampler CSOs, - // sampler views, shader CSOs - are deliberately NOT here: a texture lives in a share - // group exactly as a buffer does, and its record is where the extent, the parameters - // and the pending-upload set the backend reads now live. - g_applier.DrawFramebuffer = MGPFramebufferState{}; - g_applier.ReadFramebuffer = MGPFramebufferState{}; + // sampler views, shader CSOs and, since ID-19(b), the FRAMEBUFFER RECORDS - are + // deliberately NOT here: a texture lives in a share group exactly as a buffer does, and + // its record is where the extent, the parameters and the pending-upload set the backend + // reads now live. + // + // WHAT A MAKE-CURRENT TAKES OF THE FRAMEBUFFER FAMILY IS THE TWO BOUND HANDLES AND + // NOTHING ELSE, and that is the same observable it always took: DrawFramebuffer() / + // ReadFramebuffer() answer null after this, exactly as the zeroed records used to answer + // a null Fbo. FramebufferRecords stays because a framebuffer that is only ever addressed + // BY NAME - the DSA blit and clear entry points - has no re-emission trigger at all, so + // dropping its record here would lose it for the life of the process. + g_applier.BoundFramebuffer = {}; g_applier.BoundSamplerViews = {}; g_applier.SamplerViewStart = 0; g_applier.SamplerViewCount = 0; @@ -1154,21 +1161,27 @@ namespace MobileGL::MG_Pipe { g_applier.SamplerViewCsos.clear(); g_applier.ShaderCsos.clear(); g_applier.CompositeShaderCsos.clear(); + // AND THE FRAMEBUFFER RECORDS, which MGPipeApplierReset above deliberately keeps. Here + // there is nothing for them to outlive: the served context is going away and the + // framebuffers it owned are container objects that go with it. A table left standing + // would answer a lookup from the NEXT context on a slot the allocator has re-minted - + // with the same generation, if the next context happens to reach the same one - and + // that is the aliasing every other table is cleared to prevent. + g_applier.FramebufferRecords.clear(); g_applier.DrawProgram = kMGPipeNullHandle; g_applier.DispatchProgram = kMGPipeNullHandle; g_applier.BoundShaderCso = kMGPipeNullHandle; // AND "THE WORKING HANDLES THEY COULD NAME" IS ALL OF THEM, NOT JUST THE THREE ABOVE. - // The two framebuffer records hold eleven MGPSurface::Res naming texture and - // renderbuffer records this function has just dropped, and the three unit windows hold - // entries naming sampler-view, sampler-CSO and texture records it has just dropped. A - // window left populated here is a set of handles into empty tables: the next resolve - // either refuses and counts (a teardown-time refusal storm nothing asked for) or, on a - // slot the next context re-mints, resolves to somebody else's record. So this takes the - // same working state MGPipeApplierReset does - which is the honest reading of the rule - // as well: Reset keeps the object records BECAUSE they outlive a make-current, and here - // they do not outlive anything. - g_applier.DrawFramebuffer = MGPFramebufferState{}; - g_applier.ReadFramebuffer = MGPFramebufferState{}; + // Every framebuffer record holds eleven MGPSurface::Res naming texture and renderbuffer + // records this function has just dropped - which is why the table itself goes above - + // and the three unit windows hold entries naming sampler-view, sampler-CSO and texture + // records it has just dropped. A window left populated here is a set of handles into + // empty tables: the next resolve either refuses and counts (a teardown-time refusal + // storm nothing asked for) or, on a slot the next context re-mints, resolves to somebody + // else's record. So this takes the same working state MGPipeApplierReset does - which is + // the honest reading of the rule as well: Reset keeps the object records BECAUSE they + // outlive a make-current, and here they do not outlive anything. + g_applier.BoundFramebuffer = {}; g_applier.BoundSamplerViews = {}; g_applier.SamplerViewStart = 0; g_applier.SamplerViewCount = 0; @@ -2037,15 +2050,32 @@ namespace MobileGL::MG_Pipe { // ================================================================================ void MGPipeApplySetFramebufferState(const MGPFramebufferState& state) { - // GL HAS TWO INDEPENDENT FRAMEBUFFER BINDINGS AND THIS RECORD DESCRIBES ONE, so Target - // is what says which - and Both is one object bound to both, which writes both records - // from one call. A value outside the three is not a binding this server has, and - // guessing one would put a draw's attachments into the read record or the other way - // round, which is the defect class the per-target emission exists to close. + // GL HAS TWO INDEPENDENT FRAMEBUFFER BINDINGS AND THIS RECORD DESCRIBES ONE FRAMEBUFFER, + // so Target says which binding - if any - the description also BINDS it to. Both is one + // object bound to both and moves both handles from one call; Named + // (kMGPipeFramebufferTargetNamed) moves neither, which is how the DSA entry points + // describe a framebuffer they are about to blit into or clear without claiming it is + // bound (ID-19). A value above Named is not a target this server has, and guessing one + // would put a draw's attachments into the read binding or the other way round. const char* fault = nullptr; - if (state.Target >= static_cast(MGPipeFramebufferTarget::Count)) { + if (state.Target > kMGPipeFramebufferTargetNamed) { fault = "the record names no framebuffer binding target"; } + // THE RECORD IS ADDRESSED BY THE FRAMEBUFFER HANDLE NOW, so the handle has to be one. + // The null handle is refused rather than stored: {0,0} is what "nothing is bound to this + // binding" reads as on the other side of BoundFramebuffer, and a record installed there + // would be answered to every caller asking about an EMPTY binding. Every emitter has a + // handle for every framebuffer it describes - kMGPipeDefaultFramebuffer {0,1} for the + // default one, a minted {slot, gen} otherwise - so a null here is a malformed record. + else if (MGPipeHandleIsNull(state.Fbo)) { + fault = "the record names the null framebuffer handle"; + } + // And the same bound gate the five object tables carry, for the same reason: Slot is a + // client-supplied Uint32 arriving in a payload and this is the one number in the call + // that reaches an allocator. + else if (state.Fbo.Slot >= kMGPipeMaxFramebufferSlots) { + fault = "the framebuffer slot is outside the record table's bound"; + } // THE DRAW-BUFFER ARRAY IS AN INDEX INTO THIS RECORD'S OWN Color[], and -1 is NONE. An // entry outside that range would have the server read a colour attachment the record // does not carry: the wire array is 8 wide, the driver's MaxColorAttachments is not @@ -2069,23 +2099,86 @@ namespace MobileGL::MG_Pipe { // NO HANDLE IS RESOLVED HERE AND NONE MAY BE. A framebuffer has a handle but no wire // lifetime - the catalogue has no framebuffer create and no framebuffer destroy, // because a framebuffer is state and this call is the only one that names one - so - // there is no record to refuse against and this entry point never counts a refusal. + // there is nothing to look up, this entry point never counts a refusal, and the record + // it writes OVERWRITES whatever the slot held. That last part is the point rather than + // a compromise: the slot's previous occupant is dead by construction (the client frees + // a framebuffer's slot when the object dies and the allocator bumps the generation on + // reuse), and the record arriving now describes whatever holds the slot today. + // // The surfaces' Res handles are not resolved either: an attachment PINS its texture, // and in monolith the frontend's own SharedPtr is that keep-alive, so a refusal here // would be enforcing a lifetime rule monolith cannot need and split has not defined. - if (state.Target != static_cast(MGPipeFramebufferTarget::Read)) { - g_applier.DrawFramebuffer = state; + MGPipeFramebufferRecord* record = + RecordAt(g_applier.FramebufferRecords, state.Fbo.Slot, kMGPipeMaxFramebufferSlots); + // Unreachable: the bound was checked above, before anything moved. The null check is + // what makes that an argument rather than an assumption. + if (record == nullptr) return; + record->Gen = state.Fbo.Gen; + record->Live = true; + record->State = state; + + // AND ONLY THEN THE BINDINGS. Named moves neither, which is the whole of the fourth + // target: "this record describes the framebuffer it names; no binding changes". + if (state.Target == static_cast(MGPipeFramebufferTarget::Draw) || + state.Target == static_cast(MGPipeFramebufferTarget::Both)) { + g_applier.BoundFramebuffer[static_cast(MGPipeFramebufferTarget::Draw)] = state.Fbo; } - if (state.Target != static_cast(MGPipeFramebufferTarget::Draw)) { - g_applier.ReadFramebuffer = state; + if (state.Target == static_cast(MGPipeFramebufferTarget::Read) || + state.Target == static_cast(MGPipeFramebufferTarget::Both)) { + g_applier.BoundFramebuffer[static_cast(MGPipeFramebufferTarget::Read)] = state.Fbo; } - // ONE SERIAL FOR THE PAIR, and it moves once per applied record - a Both record is one - // record. It is what retires the four g_fboSynced* arrays and the twin's own - // {slot version, object version, backend id generation} quadruple: a compare that used - // to ask "is my memo still the frontend's" asks "is my serial still the server's". + // ONE SERIAL FOR THE FAMILY, and it moves once per applied record - a Both record is one + // record and a Named record is one record. It is what retires the four g_fboSynced* + // arrays and the twin's own {slot version, object version, backend id generation} + // quadruple: a compare that used to ask "is my memo still the frontend's" asks "is my + // serial still the server's". A Named record moves it for the same reason a bound one + // does - a twin that memoised a framebuffer's attachments has to hear that they moved, + // and whether the framebuffer happens to be bound is not that question. ++g_applier.FramebufferSerial; } + // THE READ SIDE OF THE TABLE (ID-19(b)/(d)). Package D asks it by handle at every DSA entry + // point; the two accessors under it ask it through a binding, which is the question package + // E's SyncCurrentFBOByRecord has always asked. + const MGPFramebufferState* MGPipeApplierState::FramebufferRecordFor(MGPipeHandle fbo) const { + // "Nothing is bound" and "no record" are BOTH null and only the second of them is worth + // a word: the null handle is what a make-current leaves in BoundFramebuffer and what an + // unbound target reads as, it arrives here on every draw of a context that has not + // described its framebuffers yet, and logging it would be a storm about the ordinary + // transitional state. It is separated first so the generation test below can be loud. + if (MGPipeHandleIsNull(fbo)) return nullptr; + if (fbo.Slot >= FramebufferRecords.size()) return nullptr; + const MGPipeFramebufferRecord& record = FramebufferRecords[fbo.Slot]; + // Nothing has been written at this slot. Also not an error, and also silent: it is what + // every framebuffer looks like before its first set_framebuffer_state, and the caller's + // answer to it is to run its pre-handle arm. + if (!record.Live) return nullptr; + if (record.Gen != fbo.Gen) { + // AND THIS ONE IS. A record exists at the slot and it describes a DIFFERENT object: + // either the caller is holding a handle whose framebuffer died and whose slot has + // been recycled, or an emitter minted the successor without ever describing it. On + // the handle arm the visible consequence is a DSA blit or clear into a driver + // framebuffer that never got its attachments, which is precisely the failure + // ID-19 exists to close - so this is the loud refusal P3a's lesson demands and + // never a quiet fall-back. + ++StaleFramebufferRecordLookups; + MGLOG_E_ONCE("MGPipe: the framebuffer record at slot %u is generation %u and the " + "lookup named generation %u; refusing it rather than answering with " + "another framebuffer's attachments", + fbo.Slot, record.Gen, fbo.Gen); + return nullptr; + } + return &record.State; + } + + const MGPFramebufferState* MGPipeApplierState::DrawFramebuffer() const { + return FramebufferRecordFor(BoundFramebuffer[static_cast(MGPipeFramebufferTarget::Draw)]); + } + + const MGPFramebufferState* MGPipeApplierState::ReadFramebuffer() const { + return FramebufferRecordFor(BoundFramebuffer[static_cast(MGPipeFramebufferTarget::Read)]); + } + // ================================================================================ // w2: sampler CSOs, sampler views and the three unit sets // ================================================================================ diff --git a/MobileGL/MG_Pipe/PipeApply.h b/MobileGL/MG_Pipe/PipeApply.h index ae192748..7c31f145 100644 --- a/MobileGL/MG_Pipe/PipeApply.h +++ b/MobileGL/MG_Pipe/PipeApply.h @@ -147,6 +147,42 @@ namespace MobileGL::MG_Pipe { static_assert(kMGPipeMaxShaderCsoSlots > kMGPipeShaderCsoCompositeSlotBase, "the ShaderCso bound must contain the composite band, or a composite handle " "is refused as out of range on arrival"); + // ID-19(b): the framebuffer record is now PER OBJECT and its table is slot-indexed like the + // five above, so it takes a bound on the same terms. A framebuffer record is 304 bytes and + // an FBO is a CONTAINER object - not shared between contexts, minted a few dozen at a time + // by a renderer and a few hundred by a shader pack - so 1<<16 is orders of magnitude above + // any live population and still turns a corrupt Uint32 into a refusal rather than a + // 4-billion-entry resize. + inline constexpr Uint32 kMGPipeMaxFramebufferSlots = 1u << 16; + + // THE FOURTH set_framebuffer_state TARGET, AND IT IS THE CONTRACT'S TO MINT (c0e: + // MGPipeFramebufferTarget::Named = 3). It is declared here as a plain constant because wire + // v3 and c0e run in parallel: the applier must ADMIT the value now, and this package may not + // write MGPipeTypes.h. When c0e lands, this constant is deleted and every use below becomes + // static_cast(MGPipeFramebufferTarget::Named) - wire's verification round retires it, + // and the static_assert underneath is what makes forgetting impossible: the day the + // enumerator exists, MGPipeFramebufferTarget::Count becomes 4 and this fires. + // + // ITS MEANING: "this record describes the framebuffer it names; no binding changes." Draw / + // Read / Both write the record AND set the bound handle(s); Named writes the record only. + // That is what lets the DSA entry points - BlitNamedFramebuffer and the four + // ClearNamedFramebuffer* - be handed a record for a framebuffer that is bound to neither + // binding, which is the hole esprytobj's C-1 found: the applier used to hold the two BOUND + // records only, so a named blit or clear reached a driver FBO that never got its + // attachments. + inline constexpr Uint8 kMGPipeFramebufferTargetNamed = 3; + static_assert(static_cast(MGPipeFramebufferTarget::Count) == kMGPipeFramebufferTargetNamed, + "c0e has landed MGPipeFramebufferTarget::Named: delete kMGPipeFramebufferTargetNamed " + "and spell the enumerator (wire's verification round, ID-21)"); + + // The two framebuffer BINDINGS, and there are two rather than three: Both and Named are + // things a RECORD says, not bindings a server has. MGPipeApplierState::BoundFramebuffer is + // indexed by MGPipeFramebufferTarget::Draw / ::Read, which is what makes package D's + // "is this framebuffer the one bound to target t" one array compare (ID-19(d)). + inline constexpr Uint32 kMGPipeFramebufferBindingCount = 2; + static_assert(static_cast(MGPipeFramebufferTarget::Draw) == 0 && + static_cast(MGPipeFramebufferTarget::Read) == 1, + "BoundFramebuffer is indexed by the target byte; Draw and Read must be 0 and 1"); // ---- P4a's SHAPE bounds, and they are the same argument the slot bounds above make, one // level down: every number below arrives inside a payload, every one of them decides how @@ -343,6 +379,27 @@ namespace MobileGL::MG_Pipe { Uint64 ContentSerial = 0; }; + // set_framebuffer_state's record, HELD PER FRAMEBUFFER OBJECT and indexed by the handle's + // slot (ID-19(b)). It is the one record kind in this file whose object has NO WIRE LIFETIME: + // the catalogue has no framebuffer create and no framebuffer destroy, because a framebuffer + // is state and set_framebuffer_state is the only call that names one (D-I2). So there is + // nothing to mark dead and nothing to refuse against, and a slot is simply OVERWRITTEN by + // its successor's record - which is correct rather than merely tolerable, since the record + // that reaches this table is the description of whatever object holds the slot NOW. + // + // `Live` is therefore NOT a lifetime. It means "a record has been written at this slot", + // which is the only question a reader can ask: it separates a table entry that exists + // because the vector grew past it from one an emission actually wrote. The GENERATION is + // still checked on every lookup (P3a contract-review M2), and a mismatch is a LOUD refusal - + // it means an emitter handed a stale handle, or minted a successor without describing it, + // which is exactly the seam defect the DSA arm would otherwise turn into a blit into a + // driver framebuffer with no attachments. + struct MGPipeFramebufferRecord { + Uint32 Gen = 0; + Bool Live = false; + MGPFramebufferState State{}; + }; + struct MGPipeApplierState { // Indexed by slot; slot 0 is the reserved null handle and is never live // (MGPipeHandles.h kMGPipeFirstAllocatableSlot). @@ -432,6 +489,19 @@ namespace MobileGL::MG_Pipe { // handle is an ordinary ShaderCso handle, and create/bind/delete_shader_state name it // exactly as they name any other program. Vector CompositeShaderCsos; + // AND THE SIXTH, WHICH IS THE ONE ID-19 ADDED. Keyed by the FRAMEBUFFER HANDLE's slot, + // for the reason MGPipeFramebufferRecord states: the two bound-target records the phase + // started with could not describe a framebuffer that is bound to neither binding, and + // the five DSA entry points (BlitNamedFramebuffer, the four ClearNamedFramebuffer*) hand + // Espryt exactly that. + // + // IT IS AN OBJECT TABLE AND IT LIVES WHERE THE OTHER OBJECT TABLES LIVE, which is also + // its make-current rule: MGPipeApplierReset does NOT clear it. An FBO is not shared + // between contexts, but its record is addressed by a slot out of one global allocator, + // so nothing aliases across a switch - and dropping the table would leave a + // DSA-only framebuffer with no record and no event that would ever re-emit one (the + // client's suppressor invalidation re-emits the two BOUND records and nothing else). + Vector FramebufferRecords; // Every call this applier REFUSED because it named a record this applier does not // have: an unknown slot, a slot that is not live, or a generation that has moved on @@ -456,7 +526,10 @@ namespace MobileGL::MG_Pipe { // MGPSurface::Res is likewise left unresolved on purpose (D-I3: the keep-alives are the // frontend's SharedPtrs and enforcing them is a later phase's). Its only verdict is // Fatal{ProtocolCorruption} on a malformed record, and this counter must stay at 0 - // across every framebuffer call in every build. + // across every framebuffer call in every build. ID-19(b) does not change that: the + // per-object table is WRITTEN by that call and never looked up by it, and the refusal + // that the table CAN produce - a lookup whose generation has moved on - happens on the + // server's own read path and is counted apart, in StaleFramebufferRecordLookups. // // THE OTHER CLASS IS NOT COUNTED HERE AND MUST NOT BE: a var-tail window outside its // bound, or a set_texture_params whose BuiltinSampler is the null handle, would make @@ -513,13 +586,32 @@ namespace MobileGL::MG_Pipe { // restarts walks back through values already stamped into a twin that outlived the // switch, and P4a deletes the identity patches that used to close that hole. - // set_framebuffer_state, per bound target (D-C2). Target = Both writes both. The - // record is fully resolved: ReadSurface comes from the READ framebuffer's own read - // buffer, so the shared-FBO case cannot lose it, and DrawBuffers[] is applied only for - // a record whose Target is not Read. - MGPFramebufferState DrawFramebuffer{}; - MGPFramebufferState ReadFramebuffer{}; + // WHICH FRAMEBUFFER IS BOUND TO EACH BINDING, and that is ALL this pair is since + // ID-19(b): the record itself lives in FramebufferRecords above, keyed by the handle. + // Indexed by MGPipeFramebufferTarget::Draw / ::Read. kMGPipeNullHandle means "nothing + // described this binding yet", which is what a make-current leaves behind. + // + // set_framebuffer_state Draw / Read / Both writes the RECORD at state.Fbo's slot AND + // sets the handle(s) here; Named (kMGPipeFramebufferTargetNamed) writes the record and + // touches nothing here at all - that is the whole of the fourth target's meaning. + Array BoundFramebuffer{}; + // ONE SERIAL FOR THE FAMILY, and it moves on EVERY write - a Named record's included, + // because a twin memoising "the framebuffer state I have seen" has to hear about a + // named framebuffer's attachments exactly as it hears about a bound one's. It is the + // number that retires the four g_fboSynced* arrays and the twin's {slot version, object + // version, backend id generation} triple. Uint64 FramebufferSerial = 0; + // Every FramebufferRecordFor() that found a record at the slot whose GENERATION had + // moved on. It is NOT RefusedObjectCalls: this is a READ by the server's own sync path + // and not a call this applier refused, and set_framebuffer_state's counter contract + // (below) is that no framebuffer call ever moves that one. A non-zero value here is a + // seam defect - an emitter minted a successor for a recycled slot and never described + // it, or handed out a handle it had already retired - so it is counted AND logged, and + // a unit case reads it in every build for the reason the other counters exist. + // + // `mutable` because the three accessors below are const: package E holds the applier + // through a `const auto&` and must keep doing so. + mutable Uint64 StaleFramebufferRecordLookups = 0; // The three kVarTail unit sets, as received. NO STAGE DIMENSION: MobileGL's // texture-unit space is one merged array of 192, the same unit may be sampled from two @@ -551,6 +643,26 @@ namespace MobileGL::MG_Pipe { MGPipeHandle DispatchProgram = kMGPipeNullHandle; MGPipeHandle BoundShaderCso = kMGPipeNullHandle; Uint64 ProgramBindingSerial = 0; + + // ---- THE THREE FRAMEBUFFER ACCESSORS (ID-19(b)/(d)). They are functions rather than + // members because the storage moved under them and their callers must not have to know + // it did: `DrawFramebuffer()` / `ReadFramebuffer()` answer the question the two members + // used to answer - "which record describes the framebuffer bound to this binding" - by + // resolving BoundFramebuffer[t] through FramebufferRecords. + // + // NULL IS A REAL ANSWER AND HAS EXACTLY THREE CAUSES: nothing is bound to that binding + // (the null handle, which is what a make-current leaves and is NOT an error), no record + // has been written at that slot, or the slot's generation has moved on under the handle + // (which IS an error and is counted and logged - see StaleFramebufferRecordLookups). A + // caller that used to test `MGPipeHandleIsNull(st.DrawFramebuffer.Fbo)` tests the + // pointer instead; the two are the same question. + // + // Defined in PipeApply.cpp rather than inline HERE so this header keeps its include + // closure: the stale-generation path logs, and MG_Util/Debug/Log.h is not in this + // header's closure and may not become part of it. + const MGPFramebufferState* FramebufferRecordFor(MGPipeHandle fbo) const; + const MGPFramebufferState* DrawFramebuffer() const; + const MGPFramebufferState* ReadFramebuffer() const; }; // The monolith's single applier. Under split there is one per served context. @@ -570,11 +682,21 @@ namespace MobileGL::MG_Pipe { // destroy, and dropping them is a dropped write on the far side of it. // // P4a EXTENDS BOTH HALVES AND THE RULE IS UNCHANGED (D-J4). Cleared: the two framebuffer - // records, the three unit sets, DrawProgram / DispatchProgram / BoundShaderCso - all of it + // BINDINGS, the three unit sets, DrawProgram / DispatchProgram / BoundShaderCso - all of it // per-context working state - with their serials ADVANCED and never zeroed. Not cleared: - // texture and renderbuffer resources, sampler CSOs, sampler views, shader CSOs, and the - // texture params and pending uploads that ride on a resource record, because a texture - // lives in a share group exactly as a buffer does. + // texture and renderbuffer resources, sampler CSOs, sampler views, shader CSOs, the + // framebuffer RECORDS, and the texture params and pending uploads that ride on a resource + // record, because a texture lives in a share group exactly as a buffer does. + // + // ID-19(b) MOVED THE FRAMEBUFFER RECORD ACROSS THAT LINE and the reason is worth stating. + // Before it, the whole framebuffer state was working state and a make-current took it. Now + // the RECORD is an object record and only the two BOUND HANDLES are working state, so a + // switch clears the bindings - after which DrawFramebuffer() / ReadFramebuffer() answer + // null, exactly as the cleared records used to answer a null Fbo - and leaves the table + // standing. Dropping the table instead would silently lose the record of every framebuffer + // that is described by NAME and never bound, because the client's re-emission on a fresh + // context is driven by MGPipeSetHashSuppressor::InvalidateAll, which re-sends the two bound + // records and nothing else. // // AND THEREFORE NO P4a TRACKER NEEDS A RE-PUBLICATION PATH ON FreshlyPrimed, AND NONE MAY // HAVE ONE: re-emitting create_sampler_state for a record the applier still holds would @@ -781,11 +903,29 @@ namespace MobileGL::MG_Pipe { // follow this one on the same branch. // set_framebuffer_state. Fully resolved - nothing in the record requires a lookup on the - // far side. `state.Target` says which binding it describes (Draw / Read / Both) and the - // applier keeps the two records apart; Both writes both. ContentHash covers every field - // including Fbo and DrawBuffers[8], which is what makes a suppressed record provably mean - // "the draw-buffer array did not move" and therefore "the fragColor broadcast count did - // not move". + // far side. ContentHash covers every field including Fbo and DrawBuffers[8], which is what + // makes a suppressed record provably mean "the draw-buffer array did not move" and + // therefore "the fragColor broadcast count did not move". + // + // `state.Target` NOW SAYS TWO THINGS AT ONCE (ID-19(b)), and the record always does the + // first of them: + // + // - THE RECORD IS ALWAYS WRITTEN, at FramebufferRecords[state.Fbo.Slot], whatever the + // target is. The table is keyed by the framebuffer HANDLE, so one framebuffer's record + // can never displace another's, and a slot whose object has been recycled is simply + // overwritten by its successor's record (D-I2: no wire lifetime, so nothing to retire). + // - Draw / Read / Both ADDITIONALLY set BoundFramebuffer[Draw] / [Read] / both. + // kMGPipeFramebufferTargetNamed sets NEITHER: it is how a DSA entry point hands Espryt + // a framebuffer it is about to blit into or clear WITHOUT claiming it is bound. + // + // FramebufferSerial advances on every applied record, Named included. + // + // Two refusals, both Fatal{ProtocolCorruption} and neither counted (see RefusedObjectCalls: + // this entry point resolves nothing and can only ever fault): a target above Named, a + // draw-buffer entry outside the record's own Color[], a slot at or above + // kMGPipeMaxFramebufferSlots, and the NULL HANDLE - a record that named {0,0} would install + // itself where "nothing is bound" is read, and every emitter has a handle for every + // framebuffer it describes (kMGPipeDefaultFramebuffer {0,1} for the default one). void MGPipeApplySetFramebufferState(const MGPFramebufferState& state); // create_sampler_state. `parameters` is the client's canonical SamplerParameters copy, diff --git a/MobileGL/MG_Test/Pipe/FramebufferEmitTest.cpp b/MobileGL/MG_Test/Pipe/FramebufferEmitTest.cpp index dcf3aec3..87a4d521 100644 --- a/MobileGL/MG_Test/Pipe/FramebufferEmitTest.cpp +++ b/MobileGL/MG_Test/Pipe/FramebufferEmitTest.cpp @@ -221,32 +221,42 @@ TEST(FramebufferEmit, ADrawRecordAndAReadRecordAreKeptApartAndBothWritesBoth) { const Uint64 serialAtStart = MGPipeApplier().FramebufferSerial; MGPipeApplySetFramebufferState(FramebufferRecord(MGPipeHandle{4, 1}, MGPipeFramebufferTarget::Draw, 100)); - EXPECT_EQ(MGPipeApplier().DrawFramebuffer.Fbo, (MGPipeHandle{4, 1})); - EXPECT_EQ(MGPipeApplier().DrawFramebuffer.Width, 100u); - EXPECT_EQ(MGPipeApplier().DrawFramebuffer.Color[0].InternalFormat, 0x8058u); - EXPECT_EQ(MGPipeApplier().DrawFramebuffer.DrawBuffers[0], 0); - EXPECT_EQ(MGPipeApplier().ReadFramebuffer.Fbo, kMGPipeNullHandle) + ASSERT_NE(MGPipeApplier().DrawFramebuffer(), nullptr); + EXPECT_EQ(MGPipeApplier().DrawFramebuffer()->Fbo, (MGPipeHandle{4, 1})); + EXPECT_EQ(MGPipeApplier().DrawFramebuffer()->Width, 100u); + EXPECT_EQ(MGPipeApplier().DrawFramebuffer()->Color[0].InternalFormat, 0x8058u); + EXPECT_EQ(MGPipeApplier().DrawFramebuffer()->DrawBuffers[0], 0); + EXPECT_EQ(MGPipeApplier().ReadFramebuffer(), nullptr) << "a Draw record landed in the read binding as well"; const Uint64 afterDraw = MGPipeApplier().FramebufferSerial; EXPECT_GT(afterDraw, serialAtStart) << "an applied record must move the serial the twin memoises"; MGPipeApplySetFramebufferState(FramebufferRecord(MGPipeHandle{5, 2}, MGPipeFramebufferTarget::Read, 200)); - EXPECT_EQ(MGPipeApplier().ReadFramebuffer.Fbo, (MGPipeHandle{5, 2})); - EXPECT_EQ(MGPipeApplier().ReadFramebuffer.Width, 200u); - EXPECT_EQ(MGPipeApplier().DrawFramebuffer.Fbo, (MGPipeHandle{4, 1})) - << "a Read record overwrote the draw binding"; - EXPECT_EQ(MGPipeApplier().DrawFramebuffer.Width, 100u); + ASSERT_NE(MGPipeApplier().ReadFramebuffer(), nullptr); + EXPECT_EQ(MGPipeApplier().ReadFramebuffer()->Fbo, (MGPipeHandle{5, 2})); + EXPECT_EQ(MGPipeApplier().ReadFramebuffer()->Width, 200u); + ASSERT_NE(MGPipeApplier().DrawFramebuffer(), nullptr) << "a Read record overwrote the draw binding"; + EXPECT_EQ(MGPipeApplier().DrawFramebuffer()->Fbo, (MGPipeHandle{4, 1})); + EXPECT_EQ(MGPipeApplier().DrawFramebuffer()->Width, 100u); EXPECT_GT(MGPipeApplier().FramebufferSerial, afterDraw); // Both: one record, one serial bump, two destinations. const Uint64 beforeBoth = MGPipeApplier().FramebufferSerial; MGPipeApplySetFramebufferState(FramebufferRecord(MGPipeHandle{6, 3}, MGPipeFramebufferTarget::Both, 300)); - EXPECT_EQ(MGPipeApplier().DrawFramebuffer.Fbo, (MGPipeHandle{6, 3})); - EXPECT_EQ(MGPipeApplier().ReadFramebuffer.Fbo, (MGPipeHandle{6, 3})); - EXPECT_EQ(MGPipeApplier().DrawFramebuffer.Width, 300u); - EXPECT_EQ(MGPipeApplier().ReadFramebuffer.Width, 300u); + ASSERT_NE(MGPipeApplier().DrawFramebuffer(), nullptr); + ASSERT_NE(MGPipeApplier().ReadFramebuffer(), nullptr); + EXPECT_EQ(MGPipeApplier().DrawFramebuffer()->Fbo, (MGPipeHandle{6, 3})); + EXPECT_EQ(MGPipeApplier().ReadFramebuffer()->Fbo, (MGPipeHandle{6, 3})); + EXPECT_EQ(MGPipeApplier().DrawFramebuffer()->Width, 300u); + EXPECT_EQ(MGPipeApplier().ReadFramebuffer()->Width, 300u); EXPECT_EQ(MGPipeApplier().FramebufferSerial, beforeBoth + 1) << "a Both record is ONE record and moves the serial once"; + // The two earlier framebuffers keep their own records - the table is keyed by the handle, + // so binding a third displaced neither (ID-19(b)). + ASSERT_NE(MGPipeApplier().FramebufferRecordFor(MGPipeHandle{4, 1}), nullptr); + EXPECT_EQ(MGPipeApplier().FramebufferRecordFor(MGPipeHandle{4, 1})->Width, 100u); + ASSERT_NE(MGPipeApplier().FramebufferRecordFor(MGPipeHandle{5, 2}), nullptr); + EXPECT_EQ(MGPipeApplier().FramebufferRecordFor(MGPipeHandle{5, 2})->Width, 200u); // A framebuffer has a handle but NO wire lifetime, so there is no record to refuse against // and this entry point never counts an object refusal. @@ -254,22 +264,25 @@ TEST(FramebufferEmit, ADrawRecordAndAReadRecordAreKeptApartAndBothWritesBoth) { #endif } -// A target outside the three is not a binding this server has, and guessing one would put a -// draw's attachments into the read record or the other way round. +// A target outside the FOUR is not a target this server has, and guessing one would put a +// draw's attachments into the read binding or the other way round. Named (3) is legal since +// ID-19(b) and has its own case below; the first refused value is the one above it. TEST(FramebufferEmit, ATargetOutsideTheThreeBindingsIsRefusedNamingTheRecord) { #if !MOBILEGL_PIPE_PUSH GTEST_SKIP() << "MOBILEGL_PIPE_PUSH is off: there is no applier in this build"; #else ApplierGuard guard; MGPFramebufferState bad = FramebufferRecord(MGPipeHandle{7, 4}, MGPipeFramebufferTarget::Draw, 100); - bad.Target = static_cast(MGPipeFramebufferTarget::Count); + bad.Target = static_cast(kMGPipeFramebufferTargetNamed + 1); const Uint64 serialBefore = MGPipeApplier().FramebufferSerial; - ExpectRefusedNaming("set_framebuffer_state {slot=7, gen=4, target=3}: the record names no framebuffer " + ExpectRefusedNaming("set_framebuffer_state {slot=7, gen=4, target=4}: the record names no framebuffer " "binding target", [&bad]() { MGPipeApplySetFramebufferState(bad); }); - EXPECT_EQ(MGPipeApplier().DrawFramebuffer.Fbo, kMGPipeNullHandle); - EXPECT_EQ(MGPipeApplier().ReadFramebuffer.Fbo, kMGPipeNullHandle); + EXPECT_EQ(MGPipeApplier().DrawFramebuffer(), nullptr); + EXPECT_EQ(MGPipeApplier().ReadFramebuffer(), nullptr); + EXPECT_EQ(MGPipeApplier().FramebufferRecordFor(MGPipeHandle{7, 4}), nullptr) + << "a refused record was written into the per-object table anyway"; EXPECT_EQ(MGPipeApplier().FramebufferSerial, serialBefore) << "a refused record must not move the serial"; #endif @@ -289,7 +302,8 @@ TEST(FramebufferEmit, ADrawBufferEntryOutsideTheRecordsOwnArrayIsRefusedRatherTh MGPFramebufferState legal = FramebufferRecord(MGPipeHandle{8, 1}, MGPipeFramebufferTarget::Draw, 100); legal.DrawBuffers[7] = static_cast(kMGPipeMaxColorAttachments - 1); MGPipeApplySetFramebufferState(legal); - ASSERT_EQ(MGPipeApplier().DrawFramebuffer.Fbo, (MGPipeHandle{8, 1})); + ASSERT_NE(MGPipeApplier().DrawFramebuffer(), nullptr); + ASSERT_EQ(MGPipeApplier().DrawFramebuffer()->Fbo, (MGPipeHandle{8, 1})); const Uint64 serialBefore = MGPipeApplier().FramebufferSerial; MGPFramebufferState past = FramebufferRecord(MGPipeHandle{8, 1}, MGPipeFramebufferTarget::Draw, 111); @@ -304,15 +318,19 @@ TEST(FramebufferEmit, ADrawBufferEntryOutsideTheRecordsOwnArrayIsRefusedRatherTh "colour attachment outside the record's own array", [&negative]() { MGPipeApplySetFramebufferState(negative); }); - EXPECT_EQ(MGPipeApplier().DrawFramebuffer.Width, 100u) << "a refused record was stored anyway"; + ASSERT_NE(MGPipeApplier().DrawFramebuffer(), nullptr); + EXPECT_EQ(MGPipeApplier().DrawFramebuffer()->Width, 100u) << "a refused record was stored anyway"; EXPECT_EQ(MGPipeApplier().FramebufferSerial, serialBefore); #endif } -// D-J4. The two framebuffer records are per-context WORKING state and a make-current takes -// them - but their serial ADVANCES rather than restarting, because a counter that walks back -// through values it has already stamped into a twin that outlived the switch is not a -// generation at all. Restoring `= 0` anywhere in the reset leaves this red. +// D-J4, as ID-19(b) leaves it. The two framebuffer BINDINGS are per-context working state and a +// make-current takes them - so both accessors answer null afterwards, exactly as the zeroed +// records used to answer a null Fbo - while the per-object RECORD survives, because a +// framebuffer that is only ever addressed BY NAME has no re-emission trigger at all. The serial +// ADVANCES rather than restarting, because a counter that walks back through values it has +// already stamped into a twin that outlived the switch is not a generation at all. Restoring +// `= 0` anywhere in the reset, or clearing the table there, leaves this red. TEST(FramebufferEmit, AMakeCurrentClearsBothRecordsAndAdvancesTheSerialRatherThanZeroingIt) { #if !MOBILEGL_PIPE_PUSH GTEST_SKIP() << "MOBILEGL_PIPE_PUSH is off: there is no applier in this build"; @@ -320,13 +338,18 @@ TEST(FramebufferEmit, AMakeCurrentClearsBothRecordsAndAdvancesTheSerialRatherTha ApplierGuard guard; MGPipeApplySetFramebufferState(FramebufferRecord(MGPipeHandle{4, 1}, MGPipeFramebufferTarget::Both, 100)); const Uint64 serialBefore = MGPipeApplier().FramebufferSerial; - ASSERT_EQ(MGPipeApplier().DrawFramebuffer.Width, 100u); + ASSERT_NE(MGPipeApplier().DrawFramebuffer(), nullptr); + ASSERT_EQ(MGPipeApplier().DrawFramebuffer()->Width, 100u); MGPipeApplierReset(); // the make-current - EXPECT_EQ(MGPipeApplier().DrawFramebuffer.Fbo, kMGPipeNullHandle); - EXPECT_EQ(MGPipeApplier().ReadFramebuffer.Fbo, kMGPipeNullHandle); - EXPECT_EQ(MGPipeApplier().DrawFramebuffer.Width, 0u); + EXPECT_EQ(MGPipeApplier().DrawFramebuffer(), nullptr); + EXPECT_EQ(MGPipeApplier().ReadFramebuffer(), nullptr); + EXPECT_EQ(MGPipeApplier().BoundFramebuffer[0], kMGPipeNullHandle); + EXPECT_EQ(MGPipeApplier().BoundFramebuffer[1], kMGPipeNullHandle); + ASSERT_NE(MGPipeApplier().FramebufferRecordFor(MGPipeHandle{4, 1}), nullptr) + << "the per-object record is not working state and a make-current may not take it"; + EXPECT_EQ(MGPipeApplier().FramebufferRecordFor(MGPipeHandle{4, 1})->Width, 100u); EXPECT_GT(MGPipeApplier().FramebufferSerial, serialBefore) << "the serial was carried over or restarted; the cleared window is itself a change the " "twin has to hear about, and no stamped value may ever recur"; @@ -369,7 +392,8 @@ TEST(FramebufferEmit, AReleaseOfTheObjectRecordsAlsoClearsTheWorkingHandlesThatC image.InternalFormat = 0x8058u; // GL_RGBA8 MGPipeApplySetShaderImages(MGPShaderImages{2, 1, 0xCCCCu}, &image); - ASSERT_EQ(MGPipeApplier().DrawFramebuffer.Color[0].Res, (MGPipeHandle{9, 1})); + ASSERT_NE(MGPipeApplier().DrawFramebuffer(), nullptr); + ASSERT_EQ(MGPipeApplier().DrawFramebuffer()->Color[0].Res, (MGPipeHandle{9, 1})); ASSERT_EQ(MGPipeApplier().SamplerViewCount, 1u); ASSERT_EQ(MGPipeApplier().BoundSamplerViews[2].View, (MGPipeHandle{3, 1})); ASSERT_EQ(MGPipeApplier().SamplerStateCount, 1u); @@ -379,10 +403,12 @@ TEST(FramebufferEmit, AReleaseOfTheObjectRecordsAlsoClearsTheWorkingHandlesThatC MGPipeApplierReleaseObjectRecords(); - EXPECT_EQ(MGPipeApplier().DrawFramebuffer.Fbo, kMGPipeNullHandle); - EXPECT_EQ(MGPipeApplier().ReadFramebuffer.Fbo, kMGPipeNullHandle); - EXPECT_EQ(MGPipeApplier().DrawFramebuffer.Color[0].Res, kMGPipeNullHandle) - << "a surface handle into an emptied texture table survived the teardown"; + EXPECT_EQ(MGPipeApplier().DrawFramebuffer(), nullptr); + EXPECT_EQ(MGPipeApplier().ReadFramebuffer(), nullptr); + EXPECT_EQ(MGPipeApplier().FramebufferRecordFor(MGPipeHandle{4, 1}), nullptr) + << "a framebuffer record holding eleven MGPSurface::Res into the emptied texture and " + "renderbuffer tables survived the teardown"; + EXPECT_TRUE(MGPipeApplier().FramebufferRecords.empty()); EXPECT_EQ(MGPipeApplier().SamplerViewStart, 0u); EXPECT_EQ(MGPipeApplier().SamplerViewCount, 0u); EXPECT_EQ(MGPipeApplier().BoundSamplerViews[2].View, kMGPipeNullHandle);