diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/MagmaPipeArms.h b/MobileGL/MG_Backend/DirectVulkan/Renderer/MagmaPipeArms.h index 95ee39c7..6ccb51dd 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/MagmaPipeArms.h +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/MagmaPipeArms.h @@ -11,8 +11,9 @@ #include #if MOBILEGL_PIPE_PUSH -// kMGPipeSubsystem* - the runtime bitmask's named bits. Push-only, so the pull build's -// include graph is unchanged. +// kMGPipeSubsystem* - the runtime bitmask's named bits - and the client slot allocator that +// mints every MGPipeHandle. Push-only, so the pull build's include graph is unchanged. +#include #include #endif @@ -59,5 +60,20 @@ namespace MobileGL::MG_Backend::DirectVulkan { #endif std::abort(); } + + // The {slot, gen} of a frontend object, minted on first sight and stable for that + // object's whole life (ARCHITECTURE.md 4.2). `lifetimeId` is the client's own identity + // for the object - never a GL name, never a heap address - so a deleted-and-recreated + // object at the same address cannot reproduce a handle, which is precisely the ABA + // HandleRecycleScenario reproduces. + // + // A VAO is kind VertexElementsCso: that is the gallium-shaped CSO a vertex array + // resolves to, and it is the only kind in MGPipeKind that names vertex-input state. + // Magma acquires the handle itself in P2 because the tracker does not emit object-class + // state yet (P2 emits for dirty bits 0-4 only); when it does, this becomes a read of what + // the client already sent. + inline MG_Pipe::MGPipeHandle MagmaPipeHandleOf(MG_Pipe::MGPipeKind kind, Uint64 lifetimeId) { + return MG_Pipe::MGPipeSlots().Acquire(kind, lifetimeId); + } #endif // MOBILEGL_PIPE_PUSH } // namespace MobileGL::MG_Backend::DirectVulkan diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VertexInputStateFactory.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/VertexInputStateFactory.cpp index 62b68c6d..c1bc968f 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VertexInputStateFactory.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VertexInputStateFactory.cpp @@ -7,6 +7,7 @@ // End of Source File Header #include "VertexInputStateFactory.h" +#include "MagmaPipeArms.h" #include "MG_Util/Converters/MGToStr/DataTypeConverter.h" #include #include @@ -45,7 +46,31 @@ namespace MobileGL::MG_Backend::DirectVulkan { // capture came back holding a dead VAO's vertex data (0,0,0,1 - the previous // test's positions) instead of its own. // Zero for client memory (no buffer), which is a distinct identity of its own. - const Uint64 bufferKey = attr.Buffer ? attr.Buffer->GetLifetimeId() : 0; + // + // P2 D12.4 / ARCHITECTURE.md 9.5: under the handle arm the identity is the + // buffer's {slot, gen} rather than its lifetime id - "lifetimeId -> gen mixed + // into every server-side content hash". The two are equally ABA-proof (the + // allocator maps one onto the other and bumps Gen only on slot REUSE); what + // changes is that the key is now the identity the SERVER will be handed once + // buffers travel as handles, instead of a number only the client can mint. + Uint64 bufferKey = attr.Buffer ? attr.Buffer->GetLifetimeId() : 0; +#if MOBILEGL_PIPE_PUSH + if (attr.Buffer) { + if (MagmaPipeSubsystemOn(MG_Pipe::kMGPipeSubsystemMagmaVertexInput)) { + const MG_Pipe::MGPipeHandle handle = + MagmaPipeHandleOf(MG_Pipe::MGPipeKind::Buffer, attr.Buffer->GetLifetimeId()); + bufferKey = static_cast(handle.Slot) | (static_cast(handle.Gen) << 32); + } else if (MG_Config::Features.PipeHandleAbaControl) { + // Negative control C (P2 brief D18), and it applies to the PRE-HANDLE arm + // on purpose: hash the raw BufferObject* the way this did before the + // lifetime-id fix, so HandleRecycleScenario.AbaControl can reproduce the + // ABA and assert the WRONG pixels. That arm is what proves the reproducer + // still reproduces; if the allocator stops handing the address back, it + // fails instead of passing for the wrong reason. + bufferKey = static_cast(reinterpret_cast(attr.Buffer.get())); + } + } +#endif XXHASH_VERIFY(XXH64_update(m_hashState, &bufferKey, sizeof(bufferKey))); } diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp index f39c6346..02fdf4c5 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp @@ -3625,6 +3625,43 @@ void main() { if (m_vaoDrawMemoTable.empty()) { m_vaoDrawMemoTable.resize(kVaoDrawMemoSlotCount); } +#if MOBILEGL_PIPE_PUSH + // ---- P2 D12.4, the handle arm ---- + // + // The slot IS the index. No Fibonacci mix of an address, no two-way probe, no + // frame-serial recycling choice: slots are dense by construction (the allocator has a + // free list plus a high-water mark), so consecutive VAOs land in consecutive entries + // and the collision the address hash existed to spread does not arise below the table + // size. The whole validation is one handle compare, and a handle cannot alias - Gen + // moves on slot REUSE, so a deleted VAO's successor never matches its predecessor's + // entry even at the same address and with a byte-identical configuration. + if (MagmaPipeSubsystemOn(MG_Pipe::kMGPipeSubsystemMagmaVertexInput)) { + const MG_Pipe::MGPipeHandle handle = ResolveVaoHandle(*vao); + // Fixed table, so the index wraps rather than growing: nothing in P2 frees a + // VertexElementsCso slot yet (the frontend death notification is Espryt 0b's e2, + // and buffers are the only kind with one today), so a grow-on-demand vector would + // hold one ~1 KB VaoDrawMemo per VAO EVER created. Above the table size this + // degrades to a direct-mapped cache validated by the full {slot, gen}, which is + // strictly better than the address hash it replaces - never wrong, only colder. + const Uint32 index = handle.Slot & (kVaoDrawMemoSlotCount - 1); + VaoDrawMemo& entry = m_vaoDrawMemoTable[index]; + if (entry.vaoHandle == handle) { + return &entry; + } + entry.vaoHandle = handle; + entry.vaoKey = vao; + entry.vaoLifetimeId = vao->GetLifetimeId(); + entry.contentHash = 0; + entry.layoutFactsValid = false; + // Unmatchable until a resolve completes (same rule as the legacy arm: a bailed-out + // resolve must never leave stale contents matchable). + entry.bindings.frameSerial = 0; + entry.bindings.indexFrameSerial = 0; + entry.bindings.indexBuffer = nullptr; + return &entry; + } + MagmaPipeRequireLegacyArm("LookupVaoDrawMemo"); +#endif // Multiplicative mix of the (16-byte-aligned) address; take high bits, they // carry the most entropy of a multiply. const Uint64 mixed = static_cast(reinterpret_cast(vao) >> 4) * 0x9E3779B97F4A7C15ull; @@ -3634,12 +3671,22 @@ void main() { // its own is recycled, and a slot matched on a recycled address hands the new VAO // the dead one's resolved bindings. const Uint64 lifetimeId = vao->GetLifetimeId(); +#if MOBILEGL_PIPE_PUSH + // Negative control C (P2 brief D18): with MOBILEGL_PIPE_HANDLE_ABA_CONTROL=1 the + // lifetime-id half of the compare is defeated, leaving the recycled address as the + // whole key - exactly the state this table was in before the ABA fix. That is what + // lets HandleRecycleScenario.AbaControl assert the WRONG pixels and so prove that its + // reproducer still reproduces. + const Bool compareLifetimeId = !MG_Config::Features.PipeHandleAbaControl; +#else + constexpr Bool compareLifetimeId = true; +#endif VaoDrawMemo& first = m_vaoDrawMemoTable[index]; - if (first.vaoKey == vao && first.vaoLifetimeId == lifetimeId) { + if (first.vaoKey == vao && (!compareLifetimeId || first.vaoLifetimeId == lifetimeId)) { return &first; } VaoDrawMemo& second = m_vaoDrawMemoTable[index ^ 1u]; - if (second.vaoKey == vao && second.vaoLifetimeId == lifetimeId) { + if (second.vaoKey == vao && (!compareLifetimeId || second.vaoLifetimeId == lifetimeId)) { return &second; } // Miss: recycle a slot. Prefer an empty one; otherwise evict the entry whose @@ -6234,9 +6281,23 @@ void main() { // draw of a VAO-cycling stream (Minecraft chunk rendering) through the full // path, re-resolving descriptors and texture layouts nothing invalidated. const auto& vao = *MGB_CTX->GetBoundVertexArray(); +#if MOBILEGL_PIPE_PUSH + // P2 D12.4: the handle replaces the (address, lifetime id) pair here too - one + // compare instead of two, and the same identity the VAO draw memo is keyed on, so + // the two cannot disagree about whether "the VAO moved". The config version stays: + // it answers a different question (did this same object's layout change). + const Bool vaoMoved = + MagmaPipeSubsystemOn(MG_Pipe::kMGPipeSubsystemMagmaVertexInput) + ? (!(ResolveVaoHandle(vao) == snap.vaoHandle) || + vao.GetConfigVersion() != snap.vaoConfigVersion) + : (static_cast(&vao) != snap.vao || + vao.GetLifetimeId() != snap.vaoLifetimeId || + vao.GetConfigVersion() != snap.vaoConfigVersion); +#else const Bool vaoMoved = static_cast(&vao) != snap.vao || vao.GetLifetimeId() != snap.vaoLifetimeId || vao.GetConfigVersion() != snap.vaoConfigVersion; +#endif const auto& drawFbo = MGB_CTX->GetFramebufferBindingSlot(FramebufferTarget::Draw).GetBoundObject(); if (static_cast(drawFbo.get()) != snap.drawFbo || @@ -6538,6 +6599,9 @@ void main() { snap.bindGeneration = bindGeneration; snap.vao = static_cast(&vao); snap.vaoLifetimeId = vao.GetLifetimeId(); +#if MOBILEGL_PIPE_PUSH + snap.vaoHandle = ResolveVaoHandle(vao); +#endif snap.vaoConfigVersion = vao.GetConfigVersion(); snap.vaoLayoutHash = vaoLayoutHash; snap.pipeline = pipeline; @@ -7119,6 +7183,9 @@ void main() { snap.programVersion = program.GetBackendStateVersion(); snap.vao = &vao; snap.vaoLifetimeId = vao.GetLifetimeId(); +#if MOBILEGL_PIPE_PUSH + snap.vaoHandle = ResolveVaoHandle(vao); +#endif snap.vaoConfigVersion = vao.GetConfigVersion(); snap.drawFbo = drawFbo.get(); snap.drawFboLifetimeId = drawFbo->GetLifetimeId(); diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.h b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.h index 46a8a162..f992b9e5 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.h +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.h @@ -1045,6 +1045,13 @@ namespace MobileGL::MG_Backend::DirectVulkan { // common shape), and "the VAO did not move" would then skip the layout // re-resolve for a different VAO. Uint64 vaoLifetimeId = 0; +#if MOBILEGL_PIPE_PUSH + // P2 D12.4: the handle arm's answer to the same question, and one compare rather + // than the pair above. Kept BESIDE them rather than replacing them because the + // pre-handle arm is still compiled (MOBILEGL_PIPE_LEGACY_MEMOS) and this snapshot + // is a value struct, not a wire type. + MG_Pipe::MGPipeHandle vaoHandle = MG_Pipe::kMGPipeNullHandle; +#endif Uint32 vaoConfigVersion = 0; const void* drawFbo = nullptr; // Never-reused lifetime id beside the raw pointer + Uint16 version: a @@ -1319,6 +1326,13 @@ namespace MobileGL::MG_Backend::DirectVulkan { // - bindings revalidates per draw exactly as before (frame serial, content // hash, per-binding live buffer pointers and slice epochs). struct alignas(64) VaoDrawMemo { +#if MOBILEGL_PIPE_PUSH + // P2 D12.4: the handle arm's key, and the ONLY key it needs. {slot, gen} is an + // identity, so the pointer-plus-lifetime-id pair below stops being a key here; + // the slot also picks the table entry, so the address hash and the two-way probe + // go with it. Null in an entry that has never been claimed. + MG_Pipe::MGPipeHandle vaoHandle = MG_Pipe::kMGPipeNullHandle; +#endif const MG_State::GLState::VertexArrayObject* vaoKey = nullptr; // The VAO's never-reused lifetime id, checked alongside vaoKey. The pointer // ALONE is not an identity: a deleted VAO's heap address is handed straight @@ -1347,6 +1361,32 @@ namespace MobileGL::MG_Backend::DirectVulkan { // (m_currentDrawResolvedEntry) relies on. static constexpr Uint32 kVaoDrawMemoSlotCount = 2048; // power of two Vector m_vaoDrawMemoTable; +#if MOBILEGL_PIPE_PUSH + // One-entry memo in front of the slot allocator's lifetimeId -> handle map (P2 + // D12.4). Acquiring a handle is a hash probe, and LookupVaoDrawMemo runs per draw, so + // the arm would otherwise have swapped one probe (the address hash it deletes) for + // another. A run of draws over one VAO - the common intra-batch shape - pays a single + // Uint64 compare instead. + // + // A lifetime id is never reused, so a hit can only ever be this same object; the + // valid flag exists rather than a zero sentinel because nothing promises the frontend + // counter starts above zero. + Uint64 m_lastVaoHandleLifetimeId = 0; + MG_Pipe::MGPipeHandle m_lastVaoHandle = MG_Pipe::kMGPipeNullHandle; + Bool m_lastVaoHandleValid = false; + MG_Pipe::MGPipeHandle ResolveVaoHandle(const MG_State::GLState::VertexArrayObject& vao) { + const Uint64 lifetimeId = vao.GetLifetimeId(); + if (m_lastVaoHandleValid && m_lastVaoHandleLifetimeId == lifetimeId) { + return m_lastVaoHandle; + } + const MG_Pipe::MGPipeHandle handle = + MagmaPipeHandleOf(MG_Pipe::MGPipeKind::VertexElementsCso, lifetimeId); + m_lastVaoHandleLifetimeId = lifetimeId; + m_lastVaoHandle = handle; + m_lastVaoHandleValid = true; + return handle; + } +#endif // Finds the slot holding `vao`, or recycles the older of its two candidate // slots into an empty memo keyed on `vao`. Never returns null. VaoDrawMemo* LookupVaoDrawMemo(const MG_State::GLState::VertexArrayObject* vao);