diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.cpp b/MobileGL/MG_Backend/DirectGLES/Managers.cpp index f18f0f50..e0012f3c 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.cpp +++ b/MobileGL/MG_Backend/DirectGLES/Managers.cpp @@ -1081,6 +1081,9 @@ namespace MobileGL::MG_Backend::DirectGLES { if (resource->id != 0 && CanTouchGLNow() && resource->contextGeneration == g_bufferContextGeneration) { NoteBufferIdDeleted(resource->id); + // Frontend VAO bindings survive respecification; force their + // backend twins to bind the replacement buffer name. + ++g_bufferBackendIdGeneration; g_GLESFuncs.glDeleteBuffers(1, &resource->id); resource->id = 0; resource->immutableStorage = false; @@ -1350,7 +1353,7 @@ namespace MobileGL::MG_Backend::DirectGLES { } // See the declaration: re-mints of a live resource's driver id. Written only on - // the context thread (both re-mint sites run there), read only by the VAO sync. + // the context thread (all re-mint sites run there), read only by the VAO sync. Uint64 g_bufferBackendIdGeneration = 0; void RegisterBufferBackendOps() { diff --git a/MobileGL/MG_IntegrationTest/Scenarios/LargeArenaAdoptionScenario.cpp b/MobileGL/MG_IntegrationTest/Scenarios/LargeArenaAdoptionScenario.cpp index 6aa0d23d..10bee4ba 100644 --- a/MobileGL/MG_IntegrationTest/Scenarios/LargeArenaAdoptionScenario.cpp +++ b/MobileGL/MG_IntegrationTest/Scenarios/LargeArenaAdoptionScenario.cpp @@ -102,6 +102,12 @@ void main() { word = 0xC0FFEEu; } // The NULL-data definition is the adoption point (and Minecraft's // arena-creation idiom). glBufferData(GL_ARRAY_BUFFER, kArenaBytes, nullptr, GL_DYNAMIC_DRAW); + ConfigureVertexArray(m_vao); + } + + void ConfigureVertexArray(GLuint vao) { + glBindVertexArray(vao); + glBindBuffer(GL_ARRAY_BUFFER, m_arena); glVertexAttribPointer(0, 2, GL_FLOAT, GL_FALSE, sizeof(Vertex), reinterpret_cast(kVertexOffset)); glVertexAttribPointer(1, 3, GL_FLOAT, GL_FALSE, sizeof(Vertex), @@ -173,12 +179,12 @@ void main() { word = 0xC0FFEEu; } GLsizeiptr(vertices.size() * sizeof(Vertex)), vertices.data()); } - void DrawQuad() { + void DrawQuad(GLuint vao = 0) { glViewport(0, 0, Gl().Width(), Gl().Height()); glClearColor(0.f, 0.f, 0.f, 1.f); glClear(GL_COLOR_BUFFER_BIT); glUseProgram(m_program); - glBindVertexArray(m_vao); + glBindVertexArray(vao != 0 ? vao : m_vao); glDrawArrays(GL_TRIANGLES, 0, 6); } @@ -222,6 +228,97 @@ void main() { word = 0xC0FFEEu; } EXPECT_LT(px[0], 50) << "the draw still shows the previous frame's bytes"; } + // Respecifying a frontend buffer preserves its VAO attachments even when the + // backend replaces the adopted store's GL name. Keep every attribute binding + // unchanged so a stale backend VAO cannot be repaired by a frontend rebind. + TEST_F(LargeArenaAdoptionScenario, RespecifiedVertexArenaKeepsVaoBindings) { + if (!Ready() || IsSkipped()) return; + + UploadQuad(1.f, 0.f, 0.f); + DrawQuad(); + ASSERT_GT(CenterPixel()[0], 200); + ASSERT_EQ(FirstGLError(), 0u); + + GLuint otherVao = 0; + glGenVertexArrays(1, &otherVao); + ConfigureVertexArray(otherVao); + DrawQuad(otherVao); + EXPECT_GT(CenterPixel()[0], 200); + EXPECT_EQ(FirstGLError(), 0u); + + constexpr std::array sizes = { + kArenaBytes, kArenaBytes + 4096, kArenaBytes - 4096, + }; + constexpr std::array, 3> colors = {{ + {0.f, 1.f, 0.f}, {0.f, 0.f, 1.f}, {1.f, 0.f, 0.f}, + }}; + for (std::size_t i = 0; i < sizes.size(); ++i) { + SCOPED_TRACE(sizes[i]); + glBindBuffer(GL_ARRAY_BUFFER, m_arena); + glBufferData(GL_ARRAY_BUFFER, sizes[i], nullptr, GL_DYNAMIC_DRAW); + UploadQuad(colors[i][0], colors[i][1], colors[i][2]); + // The unbound VAO can retain the deleted store; the current VAO's + // attachments can be cleared by deletion. Both must be repaired. + for (GLuint vao : {m_vao, otherVao}) { + SCOPED_TRACE(vao); + DrawQuad(vao); + const auto px = CenterPixel(); + EXPECT_EQ(FirstGLError(), 0u); + for (std::size_t channel = 0; channel < 3; ++channel) { + if (colors[i][channel] != 0.f) { + EXPECT_GT(px[channel], 200) << "VAO did not fetch the replacement vertex store"; + } else { + EXPECT_LT(px[channel], 50) << "VAO still fetched the previous vertex store"; + } + } + } + } + glDeleteVertexArrays(1, &otherVao); + } + + TEST_F(LargeArenaAdoptionScenario, RespecifiedIndexArenaKeepsVaoBinding) { + if (!Ready() || IsSkipped()) return; + + auto vertices = QuadVertices(1.f, 0.f, 0.f); + const auto green = QuadVertices(0.f, 1.f, 0.f); + vertices.insert(vertices.end(), green.begin(), green.end()); + glBindBuffer(GL_ARRAY_BUFFER, m_arena); + glBufferSubData(GL_ARRAY_BUFFER, kVertexOffset, + GLsizeiptr(vertices.size() * sizeof(Vertex)), vertices.data()); + + GLuint indices = 0; + glGenBuffers(1, &indices); + glBindVertexArray(m_vao); + glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, indices); + // Redefine through COPY_WRITE_BUFFER so the element binding slot never + // changes. The small final store also exercises returning to shadow storage. + glBindBuffer(GL_COPY_WRITE_BUFFER, indices); + constexpr std::array sizes = { + kArenaBytes, kArenaBytes, kArenaBytes + 4096, 4096, + }; + for (std::size_t i = 0; i < sizes.size(); ++i) { + SCOPED_TRACE(sizes[i]); + const GLuint first = (i % 2) == 0 ? 0u : 6u; + const std::array elements = { + first, first + 1, first + 2, first + 3, first + 4, first + 5, + }; + glBufferData(GL_COPY_WRITE_BUFFER, sizes[i], nullptr, GL_DYNAMIC_DRAW); + glBufferSubData(GL_COPY_WRITE_BUFFER, 0, sizeof(elements), elements.data()); + glViewport(0, 0, Gl().Width(), Gl().Height()); + glClearColor(0.f, 0.f, 0.f, 1.f); + glClear(GL_COLOR_BUFFER_BIT); + glUseProgram(m_program); + glDrawElements(GL_TRIANGLES, 6, GL_UNSIGNED_INT, nullptr); + const auto px = CenterPixel(); + EXPECT_EQ(FirstGLError(), 0u); + EXPECT_GT(px[first == 0 ? 0 : 1], 200) << "VAO did not fetch the replacement index store"; + EXPECT_LT(px[first == 0 ? 1 : 0], 50) << "VAO still fetched the previous index store"; + } + glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, 0); + glBindBuffer(GL_COPY_WRITE_BUFFER, 0); + glDeleteBuffers(1, &indices); + } + // The shadow IS the mapping: a readback straight after a CPU write must hand // back exactly those bytes. TEST_F(LargeArenaAdoptionScenario, ReadbackSeesTheLatestCpuWrite) {