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

@@ -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) {

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<Bool>(m_attributes[index].Buffer) != static_cast<Bool>(buffer);
         m_attributes[index].Buffer = buffer;
-        BumpAttributeBufferVersion(index);
+        BumpAttributeBufferVersion(index, presenceChanged);
     }
 
     BindingSlot<BufferObject>& VertexArrayObject::GetIndexBufferBindingSlot() {
@@ -249,8 +250,9 @@ namespace MobileGL::MG_State::GLState {
         }
 
         if (attr.Buffer != binding.Buffer) {
+            const Bool presenceChanged = static_cast<Bool>(attr.Buffer) != static_cast<Bool>(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);
     }
 

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<Bool, MAX_VERTEX_ATTRIBS> m_attributeUsesBindingModel = {};
 
                 Uint32 m_configVersion = 0;
+                Uint32 m_elementsVersion = 0;
             };
         } // namespace GLState
     } // namespace MG_State

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<Uint> 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 {