[Fix] (Pipe): stop claiming set_pixel_pack_state supplies a field it only half writes, and fold the chunk boundaries into the subset hash's seed

- Coverage.def's emitted list named GetPixelStoreParameters, but the field is
  PipeInputs::m_pixelStore[2] - pack AND unpack - and set_pixel_pack_state carries the pack half
  only, deliberately and permanently. An emitted row is a licence for the residual fill loop to
  skip the field, so the moment the render-state bitmask has its bit set the unpack half would
  be written by nothing while its poison stamp said it was published, invisible to the poison
  and to the verify comparator alike. The row is gone and the reason is in the file; the pack
  half is simply written twice until the field is split.
- kMGPipeRenderStateChunkTableVersion was a promise nobody enforced: a boundary could move,
  the two byte-count assertions be updated, and every persisted key stay valid. The hash is now
  seeded with the version XOR a compile-time checksum of the boundary table, so a moved boundary
  invalidates the keys whether or not anyone remembered - and without a static_assert on the
  boundaries, which would turn G7's negative control into a build break instead of a red test.
This commit is contained in:
2026-09-06 09:51:14 -04:00
parent ce370a3e84
commit d1a7c5f159
4 changed files with 33 additions and 9 deletions
+10 -1
View File
@@ -141,6 +141,16 @@
// The one row whose call differs from the accessor list's is GetPrimitiveRestartIndex:
// coverage maps it onto draw_vbo because that is where a backend reads it, but the VALUE
// travels in dynamic chunk D6, so set_dynamic_state is what supplies it.
//
// GetPixelStoreParameters is DELIBERATELY ABSENT, and the reason is the shape of the field
// rather than of the call. The field is PipeInputs::m_pixelStore[2] - pack AND unpack - and
// set_pixel_pack_state carries the PACK half only, deliberately and permanently (D10,
// ARCHITECTURE.md 4.6 D5: nothing on the far side of the boundary reads unpack state). A row
// here says "this field is supplied, the fill loop may skip it", which would be a half-truth:
// the moment the render-state bitmask has its bit set, the unpack half would be written by
// nothing while its poison stamp said it was published, so neither the poison nor the verify
// comparator could see it. Until the field is split, the whole of it keeps going through the
// fill loop and the pack half is simply written twice.
#define MGP_COVERAGE_EMITTED_LIST(X) \
X(GetBlendColor, SetDynamicState) \
X(GetBlendEquationIndexed, CreateRenderState) \
@@ -162,7 +172,6 @@
X(GetPatchDefaultOuterLevel, SetPatchState) \
X(GetPatchVertices, SetPatchState) \
X(GetPipelineStateVersion, BindRenderState) \
X(GetPixelStoreParameters, SetPixelPackState) \
X(GetPolygonModeFront, CreateRenderState) \
X(GetPolygonOffsetFactor, SetDynamicState) \
X(GetPolygonOffsetUnits, SetDynamicState) \
+1 -1
View File
@@ -182,7 +182,7 @@ namespace MobileGL::MG_Pipe {
Uint64 MGPipeHashPipelineBytes(const void* bytes) {
return static_cast<Uint64>(
XXH64(bytes, kMGPipePipelineChunkBytes, kMGPipeRenderStateChunkTableVersion));
XXH64(bytes, kMGPipePipelineChunkBytes, kMGPipeRenderStateChunkTableSeed));
}
Uint64 MGPipeComputePipelineSubsetHash(const RenderStateParameters& params) {
+20 -3
View File
@@ -135,11 +135,28 @@ namespace MobileGL::MG_Pipe {
inline constexpr SizeT kMGPipePipelineChunkBytes = MGPipeRenderStateChunkDetail::BytesOfHalf(true);
inline constexpr SizeT kMGPipeDynamicChunkBytes = MGPipeRenderStateChunkDetail::BytesOfHalf(false);
// Seeds MGPipeComputePipelineSubsetHash, so a chunk-table change invalidates every
// persisted key rather than silently aliasing an old one. BUMP IT whenever a boundary,
// an ordering or the halves' membership moves.
// Bumped by hand when something about the table changes that its BYTES do not show -
// the halves' membership, the meaning of a chunk, the gather order.
inline constexpr Uint64 kMGPipeRenderStateChunkTableVersion = 1;
// What actually seeds MGPipeComputePipelineSubsetHash. The version above is a promise a
// reader has to keep; this is the part that keeps itself. Folding the boundary table into
// the seed means a moved boundary invalidates every persisted key whether or not anyone
// remembered to bump the version - and it does so WITHOUT a static_assert on the
// boundaries, which would turn G7's negative control (which moves a boundary on purpose
// and must still compile) into a build break.
namespace MGPipeRenderStateChunkDetail {
constexpr Uint64 BoundaryChecksum() {
Uint64 hash = 0xcbf29ce484222325ull; // FNV-1a, 64-bit
for (SizeT i = 0; i <= kMGPipeRenderStateChunkCount; ++i) {
hash = (hash ^ static_cast<Uint64>(kMGPipeRenderStateChunkBoundaries[i])) * 0x100000001b3ull;
}
return hash;
}
} // namespace MGPipeRenderStateChunkDetail
inline constexpr Uint64 kMGPipeRenderStateChunkTableSeed =
kMGPipeRenderStateChunkTableVersion ^ MGPipeRenderStateChunkDetail::BoundaryChecksum();
// ---- the trip wires. A mistake in the table is a build break, here. ----
static_assert(kMGPipeRenderStateChunkBoundaries[0] == 0,
"the chunk table must start at byte 0 of RenderStateParameters");
+2 -4
View File
@@ -309,7 +309,6 @@ enum class MGPipeFieldEmitter : Uint8 {
CreateRenderState,
SetDynamicState,
SetPatchState,
SetPixelPackState,
SetVertexAttribDefaults,
};
@@ -319,7 +318,6 @@ inline constexpr const char* kMGPipeFieldEmitterNames[] = {
"CreateRenderState",
"SetDynamicState",
"SetPatchState",
"SetPixelPackState",
"SetVertexAttribDefaults",
};
@@ -354,7 +352,7 @@ inline constexpr MGPipeFieldEmitter kMGPipeFieldEmittedBy[kMGPipeInputFieldCount
MGPipeFieldEmitter::SetPatchState, // GetPatchDefaultOuterLevel
MGPipeFieldEmitter::SetPatchState, // GetPatchVertices
MGPipeFieldEmitter::BindRenderState, // GetPipelineStateVersion
MGPipeFieldEmitter::SetPixelPackState, // GetPixelStoreParameters
MGPipeFieldEmitter::kNone, // GetPixelStoreParameters
MGPipeFieldEmitter::CreateRenderState, // GetPolygonModeFront
MGPipeFieldEmitter::SetDynamicState, // GetPolygonOffsetFactor
MGPipeFieldEmitter::SetDynamicState, // GetPolygonOffsetUnits
@@ -388,7 +386,7 @@ inline constexpr MGPipeFieldEmitter kMGPipeFieldEmittedBy[kMGPipeInputFieldCount
MGPipeFieldEmitter::kNone, // GetBoundTransformFeedbackLifetimeId
MGPipeFieldEmitter::kNone, // HasOpenTransformFeedbackSpan
};
inline constexpr SizeT kMGPipeEmittedFieldCount = 34;
inline constexpr SizeT kMGPipeEmittedFieldCount = 33;
struct MGPipeFilledState {
Uint64 CurrentVerbSerial;