From bd2b4158e040ab0afc387dba819e0b321d62b9df Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Sat, 5 Sep 2026 14:04:07 -0400 Subject: [PATCH] [Fix] (DirectVulkan, State): key the transform-feedback counter slots on the object's identity, not on its recycled GL name - VulkanRenderer::CurrentXfbCounterSlot keyed m_xfbCounterSlotByObject on GetBoundTransformFeedbackName(). glGenTransformFeedbacks hands a deleted name straight back (IndexGenerator is LIFO) and nothing ever removed a map entry, so a transform feedback object created on a recycled name was served the DEAD object's counter group - and with it that group's m_xfbCountersValid and m_xfbLastSeenGeneration entries, which are the resume/fresh decision for vkCmdBeginTransformFeedbackEXT. This is D21 in plan B v2 4.7.3, the one entry in that table whose today-key "guards nothing", and 10.4-5 asks for it to land on dev on its own - hence this separate commit, kept in files no other commit on this branch touches so the cherry-pick applies unaided. - Frontend: TransformFeedbackObjectState gains a never-reused `lifetimeId` through a default member initialiser, so every route into existence (operator[] materialisation, `= {}` in GenTransformFeedbackNames and CreateTransformFeedbackObject) mints a fresh one and a recycled name cannot carry the dead object's id back. The allocator is the same shape as BufferObject::AllocateLifetimeId (atomic, starts at 1 so a zeroed backend slot is never a live object). - The bound object's id is mirrored in m_boundTransformFeedbackLifetimeId, refreshed by RestoreBoundTransformFeedbackState - which every bind, and the revert that deleting the bound object performs, goes through - and seeded for the default object by the GLContext constructor. GetBoundTransformFeedback LifetimeId is therefore a const load. Reading it through operator[] instead would have been an INSERT on the per-draw path, and UnorderedMap is ska::flat_hash_map, whose rehash invalidates every reference into the container, not just its iterators. - Backend: the UnorderedMap is replaced by a fixed 16-entry owner table, which fixes the second half of the same defect - the map was keyed on a value that recycles yet was never pruned, so it grew for the life of the context. With lifetime ids as keys a map would have grown without bound instead, so the bounded table is required, not cosmetic. - Slot exhaustion: past sixteen owners a group has to be taken over, and the victim is chosen among owners with NO OPEN SPAN, which GLContext::HasOpenTransformFeedbackSpan answers; an identity no live object carries any more answers false, and that is what lets a dead owner's group come back. Least-recently-used ALONE would have been exactly the wrong rule: GL only permits another object to capture while this one is PAUSED, so the paused span these groups exist to protect is by construction the least recently used entry, and an LRU takeover would reset the one resume offset that still matters. LRU is now only the tie-break among reclaimable groups. Sixteen genuinely open spans at once is reported (MGLOG_E_ONCE) rather than resolved silently, because whatever is taken then restarts at offset 0. - Not done, and why: the natural place to hand a group back is glEndTransformFeedback, but registering DirectVulkan's EndTransformFeedback table entry would flip the test FixupGsStripCaptureOrder makes of that same pointer (GL_Drawing.cpp:1255) to decide whether the backend already captured in GL's vertex order, silently disabling the geometry-stage strip fixup for DirectVulkan. Giving that discriminator a name of its own is a separate change; until then the no-open-span rule is what keeps the table honest. - CurrentXfbCounterSlot asserts the identity is never 0. Zero is the free-slot sentinel, so an identity of 0 would match every free slot as "mine" without ever claiming one - this bug reintroduced, with no symptom at the call site. - TransformFeedbackLifetimeIdTest, in its own translation unit, pins the frontend halves: an object created on a recycled name must not report the dead object's id, the default object has an identity before anything binds it, and a PAUSED span still reads as open while another object is bound and capturing - which is the whole correctness argument for the eviction rule. The name reuse is not simulated: the test asks the real generator and skips (loudly) if it never recycled. Still untested: the >16-owners path itself, which needs a backend scenario with seventeen capturing objects and there is none. - Negative controls, each applied then reverted: making a non-bound object's span read as closed reddens APausedSpanStaysOpenWhileAnotherObjectCaptures; making a vanished identity read as open reddens the same case on its delete assertion; dropping the constructor's seeding reddens AnObjectAtARecycledNameCarriesAFreshLifetimeId. - Tested: cmake --build build-linux -j 24 (clean, 166 targets); ctest -L unit -j 12 -> 1386/1386 passed; ctest -L integration-gpu -> 866/866 passed serially, and 866/866 on one of two -j 8 runs. The other -j 8 run failed DirectGLES.PointSizeDemotionScenario.TheDemotionIsActuallyArmedWhenTheEnviron mentPinsItOn, a member of the pre-existing parallel-ctest flake family: it passes in isolation here, and the unmodified parent tree (~/w7/p0-noop-wins-base) reproduces the same family under -j 8. --- .../DirectVulkan/Renderer/VulkanRenderer.cpp | 71 +++++++-- .../DirectVulkan/Renderer/VulkanRenderer.h | 14 +- MobileGL/MG_State/GLState/Core.cpp | 32 ++++ MobileGL/MG_State/GLState/Core.h | 34 ++++- MobileGL/MG_Test/State/CMakeLists.txt | 25 +++ .../State/TransformFeedbackLifetimeIdTest.cpp | 143 ++++++++++++++++++ 6 files changed, 306 insertions(+), 13 deletions(-) create mode 100644 MobileGL/MG_Test/State/TransformFeedbackLifetimeIdTest.cpp diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp index e365603d..289d2c98 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp @@ -3267,8 +3267,9 @@ void main() { } m_vertexInputStateFactory.reset(); m_xfbCounterBuffer.Destroy(); - m_xfbCounterSlotByObject.clear(); - m_xfbNextCounterSlot = 0; + m_xfbCounterSlotOwner.fill(0); + m_xfbCounterSlotLastUse.fill(0); + m_xfbCounterSlotUseSerial = 0; m_xfbCountersValid.fill(false); m_xfbLastSeenGeneration.fill(0); if (m_occlusionQueryPool != VK_NULL_HANDLE) { @@ -11199,16 +11200,66 @@ void main() { } } + // Keyed on the frontend's never-reused lifetime id, NOT on the GL name. The name is + // recycled the moment glDeleteTransformFeedbacks gives it back, so a name-keyed slot + // handed a brand-new object the counter group - and the m_xfbCountersValid / + // m_xfbLastSeenGeneration entries - of the object that died under that name. + // + // Slots are never handed back (there is no backend entry telling this renderer that a span + // closed - registering the EndTransformFeedback one would flip the "captures through its own + // driver" test FixupGsStripCaptureOrder makes of it), so once all sixteen are owned a new + // object has to take one over. The victim is chosen among owners with NO OPEN SPAN: an object + // whose span is closed, or which no longer exists at all, can never resume, so its counter + // bytes are dead. Least-recently-used ALONE would be exactly the wrong rule - GL only permits + // another object to capture while this one is PAUSED, so the paused span whose counters the + // slots exist to protect is by construction the least recently used entry. Taking a group over + // resets its counter state, because those bytes describe the previous owner's span. Uint32 VulkanRenderer::CurrentXfbCounterSlot() { - const Uint name = MG_State::pGLContext->GetBoundTransformFeedbackName(); - const auto it = m_xfbCounterSlotByObject.find(name); - if (it != m_xfbCounterSlotByObject.end()) { - return it->second; + constexpr Uint32 kNoSlot = static_cast(kXfbCounterObjectSlots); + const Uint64 identity = MG_State::pGLContext->GetBoundTransformFeedbackLifetimeId(); + MOBILEGL_ASSERT(identity != 0, + "transform feedback object reported the free-slot sentinel (0) as its identity - " + "every slot would then read as 'mine' without ever being claimed"); + Uint32 freeSlot = kNoSlot; + for (Uint32 slot = 0; slot < kNoSlot; ++slot) { + if (m_xfbCounterSlotOwner[slot] == identity) { + m_xfbCounterSlotLastUse[slot] = ++m_xfbCounterSlotUseSerial; + return slot; + } + if (m_xfbCounterSlotOwner[slot] == 0 && freeSlot == kNoSlot) { + freeSlot = slot; + } } - // Past the tracked set every object shares slot group 0. Only concurrently-paused - // spans need distinct groups, and applications do not keep sixteen of those open. - const Uint32 slot = m_xfbNextCounterSlot < kXfbCounterObjectSlots ? m_xfbNextCounterSlot++ : 0; - m_xfbCounterSlotByObject[name] = slot; + Uint32 slot = freeSlot; + if (slot == kNoSlot) { + for (Uint32 candidate = 0; candidate < kNoSlot; ++candidate) { + if (MG_State::pGLContext->HasOpenTransformFeedbackSpan(m_xfbCounterSlotOwner[candidate])) { + continue; + } + if (slot == kNoSlot || m_xfbCounterSlotLastUse[candidate] < m_xfbCounterSlotLastUse[slot]) { + slot = candidate; + } + } + } + if (slot == kNoSlot) { + // Sixteen capture spans open at once. Whatever is taken loses its resume offset and + // restarts at byte 0 of its capture buffers, which is a wrong picture rather than a + // slow one - hence a report rather than a silent choice. + MGLOG_E_ONCE("CurrentXfbCounterSlot: all %zu counter groups belong to transform feedback objects " + "with an open capture span; the least recently used one is taken over and that span " + "will restart at offset 0 instead of appending", + kXfbCounterObjectSlots); + slot = 0; + for (Uint32 candidate = 1; candidate < kNoSlot; ++candidate) { + if (m_xfbCounterSlotLastUse[candidate] < m_xfbCounterSlotLastUse[slot]) { + slot = candidate; + } + } + } + m_xfbCounterSlotOwner[slot] = identity; + m_xfbCounterSlotLastUse[slot] = ++m_xfbCounterSlotUseSerial; + m_xfbCountersValid[slot] = false; + m_xfbLastSeenGeneration[slot] = 0; return slot; } diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.h b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.h index c3d30855..f8498a3a 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.h +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.h @@ -675,8 +675,18 @@ namespace MobileGL::MG_Backend::DirectVulkan { // per object: one group of four slots each, handed out on first use. static constexpr SizeT kXfbCounterObjectSlots = 16; VkBufferObject m_xfbCounterBuffer; - UnorderedMap m_xfbCounterSlotByObject; - Uint32 m_xfbNextCounterSlot = 0; + // Which transform feedback object owns each slot group, by the frontend's never-reused + // lifetime id (0 = the slot is free). This used to be an UnorderedMap keyed on the GL + // NAME, which is recycled by glGenTransformFeedbacks: a deleted-and-recreated object + // inherited the dead one's slot, and since nothing ever removed an entry the map also + // grew for the life of the context. A fixed table cannot do either: a group is taken over + // only from an owner with no OPEN span (see CurrentXfbCounterSlot), so an object whose + // counters can still be resumed never loses them, and a dead object's group comes back. + Array m_xfbCounterSlotOwner{}; + // Tie-break among reclaimable groups only; never on its own, because the paused span the + // groups exist for is by construction the least recently used one. + Array m_xfbCounterSlotLastUse{}; + Uint64 m_xfbCounterSlotUseSerial = 0; // Set for a slot once a captured draw has been recorded into its span; selects // counter-buffer resume on the next captured draw of the same span. Array m_xfbCountersValid{}; diff --git a/MobileGL/MG_State/GLState/Core.cpp b/MobileGL/MG_State/GLState/Core.cpp index fd4207da..952e1da8 100644 --- a/MobileGL/MG_State/GLState/Core.cpp +++ b/MobileGL/MG_State/GLState/Core.cpp @@ -14,6 +14,8 @@ #include #include +#include + namespace MobileGL::MG_State { void Init() { MGLOG_D("Initializing MobileGL State..."); @@ -1259,6 +1261,33 @@ namespace MobileGL::MG_State { return m_renderbufferState.ValidateRenderbufferObject(index); } + Uint64 GLContext::AllocateTransformFeedbackLifetimeId() { + // Starts at 1 so a zero-initialised backend slot can never carry a live object's id. + static std::atomic nextId{1}; + return nextId.fetch_add(1, std::memory_order_relaxed); + } + + GLContext::GLContext() { + // The default transform feedback object (name 0) exists from the start of the context + // (GL 4.6 core 13.2.1), but nothing binds it, so nothing else would materialise it. + // Materialising it here is what lets GetBoundTransformFeedbackLifetimeId() be a plain + // const read instead of an operator[] insert on the draw path. + m_boundTransformFeedbackLifetimeId = m_transformFeedbackObjects[0].lifetimeId; + } + + Bool GLContext::HasOpenTransformFeedbackSpan(Uint64 lifetimeId) const { + if (lifetimeId == 0) return false; + for (const auto& [name, object] : m_transformFeedbackObjects) { + if (object.lifetimeId != lifetimeId) continue; + // The bound object's span state is live in the context; its saved copy is only + // written when a bind swaps it out. + return name == m_boundTransformFeedback ? m_transformFeedbackActive : object.active; + } + // No object carries this identity any more: it was deleted, and a deleted object can + // never resume. + return false; + } + void GLContext::SaveBoundTransformFeedbackState() { auto& object = m_transformFeedbackObjects[m_boundTransformFeedback]; for (Uint i = 0; i < MAX_TRANSFORM_FEEDBACK_BUFFERS; ++i) { @@ -1295,6 +1324,9 @@ namespace MobileGL::MG_State { m_transformFeedbackGeneration = object.generation; m_transformFeedbackCapturedVertices = object.capturedVertices; m_transformFeedbackInputPrimitives = object.inputPrimitives; + // Every route that changes which object is bound - BindTransformFeedbackObject and the + // revert a delete of the bound object performs - comes through here. + m_boundTransformFeedbackLifetimeId = object.lifetimeId; } void GLContext::GenTransformFeedbackNames(Uint number, Vector& ids) { diff --git a/MobileGL/MG_State/GLState/Core.h b/MobileGL/MG_State/GLState/Core.h index 73a8f799..3b7e4802 100644 --- a/MobileGL/MG_State/GLState/Core.h +++ b/MobileGL/MG_State/GLState/Core.h @@ -60,7 +60,7 @@ namespace MobileGL { class GLContext { public: - GLContext() = default; + GLContext(); // Error void RecordError(ErrorCode code, UniquePtr info); @@ -427,6 +427,27 @@ namespace MobileGL { void BindTransformFeedbackObject(Uint index); void MarkTransformFeedbackObjectForDeletion(Uint index); Uint GetBoundTransformFeedbackName() const { return m_boundTransformFeedback; } + // The bound object's never-reused identity, for a backend that keys a per-object + // resource on it. The NAME is not an identity: glGenTransformFeedbacks recycles a + // deleted one (LIFO), so a memo keyed on the name hands a brand-new object the dead + // one's slot. Cached rather than looked up on demand: the backend asks twice per + // captured draw, and an operator[] on m_transformFeedbackObjects would be an + // INSERT on the draw path - ska::flat_hash_map invalidates every reference into + // itself when it rehashes. The cache is refreshed by + // RestoreBoundTransformFeedbackState, which every bind (and the revert a delete + // performs) goes through, and seeded for the default object by the constructor. + // Never returns 0 - the counter starts at 1 so a zero-initialised memo slot cannot + // be mistaken for a live object. + Uint64 GetBoundTransformFeedbackLifetimeId() const { return m_boundTransformFeedbackLifetimeId; } + // Whether the object carrying this identity still has an OPEN capture span - one + // that glBeginTransformFeedback started and glEndTransformFeedback has not closed, + // paused or not. A backend that hands out a bounded set of per-object slots must + // never take one of these over: a paused span's counters are precisely what its + // resume reads, and GL only lets other objects capture WHILE it is paused, so the + // paused object is also the one that looks idle. An identity no live object + // carries any more (its object was deleted) answers false, which is what makes + // such a slot reclaimable. + Bool HasOpenTransformFeedbackSpan(Uint64 lifetimeId) const; // Vertices the object captured in its last completed span; the vertex count // glDrawTransformFeedback replays. Uint64 GetTransformFeedbackRecordedVertices(Uint index) const; @@ -514,6 +535,9 @@ namespace MobileGL { GLuint m_conditionalRenderQuery = 0; GLenum m_conditionalRenderMode = GL_NONE; + // Process-wide, never-reused. See GetBoundTransformFeedbackLifetimeId(); same + // contract as BufferObject::AllocateLifetimeId(). + static Uint64 AllocateTransformFeedbackLifetimeId(); // Everything a transform feedback object owns while it is NOT the bound one. struct TransformFeedbackObjectState { struct SavedBufferBinding { @@ -532,6 +556,10 @@ namespace MobileGL { Uint64 recordedVertices = 0; Bool hasCompletedSpan = false; Bool everBound = false; + // Assigned by the default member initialiser, so every way an object comes into + // being - operator[] materialisation, `= {}` in Gen/Create - gets a fresh one, + // and a recycled NAME never brings the dead object's id back with it. + Uint64 lifetimeId = AllocateTransformFeedbackLifetimeId(); }; void SaveBoundTransformFeedbackState(); void RestoreBoundTransformFeedbackState(); @@ -540,6 +568,10 @@ namespace MobileGL { UnorderedMap m_transformFeedbackObjects; IndexGenerator m_transformFeedbackNames; Uint m_boundTransformFeedback = 0; + // Mirror of m_transformFeedbackObjects[m_boundTransformFeedback].lifetimeId, so + // the per-draw read is a load rather than a hash lookup that could insert. + // Seeded by the constructor and rewritten by RestoreBoundTransformFeedbackState. + Uint64 m_boundTransformFeedbackLifetimeId = 0; // Map membership is object EXISTENCE, which is not the same as the answer // glIsProgramPipeline gives: any command that needs somewhere to put state // materializes a reserved name, so the object can exist well before it is diff --git a/MobileGL/MG_Test/State/CMakeLists.txt b/MobileGL/MG_Test/State/CMakeLists.txt index c04845c5..52d7cb9c 100644 --- a/MobileGL/MG_Test/State/CMakeLists.txt +++ b/MobileGL/MG_Test/State/CMakeLists.txt @@ -45,6 +45,31 @@ endif() include(GoogleTest) gtest_discover_tests(ObjectLifetimeIdTest DISCOVERY_TIMEOUT 30 PROPERTIES LABELS unit) +add_executable( + TransformFeedbackLifetimeIdTest + TransformFeedbackLifetimeIdTest.cpp +) + +target_include_directories(TransformFeedbackLifetimeIdTest PRIVATE + ${MGL_ROOT}/include + ${MGL_ROOT}/MobileGL + ${MGL_ROOT}/3rdparty/xxHash + ${MGL_ROOT}/3rdparty/Vulkan-Headers/include + ${MGL_ROOT}/3rdparty/SPIRV-Reflect +) + +target_link_libraries( + TransformFeedbackLifetimeIdTest PRIVATE + GTest::gtest_main + ${LINK_LIBRARIES} +) + +if (MSVC) + target_compile_options(TransformFeedbackLifetimeIdTest PRIVATE /Zc:preprocessor) +endif() + +gtest_discover_tests(TransformFeedbackLifetimeIdTest DISCOVERY_TIMEOUT 30 PROPERTIES LABELS unit) + add_executable( RenderStateTest RenderStateTest.cpp diff --git a/MobileGL/MG_Test/State/TransformFeedbackLifetimeIdTest.cpp b/MobileGL/MG_Test/State/TransformFeedbackLifetimeIdTest.cpp new file mode 100644 index 00000000..1b0274fa --- /dev/null +++ b/MobileGL/MG_Test/State/TransformFeedbackLifetimeIdTest.cpp @@ -0,0 +1,143 @@ +// MobileGL - MobileGL/MG_Test/State/TransformFeedbackLifetimeIdTest.cpp +// Copyright (c) 2025-2026 MobileGL-Dev +// Licensed under the GNU Lesser General Public License v3.0: +// https://www.gnu.org/licenses/gpl-3.0.txt +// https://www.gnu.org/licenses/lgpl-3.0.txt +// SPDX-License-Identifier: LGPL-3.0-only +// End of Source File Header +// +// D21 (plan B v2 §4.7.3): DirectVulkan hands every transform feedback object one of sixteen +// counter-buffer groups, and the group carries that span's resume offset. The map was keyed on +// the GL NAME, which glGenTransformFeedbacks recycles the moment the object is deleted, so an +// object created on a recycled name was served the DEAD object's group together with its +// m_xfbCountersValid / m_xfbLastSeenGeneration entries. +// +// The transform feedback object is a plain struct inside a map rather than a heap object, so the +// reuse to defend against is the NAME's, not an address's - which is why these cases live here +// and not in ObjectLifetimeIdTest.cpp with the heap-allocated object types. Keeping them in +// their own translation unit also keeps the D21 commit textually independent of the rest of the +// branch, which plan B §10.4-5 asks for so it can be cherry-picked to dev on its own. +// +// The second case pins the OTHER half of the backend contract: a bounded slot table has to be +// able to tell an object whose span is still open (and may yet resume) from one whose span is +// closed or whose object is gone. + +#include + +#include "Includes.h" +#include "Init.h" + +#include + +using namespace MobileGL; + +namespace { + + MG_State::GLState::GLContext& FreshContext() { + MobileGL::Initialize(); + MG_State::pGLContext = MakeUnique(); + return *MG_State::pGLContext; + } + +} // namespace + +TEST(TransformFeedbackLifetimeIdTest, AnObjectAtARecycledNameCarriesAFreshLifetimeId) { + auto& context = FreshContext(); + + // Before anything is bound. The default object exists from the start of the context, and the + // identity has to exist with it: a backend reading 0 here would match every FREE slot in its + // table without ever claiming one, which is the same bug this id was added to remove. + EXPECT_NE(context.GetBoundTransformFeedbackLifetimeId(), 0u) + << "the default transform feedback object has no identity until something binds it"; + + Vector names; + context.GenTransformFeedbackNames(1, names); + ASSERT_EQ(names.size(), 1u); + const Uint name = names[0]; + ASSERT_NE(name, 0u); + + context.BindTransformFeedbackObject(name); + const Uint64 firstId = context.GetBoundTransformFeedbackLifetimeId(); + EXPECT_NE(firstId, 0u) << "a live transform feedback object answered to id 0, which is the value a " + "zero-initialised backend slot already carries"; + + // The default object is a different object and must not share the id. + context.BindTransformFeedbackObject(0); + EXPECT_NE(context.GetBoundTransformFeedbackLifetimeId(), firstId) + << "the default transform feedback object shares an identity with a generated one"; + + // Deleting while bound reverts to the default object (GL 4.6 core 13.2.1), which is the + // shape the backend sees; delete from there anyway so the test does not depend on it. + context.BindTransformFeedbackObject(name); + context.MarkTransformFeedbackObjectForDeletion(name); + + Vector reborn; + context.GenTransformFeedbackNames(1, reborn); + ASSERT_EQ(reborn.size(), 1u); + if (reborn[0] != name) { + GTEST_SKIP() << "inconclusive, not proven: the name generator did not hand the deleted name back, so " + "the recycled-name case was never exercised"; + } + + context.BindTransformFeedbackObject(reborn[0]); + EXPECT_NE(context.GetBoundTransformFeedbackLifetimeId(), firstId) + << "a transform feedback object created on a recycled name reports the DEAD object's lifetime id - " + "DirectVulkan would hand it the dead span's counter slot, and with it that span's resume state"; +} + +// The predicate DirectVulkan's slot table asks before it takes a group over. The case that +// matters is the middle one: object A is PAUSED and another object is bound and capturing, so A +// looks completely idle to a least-recently-used rule while being exactly the object whose +// counters must survive. +TEST(TransformFeedbackLifetimeIdTest, APausedSpanStaysOpenWhileAnotherObjectCaptures) { + auto& context = FreshContext(); + + Vector names; + context.GenTransformFeedbackNames(2, names); + ASSERT_EQ(names.size(), 2u); + const Uint nameA = names[0]; + const Uint nameB = names[1]; + + context.BindTransformFeedbackObject(nameA); + const Uint64 idA = context.GetBoundTransformFeedbackLifetimeId(); + EXPECT_FALSE(context.HasOpenTransformFeedbackSpan(idA)) << "an object that never began a span reads as open"; + + context.BeginTransformFeedback(GL_POINTS, nullptr); + EXPECT_TRUE(context.HasOpenTransformFeedbackSpan(idA)); + + // Pausing is what makes interleaving legal (ARB_transform_feedback2); the span is still open. + context.SetTransformFeedbackPaused(true); + EXPECT_TRUE(context.HasOpenTransformFeedbackSpan(idA)); + + // Now the shape the slot table sees: B is bound and capturing, A is paused and untouched. + context.BindTransformFeedbackObject(nameB); + const Uint64 idB = context.GetBoundTransformFeedbackLifetimeId(); + EXPECT_NE(idB, idA); + context.BeginTransformFeedback(GL_POINTS, nullptr); + EXPECT_TRUE(context.HasOpenTransformFeedbackSpan(idA)) + << "a paused span stopped reading as open the moment another object was bound - a backend " + "reclaiming slots by 'is this owner still going' would take A's counters away"; + EXPECT_TRUE(context.HasOpenTransformFeedbackSpan(idB)); + + context.EndTransformFeedback(); + EXPECT_FALSE(context.HasOpenTransformFeedbackSpan(idB)) << "a closed span still reads as open"; + EXPECT_TRUE(context.HasOpenTransformFeedbackSpan(idA)); + + // A closes its own span; its slot becomes reclaimable. + context.BindTransformFeedbackObject(nameA); + context.EndTransformFeedback(); + EXPECT_FALSE(context.HasOpenTransformFeedbackSpan(idA)); + + // A deleted object can never resume, so its identity must not hold a slot either. + context.BindTransformFeedbackObject(nameB); + context.BeginTransformFeedback(GL_POINTS, nullptr); + EXPECT_TRUE(context.HasOpenTransformFeedbackSpan(idB)); + context.MarkTransformFeedbackObjectForDeletion(nameB); + EXPECT_FALSE(context.HasOpenTransformFeedbackSpan(idB)) + << "the identity of a deleted transform feedback object still claims an open span, so its counter " + "group would be pinned for the life of the context"; + + // Identities the context never issued, and the free-slot sentinel, are not open spans. + EXPECT_FALSE(context.HasOpenTransformFeedbackSpan(0)); + EXPECT_FALSE(context.HasOpenTransformFeedbackSpan(~0ull)); +}