From e2c90986488c7ef30f800163c95608e224c8e9e6 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Tue, 6 Oct 2026 03:56:47 -0400 Subject: [PATCH] [Refactor] (P13 W3c): delete ResidualValueBlock, op 46 becomes a retired declined row and the verify comparator owns the capability check --- .github/workflows/test.yml | 23 +++ MobileGL/MG_Impl/Pipe/PipeFill.cpp | 90 +----------- MobileGL/MG_Pipe/MGPipeTypes.h | 46 ++---- MobileGL/MG_Pipe/MGPipeValueTypes.h | 2 +- MobileGL/MG_Pipe/PipeApply.cpp | 55 ------- MobileGL/MG_Pipe/PipeApply.h | 11 -- MobileGL/MG_Pipe/PipeCalls.def | 5 +- MobileGL/MG_Pipe/PipeFields.def | 9 +- MobileGL/MG_Pipe/PipeRoute.cpp | 24 +--- MobileGL/MG_Pipe/PipeRoute.h | 7 - MobileGL/MG_Pipe/generated/PipeVerify.inc | 10 +- MobileGL/MG_Pipe/generated/PipeWire.inc | 8 -- MobileGL/MG_Pipe/generated/PipeWireLayout.inc | 2 - MobileGL/MG_Remote/Client/WireTables.cpp | 25 +--- MobileGL/MG_Remote/Wire/PipeWireCodec.cpp | 33 +---- MobileGL/MG_Test/Pipe/PipeCatalogueTest.cpp | 39 ++--- .../MG_Test/Pipe/RenderStateSpansTest.cpp | 134 +----------------- MobileGL/MG_Test/Pipe/TrackerTest.cpp | 16 +-- MobileGL/MG_Test/Wire/PeerLatchTest.cpp | 7 - MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp | 49 +++---- MobileGL/MG_Util/Metrics/PipeStats.h | 4 +- docs/ci/ci-cleanup-log.md | 1 + scripts/gen_pipe.py | 23 +-- 23 files changed, 112 insertions(+), 511 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index a1c958a9..c1b7c0e5 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -881,6 +881,29 @@ jobs: fi echo "the corrupted field turned ${matched} selected entries red, as it must" + # NEGATIVE CONTROL A2 (P13 W3c): THE CAPABILITY CROSS-CHECK MOVED HERE. ResidualValueBlock + # carried the 35 capability bits redundantly so the applier could trip on a capability no + # record carried; P13 deleted it, and the intent is the comparator's now. So a corrupted + # capability field must turn the same selection red, or the block's deletion lost its + # check rather than moving it. + - name: Negative control A2 (corrupted capability) + working-directory: build-verify + env: + MOBILEGL_ITEST_REQUIRE_GPU: "1" + MOBILEGL_PIPE_VERIFY_CORRUPT: IsCapabilityEnabled + run: | + FILTER='DirectGLES\.Verify\..*ClearThenReadPixels' + matched=$(ctest -N -L integration-verify -R "${FILTER}" | grep -cE '^ *Test *#[0-9]+:') + if [ "${matched}" -lt 1 ]; then + echo "::error::negative control A2 selected ${matched} tests; its filter no longer matches anything" + exit 1 + fi + if ctest --output-on-failure -L integration-verify -R "${FILTER}" --no-tests=error; then + echo "::error::MOBILEGL_PIPE_VERIFY_CORRUPT=IsCapabilityEnabled left ${matched} verify entries GREEN. The comparator does not compare the capability field, so deleting the residual block (P13 W3c) lost the check it carried." + exit 1 + fi + echo "the corrupted capability field turned ${matched} selected entries red, as it must" + # NEGATIVE CONTROL B (gate G5). The omission skips the STAMP of one field for one verb while # still copying its value - indistinguishable from a fill row nobody wrote - so the poison # must abort the glGenerateMipmap. Again: this step passes when ctest fails. diff --git a/MobileGL/MG_Impl/Pipe/PipeFill.cpp b/MobileGL/MG_Impl/Pipe/PipeFill.cpp index afc7f3eb..bb45694f 100644 --- a/MobileGL/MG_Impl/Pipe/PipeFill.cpp +++ b/MobileGL/MG_Impl/Pipe/PipeFill.cpp @@ -2798,7 +2798,7 @@ namespace MobileGL::MG_Pipe { // and the static_assert below is that fact's trip wire rather than a claim in prose. // // NOT THREAD-LOCAL, AND THAT IS THIS FILE'S EXISTING RULE RATHER THAN A NEW ONE: - // g_residualDue, g_omission and the verify latches beside it are file-scope too, and what + // g_omission and the verify latches beside it are file-scope too, and what // keeps a single writer on them is the verb barrier (ROADMAP.md's G1 row states it for // gPipeInputs itself). A validate that runs on the apply thread is inside that barrier by // construction. @@ -3186,67 +3186,6 @@ namespace MobileGL::MG_Pipe { #endif - // set_residual_value_state (P2 brief D9, ARCHITECTURE.md 9.4). - // - // Since P2 the block is one Uint64 of capability bits, and every one of the 35 is - // ALSO answerable from the assembled working block now that the contract closed the - // FramebufferSrgb / DepthClamp / TextureCubeMapSeamless storage holes. That - // redundancy is the whole point: the bits are read HERE from the frontend, and the - // applier compares them against the assembled answer, so the day a later call takes - // a capability over and forgets to carry it the block says so on the next draw - // (Fatal{PipeResidualDiverged, ""}). - // - // Building the carried bits from the ASSEMBLED block instead would make the trip - // wire a tautology, which is exactly the failure P1's entry compare had and P2 is - // paying to remove. - // - // ON THIS BRANCH IT IS STILL HALF A TAUTOLOGY, and saying so is part of the honesty - // the trip wire is for: the applier compares these bits against gPipeInputs' - // capability mirror, and while MGPipeDeriveRenderStateFields is a stub that mirror is - // filled by the residual fill from the SAME IsCapabilityEnabled accessor a few lines - // below. It becomes an independent oracle the moment package A's c1 lands and the - // fill stops copying those fields. What it proves already is that the block is - // emitted, sized and suppressed - the resid= byte class and the one divergence it - // caught during development (GL_DITHER) are that evidence. - // - // Emitted once per context and again whenever the capability set may have moved - // (D9). THE SHUTTER FOR THAT IS NEW_RENDER_STATE, NOT NEW_PIPELINE_STATE, and the - // difference is a hole rather than a nicety: SetCapability's ClipDistance0..7 arms - // are deliberately NOT BumpVersions() (RenderState.cpp says so in as many words), so - // glEnable(GL_CLIP_DISTANCE0) moves m_version alone - and ClipDistance0..7 are 8 of - // the 35 CapabilityInputs this block carries. Arming on the pipeline version would - // leave the trip wire disarmed for those eight for an unbounded window, which is the - // under-firing direction ARCHITECTURE.md 13.2 names as the dangerous one, and no gate - // could see it: a block that is never emitted cannot diverge. - // - // So the arming is the coarsest always-true shutter - either render-state counter - // moved - which is the same answer DirtySurface.def's derivation gives SetCapability. - // It over-fires (a glViewport re-sends 8 bytes and re-runs the compare) and that is - // the intended trade: over-firing costs one 35-bit loop on a verb that already moved - // render state, under-firing renders stale. - Uint64 EmitResidualValueState(GLContext& ctx) { - ResidualValueBlock block{}; - constexpr SizeT kCapabilityCount = static_cast(CapabilityInput::CapabilityInputCount); - static_assert(kCapabilityCount <= 64, "CapabilityBits is a Uint64"); - for (SizeT i = 0; i < kCapabilityCount; ++i) { - if (ctx.IsCapabilityEnabled(static_cast(i))) { - block.CapabilityBits |= Uint64{1} << i; - } - } - MGPipeRouteSetResidualValueState(block); - if (MG_Util::PipeStats::Enabled()) { - // ByteClass::ResidualValueBlock has been a placeholder that "stays at 0 - // until P2" since P0. This is what makes it non-zero. - MG_Util::PipeStats::AddBytes(MG_Util::PipeStats::ByteClass::ResidualValueBlock, - sizeof(ResidualValueBlock)); - } - return sizeof(MGPResidualValueState) + sizeof(ResidualValueBlock); - } - - // Set when the capability set may have moved, cleared when the block goes out. It is - // not part of the tracker because it is emission state, not a shutter: the shutter - // (the render-state counter) has already been consumed by the time this is read. - Bool g_residualDue = true; // The residual block is the ONE emission whose gate names a subsystem constant // directly instead of going through MGPipeSubsystemForDirty, and the reason is that @@ -3701,7 +3640,6 @@ namespace MobileGL::MG_Pipe { // would be suppressed as unchanged and the server would draw against a window it // had just been told to empty. MGPipeShaderBufferEmitterInstance().Reset(); - g_residualDue = true; } // P4a's segment, in ARCHITECTURE.md 5.4's RECOMMENDED order - framebuffer, then @@ -3869,31 +3807,6 @@ namespace MobileGL::MG_Pipe { if (!IsOmitted(verb, field)) filled.FilledGen[i] = filled.CurrentVerbSerial; #endif } - // ---- step 4b: the residual value block, and it goes out HERE ---- - // ARMED OUTSIDE THE SUBSYSTEM GATE: whether the capability set may have moved is a - // fact about the frontend, not about which subsystems this build pushes, and a - // per-subsystem A/B that turns the block off must not also lose the record that one - // is owed. - if ((dirty & (MGPipeDirtyBit(MGPipeDirty::NewRenderState) | - MGPipeDirtyBit(MGPipeDirty::NewPipelineState))) != 0) { - g_residualDue = true; - } - // Its trip wire compares the carried bits against the ASSEMBLED capability mirror, - // and that mirror is written either by the applier's derivation or by the fill loop - // above - so the block is only meaningful once step 4 has run. Emitting it with the - // other calls would compare against the previous verb's answer. - if ((pushMask & kMGPipeSubsystemResidualValues) != 0) { - // The trip wire compares against the ASSEMBLED capability mirror, so it can only - // run at a verb whose class actually carries that mirror - IsCapabilityEnabled is - // in seven of the nine class masks and kQuery and kXfbSpan do not read it, so at - // those verbs the mirror is whatever the last verb that did read it left behind. - // The change is HELD rather than dropped: dropping it would silently disarm the - // wire for a capability that moved between two queries. - if (g_residualDue && MGPipeFieldMaskHas(mask, MGPipeInputField::IsCapabilityEnabled)) { - payloadBytes += EmitResidualValueState(*ctx); - g_residualDue = false; - } - } if (payloadBytes != 0 && MG_Util::PipeStats::Enabled()) { // PipeStats::RecordDrawPayloadBytes has been implemented and unit-tested since // P0 and called by nothing; this is its first emitter, and the 24-bucket @@ -3945,7 +3858,6 @@ namespace MobileGL::MG_Pipe { MGPipeImageEmitterInstance().Reset(); MGPipeProgramEmitterInstance().Reset(); MGPipeShaderBufferEmitterInstance().Reset(); - g_residualDue = true; g_attribDefaultLastHeader = MGPVertexAttribDefaults{}; g_lastFillWasBarriered = true; } diff --git a/MobileGL/MG_Pipe/MGPipeTypes.h b/MobileGL/MG_Pipe/MGPipeTypes.h index 339e8d7b..ffbcd8fb 100644 --- a/MobileGL/MG_Pipe/MGPipeTypes.h +++ b/MobileGL/MG_Pipe/MGPipeTypes.h @@ -22,7 +22,7 @@ // these structs (generated/PipeWire.inc) are memcpy'd; a field silently changing width is a // protocol break that no test would otherwise see. // -// P0.5 DEBT, half repaid. The MG_State half is gone: ResidualValueBlock's +// P0.5 DEBT, half repaid. The MG_State half is gone: the (since deleted) ResidualValueBlock's // RenderStateParameters and PixelStoreParameters now come from MGPipeValueTypes.h, so // this header no longer reaches RenderState.h (its closure still touches TextureEnum.h, // through BackendObject.h, for the reason in the next sentence). What remains is MGPCaps embedding @@ -1236,42 +1236,14 @@ namespace MobileGL::MG_Pipe { }; MGP_ASSERT_POD(MGPPatchState, 40); - // Migration-only (section 6.3). Every stage removes fields and lowers - // MGL_RESIDUAL_BLOCK_SIZE; P13 asserts it is zero, which is the retirement trip wire. - // - // Layout must be asserted MEMBER BY MEMBER, not only by sizeof: a heterogeneous POD - // union is where padding differs across ABIs, and the monolith verify harness is blind - // to it because both sides are the same translation unit. G3 emits the offsetof - // assertions; under split the block is serialized field-wise rather than memcpy'd. - struct ResidualValueBlock { - // The 35 CapabilityInput bits, packed in enum order. P2 retired everything else: - // RenderStateParameters to create/bind_render_state + set_dynamic_state, Pack to - // set_pixel_pack_state, and the patch quintet to set_patch_state. - // - // What is left is deliberately REDUNDANT. Every one of the 35 capabilities is - // answerable from the assembled working block now that P2 gave FramebufferSrgb, - // DepthClamp and TextureCubeMapSeamless real storage - which is the point: the - // applier compares the two answers bit by bit, so the day a later call takes a - // capability over and forgets to carry it, the block says so on the next draw - // (Fatal{PipeResidualDiverged, ""}, MG_Pipe/PipeApply.cpp). - Uint64 CapabilityBits; - }; - static_assert(std::is_trivially_copyable_v); -// The retirement ratchet. This number only ever goes DOWN: every stage that lands a real -// set_* call deletes fields here and lowers it, and P13 replaces it with -// static_assert(sizeof(ResidualValueBlock) == 0), which stays red until the last field is -// gone. Shrinking the block without lowering the number, or growing it at all, is a build -// break - which is the point. -// -// Stable across the ABIs MobileGL ships on: the one member is a fixed-width scalar. -// -// P2: 1248 -> 8. RenderStateParameters (1168) retired to create/bind_render_state and -// set_dynamic_state, PixelStoreParameters (28) to set_pixel_pack_state, and the patch -// quintet (52 with its padding) to set_patch_state. -#define MGL_RESIDUAL_BLOCK_SIZE 8 - static_assert(sizeof(ResidualValueBlock) == MGL_RESIDUAL_BLOCK_SIZE, - "the residual value block changed size; lower MGL_RESIDUAL_BLOCK_SIZE if a field " - "retired, and do not raise it"); + // P13 W3c: ResidualValueBlock - the migration-only carrier of the 35 capability bits - is + // DELETED. Its last member was redundant by design (every capability is answerable from the + // assembled working block) and existed only so the applier could trip on a capability no + // record carried; that cross-check is the verify comparator's job now (integration-verify's + // corrupted-capability control). `static_assert(sizeof(ResidualValueBlock) == 0)` cannot be + // written in C++ (a complete type is at least one byte), so the exit gate is the type's + // absence. The record row below stays: opcodes only ever grow, no client emits op 46 and + // every decoder declines it. struct MGPResidualValueState { Uint32 Version; diff --git a/MobileGL/MG_Pipe/MGPipeValueTypes.h b/MobileGL/MG_Pipe/MGPipeValueTypes.h index 3b04eb83..19b2785e 100644 --- a/MobileGL/MG_Pipe/MGPipeValueTypes.h +++ b/MobileGL/MG_Pipe/MGPipeValueTypes.h @@ -660,7 +660,7 @@ namespace MobileGL { static_assert(std::is_trivially_copyable_v); static_assert(std::is_standard_layout_v); // offsetof legality static_assert(sizeof(RenderStateParameters) == 1168, - "RenderStateParameters changed size; MGL_RESIDUAL_BLOCK_SIZE and the Espryt spans depend on it"); + "RenderStateParameters changed size; the Espryt spans depend on it"); static_assert(offsetof(RenderStateParameters, BlendStates) < offsetof(RenderStateParameters, LogicOp)); static_assert(std::tuple_size_v == kMGMaxDrawBuffers); static_assert(std::is_trivially_copyable_v && sizeof(SamplerParameters) == 100); diff --git a/MobileGL/MG_Pipe/PipeApply.cpp b/MobileGL/MG_Pipe/PipeApply.cpp index a035003b..ebe76afd 100644 --- a/MobileGL/MG_Pipe/PipeApply.cpp +++ b/MobileGL/MG_Pipe/PipeApply.cpp @@ -585,8 +585,6 @@ namespace MobileGL::MG_Pipe { static_assert(sizeof(kCapabilityNames) / sizeof(kCapabilityNames[0]) == kCapabilityCount, "CapabilityInput gained a value; name it here or the residual trip wire " "cannot say which capability diverged"); - static_assert(kCapabilityCount <= 64, - "ResidualValueBlock::CapabilityBits is a Uint64; 35 bits fit, 65 would not"); // ============================================================================ // P14 S5: THE APPLIER REGISTRY. @@ -1793,11 +1791,7 @@ namespace MobileGL::MG_Pipe { #endif MGPipeApplier().RenderStateCsos.clear(); MGPipeApplier().BoundRenderStateCso = kMGPipeNullHandle; - MGPipeApplier().Residual = ResidualValueBlock{}; - MGPipeApplier().HasResidual = false; MGPipeApplier().ScatteredChunkBits = 0; - MGPipeApplier().ResidualCapabilitiesCompared = 0; - MGPipeApplier().ResidualDivergences = 0; MGPipeApplier().PatchCarrierComparisons = 0; MGPipeApplier().PatchCarrierDivergences = 0; // P3a. THIS RUNS AT EVERY CHANGE OF THE CURRENT CONTEXT, not once per fresh one: @@ -2234,55 +2228,6 @@ namespace MobileGL::MG_Pipe { } } - void MGPipeApplySetResidualValueState(const ResidualValueBlock& block) { - MGPipeApplier().Residual = block; - MGPipeApplier().HasResidual = true; - MGPipeApplier().ResidualCapabilitiesCompared = 0; - - // THE TRIP WIRE (ARCHITECTURE.md 9.4, P2 brief D9). CapabilityBits is redundant with - // the assembled working block by design: every one of the 35 capabilities is - // answerable from RenderStateParameters now that P2 closed the three storage holes. - // So the day a later call takes a capability over and forgets to carry it, the two - // answers part and this says so on the next draw - which is what a migration carrier - // is for. - // - // THE ORACLE IS THE WORKING BLOCK, AND THE WIRE IS ARMED PER CAPABILITY by the - // applier's own scatter ledger: capability i is compared only once every chunk its - // answer is read out of has been scattered by this applier. That is not a weakening, - // it is the wire's whole precondition: - // - // - with the render-state subsystem off (MOBILEGL_PIPE_PUSH=0x10 is a legal - // configuration - D14's per-subsystem A/B) the ledger is empty and the wire says - // nothing at all, which is right: the working block is then the per-verb fill - // loop's, published per verb CLASS, so at a kDispatch or kTextureOp verb it holds - // the previous draw's bytes and disagreeing with it means nothing; - // - with it on, the applier is the block's only writer and its bytes are current at - // every verb of every class - including the two above, which is the case a - // RenderStateSpansTest case drives on purpose. - // - // NOT PipeInputs::m_capability, which the earlier form compared against and which - // FillPoints.def does publish at seven classes rather than five: where that - // publication is what makes m_capability fresh, the fill loop filled it out of the - // same GLContext the client built CapabilityBits from, so the comparison is a - // tautology. The redundancy this wire exists to check is between the CARRIED bits and - // the ASSEMBLED block. - const RenderStateParameters& working = MGPipeApplyAccess::RenderState(gPipeInputs); - const Uint32 owned = MGPipeApplier().ScatteredChunkBits; - for (SizeT i = 0; i < kCapabilityCount; ++i) { - const CapabilityInput cap = static_cast(i); - const Uint32 sources = CapabilitySourceChunks(cap); - if ((owned & sources) != sources) continue; - ++MGPipeApplier().ResidualCapabilitiesCompared; - const Bool carried = ((block.CapabilityBits >> i) & 1ull) != 0; - const Bool assembledBit = MGPipeApplyAccess::DeriveCapability(working, cap); - if (carried == assembledBit) continue; - ++MGPipeApplier().ResidualDivergences; - MGP_TRIP_WIRE_REPORT("MGPipe: " MGP_TRIP_WIRE_TAG("PipeResidualDiverged, \"%s\"") - " carried=%d assembled=%d", - kCapabilityNames[i], static_cast(carried), - static_cast(assembledBit)); - } - } // ================================================================================ // P3a: the nine resource entry points (D-A1, D-A2). diff --git a/MobileGL/MG_Pipe/PipeApply.h b/MobileGL/MG_Pipe/PipeApply.h index 63c6dc54..cb8437e4 100644 --- a/MobileGL/MG_Pipe/PipeApply.h +++ b/MobileGL/MG_Pipe/PipeApply.h @@ -612,11 +612,6 @@ namespace MobileGL::MG_Pipe { Vector RenderStateCsos; // The last bind, so a rebind of the same handle can be answered without a scatter. MGPipeHandle BoundRenderStateCso = kMGPipeNullHandle; - // The residual block as last received. Compared against the assembled state on every - // set_residual_value_state; a disagreement is the D9 trip wire. - ResidualValueBlock Residual{}; - Bool HasResidual = false; - // The GLOBAL chunk bits (MGPipeRenderStateSpans.h's numbering) this applier has // itself scattered into the working block since the last reset - its own ledger of // which bytes of PipeInputs::m_renderState are the APPLIER'S rather than the per-verb @@ -641,8 +636,6 @@ namespace MobileGL::MG_Pipe { // go red for the reason it exists (ROADMAP.md), and only a poison or verify build // aborts: the shipped push build counts and logs, so these counters are how a unit // case sees the wire fire in EVERY build rather than in one. - Uint32 ResidualCapabilitiesCompared = 0; // of the 35, at the last set_residual_value_state - Uint32 ResidualDivergences = 0; // cumulative Uint32 PatchCarrierComparisons = 0; // cumulative, armed set_patch_state calls only Uint32 PatchCarrierDivergences = 0; // cumulative @@ -1282,10 +1275,6 @@ namespace MobileGL::MG_Pipe { // set_vertex_attrib_defaults: `tail` is hdr.Count MGPAttribValues for the attributes // named by hdr.Mask, in ascending location order. void MGPipeApplySetVertexAttribDefaults(const MGPVertexAttribDefaults& hdr, const MGPAttribValue* tail); - // set_residual_value_state: what has no call of its own. Since P2 that is one Uint64 of - // capability bits, and every one of them is ALSO answerable from the assembled working - // block - which is the point. A disagreement is Fatal{PipeResidualDiverged, ""}. - void MGPipeApplySetResidualValueState(const ResidualValueBlock& block); // --------------------------------------------------------------------------------- // P3a: the nine resource entry points (D-A1, D-A2) diff --git a/MobileGL/MG_Pipe/PipeCalls.def b/MobileGL/MG_Pipe/PipeCalls.def index 2b33c513..a070bc92 100644 --- a/MobileGL/MG_Pipe/PipeCalls.def +++ b/MobileGL/MG_Pipe/PipeCalls.def @@ -233,9 +233,8 @@ X(SetPatchState, MGPPatchState, kCtxState, kNone, kWaitNone) \ X(SetDrawProgram, MGPHandleOnly, kCtxState, kNone, kWaitNone) \ X(SetDispatchProgram, MGPHandleOnly, kCtxState, kNone, kWaitNone) \ - /* Migration-only carrier for Track V, retired field by field across P2..P13. Its */ \ - /* retirement is a compile error: MGL_RESIDUAL_BLOCK_SIZE only ever goes DOWN and the */ \ - /* final step asserts sizeof(ResidualValueBlock) == 0 (plan 6.3). */ \ + /* RETIRED (P13 W3c): the migration-only Track V carrier. ResidualValueBlock is deleted; */ \ + /* the row keeps its opcode (46), no client emits it and every decoder declines it. */ \ X(SetResidualValueState, MGPResidualValueState, kCtxState, kHasBlob, kWaitNone) \ /* ---- context: per-object state and transfer (plan 4.4.3 set_texture_params, 4.4.4) ---- */ \ X(SetTextureParams, MGPTextureParams, kCtxObject, kReplySlot, kWaitReply) \ diff --git a/MobileGL/MG_Pipe/PipeFields.def b/MobileGL/MG_Pipe/PipeFields.def index 731826c7..a86de744 100644 --- a/MobileGL/MG_Pipe/PipeFields.def +++ b/MobileGL/MG_Pipe/PipeFields.def @@ -173,11 +173,8 @@ #define MGP_FIELDS_MGPPatchState(F) \ F(Vertices) F(Outer) F(Inner) -// P2 ratcheted this block from six rows to one: RenderStateParameters retired to -// create/bind_render_state + set_dynamic_state, Pack to set_pixel_pack_state and the -// patch trio to set_patch_state. What is left is the redundant capability trip wire. -#define MGP_FIELDS_ResidualValueBlock(F) \ - F(CapabilityBits) +// P2 ratcheted ResidualValueBlock from six rows to one and P13 W3c deleted it; what stays is +// the retired op 46's payload, which the record catalogue still has to lay out. #define MGP_FIELDS_MGPResidualValueState(F) \ F(Version) F(Blob) @@ -422,7 +419,7 @@ P(MGPVertexBuffers) P(MGPIndexBuffer) P(MGPIndirectBuffers) P(MGPBoundView) P(MGPSamplerViews) \ P(MGPSamplerStates) P(MGPImageView) P(MGPShaderImages) P(MGPBufferRange) P(MGPShaderBuffers) \ P(MGPStreamOutputTargets) P(MGPGlobalConstants) P(MGPAttribValue) P(MGPVertexAttribDefaults) \ - P(MGPPixelPackState) P(MGPPatchState) P(ResidualValueBlock) P(MGPResidualValueState) \ + P(MGPPixelPackState) P(MGPPatchState) P(MGPResidualValueState) \ P(MGPSubRegion) P(MGPSubData) P(MGPSubDataComplete) P(MGPFlushRange) P(MGPReadback) \ P(MGPCopyRegion) P(MGPBlit) P(MGPClear) P(MGPMipPlan) P(MGPReadbackInfo) P(MGPReadbackToBuffer) \ P(MGPDrawInfo) \ diff --git a/MobileGL/MG_Pipe/PipeRoute.cpp b/MobileGL/MG_Pipe/PipeRoute.cpp index c69d1248..6cea73c5 100644 --- a/MobileGL/MG_Pipe/PipeRoute.cpp +++ b/MobileGL/MG_Pipe/PipeRoute.cpp @@ -247,24 +247,12 @@ namespace MobileGL::MG_Pipe { static_cast(blobBytes)); } - // set_residual_value_state: CONTRACT-P5 table 1 row 6, "the hardest row in the table". - // The applier takes `const ResidualValueBlock&` - a frontend type - and - // MGPResidualValueState is never instantiated on the live path. So the BLOCK IS THE - // BLOB, in both arms, and the adapter copies it back out. Requiring exact equality - // rather than ">=" is the decoder's rule too (PipeWireCodec.cpp): a size that only - // ever ratchets down makes a short read silently lose CapabilityBits. - void Mono_SetResidualValueState(const MGPResidualValueState*, const void* blobBytes, - Uint64 blobByteCount) { - if (blobByteCount != sizeof(ResidualValueBlock) || blobBytes == nullptr) { - MGLOG_F("MGPipe: Fatal{ResidualBlockSize} - set_residual_value_state carries %llu " - "bytes, the block is %llu", - static_cast(blobByteCount), - static_cast(sizeof(ResidualValueBlock))); - std::abort(); - } - ResidualValueBlock block{}; - std::memcpy(&block, blobBytes, sizeof(block)); - MGPipeApplySetResidualValueState(block); + // set_residual_value_state: RETIRED (P13 W3c). Nothing emits it any more; the row stays + // installed so the table is total, and reaching it is a named death. + void Mono_SetResidualValueState(const MGPResidualValueState*, const void*, Uint64) { + MGLOG_F("MGPipe: Fatal{ResidualBlockSize} - set_residual_value_state reached the monolith " + "route, but %s", "the residual value block was deleted in P13 W3c (op 46 is a retired row; the verify comparator owns the capability cross-check)"); + std::abort(); } // resource_readback carries kReplySlot but its answer is COMPLETION, not a value: the diff --git a/MobileGL/MG_Pipe/PipeRoute.h b/MobileGL/MG_Pipe/PipeRoute.h index f766038b..c3071b2e 100644 --- a/MobileGL/MG_Pipe/PipeRoute.h +++ b/MobileGL/MG_Pipe/PipeRoute.h @@ -448,13 +448,6 @@ namespace MobileGL::MG_Pipe { const MGPAttribValue* tail) { MGP_SetVertexAttribDefaults(&hdr, tail, hdr.Count); } - // The block IS the blob (table 1 row 6) and its size is a header constant both sides - // read, so no call site has to know it. - inline void MGPipeRouteSetResidualValueState(const ResidualValueBlock& block) { - MGPResidualValueState record{}; - record.Version = 0; - MGP_SetResidualValueState(&record, &block, sizeof(ResidualValueBlock)); - } // ---- vertex input ------------------------------------------------------------------ inline void MGPipeRouteCreateVertexElements(const MGPVertexElements& desc, diff --git a/MobileGL/MG_Pipe/generated/PipeVerify.inc b/MobileGL/MG_Pipe/generated/PipeVerify.inc index e386af22..76e560b8 100644 --- a/MobileGL/MG_Pipe/generated/PipeVerify.inc +++ b/MobileGL/MG_Pipe/generated/PipeVerify.inc @@ -83,7 +83,6 @@ inline Bool MGPipeVerify(const MGPAttribValue& a, const MGPAttribValue& b, const inline Bool MGPipeVerify(const MGPVertexAttribDefaults& a, const MGPVertexAttribDefaults& b, const char** outField); inline Bool MGPipeVerify(const MGPPixelPackState& a, const MGPPixelPackState& b, const char** outField); inline Bool MGPipeVerify(const MGPPatchState& a, const MGPPatchState& b, const char** outField); -inline Bool MGPipeVerify(const ResidualValueBlock& a, const ResidualValueBlock& b, const char** outField); inline Bool MGPipeVerify(const MGPResidualValueState& a, const MGPResidualValueState& b, const char** outField); inline Bool MGPipeVerify(const MGPSubRegion& a, const MGPSubRegion& b, const char** outField); inline Bool MGPipeVerify(const MGPSubData& a, const MGPSubData& b, const char** outField); @@ -209,8 +208,6 @@ struct MGPipeHasFieldVerifier : std::true_type {}; template <> struct MGPipeHasFieldVerifier : std::true_type {}; template <> -struct MGPipeHasFieldVerifier : std::true_type {}; -template <> struct MGPipeHasFieldVerifier : std::true_type {}; template <> struct MGPipeHasFieldVerifier : std::true_type {}; @@ -541,11 +538,6 @@ inline Bool MGPipeVerify(const MGPPatchState& a, const MGPPatchState& b, const c return true; } -inline Bool MGPipeVerify(const ResidualValueBlock& a, const ResidualValueBlock& b, const char** outField) { - MGP_FIELDS_ResidualValueBlock(MGP_VERIFY_FIELD) - return true; -} - inline Bool MGPipeVerify(const MGPResidualValueState& a, const MGPResidualValueState& b, const char** outField) { MGP_FIELDS_MGPResidualValueState(MGP_VERIFY_FIELD) return true; @@ -773,4 +765,4 @@ inline Bool MGPipeVerify(const MGPVertexBindingPointWire& a, const MGPVertexBind #undef MGP_VERIFY_FIELD -inline constexpr SizeT kMGPipeVerifiedPayloadCount = 85; +inline constexpr SizeT kMGPipeVerifiedPayloadCount = 84; diff --git a/MobileGL/MG_Pipe/generated/PipeWire.inc b/MobileGL/MG_Pipe/generated/PipeWire.inc index ddf80dcf..2836d756 100644 --- a/MobileGL/MG_Pipe/generated/PipeWire.inc +++ b/MobileGL/MG_Pipe/generated/PipeWire.inc @@ -1540,11 +1540,3 @@ inline Bool MGPipeApplyWireRecord(MGPWireOp op, const void* record, Uint64 size, } #undef MGP_WIRE_CHECK_BOUNDS - -// The ResidualValueBlock layout, from PipeFields.def's -// MGP_FIELDS_ResidualValueBlock. Retiring a field without lowering -// MGL_RESIDUAL_BLOCK_SIZE is a build break, which is the point. -static_assert(offsetof(ResidualValueBlock, CapabilityBits) == 0, - "the residual block's first member must sit at offset 0"); -static_assert(sizeof(ResidualValueBlock) == MGL_RESIDUAL_BLOCK_SIZE, - "the residual ratchet only ever goes down"); diff --git a/MobileGL/MG_Pipe/generated/PipeWireLayout.inc b/MobileGL/MG_Pipe/generated/PipeWireLayout.inc index 1e80dfff..27e4cd9b 100644 --- a/MobileGL/MG_Pipe/generated/PipeWireLayout.inc +++ b/MobileGL/MG_Pipe/generated/PipeWireLayout.inc @@ -237,8 +237,6 @@ inline constexpr WireLayoutMember kMGPipeWireLayoutMembers[] = { {"MGPPatchState.Vertices", offsetof(MGPPatchState, Vertices), sizeof(((MGPPatchState*)nullptr)->Vertices)}, {"MGPPatchState.Outer", offsetof(MGPPatchState, Outer), sizeof(((MGPPatchState*)nullptr)->Outer)}, {"MGPPatchState.Inner", offsetof(MGPPatchState, Inner), sizeof(((MGPPatchState*)nullptr)->Inner)}, - {"ResidualValueBlock", 0, sizeof(ResidualValueBlock)}, - {"ResidualValueBlock.CapabilityBits", offsetof(ResidualValueBlock, CapabilityBits), sizeof(((ResidualValueBlock*)nullptr)->CapabilityBits)}, {"MGPResidualValueState", 0, sizeof(MGPResidualValueState)}, {"MGPResidualValueState.Version", offsetof(MGPResidualValueState, Version), sizeof(((MGPResidualValueState*)nullptr)->Version)}, {"MGPResidualValueState.Blob", offsetof(MGPResidualValueState, Blob), sizeof(((MGPResidualValueState*)nullptr)->Blob)}, diff --git a/MobileGL/MG_Remote/Client/WireTables.cpp b/MobileGL/MG_Remote/Client/WireTables.cpp index fc46c9b5..fd6e9fc4 100644 --- a/MobileGL/MG_Remote/Client/WireTables.cpp +++ b/MobileGL/MG_Remote/Client/WireTables.cpp @@ -288,25 +288,12 @@ namespace MobileGL::MG_Remote::Client { ++g_emitted; } - // set_residual_value_state. CONTRACT-P5 table 1 row 6: the applier takes a frontend - // `ResidualValueBlock&` and `MGPResidualValueState` is never instantiated on the live - // path, so the encoder invents BOTH the record fill and the blob fill. The block IS - // the blob, whole - the decoder requires exactly sizeof(ResidualValueBlock) and says - // why ("a size that only ever ratchets down makes a short read silently lose - // CapabilityBits"), so the two sides state the same number from the same header. - void Wire_SetResidualValueState(const MG_Pipe::MGPResidualValueState* payload, - const void* blobBytes, Uint64 blobByteCount) { - if (RunsAsTheServerRole()) { - MG_Pipe::MGPipeMonolithContext().SetResidualValueState(payload, blobBytes, - blobByteCount); - return; - } - ClientSession& session = RequireSession("SetResidualValueState"); - MG_Pipe::MGPResidualValueState record = *payload; - record.Blob = StageRequired(session, "SetResidualValueState", blobBytes, blobByteCount); - session.EmitAndWait(MGPWireOp::SetResidualValueState, &record, sizeof(record), nullptr, - 0, nullptr, 0, nullptr); - ++g_emitted; + // set_residual_value_state: RETIRED (P13 W3c). Nothing calls it; the row stays installed so + // the table is total, and reaching it is a named death rather than a record on the wire. + void Wire_SetResidualValueState(const MG_Pipe::MGPResidualValueState*, const void*, Uint64) { + MGLOG_F("MGPipe: Fatal{ResidualBlockSize} - set_residual_value_state reached the wire table, " + "but %s", "the residual value block was deleted in P13 W3c (op 46 is a retired row; the verify comparator owns the capability cross-check)"); + std::abort(); } // resource_readback. kReplySlot, and the answer is COMPLETION only: the bytes travel diff --git a/MobileGL/MG_Remote/Wire/PipeWireCodec.cpp b/MobileGL/MG_Remote/Wire/PipeWireCodec.cpp index 53d16800..a1ba7d09 100644 --- a/MobileGL/MG_Remote/Wire/PipeWireCodec.cpp +++ b/MobileGL/MG_Remote/Wire/PipeWireCodec.cpp @@ -312,9 +312,6 @@ namespace MobileGL::MG_Remote::Wire { // corrupt Count can never wrap the arithmetic that is supposed to catch it. constexpr Uint64 kMaxRecordBytesOnTheWire = 0xFFFFFFFFull; - // MGPResidualValueState's blob is the block itself, which only ever ratchets DOWN. - static_assert(sizeof(ResidualValueBlock) == MGL_RESIDUAL_BLOCK_SIZE); - // CreateShaderState's seven blob members are one contiguous run, which is what lets // the honesty pass walk them as an array. static_assert(offsetof(MGPProgramDesc, Reflection) == @@ -2265,31 +2262,11 @@ namespace MobileGL::MG_Remote::Wire { MGPipeApplySetDispatchProgram(*static_cast(payload)); return true; - case MGPWireOp::SetResidualValueState: { - // THE HARDEST ROW IN TABLE 1, and it is hard for a reason that does not show in - // the payload: MGPipeApplySetResidualValueState takes `const ResidualValueBlock&` - // - not a payload, not a const void* - and MGPResidualValueState is NEVER - // INSTANTIATED on the live path (PipeFill.cpp:2184 passes the block straight to - // the applier). So the encoder had to invent both the record fill and the blob - // fill, and this is the first code in the tree that reads either. - // - // The block is the blob, whole, and its size only ever ratchets DOWN - // (MGL_RESIDUAL_BLOCK_SIZE, 1248 -> 8 at P2, 0 at P13). Requiring exact equality - // rather than ">=" is what makes a client built against an older block a loud - // mismatch instead of a silently short read of CapabilityBits. - const auto& rec = *static_cast(payload); - if (!RequireDeclaredBlob(op, rec.Blob, *m_segments)) return false; - if (rec.Blob.Size != sizeof(ResidualValueBlock)) { - return WireProtocolLatchAt("SetResidualValueState.Blob", rec.Blob.Size, - sizeof(ResidualValueBlock)); - } - const void* bytes = ResolveOrFatal(op, rec.Blob); - if (bytes == nullptr) return false; - ResidualValueBlock block{}; - std::memcpy(&block, bytes, sizeof(block)); - MGPipeApplySetResidualValueState(block); - return true; - } + case MGPWireOp::SetResidualValueState: + // RETIRED (P13 W3c), on op 50's precedent: ResidualValueBlock is deleted, no client + // emits this row, and a record that claims it is declined after the bounds gate and + // the blob honesty pass above have run. The row stays because opcodes only grow. + return false; case MGPWireOp::SetTextureParams: noteAcceptance(MGPipeApplySetTextureParams(*static_cast(payload))); diff --git a/MobileGL/MG_Test/Pipe/PipeCatalogueTest.cpp b/MobileGL/MG_Test/Pipe/PipeCatalogueTest.cpp index 01760637..c9171116 100644 --- a/MobileGL/MG_Test/Pipe/PipeCatalogueTest.cpp +++ b/MobileGL/MG_Test/Pipe/PipeCatalogueTest.cpp @@ -294,18 +294,14 @@ TEST(PipeCatalogue, ExactlyTheRoutedRowsAreInstalledAndTheRestAreStillNull) { EXPECT_EQ(gMGPipeContext.SetProgramBindings, nullptr); } -// The retirement ratchet of the migration carrier (section 6.3): the constant and the -// struct must agree, and the constant only ever goes down. +// The retirement ratchet of the migration carrier (section 6.3) reached its end in P13 W3c: +// ResidualValueBlock is deleted (a sizeof of zero cannot be written in C++, so the exit gate is +// the type's absence - nothing in the tree can name it any more). What is left to pin is the +// RETIRED ROW: op 46 keeps its number, because opcodes only ever grow, and its record keeps its +// layout so a decoder can bounds-check what it then declines. TEST(PipeCatalogue, ResidualBlockSizeIsPinned) { - static_assert(sizeof(ResidualValueBlock) == MGL_RESIDUAL_BLOCK_SIZE); - EXPECT_EQ(sizeof(ResidualValueBlock), static_cast(MGL_RESIDUAL_BLOCK_SIZE)); - // P2 ate 1240 of the 1248: RenderStateParameters retired to create/bind_render_state and - // set_dynamic_state, PixelStoreParameters to set_pixel_pack_state, the patch quintet to - // set_patch_state. What is left is one Uint64 of capability bits, and it is redundant on - // purpose - the applier's trip wire compares it against the assembled block. - EXPECT_EQ(sizeof(ResidualValueBlock), 8u); - EXPECT_LT(sizeof(ResidualValueBlock), sizeof(RenderStateParameters)); - EXPECT_EQ(offsetof(ResidualValueBlock, CapabilityBits), 0u); + EXPECT_EQ(static_cast(MGPWireOp::SetResidualValueState), 46u); + EXPECT_EQ(sizeof(MGPResidualValueState), 32u); } // P0.5 moved the value structs into MG_Pipe/MGPipeValueTypes.h. These are the runtime twins @@ -338,8 +334,8 @@ TEST(PipeCatalogue, ValueTypeLayoutsArePinned) { // word, at offset 0, and the members it used to carry are gone rather than merely moved - // which is exactly what "MGL_RESIDUAL_BLOCK_SIZE only ever goes down" has to mean. TEST(PipeCatalogue, ResidualBlockIsExactlyItsTwoValueStructsPlusPatchTail) { - EXPECT_EQ(offsetof(ResidualValueBlock, CapabilityBits), 0u); - EXPECT_EQ(sizeof(ResidualValueBlock), sizeof(Uint64)); + // P13 W3c deleted the last capability word, so the carrier is gone altogether; the members + // it used to carry are pinned on the carriers that took them over. // The three carriers that took the retired members over. EXPECT_EQ(sizeof(MGPPixelPackState), sizeof(PixelStoreParameters)); EXPECT_EQ(sizeof(MGPPatchState), 40u); @@ -993,17 +989,8 @@ TEST(PipeCatalogue, FloatVectorsCompareBitwise) { EXPECT_TRUE(MGPipeFieldEqual(1.5f, 1.5f)); EXPECT_FALSE(MGPipeFieldEqual(-0.f, 0.f)); - // The residual carrier is one field since P2, so the nested-struct case it used to - // demonstrate is demonstrated on RenderStateParameters directly - which is where it - // actually matters now that the block travels as create/bind_render_state chunks. - ResidualValueBlock left{}; - ResidualValueBlock right{}; - const char* field = nullptr; - EXPECT_TRUE(MGPipeVerify(left, right, &field)); - right.CapabilityBits = 1ull << static_cast(CapabilityInput::FramebufferSrgb); - EXPECT_FALSE(MGPipeVerify(left, right, &field)); - EXPECT_STREQ(field, "CapabilityBits"); - + // The nested-struct case is demonstrated on RenderStateParameters directly - which is where + // it actually matters now that the block travels as create/bind_render_state chunks. RenderStateParameters leftState{}; RenderStateParameters rightState{}; const char* inner = nullptr; @@ -1054,8 +1041,8 @@ TEST(PipeCatalogue, SixValueStructsHaveFieldLists) { // appended bind_context's MGPBindContext (docs/Disaggregated/design/11-state-ownership.md): // 84. A payload without a list here is one the comparator cannot see, so the row is not // optional bookkeeping - gen_pipe.py refuses the build without it. - // + shared_image's MGPSharedImageOp: 85. - EXPECT_EQ(kMGPipeVerifiedPayloadCount, 85u); + // + shared_image's MGPSharedImageOp: 85. P13 W3c deleted ResidualValueBlock and its list: 84. + EXPECT_EQ(kMGPipeVerifiedPayloadCount, 84u); static_assert(MGPipeHasFieldVerifier::value); static_assert(MGPipeHasFieldVerifier::value); static_assert(MGPipeHasFieldVerifier::value); diff --git a/MobileGL/MG_Test/Pipe/RenderStateSpansTest.cpp b/MobileGL/MG_Test/Pipe/RenderStateSpansTest.cpp index 886efafb..414b640a 100644 --- a/MobileGL/MG_Test/Pipe/RenderStateSpansTest.cpp +++ b/MobileGL/MG_Test/Pipe/RenderStateSpansTest.cpp @@ -908,8 +908,8 @@ namespace { } // ===================================================================================== - // 6. THE APPLIER'S OWN CASES: the two redundancy trip wires, and the entry points - // nothing else drives. + // 6. THE APPLIER'S OWN CASES: the patch-carrier trip wire (the residual one retired with + // its block in P13 W3c), and the entry points nothing else drives. // // ROADMAP.md: every gate must be able to go red for the reason it exists. Both wires // are therefore driven in three states - DISARMED (the applier has not scattered the @@ -968,20 +968,6 @@ namespace { MGPipeApplySetDynamicState(dyn, dynamicBytes.data()); } - // The residual block the tracker would emit for this context: the 35 capability answers - // the FRONTEND gives, packed in enum order. The wire's job is to disagree with the - // assembled block when the two have parted, so the carried side has to come from the - // frontend and not from the block. - ResidualValueBlock CarriedBitsOf(GLContext& ctx) { - ResidualValueBlock block{}; - for (SizeT i = 0; i < kCapCount; ++i) { - if (ctx.IsCapabilityEnabled(static_cast(i))) { - block.CapabilityBits |= Uint64{1} << i; - } - } - return block; - } - MGPPatchState PatchStateOf(Uint32 vertices, const FloatVec4& outer, const FloatVec2& inner) { MGPPatchState patch{}; patch.Vertices = vertices; @@ -1038,120 +1024,10 @@ namespace { } #endif // MGTEST_HAVE_FORK - // ------------------------------------------------------------------------------------- - // 6a. The residual trip wire is SILENT until the applier owns the bytes it would compare. + // 6a-6c (the residual capability trip wire) were retired with the residual block in P13 W3c: + // the capability cross-check is the verify comparator's now (integration-verify's + // corrupted-capability control). // - // This is the shape MOBILEGL_PIPE_PUSH=0x10 has - the residual subsystem on and the - // render-state subsystem off, which D14 makes a legal per-subsystem A/B. The working - // block is then the per-verb fill loop's, published per verb CLASS, and disagreeing - // with it means nothing. An earlier form of this wire aborted here. - // ------------------------------------------------------------------------------------- - TEST(RenderStateSpans, ResidualTripWireIsSilentUntilTheApplierOwnsTheBytes) { - ApplierContextGuard guard; - GLContext& ctx = *MG_State::pGLContext; - Uint32 nextSlot = kMGPipeFirstAllocatableSlot; - - // Nothing scattered: a block that disagrees on EVERY capability must pass in silence. - ResidualValueBlock everything{}; - everything.CapabilityBits = ~Uint64{0}; - MGPipeApplySetResidualValueState(everything); - EXPECT_EQ(MGPipeApplier().ScatteredChunkBits, 0u); - EXPECT_EQ(MGPipeApplier().ResidualCapabilitiesCompared, 0u); - EXPECT_EQ(MGPipeApplier().ResidualDivergences, 0u); - - // A bind owns the PIPELINE half, which answers 27 of the 35; the eight ClipDistances - // are answered from a dynamic chunk and stay unarmed. That grain is why the arming is - // per capability and not per applier. - ApplyWholeBlockFromContext(ctx, nextSlot, /*withDynamic=*/false); - MGPipeApplySetResidualValueState(CarriedBitsOf(ctx)); - const Uint32 armedByThePipelineHalf = MGPipeApplier().ResidualCapabilitiesCompared; - // SOME, and not all. The exact number is 27 under the shipped table, but it is a - // consequence of the table rather than of the wire, so the case asserts the property - // and lets the two directions below say which capabilities are on which side - a - // hard-coded 27 would turn any legitimate boundary move into a failure here as well - // as in the two cases that exist to catch it. - EXPECT_GT(armedByThePipelineHalf, 0u); - EXPECT_LT(armedByThePipelineHalf, static_cast(kCapCount)) - << "a bind alone cannot answer a capability whose mask is in the dynamic half"; - EXPECT_EQ(MGPipeApplier().ResidualDivergences, 0u); - - // ... and a block that disagrees on a CLIP DISTANCE still says nothing, because the - // chunk its answer is read out of has not been scattered. - ResidualValueBlock clipOnly = CarriedBitsOf(ctx); - clipOnly.CapabilityBits ^= Uint64{1} << static_cast(CapabilityInput::ClipDistance3); - MGPipeApplySetResidualValueState(clipOnly); - EXPECT_EQ(MGPipeApplier().ResidualDivergences, 0u); - - // The dynamic half arms the remaining eight. - ApplyWholeBlockFromContext(ctx, nextSlot, /*withDynamic=*/true); - MGPipeApplySetResidualValueState(CarriedBitsOf(ctx)); - EXPECT_EQ(MGPipeApplier().ResidualCapabilitiesCompared, static_cast(kCapCount)); - EXPECT_EQ(MGPipeApplier().ResidualDivergences, 0u); - } - - // ------------------------------------------------------------------------------------- - // 6b. The armed wire HOLDS at a verb class that does not publish the working block. - // - // A draw, a capability change, then a DISPATCH. kDispatch publishes - // IsCapabilityEnabled and NOT GetRenderStateParameters (MG_Pipe/FillPoints.def), so - // PipeInputs::m_capability is refilled from the live context here and the working - // block is not - which is exactly why the block has to be the APPLIER'S to be an - // oracle. It is: the applier scattered it, and the emission for this verb keeps it - // current. This is the case that pins the class contract shut. - // ------------------------------------------------------------------------------------- - TEST(RenderStateSpans, ResidualTripWireHoldsAcrossAVerbClassThatDoesNotPublishTheBlock) { - ApplierContextGuard guard; - GLContext& ctx = *MG_State::pGLContext; - Uint32 nextSlot = kMGPipeFirstAllocatableSlot; - { - MG_Test::ScopedPipeVerb draw(MGPipeVerb::DrawArrays); - ApplyWholeBlockFromContext(ctx, nextSlot, /*withDynamic=*/true); - MGPipeApplySetResidualValueState(CarriedBitsOf(ctx)); - } - EXPECT_EQ(MGPipeApplier().ResidualDivergences, 0u); - - ctx.SetCapability(CapabilityInput::Dither, !ctx.IsCapabilityEnabled(CapabilityInput::Dither)); - { - MG_Test::ScopedPipeVerb dispatch(MGPipeVerb::DispatchCompute); - ApplyWholeBlockFromContext(ctx, nextSlot, /*withDynamic=*/true); - MGPipeApplySetResidualValueState(CarriedBitsOf(ctx)); - } - EXPECT_EQ(MGPipeApplier().ResidualCapabilitiesCompared, static_cast(kCapCount)); - EXPECT_EQ(MGPipeApplier().ResidualDivergences, 0u); - } - - // ------------------------------------------------------------------------------------- - // 6c. The armed wire FIRES, naming the capability. The red half of 6a/6b. - // ------------------------------------------------------------------------------------- - TEST(RenderStateSpans, ResidualTripWireFiresNamingTheCapability) { - ApplierContextGuard guard; - GLContext& ctx = *MG_State::pGLContext; - Uint32 nextSlot = kMGPipeFirstAllocatableSlot; - ApplyWholeBlockFromContext(ctx, nextSlot, /*withDynamic=*/true); - - ResidualValueBlock diverging = CarriedBitsOf(ctx); - diverging.CapabilityBits ^= Uint64{1} << static_cast(CapabilityInput::Dither); - -#if MOBILEGL_PIPE_POISON || MOBILEGL_PIPE_VERIFY -#if MGTEST_HAVE_FORK - const ChildResult child = RunInChild([&diverging]() { MGPipeApplySetResidualValueState(diverging); }); - EXPECT_TRUE(DiedOfAbort(child)) << DescribeStatus(child) << "; log: " << child.Log; - EXPECT_NE(child.Log.find("Fatal{PipeResidualDiverged, \"Dither\"}"), std::string::npos) - << "the wire fired without naming the capability; log: " << child.Log; -#else - GTEST_SKIP() << "no fork on this platform; the wire's verdict here is std::abort()"; -#endif -#else - // A shipped push build counts and logs rather than aborting, so the wire is asserted - // in the form this build gives it. - const std::string before = ReadLog(); - MGPipeApplySetResidualValueState(diverging); - EXPECT_EQ(MGPipeApplier().ResidualDivergences, 1u); - EXPECT_NE(ReadLog().substr(before.size()).find("PipeResidualDiverged, \"Dither\""), std::string::npos) - << "the wire counted a divergence without logging which capability"; -#endif - } - // ------------------------------------------------------------------------------------- // 6d. The patch-carrier trip wire, in all three states - including a NaN outer level, // which is a legal glPatchParameterfv value that must compare EQUAL to itself and diff --git a/MobileGL/MG_Test/Pipe/TrackerTest.cpp b/MobileGL/MG_Test/Pipe/TrackerTest.cpp index 71a9fcec..7e414d46 100644 --- a/MobileGL/MG_Test/Pipe/TrackerTest.cpp +++ b/MobileGL/MG_Test/Pipe/TrackerTest.cpp @@ -27,6 +27,7 @@ #include #include #include +#include #include #include #include @@ -1161,12 +1162,12 @@ namespace { // that reads the PIPELINE version - which is what this emitter used - never re-arms for // those eight, and nothing can see it downstream: a block that is not emitted cannot // diverge, so the trip wire is simply disarmed. + // P13 W3c: the residual block is gone, so the case now asserts what the re-arm stood in for - + // that the capability itself reaches the applier's assembled block on the next draw. TEST_F(TrackerShippedEmitter, AClipDistanceEnableReArmsTheResidualBlock) { Draw(); - ASSERT_TRUE(MGPipeApplier().HasResidual) << "the priming draw sent no residual block"; - // Poison the server's copy so a re-emission is the only thing that can restore it. - MGPipeApplier().Residual = ResidualValueBlock{}; - MGPipeApplier().HasResidual = false; + ASSERT_FALSE(MobileGL::MG_Pipe::gPipeInputs.IsCapabilityEnabled(CapabilityInput::ClipDistance0)) + << "the premise is gone: a fresh context already has clip distance 0 enabled"; Ctx().SetCapability(CapabilityInput::ClipDistance0, true); // The premise: this moved the render-state counter and NOT the pipeline one. @@ -1176,11 +1177,8 @@ namespace { << "the premise is gone: a clip-distance enable now moves the pipeline version"; EXPECT_EQ(static_cast(Ctx().GetPipelineStateVersion()), pipelineBefore); - ASSERT_TRUE(MGPipeApplier().HasResidual) - << "a capability change that moves only m_version never re-armed the residual block"; - const Uint64 bit = Uint64{1} << static_cast(CapabilityInput::ClipDistance0); - EXPECT_NE(MGPipeApplier().Residual.CapabilityBits & bit, 0ull) - << "the re-emitted block does not carry the capability that moved"; + EXPECT_TRUE(MobileGL::MG_Pipe::gPipeInputs.IsCapabilityEnabled(CapabilityInput::ClipDistance0)) + << "a capability change that moves only m_version never reached the applier's block"; } // MAJOR 3 of round 2's review, pinned. A fresh context resets the tracker's staging diff --git a/MobileGL/MG_Test/Wire/PeerLatchTest.cpp b/MobileGL/MG_Test/Wire/PeerLatchTest.cpp index 1f68df56..f73d37c8 100644 --- a/MobileGL/MG_Test/Wire/PeerLatchTest.cpp +++ b/MobileGL/MG_Test/Wire/PeerLatchTest.cpp @@ -1111,13 +1111,6 @@ namespace { entry.Name.Offset = staged.Offset; return Forge(c, r, P::MGPWireOp::SetProgramBindings, Bytes(p), {{}, {}, Bytes(entry)}); }}, - {"SetResidualValueStateBlobSize", "PipeWireCodec.cpp", "SetResidualValueState.Blob", - Outcome::Latched, "Fatal{ProtocolCorruption, \"SetResidualValueState.Blob\"} got=16 expected=8", false, - [](Client::ClientSession& c, PeerReport& r) { - P::MGPResidualValueState d{}; - d.Blob = Stage(c, 16); - return Forge(c, r, P::MGPWireOp::SetResidualValueState, Bytes(d)); - }}, {"ResourceSubDataLevelExtent", "PipeWireCodec.cpp", "ResourceSubData.LevelExtent", Outcome::Latched, "Fatal{ProtocolCorruption, \"ResourceSubData.LevelExtent\"}", false, [](Client::ClientSession& c, PeerReport& r) { diff --git a/MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp b/MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp index f894fe0f..4d8e3fd8 100644 --- a/MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp +++ b/MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp @@ -1754,26 +1754,24 @@ TEST_F(PipeWireCodecTest, CreateShaderStateCrossesAsOneArchiveAndSixUndeclaredRu } // ===================================================================================== -// SetResidualValueState - table 1's hardest row +// SetResidualValueState - RETIRED (P13 W3c) // ===================================================================================== TEST_F(PipeWireCodecTest, ResidualValueBlockCrossesAsItsOwnBlob) { - // The applier takes `const ResidualValueBlock&` and MGPResidualValueState is never - // instantiated on the live path, so this is the first code in the tree that fills either. + // P13 W3c deleted ResidualValueBlock and retired op 46 on op 50's precedent: the row keeps + // its number, a record that claims it still passes the bounds gate and the blob honesty + // pass, and the decoder then DECLINES it - no applier entry point exists to reach. Wire2 wire; - ResidualValueBlock block{}; - block.CapabilityBits = 0x0123456789ABCDEFull; - + const Uint64 eightBytes = 0x0123456789ABCDEFull; MGPResidualValueState record{}; record.Version = 3; - record.Blob = wire.Encoder().StageBytes(&block, sizeof(block)); - EXPECT_EQ(record.Blob.Size, static_cast(MGL_RESIDUAL_BLOCK_SIZE)); + record.Blob = wire.Encoder().StageBytes(&eightBytes, sizeof(eightBytes)); ASSERT_NE(wire.Encoder().EncodeRecord(MGPWireOp::SetResidualValueState, &record, sizeof(record)), kInvalidSeq); - bool applied = false; + bool applied = true; ASSERT_TRUE(wire.PumpOne(&applied)); - EXPECT_TRUE(applied); + EXPECT_FALSE(applied) << "a retired row must be declined, not applied"; } // ===================================================================================== @@ -2260,34 +2258,37 @@ TEST_F(PipeWireCodecTest, TheAuditFillOverwritesExactlyTheRunsTheRecordResolved) // so this case asserts the bytes, not the flag. Wire2 wire; wire.Decoder().SetAuditPoison(true); - ResidualValueBlock block{}; - block.CapabilityBits = 0x5555555555555555ull; - MGPResidualValueState record{}; - record.Blob = wire.Encoder().StageBytes(&block, sizeof(block)); - ASSERT_NE(wire.Encoder().EncodeRecord(MGPWireOp::SetResidualValueState, &record, sizeof(record)), + // P13 W3c: the vehicle used to be op 46's residual block; a live single-blob record now. + SamplerParameters params{}; + params.minLod = -1.5f; + MGPSamplerDesc record{}; + record.Cso = MakeHandle(91); + record.Parameters = wire.Encoder().StageBytes(¶ms, sizeof(params)); + ASSERT_NE(wire.Encoder().EncodeRecord(MGPWireOp::CreateSamplerState, &record, sizeof(record)), kInvalidSeq); bool applied = false; ASSERT_TRUE(wire.PumpOne(&applied)); EXPECT_TRUE(applied); - EXPECT_EQ(wire.Decoder().PoisonedStageBytes(), sizeof(ResidualValueBlock)); - const auto* staged = wire.StageBase() + record.Blob.Offset; - for (std::size_t i = 0; i < sizeof(ResidualValueBlock); ++i) { + EXPECT_EQ(wire.Decoder().PoisonedStageBytes(), sizeof(SamplerParameters)); + const auto* staged = wire.StageBase() + record.Parameters.Offset; + for (std::size_t i = 0; i < sizeof(SamplerParameters); ++i) { EXPECT_EQ(staged[i], 0xDD) << "byte " << i; } } TEST_F(PipeWireCodecTest, TheAuditFillIsOffByDefaultSoTheHotPathPaysNothing) { Wire2 wire; - ResidualValueBlock block{}; - block.CapabilityBits = 0x77ull; - MGPResidualValueState record{}; - record.Blob = wire.Encoder().StageBytes(&block, sizeof(block)); - ASSERT_NE(wire.Encoder().EncodeRecord(MGPWireOp::SetResidualValueState, &record, sizeof(record)), + SamplerParameters params{}; + params.minLod = 0.75f; + MGPSamplerDesc record{}; + record.Cso = MakeHandle(92); + record.Parameters = wire.Encoder().StageBytes(¶ms, sizeof(params)); + ASSERT_NE(wire.Encoder().EncodeRecord(MGPWireOp::CreateSamplerState, &record, sizeof(record)), kInvalidSeq); bool applied = false; ASSERT_TRUE(wire.PumpOne(&applied)); EXPECT_EQ(wire.Decoder().PoisonedStageBytes(), 0u); - const auto* staged = wire.StageBase() + record.Blob.Offset; + const auto* staged = wire.StageBase() + record.Parameters.Offset; EXPECT_NE(staged[0], 0xDD); } diff --git a/MobileGL/MG_Util/Metrics/PipeStats.h b/MobileGL/MG_Util/Metrics/PipeStats.h index 65434f24..93d34af0 100644 --- a/MobileGL/MG_Util/Metrics/PipeStats.h +++ b/MobileGL/MG_Util/Metrics/PipeStats.h @@ -70,8 +70,8 @@ namespace MobileGL::MG_Util::PipeStats { StageIndirectCmd, // Bytes pushed because a persistently mapped range was published to the backend. PersistentMapPush, - // PLACEHOLDER (plan section 6.3): the residual value block does not exist yet. The - // class is minted now so the counter names never churn; it stays at 0 until P2. + // RETIRED (P13 W3c): the residual value block was deleted. The class keeps its slot and + // its "resid" short name so the counter names never churn; nothing adds to it. ResidualValueBlock, // P4a's, and THE PUSH GUARD IS NEW ON THIS ENUM: CallClass has had one since P2 and // ByteClass has never had one, so the block is opened here rather than the member diff --git a/docs/ci/ci-cleanup-log.md b/docs/ci/ci-cleanup-log.md index b780712e..6b0c2073 100644 --- a/docs/ci/ci-cleanup-log.md +++ b/docs/ci/ci-cleanup-log.md @@ -17,6 +17,7 @@ them wrong. | 2026-10-06 | `DirectGLES.ResourceSubsystemControl.Off.*`, `DirectGLES.ResourceSubsystemOff.*`, `DirectGLES.ObjectSubsystemControl.{Off,Refused,RefusedTexture}.*` | Test / integration-gpu | they cleared subsystem bits (masks 0x7f / 0x1ff / 0x9ff / 0x5ff) to run legacy arms; since W3b bits 0-13 are fixed on (ID-P13-3) | one named-refusal control per lane, `.RefusedAtStartup`: the same scenario and mask must go red with `Fatal{PipeSubsystemsFixedOn}` naming that mask (`Harness/RefusedMaskControl.cmake`); the `On` lanes now run 0x3fff | | 2026-10-06 | `SanityTest`: `DirectGLESSlotTable.{AnArmlessKnobCombinationStopsInsteadOfSkippingTheLane, TheArmlessCasesLeaveTheLogPathAndTheConfigAsTheyFoundThem, EglBringUpUnderTheArmlessKnobPairReturnsInsteadOfStopping}` | Test / unit | they pinned the `MOBILEGL_PIPE_LEGACY_MEMOS=0` + cleared-bit knob pair; the knob is gone and the mask is refused at startup | the RefusedAtStartup controls above | | 2026-10-06 | `SanityTest`: `DirectGLESTextureSync.{UnitMemoRefusesToDriveATwinFromAnotherTexture, AnAttachmentOnlyTexturesParametersReachTheDriverWithNoSamplerView}` | Test / unit | white-box probes that only ever ran at the P2 mask (0x7f): the borrowed-slot unit memo and the pre-handle texture-parameter push; both arms are gone with the legacy memos, and at the shipping mask the fixture cannot produce the applier records the handle arm reads | `DirectGLESTextureSync`'s handle-arm ABA sibling (P5e tx2) and `TextureParamsWithoutASamplerViewScenario` (G9, every lane) | +| 2026-10-06 | unit `RenderStateSpans.{ResidualTripWireIsSilentUntilTheApplierOwnsTheBytes, ResidualTripWireHoldsAcrossAVerbClassThatDoesNotPublishTheBlock, ResidualTripWireFiresNamingTheCapability}` and PeerLatchTest's `SetResidualValueStateBlobSize` row | Test / unit | P13 W3c deleted ResidualValueBlock and its applier trip wire (op 46 is a retired, declined row) | integration-verify's new `Negative control A2 (corrupted capability)`: the verify comparator must go red on a corrupted `IsCapabilityEnabled`; `PipeWireCodecTest.ResidualValueBlockCrossesAsItsOwnBlob` now pins that op 46 is declined; `TrackerTest...AClipDistanceEnableReArmsTheResidualBlock` now asserts the capability reaches the applier's block | ## Kept after investigation diff --git a/scripts/gen_pipe.py b/scripts/gen_pipe.py index e36b7f08..653f88d6 100644 --- a/scripts/gen_pipe.py +++ b/scripts/gen_pipe.py @@ -764,7 +764,7 @@ def gen_thunks(calls): return "\n".join(out) -def gen_wire(calls, residual_fields=None): +def gen_wire(calls): out = [banner("PipeWire.inc", "G3: wire records, size assertions and the applier's bounds gate.", "PipeCalls.def")] out.append("""// Every record is a fixed header plus its payload, padded to the stream's 8-byte @@ -1045,25 +1045,6 @@ inline Bool MGPipeApplyWireRecord(MGPWireOp op, const void* record, Uint64 size, } #undef MGP_WIRE_CHECK_BOUNDS""") - # The migration carrier's layout, asserted MEMBER BY MEMBER and not only by sizeof - # (plan 6.3): a heterogeneous POD is where padding differs across ABIs, and the monolith - # verify harness is blind to it because both sides are the same translation unit. The - # first member is pinned at 0 and the rest are pinned to ascend, which is the strongest - # statement a generator that cannot see the layout can make; sizeof plus - # MGL_RESIDUAL_BLOCK_SIZE pins the rest, and the ratchet only ever goes DOWN. - if residual_fields: - out.append("") - out.append("// The ResidualValueBlock layout, from PipeFields.def's") - out.append("// MGP_FIELDS_ResidualValueBlock. Retiring a field without lowering") - out.append("// MGL_RESIDUAL_BLOCK_SIZE is a build break, which is the point.") - out.append("static_assert(offsetof(ResidualValueBlock, %s) == 0," % residual_fields[0]) - out.append(" \"the residual block's first member must sit at offset 0\");") - for previous, member in zip(residual_fields, residual_fields[1:]): - out.append("static_assert(offsetof(ResidualValueBlock, %s) >" % member) - out.append(" offsetof(ResidualValueBlock, %s)," % previous) - out.append(" \"the residual block's members must stay in declaration order\");") - out.append("static_assert(sizeof(ResidualValueBlock) == MGL_RESIDUAL_BLOCK_SIZE,") - out.append(" \"the residual ratchet only ever goes down\");") return "\n".join(out) + "\n" @@ -1724,7 +1705,7 @@ def main(): write(os.path.join(GENERATED_DIR, "PipeTables.inc"), gen_tables(calls), args.check, changed) write(os.path.join(GENERATED_DIR, "PipeThunks.inc"), gen_thunks(calls), args.check, changed) write(os.path.join(GENERATED_DIR, "PipeWire.inc"), - gen_wire(calls, parse_field_lists().get("ResidualValueBlock")), args.check, changed) + gen_wire(calls), args.check, changed) write(os.path.join(GENERATED_DIR, "PipeWireLayout.inc"), gen_wire_layout(calls, payloads), args.check, changed) write(os.path.join(GENERATED_DIR, "PipeVerify.inc"), gen_verify(payloads), args.check, changed) write(os.path.join(GENERATED_DIR, "PipeFilled.inc"), gen_filled(accessors, calls, sticky, emitted),