From 363fce247f59cd56cf641a8aee6a10876e7ffdd7 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Mon, 12 Oct 2026 05:51:34 -0400 Subject: [PATCH] [Fix] (Espryt): order a dispatch after unbarriered storage-buffer writes so adjacent components survive --- MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp | 36 +++++++++++++- .../SsboArrayDynamicIndexScenario.cpp | 47 +++++++++++++++++++ 2 files changed, 81 insertions(+), 2 deletions(-) diff --git a/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp b/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp index 2d0d0634..d0415ea8 100644 --- a/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp +++ b/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp @@ -455,6 +455,30 @@ namespace MobileGL::MG_Backend::DirectGLES { return request; } + // True from a draw or dispatch that ran with a storage buffer bound until the next + // barrier that orders storage-buffer writes. Every bound storage point counts as written, + // the client's own conservative set (see serverGpuWritten below). + // + // WHAT IT IS FOR: two dispatches with no barrier between them that write DIFFERENT + // components of one vector in the same storage buffer. GL leaves the two unordered, which + // allows them to run concurrently, but every write still lands - they are separate memory + // locations. A driver that stores a vector component as a read-modify-write of the whole + // vector loses one of them instead: KHR-GL43.shader_storage_buffer_object. + // advanced-switchBuffers-cs writes digest.x .. digest.w of one uvec4 from four + // back-to-back dispatches and read back a zero component in 8 runs of 20 on Adreno (0 of + // 20 with a storage barrier between the dispatches, 0 of 20 with uint[4]). The fix is the + // barrier the application did not owe: PrepareForCompute issues it when the previous + // shader work may have written a storage buffer and nothing ordered those writes since. + // + // Compute only. A dispatch that follows a dispatch is the shape the test has and the one + // a driver may overlap; draws keep their own ordering and are not taxed, they only SET + // the flag here - one store in the walk that already visits every bound point. + static Bool g_shaderStorageWritesUnordered = false; + + // Called by the application's own glMemoryBarrier: a barrier that orders storage writes + // already did the work. + static void NoteShaderStorageWritesOrdered() { g_shaderStorageWritesUnordered = false; } + // The record arm of SyncBufferBindingPoints. Same shape, same order, same branches; the // four frontend reads become the applier's window and a handle per entry. // @@ -502,6 +526,7 @@ namespace MobileGL::MG_Backend::DirectGLES { // point is read-only by definition. if (shaderBufferClass != MG_Pipe::kMGPipeShaderBufferClassUniform) { backendResource->serverGpuWritten = true; + g_shaderStorageWritesUnordered = true; } const auto backendBufferId = backendResource->id; if (entry.Offset == 0 && entry.Size == MG_Pipe::kMGPipeWholeBuffer) { @@ -6219,14 +6244,19 @@ namespace MobileGL::MG_Backend::DirectGLES { noFrontendProgram; const TextureImpl::DrawTextureSyncKeys textureKeys = TextureImpl::CaptureDrawTextureSyncKeys(); + // Read BEFORE this dispatch's own binding walk sets it again (see the flag's comment). + const Bool storageWritesUnordered = BufferImpl::g_shaderStorageWritesUnordered; + BufferImpl::g_shaderStorageWritesUnordered = false; BufferImpl::SyncComputeBuffers(includeDispatchIndirectBuffer); TextureImpl::SyncNeccessaryTextures(textureKeys); TextureImpl::SyncImageTextureBindings(); TextureImpl::MarkWritableImageBufferTexturesGpuWritten(); // Both syncs above may have re-uploaded a texture this dispatch reads or stores - // (Managers.h, NoteInPlaceTextureUpload). + // (Managers.h, NoteInPlaceTextureUpload), and the previous shader work may have left + // storage writes unordered; one barrier covers whichever of the two applies. { GLbitfield barriers = TextureImpl::TakeInPlaceTextureUploadBarrierBits(); + if (storageWritesUnordered) barriers |= GL_SHADER_STORAGE_BARRIER_BIT; if (barriers != 0 && g_GLESFuncs.glMemoryBarrier) g_GLESFuncs.glMemoryBarrier(barriers); } // P5e (pg), kimi row 92: the DISPATCH half of the same retirement - @@ -9992,7 +10022,9 @@ namespace MobileGL::MG_Backend::DirectGLES { } void MemoryBarrier(GLbitfield barriers) { - g_GLESFuncs.glMemoryBarrier(LowerAtomicCounterBarrierBits(barriers)); + barriers = LowerAtomicCounterBarrierBits(barriers); + if ((barriers & GL_SHADER_STORAGE_BARRIER_BIT) != 0) BufferImpl::NoteShaderStorageWritesOrdered(); + g_GLESFuncs.glMemoryBarrier(barriers); if (g_GLESCapabilities.IsAngleRenderer) { g_GLESFuncs.glFlush(); } diff --git a/MobileGL/MG_IntegrationTest/Scenarios/SsboArrayDynamicIndexScenario.cpp b/MobileGL/MG_IntegrationTest/Scenarios/SsboArrayDynamicIndexScenario.cpp index cb7948b4..cf32e660 100644 --- a/MobileGL/MG_IntegrationTest/Scenarios/SsboArrayDynamicIndexScenario.cpp +++ b/MobileGL/MG_IntegrationTest/Scenarios/SsboArrayDynamicIndexScenario.cpp @@ -186,4 +186,51 @@ void main() { glBindBufferBase(GL_SHADER_STORAGE_BUFFER, static_cast(i), 0); } } + + // Four back-to-back dispatches with NO barrier between them, each writing one component of the + // same uvec4 (KHR-GL43.shader_storage_buffer_object.advanced-switchBuffers-cs). GL leaves them + // unordered, but the four components are separate memory locations and all four writes have + // to land; a driver that stores a vector component as a read-modify-write of the whole vector + // loses some unless the backend orders the dispatches (DirectGLES's PrepareForCompute does). + TEST_F(SsboArrayDynamicIndexScenario, BackToBackDispatchesKeepEachOthersComponents) { + if (!Ready() || IsSkipped()) return; + const GLuint program = CompileComputeProgram(R"(#version 430 core +layout(local_size_x = 1) in; +layout(binding = 0, std430) buffer Input { uint cookie[4]; } g_in; +layout(binding = 1, std430) buffer Output { uvec4 digest; }; +void main() { + switch (g_in.cookie[0] + g_in.cookie[1] + g_in.cookie[2] + g_in.cookie[3]) { + case 0x000000ffu: digest.x = 0xff000000u; break; + case 0x0000ff00u: digest.y = 0x00ff0000u; break; + case 0x00ff0000u: digest.z = 0x0000ff00u; break; + case 0xff000000u: digest.w = 0x000000ffu; break; + } +} +)"); + ASSERT_NE(program, 0u) << m_buildLog; + GLuint inputs[4] = {}; + for (int i = 0; i < 4; ++i) { + // Four cookies summing to 0xff << 8i: dispatch i selects case i. + const unsigned int part = 0x11u << (8 * i); + inputs[i] = MakeStorageBuffer({part, part, part * 2, (0xffu << (8 * i)) - part * 4}); + } + const GLuint output = MakeStorageBuffer({0u, 0u, 0u, 0u}); + glBindBufferBase(GL_SHADER_STORAGE_BUFFER, 1, output); + glUseProgram(program); + for (int i = 0; i < 4; ++i) { + glBindBufferBase(GL_SHADER_STORAGE_BUFFER, 0, inputs[i]); + glDispatchCompute(1, 1, 1); + } + glMemoryBarrier(GL_BUFFER_UPDATE_BARRIER_BIT); + glUseProgram(0); + EXPECT_EQ(FirstGLError(), 0u); + const std::vector digest = ReadBuffer(output, 4); + EXPECT_EQ(digest[0], 0xff000000u); + EXPECT_EQ(digest[1], 0x00ff0000u); + EXPECT_EQ(digest[2], 0x0000ff00u); + EXPECT_EQ(digest[3], 0x000000ffu); + glBindBufferBase(GL_SHADER_STORAGE_BUFFER, 0, 0); + glBindBufferBase(GL_SHADER_STORAGE_BUFFER, 1, 0); + glDeleteProgram(program); + } } // namespace MGITest