diff --git a/MobileGL/MG_Pipe/MGPipeTypes.h b/MobileGL/MG_Pipe/MGPipeTypes.h index dff45c81..fa98a504 100644 --- a/MobileGL/MG_Pipe/MGPipeTypes.h +++ b/MobileGL/MG_Pipe/MGPipeTypes.h @@ -298,9 +298,16 @@ namespace MobileGL::MG_Pipe { Uint8 FixedSampleLocations, Immutable; Uint32 Usage; // BufferUsage Uint32 StorageFlags; // glBufferStorage flags - Uint8 HasDefinedContent; // false after a NULL-data respecify + Uint8 HasDefinedContent; // false after a NULL-data respecify - STORAGE-DEFINING Uint8 ImageBindableHint; // client-side everImageBound; pre-emptive allocation Uint16 Pad0; + // ImageBindableHint and BindMask above are the two METADATA fields the rule exists + // for: a respecify that moves only them - every storage-defining field equal to the + // stored descriptor - is a metadata update, with no reallocation ack and no + // PendingUploads clear. The rule, the full storage-defining field set and the third + // field allowed to differ (GlNameForDiag) are stated beside + // MGPipeResourceRespecifyNeedsAck below (P4a, ID-18 M4), which is where the applier + // and both twins read them from. // Diagnostics only. A GL name is NEVER an identity, never a memo key and never part // of a content hash (section 4.2.1). Widened from the plan's two bytes, which // cannot hold one. @@ -560,19 +567,52 @@ namespace MobileGL::MG_Pipe { }; MGP_ASSERT_POD(MGPSurface, 24); - // P4a, D-C2/D-C3: the record is emitted PER BOUND TARGET. + // P4a, D-C2/D-C3 and ID-19: THE RECORD DESCRIBES A FRAMEBUFFER OBJECT, and Target says + // whether it ALSO moves a binding. // - // GL has two independent framebuffer bindings and this record carries one Fbo and one - // ReadSurface, so Target says which binding it describes: 0 = Draw, 1 = Read, 2 = Both - // (one object bound to both targets). The draw-buffer array is applied only for a record - // whose Target is not Read - Espryt's own comment records the Minecraft 26.x OIT bug - // where a READ-only sync landed glDrawBuffers on the wrong framebuffer - and ReadSurface - // is resolved from the READ framebuffer's own read buffer, which is what makes the - // read-buffer-shared-FBO defect class unrepresentable rather than merely fixed. + // A `Named` record describes the framebuffer object it names (Fbo) and changes NO + // binding. `Draw` / `Read` / `Both` records describe the same object AND set the bound + // handle(s) of the target(s) they name. + // + // The applier therefore keeps records PER FRAMEBUFFER OBJECT, keyed by the handle's slot + // (the generation is checked on lookup and a stale one refuses; a framebuffer has no wire + // lifetime - D-I2, the catalogue has no framebuffer delete - so a successor's record + // simply OVERWRITES the slot), plus the two bound handles. And every DSA entry point that + // hands a framebuffer to the server BY NAME - BlitNamedFramebuffer, the four + // ClearNamedFramebuffer*, and the DSA attachment / draw-buffer / read-buffer setters at + // their validate point - is PRECEDED BY A Named RECORD, so that any framebuffer the + // server is about to receive by name already has one. + // + // That last rule is the phase's main correction, not a nicety. With only the two + // bound-target records, glClearNamedFramebufferfv(fbo) on an unbound fbo made the backend + // mint a fresh driver framebuffer with NO ATTACHMENTS, find no record for it, decline, + // and then issue the clear against it anyway - GL_INVALID_FRAMEBUFFER_OPERATION and + // nothing cleared, where the legacy arm cleared correctly. Writing such an object into + // the bound-target record instead would have been worse: the applier would then claim it + // is bound. + // + // GL has two independent framebuffer bindings and one record carries one Fbo, so a + // Draw/Read pair is two records and one object bound to both targets is one record with + // Both. The draw-buffer array belongs to the OBJECT the record names; it reaches the + // driver's bound draw framebuffer only for a record whose Target is Draw or Both - + // Espryt's own comment records the Minecraft 26.x OIT bug where a READ-only sync landed + // glDrawBuffers on the wrong framebuffer - and a Named record's draw buffers are applied + // when that object is next configured, never to whatever happens to be bound. ReadSurface + // is resolved from THAT framebuffer's own read buffer in EVERY record, Named included, + // which is what makes the read-buffer-shared-FBO defect class unrepresentable rather than + // merely fixed. + // + // Target IS A ContentHash INPUT (the hash covers the whole record), and the emitter's + // suppressor must be keyed by the framebuffer the record names, not by one global slot: + // two different objects' Named records in a row must both go out, and a Named record must + // never be suppressed against the same object's bound record or the reverse. enum class MGPipeFramebufferTarget : Uint8 { Draw = 0, Read = 1, Both = 2, + // Describes the framebuffer named by Fbo and changes no binding (ID-19). Emitted + // ahead of every DSA entry point that hands that framebuffer over by name. + Named = 3, Count, }; @@ -589,14 +629,48 @@ namespace MobileGL::MG_Pipe { static_assert(kMGPipeMaxColorAttachments == MobileGL::kMGMaxDrawBuffers, "MGPFramebufferState::Color[] and DrawBuffers[] are one array width"); + // ONE RECORD DESCRIBES ONE FRAMEBUFFER OBJECT - the one named by Fbo - and Target says + // whether it also moves a binding (P4a, D-C2 as corrected by ID-19; see + // MGPipeFramebufferTarget above for the failure that forced it). + // + // A Named record describes that object and changes NO binding. Draw / Read / Both records + // describe that object AND set the bound handle(s) of the target(s) they name. The applier + // keeps these records PER FRAMEBUFFER OBJECT, keyed by Fbo's slot (generation checked on + // lookup; a framebuffer has no wire lifetime - D-I2 - so a successor's record simply + // overwrites the slot), plus the two bound handles; every DSA entry point that hands a + // framebuffer to the server by name is preceded by a Named record. + // + // WHAT Target CHANGES, FIELD BY FIELD. NO FIELD IN THIS RECORD REFERS TO "the currently + // bound framebuffer" - every one of them describes the object named by Fbo - and that is + // the invariant a reader depends on: + // + // Fbo, Color[], Depth, Stencil, ReadSurface, Width/Height/Layers/Samples, + // FixedSampleLocations, IsDefault, Complete + // properties of the object named by Fbo, identical in meaning under every Target. + // In particular ReadSurface is resolved from THAT framebuffer's own read buffer - + // on a Named record too - never from whichever framebuffer is bound to GL_READ. + // DrawBuffers[] + // a property of the named object; it reaches the driver's bound draw framebuffer + // only when Target is Draw or Both. Under Named it is stored with the object and + // applied when that object is next configured. + // Target + // the only binding-specific field: Draw/Read/Both name the binding(s) this record + // also sets, Named names none. It is a ContentHash input. + // ContentHash + // per RECORD, not per object, and the emitter's suppressor is keyed by the + // framebuffer named: a Named record must never be suppressed against the same + // object's bound record, nor one object's Named record against another's. struct MGPFramebufferState { MGPipeHandle Fbo; // kMGPipeDefaultFramebuffer for the default framebuffer MGPSurface Color[8]; MGPSurface Depth, Stencil; - // The RESOLVED read surface, not an index. This is what structurally closes the + // The RESOLVED read surface, not an index, and it is THIS framebuffer's own read + // buffer under every Target - Named included. This is what structurally closes the // read-buffer-shared-FBO defect class. MGPSurface ReadSurface; - Int8 DrawBuffers[8]; // attachment index, -1 = NONE + // attachment index, -1 = NONE. The named object's array; applied to the bound draw + // framebuffer only when Target is Draw or Both. + Int8 DrawBuffers[8]; Uint16 Width, Height, Layers, Samples; // Complete is FramebufferObject::CheckCompleteness(), the frontend-only answer - NOT // glCheckFramebufferStatus's. CheckFramebufferStatus_State additionally consults @@ -607,12 +681,15 @@ namespace MobileGL::MG_Pipe { // answer, and glCheckFramebufferStatus keeps answering from the frontend as it does // today. Uint8 FixedSampleLocations, IsDefault, Complete; - Uint8 Target; // MGPipeFramebufferTarget, above (P4a, D-C2; was Pad0) + // MGPipeFramebufferTarget, above (P4a, D-C2; was Pad0). Draw/Read/Both also set the + // named binding(s); Named sets none (ID-19). The ONLY binding-specific field. + Uint8 Target; Uint32 Pad1; // Two jobs (section 4.5.6): the server's render-pass memo key, and the CLIENT's // emission suppressor - an unchanged hash means this record is not sent at all. // The same pattern is mandatory for every kVarTail set_* below, or 26.2's // redundant glBindSampler traffic reappears as a variable-length record per batch. + // Target is one of its inputs, and the suppressor is keyed per framebuffer. Uint64 ContentHash; }; MGP_ASSERT_POD(MGPFramebufferState, 304); @@ -1008,6 +1085,46 @@ namespace MobileGL::MG_Pipe { return desc.Immutable != 0 && desc.Target == kMGPipeResourceTargetBuffer; } + // P4a, ID-18 M4: A RESPECIFY WHOSE STORAGE-DEFINING FIELDS ALL EQUAL THE STORED + // DESCRIPTOR IS A METADATA UPDATE, NOT A REALLOCATION. + // + // MGPResourceDesc::BindMask and ImageBindableHint are STICKY facts the client discovers + // AFTER allocation - a texture first bound as a shader image, first used as a render + // target - and they ride resource_create and every resource_respecify. An IMMUTABLE + // texture never has a later respecify, so without a rule those two would reach the server + // only by accident, or never; with one, a mask change after allocation emits a + // resource_respecify that REPEATS the storage the resource already has. + // + // The applier and both twins must classify such a record as a metadata update: + // - NO reallocation acknowledgement. MGPipeResourceRespecifyNeedsAck above still + // answers the per-record question, but a metadata update allocates nothing, so a + // record it classifies as metadata is not acked even when that predicate says the + // call may require one. + // - NO PendingUploads clear - not the whole vector, and not the redefined level either. + // This REFINES the level-scoped clear: identical storage fields clear NOTHING. (The + // level-scoped rule exists because clearing the whole vector on a level-1 definition + // silently dropped level 0's accepted texels; a metadata update must drop neither.) + // - The stored descriptor's BindMask and ImageBindableHint ARE updated - BindMask is + // sticky and therefore ORed, never replaced - and the twin re-derives its storage + // flags from the new mask on its next sync, recreating backend storage only where the + // backend actually needs it. The record itself is not a request to recreate. + // + // THE STORAGE-DEFINING FIELD SET, named here so that neither side has to guess and a + // later field cannot join it by silence. It is every MGPResourceDesc member except the + // three metadata ones and the padding: + // + // Target, StorageKind, InternalFormat, Width, Height, Depth, ArrayLayers, Levels, + // Samples, FixedSampleLocations, Immutable, Usage, StorageFlags, HasDefinedContent, + // ViewOf, BufferForTexBuffer, BufOffset, BufSize. + // + // `Resource` is the identity the stored descriptor is looked up BY, not a comparand. The + // three fields that may differ on a metadata update are exactly BindMask, + // ImageBindableHint and GlNameForDiag (diagnostics only, never an identity, never a memo + // key). HasDefinedContent is storage-defining ON PURPOSE: glBufferData(size, NULL) at an + // unchanged size is an orphaning reallocation and has to keep clearing, rather than being + // mistaken for a mask change. A field added to MGPResourceDesc must be placed in one of + // the two lists in the same commit - PipeCatalogue pins the struct's size for that. + // The forward terminator for a server-initiated texture pull (section 7.1). May carry // zero regions - that is how a pull that needs nothing is answered. struct MGPSubDataComplete { diff --git a/MobileGL/MG_Test/Pipe/PipeCatalogueTest.cpp b/MobileGL/MG_Test/Pipe/PipeCatalogueTest.cpp index cf61a744..db7fbe30 100644 --- a/MobileGL/MG_Test/Pipe/PipeCatalogueTest.cpp +++ b/MobileGL/MG_Test/Pipe/PipeCatalogueTest.cpp @@ -185,13 +185,28 @@ TEST(PipeCatalogue, TextureParamsNameTheirBuiltinSamplerAndFramebufferStateNames // different owners, one byte each. EXPECT_NE(offsetof(MGPTextureParams, ForceResync), offsetof(MGPTextureParams, SamplerResync)); - // Pad0 -> Uint8 Target, and the SIZE DID NOT MOVE, which is the whole point: the record is - // emitted once per bound target that moved, or once with Both, and that costs a byte the - // struct already had. + // Pad0 -> Uint8 Target, and the SIZE DID NOT MOVE, which is the whole point: the record + // describes one framebuffer OBJECT and Target says which binding(s), if any, it also + // sets, and that costs a byte the struct already had. Named (ID-19) cost nothing at all - + // it is a fourth value of a byte that was already there, which is why the applier could + // be given a per-object table without a wire change. EXPECT_EQ(sizeof(MGPFramebufferState), 304u); EXPECT_EQ(static_cast(MGPipeFramebufferTarget::Draw), 0u); EXPECT_EQ(static_cast(MGPipeFramebufferTarget::Read), 1u); EXPECT_EQ(static_cast(MGPipeFramebufferTarget::Both), 2u); + // Named = 3, and it is pinned by VALUE rather than merely by existence: the applier + // validates a record with `Target >= Count`, so an enumerator inserted ahead of Named + // would silently re-point every Named record the client already emits at Draw or Read - + // and a Draw record for a framebuffer that is not bound is the exact corruption Named + // exists to prevent (a DSA clear/blit landing on an unattached driver framebuffer). + EXPECT_EQ(static_cast(MGPipeFramebufferTarget::Named), 3u); + // Count is the applier's refusal bound and it is 4 now, not 3: a wire that still refused + // 3 would drop every DSA record on the floor. + EXPECT_EQ(static_cast(MGPipeFramebufferTarget::Count), 4u); + // The byte must be able to hold every value, since Target is a Uint8 in the record and + // the enum is the only thing that says what fits. + EXPECT_LE(static_cast(MGPipeFramebufferTarget::Count), 256u); + EXPECT_EQ(sizeof(MGPFramebufferState::Target), 1u); // The wire's colour-attachment width is ONE width, and it is the wire's rather than the // driver's: a driver reporting more attachments than this is refused at bring-up, never // truncated into the record. @@ -886,6 +901,22 @@ TEST(PipeCatalogue, ResourceRespecifyAcksOnlyImmutableStorage) { // And the opcode did not move: a flag-word edit is not a catalogue edit. EXPECT_EQ(static_cast(MGPWireOp::ResourceRespecify), 3); + + // P4a, ID-18 M4. The metadata-update rule is a PROSE contract stated beside the predicate + // above - it compares an incoming descriptor against the applier's stored one, which this + // header cannot do - so what is pinnable here is the thing that would make the prose lie: + // a field added to MGPResourceDesc and classified into neither list. The size is the + // tripwire, and the two metadata fields are named so the classification cannot be lost to + // a rename either. + EXPECT_EQ(sizeof(MGPResourceDesc), 88u); + EXPECT_EQ(sizeof(MGPResourceDesc::BindMask), 2u); + EXPECT_EQ(sizeof(MGPResourceDesc::ImageBindableHint), 1u); + // HasDefinedContent sits next to ImageBindableHint and is deliberately on the OTHER side + // of the line: glBufferData(size, NULL) at an unchanged size is an orphaning + // reallocation, so a record that moves only it must still clear, and must never be read + // as a mask change. + EXPECT_NE(offsetof(MGPResourceDesc, HasDefinedContent), + offsetof(MGPResourceDesc, ImageBindableHint)); } // G13b, D-M: "emulation 在 split 下显式 Fatal 直到 P8" costs P4a a NAMED, GREPPABLE call site