diff --git a/MobileGL/MG_State/GLState/VertexArrayState/VertexArrayObject.h b/MobileGL/MG_State/GLState/VertexArrayState/VertexArrayObject.h index f0b67403..91d4317d 100644 --- a/MobileGL/MG_State/GLState/VertexArrayState/VertexArrayObject.h +++ b/MobileGL/MG_State/GLState/VertexArrayState/VertexArrayObject.h @@ -149,6 +149,25 @@ namespace MobileGL { m_backendStateMemoVersion = m_configVersion; } + // Backend-owned aux memo: two opaque VALUE words (no pointee, so unlike the + // state memo above they need no eviction-epoch guard), valid while the config + // version matches. They live next to m_configVersion, which every per-draw + // path already loads, so a backend can re-read small derived facts about this + // VAO's configuration (e.g. a layout hash and attribute masks) without + // chasing into its own cache's heap entry - that chase is a guaranteed cache + // miss when an app cycles hundreds of VAOs per frame. + Bool GetBackendAuxMemo(Uint64& outAux0, Uint64& outAux1) const { + if (m_backendAuxMemoVersion != m_configVersion) return false; + outAux0 = m_backendAuxMemo0; + outAux1 = m_backendAuxMemo1; + return true; + } + void SetBackendAuxMemo(Uint64 aux0, Uint64 aux1) const { + m_backendAuxMemo0 = aux0; + m_backendAuxMemo1 = aux1; + m_backendAuxMemoVersion = m_configVersion; + } + private: void BumpAttributeFormatVersion(Uint index); void BumpAttributeBufferVersion(Uint index); @@ -185,6 +204,9 @@ namespace MobileGL { mutable const void* m_backendStateMemo = nullptr; mutable Uint64 m_backendStateMemoEpoch = 0; mutable Uint32 m_backendStateMemoVersion = ~0u; + mutable Uint64 m_backendAuxMemo0 = 0; + mutable Uint64 m_backendAuxMemo1 = 0; + mutable Uint32 m_backendAuxMemoVersion = ~0u; }; } // namespace GLState } // namespace MG_State diff --git a/MobileGL/MG_State/GLState/VertexArrayState/VertexArrayState.cpp b/MobileGL/MG_State/GLState/VertexArrayState/VertexArrayState.cpp index a8139876..77fe1922 100644 --- a/MobileGL/MG_State/GLState/VertexArrayState/VertexArrayState.cpp +++ b/MobileGL/MG_State/GLState/VertexArrayState/VertexArrayState.cpp @@ -9,20 +9,26 @@ #include "VertexArrayState.h" namespace MobileGL::MG_State::GLState { + namespace { + // Shared "nothing bound" answer for GetBoundVertexArray/GetVertexArrayObject. + // Function-local statics carry a guard check per access; this one is + // constant-initialized and lives on the hot per-draw path. + const SharedPtr kNullVertexArrayObject = nullptr; + } // namespace + VertexArrayState::VertexArrayState() : m_indexGenerator(1024, 1) { // Generate default VAO at index 0, which is not valid in core profile, but still remains for // compatibility reasons. m_indexGenerator.Insert(0); auto defaultVAO = MakeShared(0); m_vertexArrays.push_back(defaultVAO); - m_boundVertexArray = defaultVAO; + m_boundIndex = 0; } const SharedPtr& VertexArrayState::GetVertexArrayObject(Uint index) { if (index >= m_vertexArrays.size()) { // FIXME: report a GL error here - static SharedPtr nullVertexArrayObject = nullptr; - return nullVertexArrayObject; + return kNullVertexArrayObject; } return m_vertexArrays[index]; @@ -34,11 +40,20 @@ namespace MobileGL::MG_State::GLState { } void VertexArrayState::Bind(Uint index) { - const auto& vertexArray = GetVertexArrayObject(index); - // Re-binding the already-current VAO is a per-batch habit of Blaze3D-style renderers; - // skip the shared_ptr store (two atomic refcount ops) when nothing changes. - if (vertexArray == m_boundVertexArray) return; - m_boundVertexArray = vertexArray; + // Per-draw-batch hot path (Blaze3D-style renderers rebind a different VAO before + // every draw): store only the slot index - no SharedPtr copy, no refcount atomics. + // The bound object's lifetime is guaranteed by its slot (see the invariant note on + // m_boundIndex in the header). + if (m_boundDetached) [[unlikely]] { + // A cold path displaced the previously bound object out of its slot; this Bind + // supersedes it, exactly like the old SharedPtr member being overwritten. + m_boundDetached = nullptr; + } + // Match the previous semantics exactly: binding an out-of-range name, or a name + // whose slot holds no object, left the old SharedPtr member null - resolve that + // NOW, so a slot created later does not silently become bound. + m_boundIndex = + (index < m_vertexArrays.size() && m_vertexArrays[index] != nullptr) ? index : kUnboundIndex; } const SharedPtr& VertexArrayState::CreateVertexArrayObject(Uint index) { @@ -48,14 +63,36 @@ namespace MobileGL::MG_State::GLState { m_vertexArrays.resize(index + 1, nullptr); } auto& vao = m_vertexArrays[index]; + if (index == m_boundIndex && vao != nullptr && !m_boundDetached) { + // Replacing the bound slot's live object: keep the OLD object alive and bound + // (that is what the previous SharedPtr member provided) until the next Bind. + // Unreachable through the GL entry points today - bind-time creation only fills + // empty slots and generated names are never in use - but the invariant is + // enforced here, not assumed. + m_boundDetached = std::move(vao); + } vao = MakeShared(index); return vao; } void VertexArrayState::MarkVertexArrayForDeletion(Uint index) { if (m_indexGenerator.IsValid(index)) { - if (m_boundVertexArray && m_boundVertexArray->GetExternalIndex() == index) { - m_boundVertexArray = GetVertexArrayObject(0); + // "Deleting the bound VAO rebinds the default VAO" needs the same answer the old + // SharedPtr compare gave: either the live bound slot is the one being deleted, or + // the bound object is a detached one that carries this external index. + const Bool deletingBound = m_boundDetached + ? m_boundDetached->GetExternalIndex() == index + : (m_boundIndex == index && m_boundIndex != kUnboundIndex); + if (deletingBound) { + m_boundDetached = nullptr; + m_boundIndex = 0; // the default VAO's slot always exists + if (index == 0) { + // Deleting slot 0 while it is bound (unreachable via the GL entry + // points, which filter name 0): the old SharedPtr member kept the + // object alive and bound across the slot null-out below; detach it + // to preserve that. + m_boundDetached = m_vertexArrays[0]; + } } if (ValidateVertexArrayObject(index)) { @@ -76,7 +113,16 @@ namespace MobileGL::MG_State::GLState { } const SharedPtr& VertexArrayState::GetBoundVertexArray() { - return m_boundVertexArray; + // NOTE: like GetVertexArrayObject, the returned reference is a slot reference and + // must not be held across CreateVertexArrayObject (vector growth) - existing + // callers bind/create first and only then take the reference. + if (m_boundDetached) [[unlikely]] { + return m_boundDetached; + } + if (m_boundIndex < m_vertexArrays.size()) { + return m_vertexArrays[m_boundIndex]; + } + return kNullVertexArrayObject; } Vector>& VertexArrayState::GetAllVertexArrays() { diff --git a/MobileGL/MG_State/GLState/VertexArrayState/VertexArrayState.h b/MobileGL/MG_State/GLState/VertexArrayState/VertexArrayState.h index a79b7cba..418813c4 100644 --- a/MobileGL/MG_State/GLState/VertexArrayState/VertexArrayState.h +++ b/MobileGL/MG_State/GLState/VertexArrayState/VertexArrayState.h @@ -29,9 +29,32 @@ namespace MobileGL { Vector>& GetAllVertexArrays(); private: + // "Nothing bound" (an out-of-range or never-created name was bound). Distinct from + // being bound to a live slot so that a slot filled AFTER such a bind does not + // retroactively become the bound VAO. + static constexpr Uint kUnboundIndex = ~static_cast(0); + Vector> m_vertexArrays; IndexGenerator m_indexGenerator; - SharedPtr m_boundVertexArray; + + // The bound VAO is represented as an INDEX into m_vertexArrays, not as an owning + // SharedPtr copy. Chunk-style renderers rebind a different VAO before every draw, + // and the SharedPtr store this replaces cost two atomic refcount ops per bind - + // the single largest line of a vanilla-Minecraft draw profile (the lock-prefixed + // refcount RMWs serialize the store buffer in the middle of command recording). + // + // LIFETIME INVARIANT this relies on (and which the cold paths below enforce + // rather than assume): the object GetBoundVertexArray() refers to is kept alive + // by its own slot in m_vertexArrays. Every path that clears or replaces a slot + // either (a) rebinds index 0 first when it targets the bound slot + // (MarkVertexArrayForDeletion), or (b) detaches the displaced object into + // m_boundDetached (CreateVertexArrayObject), which then owns it and keeps + // GetBoundVertexArray() answering with the OLD object - exactly what the previous + // SharedPtr member did - until the next Bind drops it. + Uint m_boundIndex = 0; + // Cold-path ownership backstop, see above. Null in the steady state; Bind clears + // it (one predictable branch on the hot path). + SharedPtr m_boundDetached; }; } // namespace GLState } // namespace MG_State