diff --git a/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp b/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp index 4905b4a7..571252d2 100644 --- a/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp +++ b/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp @@ -1451,9 +1451,25 @@ namespace MobileGL::MG_Backend::DirectGLES { // start at 1). This is not the twin table's {slot, gen}: a bound texture that has never // been synced has no twin and therefore no handle, so a handle-keyed snapshot would read // two never-synced textures as equal. The identity has to exist before the twin does. + // + // Split by the RUNTIME arm, not by the build, for exactly the reason g_fbSlotCache is: + // MOBILEGL_PIPE_PUSH=0 has to reproduce P1's behaviour (ConfigLoader.cpp), and a + // legacy-arm run that debounced on lifetime ids would be running P2's mechanism while + // the A/B attributed the result to P1. The two answers are equivalent - OwnerEquals on + // two empty pointers is true and LifetimeIdOf(nullptr) == 0 == 0; a live-versus-expired + // control block and two distinct lifetime ids both compare unequal - so keeping the + // legacy fields costs that arm nothing but the words, and gives the control back its + // fidelity. A build with no legacy arm compiled carries neither the fields nor the + // branch. struct UnitBindingsSnapshot { Array slotObjects{}; Uint64 samplerObject = 0; +#if MOBILEGL_PIPE_LEGACY_MEMOS + // P1's identity, kept verbatim for the legacy arm only. + Array, (SizeT)TextureTarget::TextureTargetCount> + legacySlotObjects{}; + WeakPtr legacySamplerObject{}; +#endif }; static Uint64 LifetimeIdOf(const SharedPtr& object) { @@ -1464,32 +1480,69 @@ namespace MobileGL::MG_Backend::DirectGLES { return object ? object->GetLifetimeId() : 0; } +#if MOBILEGL_PIPE_LEGACY_MEMOS +#define MGB_UNIT_BINDINGS_HANDLE_ARM (EsprytSlotTablesEnabled()) +#else +#define MGB_UNIT_BINDINGS_HANDLE_ARM (true) +#endif + static void CaptureUnitBindings(Int maxTouchedUnit, Vector& out) { + const Bool handleArm = MGB_UNIT_BINDINGS_HANDLE_ARM; out.resize(static_cast(maxTouchedUnit + 1)); for (Int unit = 0; unit <= maxTouchedUnit; ++unit) { auto& textureUnit = MGB_CTX->GetTextureUnitObject(unit); auto& snapshot = out[static_cast(unit)]; const auto& slots = textureUnit.GetAllBindingSlots(); for (SizeT i = 0; i < slots.size(); ++i) { - snapshot.slotObjects[i] = LifetimeIdOf(slots[i].GetBoundObject()); + if (handleArm) { + snapshot.slotObjects[i] = LifetimeIdOf(slots[i].GetBoundObject()); + } +#if MOBILEGL_PIPE_LEGACY_MEMOS + else { + snapshot.legacySlotObjects[i] = slots[i].GetBoundObject(); + } +#endif } - snapshot.samplerObject = LifetimeIdOf(textureUnit.GetSamplerObject()); + if (handleArm) { + snapshot.samplerObject = LifetimeIdOf(textureUnit.GetSamplerObject()); + } +#if MOBILEGL_PIPE_LEGACY_MEMOS + else { + snapshot.legacySamplerObject = textureUnit.GetSamplerObject(); + } +#endif } } static Bool UnitBindingsUnchanged(Int maxTouchedUnit, const Vector& snapshots) { if (snapshots.size() != static_cast(maxTouchedUnit + 1)) return false; + const Bool handleArm = MGB_UNIT_BINDINGS_HANDLE_ARM; for (Int unit = 0; unit <= maxTouchedUnit; ++unit) { auto& textureUnit = MGB_CTX->GetTextureUnitObject(unit); const auto& snapshot = snapshots[static_cast(unit)]; const auto& slots = textureUnit.GetAllBindingSlots(); for (SizeT i = 0; i < slots.size(); ++i) { - if (snapshot.slotObjects[i] != LifetimeIdOf(slots[i].GetBoundObject())) return false; + if (handleArm) { + if (snapshot.slotObjects[i] != LifetimeIdOf(slots[i].GetBoundObject())) return false; + } +#if MOBILEGL_PIPE_LEGACY_MEMOS + else if (!OwnerEquals(snapshot.legacySlotObjects[i], slots[i].GetBoundObject())) { + return false; + } +#endif } - if (snapshot.samplerObject != LifetimeIdOf(textureUnit.GetSamplerObject())) return false; + if (handleArm) { + if (snapshot.samplerObject != LifetimeIdOf(textureUnit.GetSamplerObject())) return false; + } +#if MOBILEGL_PIPE_LEGACY_MEMOS + else if (!OwnerEquals(snapshot.legacySamplerObject, textureUnit.GetSamplerObject())) { + return false; + } +#endif } return true; } +#undef MGB_UNIT_BINDINGS_HANDLE_ARM #else struct UnitBindingsSnapshot { Array, (SizeT)TextureTarget::TextureTargetCount> diff --git a/MobileGL/MG_Test/SanityTest.cpp b/MobileGL/MG_Test/SanityTest.cpp index 59fd3c1b..7c095758 100644 --- a/MobileGL/MG_Test/SanityTest.cpp +++ b/MobileGL/MG_Test/SanityTest.cpp @@ -3199,10 +3199,18 @@ namespace { int marker = 0; }; - // Kind Query is unused by every shipping path, so these cases cannot disturb the slot space - // any real twin table allocates out of. + // Kinds Query and Fence are unused by every shipping path, so these cases cannot disturb + // the slot space any real twin table allocates out of. + // + // TWO of them, because MGPipeSlots() is a process-global singleton and three of the cases + // below read its per-kind LiveCount / HighWater. The two-holder case is the one that can + // perturb another, so it gets a kind of its own rather than a promise about gtest's + // registration order: --gtest_shuffle, --gtest_filter and a future case are all free to + // reorder them, and a shared kind would make that a flake. using FakeSlotTable = MobileGL::MG_Backend::DirectGLES:: BackendSlotTable; + using FakeSharedKindSlotTable = MobileGL::MG_Backend::DirectGLES:: + BackendSlotTable; } // namespace // The property the whole slice exists for. The pre-P2 registry keyed twins on the frontend heap @@ -3320,18 +3328,19 @@ TEST(DirectGLESSlotTable, AWholeTableSavesResetsAndRestores) { TEST(DirectGLESSlotTable, TwoTablesOfTheSameKindShareOneSlotAndKeepTheirOwnTwin) { using namespace MobileGL; + constexpr MG_Pipe::MGPipeKind kKind = MG_Pipe::MGPipeKind::Fence; auto& slots = MG_Pipe::MGPipeSlots(); - const Uint32 liveBefore = slots.LiveCount(MG_Pipe::MGPipeKind::Query); + const Uint32 liveBefore = slots.LiveCount(kKind); - FakeSlotTable a; - FakeSlotTable b; + FakeSharedKindSlotTable a; + FakeSharedKindSlotTable b; auto object = MakeShared(0xE1u); a.GetOrCreate(object) = MakeShared(); (*a.Find(object.get()))->marker = 1; b.GetOrCreate(object) = MakeShared(); (*b.Find(object.get()))->marker = 2; - EXPECT_EQ(slots.LiveCount(MG_Pipe::MGPipeKind::Query), liveBefore + 1u) + EXPECT_EQ(slots.LiveCount(kKind), liveBefore + 1u) << "the two tables minted a slot each; Magma's table would then resolve a different " "handle for the same object than Espryt's"; @@ -3343,9 +3352,26 @@ TEST(DirectGLESSlotTable, TwoTablesOfTheSameKindShareOneSlotAndKeepTheirOwnTwin) ASSERT_NE(b.FindByHandle(handle), nullptr); EXPECT_EQ((*a.FindByHandle(handle))->marker, 1); EXPECT_EQ((*b.FindByHandle(handle))->marker, 2); - // Deliberately no sweep: two tables of ONE kind both hold this slot, and whichever swept - // first would return it to the allocator while the other still names it. The six shipping - // registries are one table per kind, so that cannot arise outside this case. + + // TWO HOLDERS OF ONE SLOT is a real configuration, not a test artefact, and this is where + // the sharp edge is: whichever holder sweeps (or is told of the death) first returns the + // slot to the allocator while the other still names it. The allocator makes that SAFE - + // Free is generation-guarded and idempotent, and FindByHandle's Gen compare turns the + // other holder's now-stale handle into a miss - but not free: if the object is still alive + // the next resolution re-Acquires it onto a NEW slot, orphaning the first table's entry + // until its own sweep. Two such configurations exist or are landing: the + // ScopedDirectGLESTextureBindings fixture above holds a second live table of kind Texture + // for the length of a test, and package D's subsystem 4 re-keys VaoDrawMemo out of this + // same per-kind allocator. Flagged for the integrator rather than defended here, because + // the fix (a refcounted slot-ownership token) belongs with whoever owns both holders. + // + // The cleanup below is what keeps that out of the OTHER cases: object first, then both + // tables, so the slot is back on the free list and this kind's LiveCount is where it was. + object.reset(); + a.CollectGarbageNow(); + b.CollectGarbageNow(); + EXPECT_EQ(slots.LiveCount(kKind), liveBefore) + << "the shared slot outlived both holders and the object"; } // The sweep has TWO drivers and the table must carry both. Nothing below calls