From 657722ff573b97ac6f72984f2f0600c6dccb164d Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Thu, 8 Oct 2026 16:11:11 -0400 Subject: [PATCH] Pipe: a buffer-only glBindVertexBuffer no longer rebuilds and compares the vertex-elements blob (VAO elements version; red-once: BindVertexBufferSwitchesAndOffsets fails on both backends). --- MobileGL/MG_Impl/Pipe/VertexInputEmit.h | 9 ++-- .../VertexArrayState/VertexArrayObject.cpp | 13 ++++-- .../VertexArrayState/VertexArrayObject.h | 8 +++- .../MG_Test/VertexArray/VertexArrayTest.cpp | 41 +++++++++++++++++++ 4 files changed, 64 insertions(+), 7 deletions(-) diff --git a/MobileGL/MG_Impl/Pipe/VertexInputEmit.h b/MobileGL/MG_Impl/Pipe/VertexInputEmit.h index 867ca8b1..8828c704 100644 --- a/MobileGL/MG_Impl/Pipe/VertexInputEmit.h +++ b/MobileGL/MG_Impl/Pipe/VertexInputEmit.h @@ -168,7 +168,11 @@ namespace MobileGL::MG_Pipe { } const Uint64 lifetimeId = vao->GetLifetimeId(); - const Uint32 configVersion = vao->GetConfigVersion(); + // P15: keyed on the elements version, which a buffer-only glBindVertexBuffer does not + // move (FCL MC 1.21.5 does one per chunk draw), so such a draw skips the blob build + // and compare below entirely. The dedup's kill switch also restores the old key. + const Bool dedup = MG_Config::Features.PipeVertexElementsDedup != MG_Config::QuirkOverride::ForceOff; + const Uint32 configVersion = dedup ? vao->GetElementsVersion() : vao->GetConfigVersion(); // The VAO's handle is the allocator's for the object's whole life (only an allocator // reset forgets it), so it is resolved once per VAO rather than per draw (VaoMemo). VaoMemo& memo = MemoFor(*vao); @@ -189,8 +193,7 @@ namespace MobileGL::MG_Pipe { // already the server's: re-creating it moved its content serial for nothing, which // sent the backend through a whole attribute walk per draw (FCL MC 1.21.5: one per // chunk draw). Kill switch MOBILEGL_PIPE_VERTEX_ELEMENTS_DEDUP=0. - const Bool unchanged = MG_Config::Features.PipeVertexElementsDedup != MG_Config::QuirkOverride::ForceOff && - latch.Published && latch.Gen == handle.Gen && + const Bool unchanged = dedup && latch.Published && latch.Gen == handle.Gen && latch.PublishedBlob.size() == kBlobBytes && std::memcmp(latch.PublishedBlob.data(), m_blob.data(), kBlobBytes) == 0; if (unchanged) { diff --git a/MobileGL/MG_State/GLState/VertexArrayState/VertexArrayObject.cpp b/MobileGL/MG_State/GLState/VertexArrayState/VertexArrayObject.cpp index 215fc436..8bd96b15 100644 --- a/MobileGL/MG_State/GLState/VertexArrayState/VertexArrayObject.cpp +++ b/MobileGL/MG_State/GLState/VertexArrayState/VertexArrayObject.cpp @@ -160,8 +160,9 @@ namespace MobileGL::MG_State::GLState { if (m_attributes[index].Buffer == buffer) return; + const Bool presenceChanged = static_cast(m_attributes[index].Buffer) != static_cast(buffer); m_attributes[index].Buffer = buffer; - BumpAttributeBufferVersion(index); + BumpAttributeBufferVersion(index, presenceChanged); } BindingSlot& VertexArrayObject::GetIndexBufferBindingSlot() { @@ -249,8 +250,9 @@ namespace MobileGL::MG_State::GLState { } if (attr.Buffer != binding.Buffer) { + const Bool presenceChanged = static_cast(attr.Buffer) != static_cast(binding.Buffer); attr.Buffer = binding.Buffer; - BumpAttributeBufferVersion(attribIndex); + BumpAttributeBufferVersion(attribIndex, presenceChanged); } } @@ -322,13 +324,17 @@ namespace MobileGL::MG_State::GLState { if (index >= MAX_VERTEX_ATTRIBS) return; ++m_attributeVersions[index].FormatVersion; ++m_configVersion; + ++m_elementsVersion; MGP_NOTE_AGGREGATE(VaoAttribute); } - void VertexArrayObject::BumpAttributeBufferVersion(Uint index) { + void VertexArrayObject::BumpAttributeBufferVersion(Uint index, Bool presenceChanged) { if (index >= MAX_VERTEX_ATTRIBS) return; ++m_attributeVersions[index].BufferVersion; ++m_configVersion; + // The record zeroes an attribute's offset when it has no buffer, so presence is part + // of what it says; which buffer is not. + if (presenceChanged) ++m_elementsVersion; MGP_NOTE_AGGREGATE(VaoAttribute); } @@ -336,6 +342,7 @@ namespace MobileGL::MG_State::GLState { if (index >= MAX_VERTEX_ATTRIBS) return; ++m_attributeVersions[index].SwitchVersion; ++m_configVersion; + ++m_elementsVersion; MGP_NOTE_AGGREGATE(VaoAttribute); } diff --git a/MobileGL/MG_State/GLState/VertexArrayState/VertexArrayObject.h b/MobileGL/MG_State/GLState/VertexArrayState/VertexArrayObject.h index d6b02b89..182de730 100644 --- a/MobileGL/MG_State/GLState/VertexArrayState/VertexArrayObject.h +++ b/MobileGL/MG_State/GLState/VertexArrayState/VertexArrayObject.h @@ -103,11 +103,16 @@ namespace MobileGL { // Aggregate of every per-attribute version bump; lets backends detect // "any vertex-input state changed" with one compare. Uint32 GetConfigVersion() const { return m_configVersion; } + // P15: moves with every bump EXCEPT a buffer change that keeps the attribute's + // buffer present (or absent) - a glBindVertexBuffer that only swaps the buffer. + // That is everything a vertex-elements record (formats, offsets, strides, + // enables, divisors) can see, so the record's emitter keys on it. + Uint32 GetElementsVersion() const { return m_elementsVersion; } private: void BumpAttributeFormatVersion(Uint index); - void BumpAttributeBufferVersion(Uint index); + void BumpAttributeBufferVersion(Uint index, Bool presenceChanged); void BumpAttributeSwitchVersion(Uint index); void ResolveAttributeFromBinding(Uint attribIndex); // Re-resolve every attribute currently pointed at `bindingIndex`. `adopt` turns @@ -143,6 +148,7 @@ namespace MobileGL { Array m_attributeUsesBindingModel = {}; Uint32 m_configVersion = 0; + Uint32 m_elementsVersion = 0; }; } // namespace GLState } // namespace MG_State diff --git a/MobileGL/MG_Test/VertexArray/VertexArrayTest.cpp b/MobileGL/MG_Test/VertexArray/VertexArrayTest.cpp index 6fb42058..050e7e3b 100644 --- a/MobileGL/MG_Test/VertexArray/VertexArrayTest.cpp +++ b/MobileGL/MG_Test/VertexArray/VertexArrayTest.cpp @@ -361,6 +361,47 @@ TEST_F(VertexArrayTest, DefaultAttributeBindingIsIdentityAcrossFullCapacity) { EXPECT_EQ(vao->GetAttribute(0).Buffer, nullptr); } +// P15: the elements version (what keys the vertex-elements record) moves on everything that +// record carries and on nothing else. A glBindVertexBuffer that only swaps the buffer moves the +// config version but not the elements version; offset, stride, presence and enables move both. +TEST_F(VertexArrayTest, ElementsVersionIgnoresABufferOnlySwapAndTracksEverythingTheRecordCarries) { + Vector vaoNames; + MG_State::pGLContext->GenVertexArrayNames(1, vaoNames); + auto vao = MG_State::pGLContext->CreateVertexArrayObject(vaoNames[0]); + MG_State::pGLContext->BindVertexArray(vaoNames[0]); + auto vboA = CreateTestVBO(); + auto vboB = CreateTestVBO(); + + vao->SetAttributeFormatSeparate(0, 3, DataType::Float32, false, false, 0); + vao->SetBindingBuffer(0, vboA, 0, 12); + vao->EnableAttribute(0); + + Uint32 config = vao->GetConfigVersion(), elements = vao->GetElementsVersion(); + const auto expectMoved = [&](Bool configMoves, Bool elementsMove, const char* what) { + EXPECT_EQ(vao->GetConfigVersion() != config, configMoves) << what; + EXPECT_EQ(vao->GetElementsVersion() != elements, elementsMove) << what; + config = vao->GetConfigVersion(); + elements = vao->GetElementsVersion(); + }; + + vao->SetBindingBuffer(0, vboB, 0, 12); + expectMoved(true, false, "buffer-only swap"); + vao->SetBindingBuffer(0, vboB, 0, 12); + expectMoved(false, false, "identical rebind"); + vao->SetBindingBuffer(0, vboA, 16, 12); + expectMoved(true, true, "offset change"); + vao->SetBindingBuffer(0, vboA, 16, 24); + expectMoved(true, true, "stride change"); + vao->SetBindingBuffer(0, nullptr, 16, 24); + expectMoved(true, true, "buffer -> none"); + vao->SetBindingBuffer(0, vboB, 16, 24); + expectMoved(true, true, "none -> buffer"); + vao->DisableAttribute(0); + expectMoved(true, true, "disable"); + vao->SetBindingDivisor(0, 1); + expectMoved(true, true, "divisor"); +} + using namespace MobileGL::MG_Impl::GLImpl; class GeneralVertexArrayTest : public ::testing::Test {