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

[Fix] (Pipe): re-arm the residual block on any render-state move and republish every vertex-attribute default on a fresh context

Browse Source 
- the residual value block armed on NEW_PIPELINE_STATE, so glEnable(GL_CLIP_DISTANCE0)
  never re-armed it: SetCapability's ClipDistance0..7 arms are deliberately not
  BumpVersions() and those eight are 8 of the 35 CapabilityInputs the block carries, so
  the D9/G10 trip wire was disarmed for them for an unbounded window - and invisibly, a
  block that is never emitted cannot diverge. It now arms on either render-state counter,
  the same answer DirtySurface.def derives for SetCapability, and the comment that
  asserted the opposite ("every SET_CAPABILITY arm calls BumpVersions") is corrected.
- the arming moved outside the residual subsystem gate: whether the capability set may
  have moved is a fact about the frontend, not about which subsystems this build pushes.
- set_vertex_attrib_defaults published NOTHING across a context change. Tracker::Reset()
  sets the staging mirror to the GL defaults and a fresh GLContext holds the same, so the
  per-attribute diff was empty on the one walk that must publish a COMPLETE state, while
  MGPipeApplierReset() leaves gPipeInputs.m_currentVertexAttribute holding the previous
  context's values - which cancelled, two lines later, the InvalidateAll() written for
  exactly that case. It now sends all 32 when the tracker is freshly primed, the arm
  EmitRenderState already had.
- the fresh-context reset of the CSO cache and the applier moved out of EmitRenderState,
  which runs only when bit 0 of MOBILEGL_PIPE_PUSH is set: the per-subsystem A/B D14
  invites gave a fresh context a never-reset applier while every suppressor slot was
  invalidated.
- MGPipeVertexAttribDefaultsLastHeader() is the observable for both properties of that
  call that cannot be read back without a poisoned read of m_currentVertexAttribute.
- the repair case now asserts the invariant (the call named exactly what moved, at most
  one repair) instead of repairs == before + 1, which pinned today's applier and would
  have gone red the day package A honours MGPAttribValue::ValueClass.
- a static_assert that no MGPipeDirty bit owns kMGPipeSubsystemResidualValues, which is
  what makes the residual block's direct subsystem test the one safe exception to
  MGPipeSubsystemForDirty, and a note that the NEW_PIPELINE_STATE/NEW_RENDER_STATE
  MOBILEGL_ASSERT is a debug/verify alarm over behaviour that is safe in every build.
This commit is contained in:
swung0x48
2026-09-07 23:18:09 -04:00
parent 067b186677
commit 8f66c374aa
3 changed files with 204 additions and 31 deletions
Download Patch File Download Diff File Expand all files Collapse all files
MobileGL/MG_Impl/Pipe/PipeFill.cpp
+97 -22
View File
@@ -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<MGPAttribValue, kAttribs> 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<SizeT>(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<MGPipeDirty>(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<Uint32>((Uint64{1} << kMGPipeDynamicChunkCount) - 1);
@@ -928,14 +984,6 @@ namespace MobileGL::MG_Pipe {
const auto pipelineVersion = static_cast<Uint16>(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
MobileGL/MG_Impl/Pipe/PipeFill.h
+9
View File
@@ -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
MobileGL/MG_Test/Pipe/TrackerTest.cpp
+98 -9
View File
@@ -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<Float, 4>{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<Uint16>(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<Uint16>(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<SizeT>(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<Float, 4>{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<GLContext>();
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<GLContext>();
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) {