diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.cpp b/MobileGL/MG_Backend/DirectGLES/Managers.cpp index aaf44b8b..76d7d494 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.cpp +++ b/MobileGL/MG_Backend/DirectGLES/Managers.cpp @@ -386,6 +386,13 @@ namespace MobileGL::MG_Backend::DirectGLES { // entry would otherwise false-skip the rebind). void ScrubBufferBindingShadowsForId(Uint id); + // Defined next to the same shadow, and the counterpart to the scrub above for a + // buffer whose STORE was re-specified rather than deleted: the binding survives - + // nothing unbound the id - but the extent the driver resolved for it at bind time + // does not. Marks those points unknown so the next sync issues a real + // glBindBufferBase/Range instead of skipping it. + void InvalidateIndexedBufferBindingShadowsForId(Uint id); + // Resources whose owning BufferObject died; ids deleted at the next // sync point with a current ES context. Vector> g_deferredBufferReleases; @@ -558,6 +565,10 @@ namespace MobileGL::MG_Backend::DirectGLES { ZoneScopedC(TRACY_ZONECOLOR_BACKEND); #endif const SizeT size = bufferObject.GetSize(); + // Read BEFORE the fields below are overwritten: whether this respecify changes + // the store's EXTENT is what decides if the indexed-binding shadow still + // describes the driver. + const Bool extentChanged = !resource.storageInitialized || resource.storageSize != size; const GLenum usage = MG_Util::ConvertBufferUsageToGLEnum(bufferObject.GetUsage()); BindBufferId(TempBufferTarget, resource.id); // An orphaning respecify (glBufferData with NULL, content never @@ -573,6 +584,17 @@ namespace MobileGL::MG_Backend::DirectGLES { resource.pendingRespecify = false; resource.pendingRanges.clear(); resource.syncedChangeSerial = bufferObject.GetChangeSerial(); + // A GROWN store keeps its indexed bindings, and BindBufferBaseCached skips a + // rebind whenever the shadow already records this id at that index - so on a + // driver that resolves a whole-buffer indexed binding's extent at BIND time + // (Adreno does; Mali does not) the shader keeps seeing the old, smaller range: + // stores past it are dropped and loads return zero. Forget what the shadow + // claims for this id so the next SyncBufferBindingPoints issues the bind for + // real. Only when the extent actually moved: an orphaning respecify at the same + // size is Minecraft's per-frame hot path and its bindings are still exact. + if (extentChanged) { + InvalidateIndexedBufferBindingShadowsForId(resource.id); + } } Bool StorageMatches(const GLESBufferResource& resource, const BufferObject& bufferObject) { @@ -1170,6 +1192,12 @@ namespace MobileGL::MG_Backend::DirectGLES { GLintptr offset = 0; GLsizeiptr size = 0; Bool isBase = true; + // False when the driver's binding at this point is no longer described by the + // fields above and the next bind must be issued whatever it asks for. Set by + // InvalidateIndexedBufferBindingShadowsForId after a store was re-specified at + // a new size: the id is still bound, so the entry must NOT be scrubbed to + // base(0) (a later bind of 0 would then be false-skipped) - only distrusted. + Bool known = true; }; constexpr SizeT kMaxIndexedBufferBindings = 64; IndexedBufferBinding g_indexedUBOBindings[kMaxIndexedBufferBindings]; @@ -1201,20 +1229,30 @@ namespace MobileGL::MG_Backend::DirectGLES { g_boundPixelUnpackBufferId = 0; } } + + void InvalidateIndexedBufferBindingShadowsForId(Uint id) { + if (id == 0) return; + for (auto& binding : g_indexedUBOBindings) { + if (binding.id == id) binding.known = false; + } + for (auto& binding : g_indexedSSBOBindings) { + if (binding.id == id) binding.known = false; + } + } } // namespace void BindBufferBaseCached(GLenum glTarget, Uint index, Uint id) { auto* s = IndexedBindingShadow(glTarget, index); - if (s && s->isBase && s->id == id) return; + if (s && s->known && s->isBase && s->id == id) return; g_GLESFuncs.glBindBufferBase(glTarget, index, id); - if (s) *s = {id, 0, 0, true}; + if (s) *s = {id, 0, 0, true, true}; } void BindBufferRangeCached(GLenum glTarget, Uint index, Uint id, GLintptr offset, GLsizeiptr size) { auto* s = IndexedBindingShadow(glTarget, index); - if (s && !s->isBase && s->id == id && s->offset == offset && s->size == size) return; + if (s && s->known && !s->isBase && s->id == id && s->offset == offset && s->size == size) return; g_GLESFuncs.glBindBufferRange(glTarget, index, id, offset, size); - if (s) *s = {id, offset, size, false}; + if (s) *s = {id, offset, size, false, true}; } void InvalidateIndexedBufferBindingCache() { diff --git a/MobileGL/MG_IntegrationTest/CMakeLists.txt b/MobileGL/MG_IntegrationTest/CMakeLists.txt index c2a01bf4..b995414c 100644 --- a/MobileGL/MG_IntegrationTest/CMakeLists.txt +++ b/MobileGL/MG_IntegrationTest/CMakeLists.txt @@ -98,6 +98,7 @@ add_executable(MobileGLIntegrationTest Scenarios/LayeredTextureReadbackScenario.cpp Scenarios/AtomicCounterScenario.cpp Scenarios/SsboArrayDynamicIndexScenario.cpp + Scenarios/StorageBufferRegrowScenario.cpp ) target_include_directories(MobileGLIntegrationTest PRIVATE diff --git a/MobileGL/MG_IntegrationTest/Scenarios/StorageBufferRegrowScenario.cpp b/MobileGL/MG_IntegrationTest/Scenarios/StorageBufferRegrowScenario.cpp new file mode 100644 index 00000000..1824ebab --- /dev/null +++ b/MobileGL/MG_IntegrationTest/Scenarios/StorageBufferRegrowScenario.cpp @@ -0,0 +1,156 @@ +// MobileGL - MobileGL/MG_IntegrationTest/Scenarios/StorageBufferRegrowScenario.cpp +// Copyright (c) 2026 MobileGL-Dev +// Licensed under the GNU Lesser General Public License v3.0: +// https://www.gnu.org/licenses/gpl-3.0.txt +// https://www.gnu.org/licenses/lgpl-3.0.txt +// SPDX-License-Identifier: LGPL-3.0-only +// End of Source File Header +// +// Scenario - glBufferData GROWS A BUFFER THAT IS ALREADY BOUND AT AN INDEXED POINT. +// +// GL says the indexed binding follows the buffer object, so after the store is re-specified the +// shader sees the NEW extent. DirectGLES shadows the indexed bindings so a redundant +// glBindBufferBase can be skipped, and nothing used to invalidate that shadow when the store was +// re-specified underneath it - so on a driver that resolves a whole-buffer indexed binding's +// extent at BIND time (Adreno does; Mali does not) the shader kept seeing the OLD, smaller range. +// Stores past it are dropped and loads return zero, which is exactly what +// KHR-GL43.compute_shader.dispatch-indirect reported: the first iteration's 6 elements correct and +// everything past byte 24 zero, after the same buffer was re-specified from 24 to 96 bytes. +// +// The assertion is deliberately on the WHOLE grown range, so a partial write names the byte the +// stale extent stopped at. + +#include +#include + +#include "../Harness/HeadlessGL.h" +#include "../Harness/ScenarioFixture.h" + +#ifdef GLAPI +#undef GLAPI +#endif +#define GL_GLEXT_PROTOTYPES +#include +#include +#undef GL_GLEXT_PROTOTYPES + +namespace MGITest { + namespace { + + constexpr const char* kComputeSource = R"(#version 430 core +layout(local_size_x = 1) in; +layout(std430, binding = 0) buffer Output { + uint g_data[]; +}; +void main() { + g_data[gl_GlobalInvocationID.x] = gl_GlobalInvocationID.x + 1u; +} +)"; + + constexpr int kSmallElements = 6; // 24 bytes - the first iteration's size + constexpr int kLargeElements = 24; // 96 bytes - what the second iteration grows to + + class StorageBufferRegrowScenario : public ScenarioTest { + protected: + void SetUp() override { + ScenarioTest::SetUp(); + if (!Ready()) return; + m_program = CompileComputeProgram(kComputeSource); + ASSERT_NE(m_program, 0u) << m_buildLog; + glGenBuffers(1, &m_buffer); + } + + void TearDown() override { + if (!Ready()) return; + if (m_buffer != 0) glDeleteBuffers(1, &m_buffer); + if (m_program != 0) glDeleteProgram(m_program); + } + + unsigned int CompileComputeProgram(const char* source) { + const GLuint shader = glCreateShader(GL_COMPUTE_SHADER); + glShaderSource(shader, 1, &source, nullptr); + glCompileShader(shader); + GLint compiled = 0; + glGetShaderiv(shader, GL_COMPILE_STATUS, &compiled); + if (compiled == GL_FALSE) { + char log[2048] = {}; + glGetShaderInfoLog(shader, sizeof(log) - 1, nullptr, log); + m_buildLog = std::string("compute shader did not compile: ") + log; + glDeleteShader(shader); + return 0; + } + const GLuint program = glCreateProgram(); + glAttachShader(program, shader); + glLinkProgram(program); + glDeleteShader(shader); + GLint linked = 0; + glGetProgramiv(program, GL_LINK_STATUS, &linked); + if (linked == GL_FALSE) { + char log[2048] = {}; + glGetProgramInfoLog(program, sizeof(log) - 1, nullptr, log); + m_buildLog = std::string("compute program did not link: ") + log; + glDeleteProgram(program); + return 0; + } + return program; + } + + void RespecifyTo(int elements) { + const std::vector zeros(static_cast(elements), 0u); + glBindBuffer(GL_SHADER_STORAGE_BUFFER, m_buffer); + glBufferData(GL_SHADER_STORAGE_BUFFER, + static_cast(zeros.size() * sizeof(unsigned int)), zeros.data(), + GL_DYNAMIC_COPY); + } + + std::vector DispatchAndRead(int elements) { + glUseProgram(m_program); + glDispatchCompute(static_cast(elements), 1, 1); + glMemoryBarrier(GL_BUFFER_UPDATE_BARRIER_BIT); + std::vector values(static_cast(elements), 0xDEADBEEFu); + glBindBuffer(GL_SHADER_STORAGE_BUFFER, m_buffer); + glGetBufferSubData(GL_SHADER_STORAGE_BUFFER, 0, + static_cast(values.size() * sizeof(unsigned int)), values.data()); + return values; + } + + unsigned int m_program = 0; + GLuint m_buffer = 0; + std::string m_buildLog; + }; + + } // namespace + + TEST_F(StorageBufferRegrowScenario, AGrownStoreIsVisibleThroughItsExistingIndexedBinding) { + if (!Ready() || IsSkipped()) return; + + // Iteration one: 24 bytes, bound once, six groups. + RespecifyTo(kSmallElements); + glBindBufferBase(GL_SHADER_STORAGE_BUFFER, 0, m_buffer); + ASSERT_EQ(FirstGLError(), 0u); + + const std::vector small = DispatchAndRead(kSmallElements); + ASSERT_EQ(FirstGLError(), 0u); + for (int i = 0; i < kSmallElements; ++i) { + ASSERT_EQ(small[static_cast(i)], static_cast(i + 1)) + << "the 24-byte iteration itself did not write element " << i; + } + + // Iteration two: the SAME buffer grows to 96 bytes with NO new glBindBufferBase, which is + // what the application is entitled to do and what the shadow used to swallow. + RespecifyTo(kLargeElements); + ASSERT_EQ(FirstGLError(), 0u); + + const std::vector large = DispatchAndRead(kLargeElements); + EXPECT_EQ(FirstGLError(), 0u); + for (int i = 0; i < kLargeElements; ++i) { + EXPECT_EQ(large[static_cast(i)], static_cast(i + 1)) + << "element " << i << " (byte " << i * 4 << ") of the grown store came back as " + << large[static_cast(i)] + << "; zero from element " << kSmallElements + << " on means the shader still saw the pre-growth extent"; + } + + glBindBufferBase(GL_SHADER_STORAGE_BUFFER, 0, 0); + } +} // namespace MGITest