From 702f937ca99cfabc32851aa2a9d4cbd7d82093f4 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Mon, 12 Oct 2026 18:55:51 -0400 Subject: [PATCH] [Fix] (Pipe): avalanche the composite shutter mix so dirty bits stop colliding --- MobileGL/MG_Impl/Pipe/ResourceTracker.h | 11 ++++-- MobileGL/MG_Impl/Pipe/Tracker.h | 34 +++++++++++++------ MobileGL/MG_Test/Pipe/TrackerTest.cpp | 24 +++++++++++++ .../recorder/BufferVertexArrayDraw.txt | 6 ++-- 4 files changed, 60 insertions(+), 15 deletions(-) diff --git a/MobileGL/MG_Impl/Pipe/ResourceTracker.h b/MobileGL/MG_Impl/Pipe/ResourceTracker.h index daf3cfa1..bb2ff1d8 100644 --- a/MobileGL/MG_Impl/Pipe/ResourceTracker.h +++ b/MobileGL/MG_Impl/Pipe/ResourceTracker.h @@ -535,8 +535,15 @@ namespace MobileGL::MG_Pipe { // The same mix Tracker.h's composite shutters use. Spelled here rather than // included so this header does not depend on the tracker. static constexpr Uint64 MGPipeMixShutterValue(Uint64 accumulator, Uint64 value) { - accumulator ^= value + 0x9e3779b97f4a7c15ull + (accumulator << 6) + (accumulator >> 2); - return accumulator; + const auto avalanche = [](Uint64 v) { + v ^= v >> 30; + v *= 0xbf58476d1ce4e5b9ull; + v ^= v >> 27; + v *= 0x94d049bb133111ebull; + v ^= v >> 31; + return v; + }; + return avalanche(accumulator ^ avalanche(value + 0x9e3779b97f4a7c15ull)); } Vector m_bySlot; diff --git a/MobileGL/MG_Impl/Pipe/Tracker.h b/MobileGL/MG_Impl/Pipe/Tracker.h index e664198b..a68c2a9a 100644 --- a/MobileGL/MG_Impl/Pipe/Tracker.h +++ b/MobileGL/MG_Impl/Pipe/Tracker.h @@ -245,17 +245,31 @@ namespace MobileGL::MG_Pipe { // compares, neither of which can collide. // // P3a's three ARE composed, so the risk is now real rather than academic, and it is - // accepted with its size stated: each mix takes a 64-bit input into a 64-bit - // accumulator, so two DIFFERENT vertex configurations collide with probability ~2^-64 - // per pair, and the inputs are a monotone lifetime id, a monotone configuration version - // and a widened slot version - none of which an application can steer. The alternative, - // comparing the whole 32-attribute configuration byte for byte on every verb, is the - // per-draw cost the shutter exists to avoid. The narrowing that removes the composition - // for bit 10 - its own slot version plus the bound object's identity - is what this - // phase already did to the one shutter that was composed over an unrelated aggregate. + // accepted with its size stated: two DIFFERENT states collide with probability ~2^-64 per + // pair, and the inputs are monotone lifetime ids, versions and generations - none of which + // an application can steer. The alternative, comparing the whole 32-attribute configuration + // byte for byte on every verb, is the per-draw cost the shutter exists to avoid. The + // narrowing that removes the composition for bit 10 - its own slot version plus the bound + // object's identity - is what this phase already did to the one shutter that was composed + // over an unrelated aggregate. + // + // That bound needs a mix that avalanches. The hash_combine step this used to be + // (acc ^= value + k + (acc << 6) + (acc >> 2)) does not: small neighbouring counters collide + // outright - (1, 64) and (2, 1) mix to the same value - and bit 12 missed a fire on exactly + // such a pair, so a draw sampled the previous set's texture + // (KHR-GL46.internalformat.copy_tex_image.rgb10_a2ui after a run of earlier cases). Each + // step here is a bijection of either argument with the other fixed, so a change to one + // input alone always moves the result, and the 64-bit finalizer spreads a change to both. + inline constexpr Uint64 MGPipeShutterAvalanche(Uint64 value) { + value ^= value >> 30; + value *= 0xbf58476d1ce4e5b9ull; + value ^= value >> 27; + value *= 0x94d049bb133111ebull; + value ^= value >> 31; + return value; + } inline constexpr Uint64 MGPipeMixShutter(Uint64 accumulator, Uint64 value) { - accumulator ^= value + 0x9e3779b97f4a7c15ull + (accumulator << 6) + (accumulator >> 2); - return accumulator; + return MGPipeShutterAvalanche(accumulator ^ MGPipeShutterAvalanche(value + 0x9e3779b97f4a7c15ull)); } // A Uint16 counter widened at the TRACKER boundary, never in MG_State diff --git a/MobileGL/MG_Test/Pipe/TrackerTest.cpp b/MobileGL/MG_Test/Pipe/TrackerTest.cpp index 6bee0bfa..ae0ca7b9 100644 --- a/MobileGL/MG_Test/Pipe/TrackerTest.cpp +++ b/MobileGL/MG_Test/Pipe/TrackerTest.cpp @@ -33,6 +33,7 @@ #include #include +#include #include #include #include @@ -46,6 +47,29 @@ namespace { EXPECT_EQ(kMGPipeSubsystemsMigratedAtP2 & kMGPipeBehaviourNoCsoContentAddressing, 0ull); } + // A composite shutter is only as good as its mix: a collision is a missed fire. The pairs + // below collided under the hash_combine step the mix used to be - the second one on a + // phone, the third is bit 12's (bind generation, program units) pair that left a draw + // sampling the previous unit set's texture. + TEST(Tracker, ShutterMixSeparatesSmallCounterPairs) { + EXPECT_NE(MGPipeMixShutter(1, 64), MGPipeMixShutter(2, 1)); + EXPECT_NE(MGPipeMixShutter(174, 37), MGPipeMixShutter(48, 8026)); + const auto bit12 = [](Uint64 content, Uint64 params, Uint64 bind, Uint64 units) { + return MGPipeMixShutter(MGPipeMixShutter(MGPipeMixShutter(content, params), bind), units); + }; + Vector seen; + for (Uint64 content = 0; content < 4; ++content) + for (Uint64 params = 0; params < 40; ++params) + for (Uint64 bind = 0; bind < 16; ++bind) + for (Uint64 program = 1; program < 8; ++program) + for (Uint64 version = 0; version < 8; ++version) { + seen.push_back(bit12(content, params, bind, MGPipeMixShutter(program, version))); + } + std::sort(seen.begin(), seen.end()); + EXPECT_EQ(std::adjacent_find(seen.begin(), seen.end()), seen.end()) + << "two distinct small states share a shutter value"; + } + using GLContext = MG_State::GLState::GLContext; using MG_State::GLState::TextureObjectBase; diff --git a/MobileGL/MG_Test/Pipe/goldens/recorder/BufferVertexArrayDraw.txt b/MobileGL/MG_Test/Pipe/goldens/recorder/BufferVertexArrayDraw.txt index 4e710ca3..19486719 100644 --- a/MobileGL/MG_Test/Pipe/goldens/recorder/BufferVertexArrayDraw.txt +++ b/MobileGL/MG_Test/Pipe/goldens/recorder/BufferVertexArrayDraw.txt @@ -24,8 +24,8 @@ 21 ResourceSubData 104 592591476e1de3eb 22 ResourceRespecify 88 d2ea8273d6969816 23 CreateSamplerView 36 92735167665158b2 -24 SetSamplerViews 40 ff12d52ac483fd59 -25 BindSamplerStates 24 8ec570815d8132c4 +24 SetSamplerViews 40 cdfff376ce9850d7 +25 BindSamplerStates 24 c15219323b7a40b1 26 SetGlobalConstants 56 3a08c059ed3fb509 27 CreateRenderState 444 cbaa7e1b0c8f56b0 28 BindRenderState 12 2df0851678bfe6a3 @@ -36,7 +36,7 @@ 33 SetContextValues 96 8d24acac68829781 34 CreateVertexElements 1320 1e6a49d992827735 35 BindVertexElements 16 b110667323a9f265 -36 SetVertexBuffers 56 98392787becb775d +36 SetVertexBuffers 56 776e3b47bef003ea 37 SetIndexBuffer 24 8b038a41009b3de1 38 DrawVbo 68 f68b4c81a25b7c80 39 DrawVbo 68 bfcdfb6905f6a2e0