diff --git a/MobileGL/MG_Impl/Pipe/PipeFill.cpp b/MobileGL/MG_Impl/Pipe/PipeFill.cpp index ad032e91..884cc69e 100644 --- a/MobileGL/MG_Impl/Pipe/PipeFill.cpp +++ b/MobileGL/MG_Impl/Pipe/PipeFill.cpp @@ -812,7 +812,14 @@ namespace MobileGL::MG_Pipe { // stops happening, with no edit here. Uint64 g_attribDefaultRepairs = 0; - Uint64 EmitVertexAttribDefaults(GLContext& ctx) { + // The header of the last set_vertex_attrib_defaults that actually went out. Count == 0 + // means none ever did, because a call that names no attribute is not emitted at all. + // It is the observable for the two things about this call that cannot be read back + // without a poisoned read of m_currentVertexAttribute: that a fresh context republishes + // the COMPLETE set, and that a single moved attribute publishes exactly that one. + MGPVertexAttribDefaults g_attribDefaultLastHeader{}; + + Uint64 EmitVertexAttribDefaults(GLContext& ctx, Bool freshlyPrimed) { MGPipeTracker& tracker = MGPipeTrackerInstance(); auto& staged = tracker.StagedAttribDefaults(); constexpr SizeT kAttribs = MG_State::GLState::VertexArrayObject::MAX_VERTEX_ATTRIBS; @@ -827,10 +834,25 @@ namespace MobileGL::MG_Pipe { return 0; } + // A FRESH CONTEXT PUBLISHES ALL 32, not the difference against a mirror that + // describes a context that is gone. Tracker::Reset() sets the staging mirror to + // AttribDefaults{}, whose NSDMIs are the GL defaults {0,0,0,1} - and a fresh + // GLContext's m_currentVertexAttributes hold exactly those, so the diff below is + // EMPTY on the one walk that must publish everything. The server's mirror is not + // default: MGPipeApplierReset() clears the CSO store and the residual block and + // leaves gPipeInputs.m_currentVertexAttribute holding the PREVIOUS context's + // defaults. So the InvalidateAll() a fresh context does to the set-hash + // suppressor would have been cancelled two lines later by this diff, and the one + // call P2 fully owns would publish nothing across a context change - exactly the + // "memo that serves a stale answer" the tracker's own COMPLETE-state rule + // (Tracker.h) exists to forbid. EmitRenderState has the same arm + // (freshlyPrimed ? kAllDynamicChunks) and the other two calls send whole values. Array tail{}; MGPVertexAttribDefaults header{}; for (SizeT i = 0; i < kAttribs; ++i) { - if (std::memcmp(&resolved[i], &staged[i], sizeof(resolved[i])) == 0) continue; + if (!freshlyPrimed && std::memcmp(&resolved[i], &staged[i], sizeof(resolved[i])) == 0) { + continue; + } // The class the frontend WROTE, and that class's own bytes. Not a literal 0 // and not ClassifyVertexAttribType's answer: that one is the SHADER's question // ("which view does this input consume"), asked at the backend read sites, and @@ -844,6 +866,7 @@ namespace MobileGL::MG_Pipe { staged[i] = resolved[i]; } if (header.Count == 0) return 0; + g_attribDefaultLastHeader = header; MGPipeApplySetVertexAttribDefaults(header, tail.data()); // Did the applier reproduce it? Byte for byte, over the attributes this call @@ -889,9 +912,21 @@ namespace MobileGL::MG_Pipe { // 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, - // which is whenever the pipeline version moved: every SET_CAPABILITY arm calls - // BumpVersions, so that shutter cannot miss one. + // 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); @@ -913,9 +948,30 @@ namespace MobileGL::MG_Pipe { // 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 pipeline version) has already been consumed by the time this is read. + // (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 + // it has no dirty bit: it carries what has no shutter of its own, which is what makes + // it the residue. That exception is safe only while no dirty bit claims the same + // subsystem - if one ever did, the block would be gated twice and that bit's own + // emission would silently inherit the residual A/B switch. Asserted rather than + // assumed, the same discipline SubsystemForEmitter's five static_asserts use. + constexpr Bool NoDirtyBitOwnsTheResidualSubsystem() { + for (SizeT i = 0; i < kMGPipeDirtyCount; ++i) { + if (MGPipeSubsystemForDirty(static_cast(i)) == + kMGPipeSubsystemResidualValues) { + return false; + } + } + return true; + } + static_assert(NoDirtyBitOwnsTheResidualSubsystem(), + "a MGPipeDirty bit now owns kMGPipeSubsystemResidualValues: route the " + "residual block's gate through MGPipeSubsystemForDirty like every other " + "emission, or the two gates will disagree"); + constexpr Uint32 kAllDynamicChunks = static_cast((Uint64{1} << kMGPipeDynamicChunkCount) - 1); @@ -928,14 +984,6 @@ namespace MobileGL::MG_Pipe { const auto pipelineVersion = static_cast(ctx.GetPipelineStateVersion()); Uint64 payloadBytes = 0; - if (freshlyPrimed) { - // A fresh context is a fresh server: the cache's handles name slots this - // client's allocator is about to hand out again, so both sides start over - // together rather than one of them remembering the other's objects. - MGPipeCsoCacheInstance().Reset(); - MGPipeApplierReset(); - } - if (dirty & MGPipeDirtyBit(MGPipeDirty::NewPipelineState)) { const MGPipeHandle cso = MGPipeCsoCacheInstance().Acquire(live, payloadBytes); MGPBindRenderState bind{}; @@ -978,6 +1026,13 @@ namespace MobileGL::MG_Pipe { // (RenderState.h), but that is an invariant of ANOTHER package's file. So it is // asserted here rather than assumed, and the assignment is narrowed to the one // bit that owns the mirror. + // + // MOBILEGL_ASSERT compiles out in Release/INFO, which is the G1/G3 + // configuration, so the assert itself is a debug/verify-only alarm. THE + // BEHAVIOUR IS SAFE IN EVERY BUILD REGARDLESS, and it is the narrowing below + // rather than the assert that makes it so: if the invariant ever broke in a + // shipping build the mirror would simply not advance, which costs a re-send of + // chunks the server already has and never claims it holds chunks it does not. MOBILEGL_ASSERT((dirty & MGPipeDirtyBit(MGPipeDirty::NewPipelineState)) == 0 || (dirty & MGPipeDirtyBit(MGPipeDirty::NewRenderState)) != 0, "NEW_PIPELINE_STATE fired without NEW_RENDER_STATE: RenderState's " @@ -990,6 +1045,7 @@ namespace MobileGL::MG_Pipe { } // namespace Uint64 MGPipeVertexAttribDefaultRepairCount() { return g_attribDefaultRepairs; } + MGPVertexAttribDefaults MGPipeVertexAttribDefaultsLastHeader() { return g_attribDefaultLastHeader; } // ---- the validate point (P2 brief D1) ---- void MGPipeValidateForVerb(MGPipeVerb verb) { @@ -1038,13 +1094,27 @@ namespace MobileGL::MG_Pipe { (dirty & MGPipeDirtyBit(bit)) != 0; }; Uint64 payloadBytes = 0; + + // A fresh context is a fresh server, and that is true of EVERY subsystem, so it is + // handled BEFORE the per-subsystem gates rather than inside one of them. It used to + // live inside EmitRenderState, which runs only when bit 0 of MOBILEGL_PIPE_PUSH is + // set - so the per-subsystem A/B D14 invites (clear bit 0, keep bits 1..3) gave a + // fresh context a never-reset applier while every other slot WAS invalidated. + // - the CSO cache's handles name slots this client's allocator is about to hand + // out again, so both sides start over together rather than one of them + // remembering the other's objects; + // - what the server has is no longer what any suppressor slot last emitted; + // - and the residual block owes a fresh publication whatever else moved. + if (tracker.FreshlyPrimed()) { + MGPipeCsoCacheInstance().Reset(); + MGPipeApplierReset(); + MGPipeSetHashSuppressorInstance().InvalidateAll(); + g_residualDue = true; + } + if (wants(MGPipeDirty::NewPipelineState) || wants(MGPipeDirty::NewRenderState)) { payloadBytes += EmitRenderState(*ctx, dirty, tracker.FreshlyPrimed()); } - if (tracker.FreshlyPrimed()) { - // A fresh context: what the server has is no longer what any slot last emitted. - MGPipeSetHashSuppressorInstance().InvalidateAll(); - } if (wants(MGPipeDirty::NewPixelPack)) { payloadBytes += EmitPixelPackState(*ctx); } @@ -1052,7 +1122,7 @@ namespace MobileGL::MG_Pipe { payloadBytes += EmitPatchState(*ctx); } if (wants(MGPipeDirty::NewVertexAttribDefaults)) { - payloadBytes += EmitVertexAttribDefaults(*ctx); + payloadBytes += EmitVertexAttribDefaults(*ctx, tracker.FreshlyPrimed()); } // ---- step 4: the residual fill, for what an emitted call did NOT supply ---- @@ -1088,14 +1158,19 @@ namespace MobileGL::MG_Pipe { #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) { - if (tracker.FreshlyPrimed() || (dirty & MGPipeDirtyBit(MGPipeDirty::NewPipelineState)) != 0) { - g_residualDue = true; - } // 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 diff --git a/MobileGL/MG_Impl/Pipe/PipeFill.h b/MobileGL/MG_Impl/Pipe/PipeFill.h index 9a8df9b1..f85a07b2 100644 --- a/MobileGL/MG_Impl/Pipe/PipeFill.h +++ b/MobileGL/MG_Impl/Pipe/PipeFill.h @@ -64,6 +64,15 @@ namespace MobileGL::MG_Pipe { // moved. Uint64 MGPipeVertexAttribDefaultRepairCount(); + // PipeFill.cpp. The header of the last set_vertex_attrib_defaults that actually went out + // - Mask, and Count == 0 for "none ever did", since a call naming no attribute is not + // emitted. Two properties of this call have no other observable, because reading + // m_currentVertexAttribute back at a verb whose class does not carry it is the poison + // violation the fill table exists to forbid: that a FRESH CONTEXT republishes all 32 + // (the server's mirror still holds the previous context's defaults), and that one moved + // attribute publishes exactly one. Eight bytes, written only when a call goes out. + MGPVertexAttribDefaults MGPipeVertexAttribDefaultsLastHeader(); + #if MOBILEGL_PIPE_VERIFY // PipeFill.cpp. The second arm of the comparator (P1 brief D8, ARCHITECTURE.md 13.2-2): // fills `snapshot` from the live GLContext the old way, for every field in `mask`. This diff --git a/MobileGL/MG_Test/Pipe/TrackerTest.cpp b/MobileGL/MG_Test/Pipe/TrackerTest.cpp index 5a3fa1b3..6cfd1453 100644 --- a/MobileGL/MG_Test/Pipe/TrackerTest.cpp +++ b/MobileGL/MG_Test/Pipe/TrackerTest.cpp @@ -79,7 +79,10 @@ namespace { X(TrackerShippedEmitter, ABlendToggleThroughTheValidatePointMintsTwoCsos) \ X(TrackerShippedEmitter, TheSteadyStateThroughTheValidatePointEmitsNothing) \ X(TrackerShippedEmitter, APushedAttributeDefaultTheApplierCannotReproduceIsRepaired) \ - X(TrackerShippedEmitter, AViewportThroughTheValidatePointMintsNoCso) + X(TrackerShippedEmitter, AViewportThroughTheValidatePointMintsNoCso) \ + X(TrackerShippedEmitter, AClipDistanceEnableReArmsTheResidualBlock) \ + X(TrackerShippedEmitter, AFreshContextRepublishesEveryVertexAttributeDefault) \ + X(TrackerShippedEmitter, AFreshContextResetsTheApplierWithTheRenderStateSubsystemOff) #define MGL_DECLARE_PULL_SKIP(Suite, Name) \ TEST(Suite, Name) { GTEST_SKIP() << "compiled only under MOBILEGL_PIPE_PUSH"; } @@ -597,12 +600,19 @@ namespace { EXPECT_EQ(Cso().Binds, 0u) << "a steady-state draw bound a render-state CSO"; } - // The window MAJOR-2's repair covers: a glVertexAttrib* write followed by a verb whose - // class does NOT read m_currentVertexAttribute. The call still goes out (the dirty bit - // and the subsystem bit are all step 3 looks at), the applier writes four words into all - // three views because it ignores ValueClass, and nothing in step 4 puts the value back - - // so the client checks and repairs. Reading the storage here to prove it would be the - // poison violation the fill table forbids, so the repair counter is the observable. + // The window the repair covers: a glVertexAttrib* write followed by a verb whose class + // does NOT read m_currentVertexAttribute. The call still goes out (the dirty bit and the + // subsystem bit are all step 3 looks at), and today's applier writes four words into all + // three views because it ignores ValueClass, so nothing in step 4 puts the converted + // value back and the client repairs the mirror itself. + // + // THE ASSERTION IS THE INVARIANT, NOT THE DEFECT. "repairs == before + 1" would pin + // today's applier and go red the day package A teaches + // MGPipeApplySetVertexAttribDefaults to switch on MGPAttribValue::ValueClass - which is + // the hand-off this package declares as blocking, and which is supposed to need no edit + // here. What must hold either way is that the call went out naming exactly the attribute + // that moved, and that the mirror ends up right by at most one repair: zero repairs once + // the applier reproduces the value, one until then. TEST_F(TrackerShippedEmitter, APushedAttributeDefaultTheApplierCannotReproduceIsRepaired) { Draw(); const Uint64 before = MGPipeVertexAttribDefaultRepairCount(); @@ -610,8 +620,87 @@ namespace { // cannot be the same four words whichever view the carrier picks. Ctx().SetCurrentVertexAttributeFloat(0, Array{1.5f, 2.5f, 3.5f, 4.5f}); MGPipeValidateForVerb(MGPipeVerb::GenerateMipmap); - EXPECT_EQ(MGPipeVertexAttribDefaultRepairCount(), before + 1) - << "the emitter accepted an applier write that cannot reproduce a converted value"; + const MGPVertexAttribDefaults header = MGPipeVertexAttribDefaultsLastHeader(); + ASSERT_EQ(header.Count, 1u) << "the moved attribute default did not go out at all"; + EXPECT_EQ(header.Mask, 1u) << "the call named an attribute that did not move"; + const Uint64 repairs = MGPipeVertexAttribDefaultRepairCount() - before; + EXPECT_LE(repairs, 1u) << "one call cannot need two repairs"; + // and the repair is not free-running: a second identical walk moves nothing, so it + // neither re-emits nor re-repairs. + MGPipeValidateForVerb(MGPipeVerb::GenerateMipmap); + EXPECT_EQ(MGPipeVertexAttribDefaultRepairCount() - before, repairs); + } + + // MAJOR 1 of round 2's review, pinned. glEnable(GL_CLIP_DISTANCE0) is one of the 35 + // capabilities the residual block carries AND one of the eight whose SetCapability arm + // deliberately does not BumpVersions(), so it moves m_version alone. An arming condition + // 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. + 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; + + Ctx().SetCapability(CapabilityInput::ClipDistance0, true); + // The premise: this moved the render-state counter and NOT the pipeline one. + const Uint16 pipelineBefore = static_cast(Ctx().GetPipelineStateVersion()); + Draw(); + ASSERT_EQ(MGPipeTrackerInstance().LastDirty() & MGPipeDirtyBit(MGPipeDirty::NewPipelineState), 0u) + << "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"; + } + + // MAJOR 3 of round 2's review, pinned. A fresh context resets the tracker's staging + // mirror to the GL defaults, which are exactly what a fresh GLContext holds - so the + // per-attribute diff is empty on the one walk that must publish everything, while the + // applier's mirror still holds the PREVIOUS context's defaults. + TEST_F(TrackerShippedEmitter, AFreshContextRepublishesEveryVertexAttributeDefault) { + // The fixture's context is itself fresh, so the priming draw is the first half of the + // same statement: a fresh context publishes the COMPLETE set, not a difference. + Draw(); + ASSERT_EQ(MGPipeVertexAttribDefaultsLastHeader().Count, 32u) + << "the first walk on a fresh context published an increment, not a complete state"; + Ctx().SetCurrentVertexAttributeFloat(3, Array{9.f, 8.f, 7.f, 6.f}); + Draw(); + ASSERT_EQ(MGPipeVertexAttribDefaultsLastHeader().Count, 1u) + << "a steady context published more than the one attribute that moved"; + + // A different context, whose 32 defaults are the value-initialised {0,0,0,1} the + // tracker's own reset produces - so a diff against the staging mirror finds nothing. + MG_State::pGLContext = MakeUnique(); + Draw(); + const MGPVertexAttribDefaults header = MGPipeVertexAttribDefaultsLastHeader(); + EXPECT_EQ(header.Count, 32u) + << "a fresh context published " << header.Count + << " attribute defaults; the server's mirror still holds the previous context's"; + EXPECT_EQ(header.Mask, 0xFFFFFFFFu); + } + + // Minor 4 of round 2's review. The fresh-context reset of the applier and the CSO cache + // used to sit inside EmitRenderState, i.e. behind bit 0 of MOBILEGL_PIPE_PUSH, so the + // per-subsystem A/B D14 invites gave a fresh context a never-reset applier holding the + // previous context's CSO records while every suppressor slot WAS invalidated. + TEST_F(TrackerShippedEmitter, AFreshContextResetsTheApplierWithTheRenderStateSubsystemOff) { + Draw(); + ASSERT_FALSE(MGPipeApplier().RenderStateCsos.empty()) + << "the priming draw created no CSO record to leak into the next context"; + + MG_Config::Features.PipePush = kMGPipeSubsystemsMigratedAtP2 & ~kMGPipeSubsystemRenderState; + MG_State::pGLContext = MakeUnique(); + Draw(); + EXPECT_TRUE(MGPipeApplier().RenderStateCsos.empty()) + << "a fresh context kept the previous context's CSO records because the reset was " + "behind the render-state subsystem bit"; + EXPECT_TRUE(MGPipeHandleIsNull(MGPipeApplier().BoundRenderStateCso)); } TEST_F(TrackerShippedEmitter, AViewportThroughTheValidatePointMintsNoCso) {