From c8a5abe5812ab19a54d2cab22e2f93d1dc4899b0 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Thu, 8 Oct 2026 12:31:57 -0400 Subject: [PATCH] [Perf] (pipe): a vertex-elements record identical to the one its VAO already published is not re-created (FCL Espryt, shadow off: host calls 3724 -> 1228/frame). --- MobileGL/Config.h | 4 ++ MobileGL/ConfigLoader.cpp | 1 + MobileGL/MG_Impl/Pipe/VertexInputEmit.h | 55 ++++++++++++++----- .../Scenarios/VaoShadowScenario.cpp | 54 ++++++++++++++++++ 4 files changed, 101 insertions(+), 13 deletions(-) diff --git a/MobileGL/Config.h b/MobileGL/Config.h index 239a4796..043187e1 100644 --- a/MobileGL/Config.h +++ b/MobileGL/Config.h @@ -144,6 +144,10 @@ namespace MobileGL::MG_Config { // pointer to the driver VAO only when they differ from what that VAO already holds (a // shadow on the VAO twin). Auto is ON; ForceOff re-emits the whole configuration per walk. QuirkOverride EsprytVaoShadow = QuirkOverride::Auto; + // MOBILEGL_PIPE_VERTEX_ELEMENTS_DEDUP: a VAO configuration change whose vertex-elements + // record is byte-identical to the one its handle last published (a glBindVertexBuffer + // that only changed the buffer) re-publishes nothing. Auto is ON; ForceOff re-creates. + QuirkOverride PipeVertexElementsDedup = QuirkOverride::Auto; // MOBILEGL_ADVERTISE_FP64: add GL_ARB_gpu_shader_fp64 to the advertised extension // string. `double` in a shader always WORKS - it is narrowed to 32 bits before any // module reaches a backend (ShaderTranspiler::DemoteFloat64Pass) - but the extension diff --git a/MobileGL/ConfigLoader.cpp b/MobileGL/ConfigLoader.cpp index 3368c5aa..ae6002f1 100644 --- a/MobileGL/ConfigLoader.cpp +++ b/MobileGL/ConfigLoader.cpp @@ -373,6 +373,7 @@ namespace MobileGL::MG_ConfigLoader { features.MagmaVertexConversionCache = QueryEnvQuirkOverride("MOBILEGL_MAGMA_VERTEX_CONVERSION_CACHE"); features.MagmaWriteRename = QueryEnvQuirkOverride("MOBILEGL_MAGMA_WRITE_RENAME"); features.EsprytVaoShadow = QueryEnvQuirkOverride("MOBILEGL_ESPRYT_VAO_SHADOW"); + features.PipeVertexElementsDedup = QueryEnvQuirkOverride("MOBILEGL_PIPE_VERTEX_ELEMENTS_DEDUP"); features.AdvertiseFp64 = QueryEnvFlag("MOBILEGL_ADVERTISE_FP64"); features.MagmaR11G11B10FFallback = QueryEnvFlag("MOBILEGL_MAGMA_R11G11B10F_FALLBACK"); features.MagmaAllowInvalidSpirv = QueryEnvFlag("MOBILEGL_MAGMA_ALLOW_INVALID_SPIRV"); diff --git a/MobileGL/MG_Impl/Pipe/VertexInputEmit.h b/MobileGL/MG_Impl/Pipe/VertexInputEmit.h index 6db0d11c..867ca8b1 100644 --- a/MobileGL/MG_Impl/Pipe/VertexInputEmit.h +++ b/MobileGL/MG_Impl/Pipe/VertexInputEmit.h @@ -8,6 +8,7 @@ #pragma once #include +#include // The CLIENT side of P3a's vertex-input family (brief D-G, D-H, D-I): the bound VAO's // format as create/bind_vertex_elements, its buffers as set_vertex_buffers with an explicit @@ -182,10 +183,25 @@ namespace MobileGL::MG_Pipe { const Bool configMoved = !latch.Published || latch.ConfigVersion != configVersion || latch.Gen != handle.Gen; if (configMoved) { - bytes += EmitCreate(*vao, handle, latch, configVersion); - // The applier drops the record's kept bindings at the same create. - memo.StoredBuffersValid = false; - memo.StoredIndexValid = false; + BuildBlob(*vao); + // P15: the version moves on every glBindVertexBuffer, but the record only names + // formats and offsets. An identical record for the same handle and generation is + // 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 && + latch.PublishedBlob.size() == kBlobBytes && + std::memcmp(latch.PublishedBlob.data(), m_blob.data(), kBlobBytes) == 0; + if (unchanged) { + latch.ConfigVersion = configVersion; + ++m_createsDeduped; + } else { + bytes += EmitCreate(handle, latch, configVersion); + // The applier drops the record's kept bindings at the same create. + memo.StoredBuffersValid = false; + memo.StoredIndexValid = false; + } } if (lifetimeId != m_boundLifetimeId || m_boundHandle != handle) { MGPipeRouteBindVertexElements(HandleOnly(handle)); @@ -385,6 +401,7 @@ namespace MobileGL::MG_Pipe { const MGPIndexBuffer& LastIndexBuffer() const { return m_lastIndex; } MGPipeHandle BoundHandle() const { return m_boundHandle; } Uint64 CreateCount() const { return m_creates; } + Uint64 DedupedCreateCount() const { return m_createsDeduped; } Uint64 BindCount() const { return m_binds; } Uint64 VertexBufferSetCount() const { return m_bufferSets; } Uint64 IndexBufferSetCount() const { return m_indexSets; } @@ -449,7 +466,7 @@ namespace MobileGL::MG_Pipe { m_serverIndexKnown = false; } - void ResetCounters() { m_creates = m_binds = m_bufferSets = m_indexSets = 0; } + void ResetCounters() { m_creates = m_createsDeduped = m_binds = m_bufferSets = m_indexSets = 0; } private: // P14: what EmitVertexElements / EmitVertexBuffers / EmitIndexBuffer resolve from a VAO, @@ -568,6 +585,11 @@ namespace MobileGL::MG_Pipe { // RecordIsPublished. Bool RecordLive = false; Uint32 RecordGen = 0; + // P15: the blob this handle last published, so a configuration version that moved + // without moving the record (a glBindVertexBuffer that only changed the buffer, + // whose identity rides set_vertex_buffers) re-publishes nothing. A full copy, not a + // hash: the compare decides what the server holds. + Vector PublishedBlob; }; static MGPHandleOnly HandleOnly(MGPipeHandle handle) { @@ -577,12 +599,15 @@ namespace MobileGL::MG_Pipe { return only; } - Uint64 EmitCreate(const VertexArrayObject& vao, MGPipeHandle handle, Latch& latch, Uint32 configVersion) { - // ALL 32 OF EACH, deliberately. The record DECLARES both counts and the applier - // refuses one whose counts do not describe its own blob, so a self-describing - // record is the cheap shape - and G6 is stated over all 32 slots, which a - // truncated set could not answer. It rides create_vertex_elements only, i.e. - // once per configuration change, never per draw. + static constexpr SizeT kAttribBytes = kAttribs * sizeof(MGPVertexAttribWire); + static constexpr SizeT kBindingBytes = kBindings * sizeof(MGPVertexBindingPointWire); + static constexpr SizeT kBlobBytes = kAttribBytes + kBindingBytes; + + // The record's blob for this configuration, into m_attributes / m_bindingPoints / m_blob. + // ALL 32 OF EACH, deliberately. The record DECLARES both counts and the applier refuses + // one whose counts do not describe its own blob, so a self-describing record is the cheap + // shape - and G6 is stated over all 32 slots, which a truncated set could not answer. + void BuildBlob(const VertexArrayObject& vao) { for (SizeT i = 0; i < kAttribs; ++i) { m_attributes[i] = MGPipeBuildVertexAttribWire(vao.GetAttribute(static_cast(i)), vao.GetAttributeBindingIndex(static_cast(i))); @@ -595,11 +620,13 @@ namespace MobileGL::MG_Pipe { if (!vao.GetBindingPoint(static_cast(i)).Buffer) m_bindingPoints[i].Offset = 0; } // Attributes first, then binding points, both ascending and contiguous. - constexpr SizeT kAttribBytes = kAttribs * sizeof(MGPVertexAttribWire); - constexpr SizeT kBindingBytes = kBindings * sizeof(MGPVertexBindingPointWire); std::memcpy(m_blob.data(), m_attributes.data(), kAttribBytes); std::memcpy(m_blob.data() + kAttribBytes, m_bindingPoints.data(), kBindingBytes); + } + // create_vertex_elements for the blob BuildBlob just made. It rides only a configuration + // change whose record differs from the one this handle published, never per draw. + Uint64 EmitCreate(MGPipeHandle handle, Latch& latch, Uint32 configVersion) { m_lastElements = MGPVertexElements{}; m_lastElements.Cso = handle; m_lastElements.AttributeCount = static_cast(kAttribs); @@ -616,6 +643,7 @@ namespace MobileGL::MG_Pipe { // only thing that makes delete_vertex_elements a legal call for this handle. latch.RecordLive = true; latch.RecordGen = handle.Gen; + latch.PublishedBlob.assign(m_blob.begin(), m_blob.begin() + kBlobBytes); return sizeof(MGPVertexElements) + kAttribBytes + kBindingBytes; } @@ -634,6 +662,7 @@ namespace MobileGL::MG_Pipe { Uint64 m_boundLifetimeId = 0; Uint64 m_creates = 0; + Uint64 m_createsDeduped = 0; Uint64 m_binds = 0; Uint64 m_bufferSets = 0; Uint64 m_indexSets = 0; diff --git a/MobileGL/MG_IntegrationTest/Scenarios/VaoShadowScenario.cpp b/MobileGL/MG_IntegrationTest/Scenarios/VaoShadowScenario.cpp index ab475c5b..679bf44e 100644 --- a/MobileGL/MG_IntegrationTest/Scenarios/VaoShadowScenario.cpp +++ b/MobileGL/MG_IntegrationTest/Scenarios/VaoShadowScenario.cpp @@ -237,4 +237,58 @@ void main() { o0 = vColor; } Destroy(s); } + // The binding-point API, as a chunk renderer uses it: one format, glBindVertexBuffer per draw. + // A buffer-only switch leaves the vertex-elements record byte-identical, so the client does + // not re-publish it (MOBILEGL_PIPE_VERTEX_ELEMENTS_DEDUP); the draw must still read the new + // buffer. An offset change does change the record and must reach the server. + TEST_F(VaoShadowScenario, BindVertexBufferSwitchesAndOffsetsAreReadByEachDraw) { + if (!Ready()) return; + std::string error; + Scene s = MakeScene(&error); + ASSERT_NE(s.program, 0u) << error; + // Interleaved {x, y, r, g, b, a}: buffer A a red left quad, buffer B a green right quad, + // buffer C two quads - the first blue on the left, the second (offset 4 vertices) a + // white right quad. + const auto interleave = [](const std::vector& pos, const std::vector& col) { + std::vector v; + for (int i = 0; i < 4; ++i) { + v.insert(v.end(), {pos[i * 2], pos[i * 2 + 1]}); + v.insert(v.end(), col.begin() + i * 4, col.begin() + i * 4 + 4); + } + return v; + }; + const GLuint a = Buffer(interleave(Quad(0.0f, 0.5f), Colors(1.0f, 0.0f, 0.0f))); + const GLuint b = Buffer(interleave(Quad(0.5f, 1.0f), Colors(0.0f, 1.0f, 0.0f))); + std::vector two = interleave(Quad(0.0f, 0.5f), Colors(0.0f, 0.0f, 1.0f)); + const std::vector second = interleave(Quad(0.5f, 1.0f), Colors(1.0f, 1.0f, 1.0f)); + two.insert(two.end(), second.begin(), second.end()); + const GLuint c = Buffer(two); + constexpr GLsizei kStride = 6 * sizeof(float); + glVertexAttribFormat(0, 2, GL_FLOAT, GL_FALSE, 0); + glVertexAttribFormat(1, 4, GL_FLOAT, GL_FALSE, 2 * sizeof(float)); + glVertexAttribBinding(0, 0); + glVertexAttribBinding(1, 0); + glEnableVertexAttribArray(0); + glEnableVertexAttribArray(1); + + glBindVertexBuffer(0, a, 0, kStride); + glDrawArrays(GL_TRIANGLE_STRIP, 0, 4); // red left + glBindVertexBuffer(0, b, 0, kStride); + glDrawArrays(GL_TRIANGLE_STRIP, 0, 4); // green right + EXPECT_TRUE(Near(At(kSize / 4, kSize / 2), 255, 0, 0)) << "buffer A"; + EXPECT_TRUE(Near(At(kSize * 3 / 4, kSize / 2), 0, 255, 0)) << "buffer B, same format"; + + glClear(GL_COLOR_BUFFER_BIT); + glBindVertexBuffer(0, c, 0, kStride); + glDrawArrays(GL_TRIANGLE_STRIP, 0, 4); // blue left (offset 0) + glBindVertexBuffer(0, c, 4 * kStride, kStride); + glDrawArrays(GL_TRIANGLE_STRIP, 0, 4); // white right (offset 4 vertices) + EXPECT_EQ(FirstGLError(), 0u); + EXPECT_TRUE(Near(At(kSize / 4, kSize / 2), 0, 0, 255)) << "buffer C at offset 0"; + EXPECT_TRUE(Near(At(kSize * 3 / 4, kSize / 2), 255, 255, 255)) << "buffer C at offset 4 vertices"; + GLuint bufs[] = {a, b, c}; + glDeleteBuffers(3, bufs); + Destroy(s); + } + } // namespace MGITest