[Fix] (Espryt): barrier re-uploads into image-accessed textures ahead of the dispatch that reads them

MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp

@@ -3094,6 +3094,12 @@ namespace MobileGL::MG_Backend::DirectGLES {
                 return;
             }
             SyncImageTextureBindings();
+            // The sweep may just have landed a re-upload into a texture the draw reaches through
+            // an image unit (the content clause above is one of the ways it gets here).
+            if (const GLbitfield barriers = TakeInPlaceTextureUploadBarrierBits();
+                barriers != 0 && g_GLESFuncs.glMemoryBarrier) {
+                g_GLESFuncs.glMemoryBarrier(barriers);
+            }
             g_imageSweepShaderImagesSerial = st.ShaderImagesSerial;
             g_imageSweepTextureShutterSerial = st.TextureShutterSerial;
             g_imageSweepContextSerial = st.ContextSerial;
@@ -6217,6 +6223,12 @@ namespace MobileGL::MG_Backend::DirectGLES {
         TextureImpl::SyncNeccessaryTextures(textureKeys);
         TextureImpl::SyncImageTextureBindings();
         TextureImpl::MarkWritableImageBufferTexturesGpuWritten();
+        // Both syncs above may have re-uploaded a texture this dispatch reads or stores
+        // (Managers.h, NoteInPlaceTextureUpload).
+        {
+            GLbitfield barriers = TextureImpl::TakeInPlaceTextureUploadBarrierBits();
+            if (barriers != 0 && g_GLESFuncs.glMemoryBarrier) g_GLESFuncs.glMemoryBarrier(barriers);
+        }
         // P5e (pg), kimi row 92: the DISPATCH half of the same retirement -
         // MGPipeApplier().DispatchProgram, which set_dispatch_program carried. A compute-only
         // pipeline has no draw program at all, which is why the two applier fields are separate.

MobileGL/MG_Backend/DirectGLES/Managers.cpp

@@ -6203,6 +6203,20 @@ namespace MobileGL::MG_Backend::DirectGLES {
             return reinterpret_cast<const void*>(static_cast<std::uintptr_t>(offset));
         }
 
+        // See Managers.h. One flag for every context, like the image-unit high-water mark: a
+        // dispatch on another context can consume an upload's flag before the uploading
+        // context's own dispatch sees it - a hole only a shared texture re-uploaded on one context
+        // and image-accessed on another can fall into, and GL wants a fence between those anyway.
+        static Bool g_inPlaceTextureUploadPending = false;
+
+        void NoteInPlaceTextureUpload() { g_inPlaceTextureUploadPending = true; }
+
+        GLbitfield TakeInPlaceTextureUploadBarrierBits() {
+            if (!g_inPlaceTextureUploadPending) return 0;
+            g_inPlaceTextureUploadPending = false;
+            return GL_TEXTURE_UPDATE_BARRIER_BIT | GL_TEXTURE_FETCH_BARRIER_BIT | GL_SHADER_IMAGE_ACCESS_BARRIER_BIT;
+        }
+
         static Uint GetNormFallbackComponentCount(TextureInternalFormat format) {
             switch (format) {
             case TextureInternalFormat::R8Snorm:
@@ -8420,6 +8434,8 @@ namespace MobileGL::MG_Backend::DirectGLES {
                             if (ringStaged) {
                                 BufferImpl::BindPixelUnpackBufferId(BufferImpl::UnpackRingBufferId());
                             }
+                            // Ring-sourced or not: see NoteInPlaceTextureUpload.
+                            NoteInPlaceTextureUpload();
                             if (MG_Util::PipeStats::Enabled()) {
                                 // One emission per (upload target, level) that ships texels;
                                 // the switch below turns it into either one union-box job or

MobileGL/MG_Backend/DirectGLES/Managers.h

@@ -2076,6 +2076,28 @@ namespace MobileGL::MG_Backend::DirectGLES {
         Bool FollowCopyImageInStagedStore(MG_Pipe::MGPipeHandle source, Uint32 sourceLevel, const IntVec3& sourceOrigin,
                                           MG_Pipe::MGPipeHandle destination, Uint32 destinationLevel,
                                           const IntVec3& destinationOrigin, const IntVec3& size);
+        // A re-upload into storage the driver already holds is a copy the driver may run on a path
+        // of its own that the next shader's image loads and stores do not wait for. On Adreno, a
+        // compute dispatch that imageStore()s over a texture whose re-upload the same Prepare had
+        // just issued read back with up to 2.4k of 4096 texels wrong, while the same dispatch over
+        // freshly created textures was exact (KHR-GL43.compute_shader.resource-image). The two
+        // differ in exactly two ways: the fresh ones are allocated and filled from client memory,
+        // the re-uploads go through the unpack ring as a buffer-to-texture copy; and the fresh
+        // ones were never used by the GPU. Both are covered - every in-place level upload is
+        // noted, ring-sourced or not.
+        //
+        // GL orders a texture update ahead of every later shader access without a barrier, so
+        // the application has nothing to fix this with - and its own barriers could not reach it
+        // anyway: the upload is applied lazily, at the dispatch that consumes it, after any
+        // barrier the application issued. So the shader-access sites that can see such a texture
+        // through an image unit - every dispatch, and a draw whose image units were just
+        // re-synced - issue ONE barrier ahead of the shader, and only when an in-place upload was
+        // issued since the last one. Sampled access from a draw does not take it: that is the
+        // order every driver keeps on its own, and it is the per-draw hot path.
+        void NoteInPlaceTextureUpload();
+        // The barrier bits owed for the uploads noted since the last call (0 when none), and
+        // the note cleared: the caller issues them, folded into any barrier of its own.
+        GLbitfield TakeInPlaceTextureUploadBarrierBits();
         // Brings every texture the next draw reads - the touched units' bindings and the draw
         // FBO's texture attachments - onto the backend, through the two borrowed-pair memos
         // documented at their definitions. Declared here so tests can drive those memos directly.

MobileGL/MG_IntegrationTest/Scenarios/PartialUploadOverGpuContentScenario.cpp

@@ -614,6 +614,51 @@ void main() {
             DestroyColorFbo(target);
         }
 
+        // The dispatch form of the case above, which is KHR-GL43.compute_shader.resource-image's
+        // shape: every pass re-uploads into the SAME image-bound texture and a dispatch stores
+        // over all of it. The upload is applied at the dispatch's own Prepare, after any barrier
+        // the application issued, so it is the backend that has to order it ahead of the stores.
+        TEST_F(PartialUploadOverGpuContentScenario, SubImageBeforeAnImageStoreDispatchLandsUnderTheStores) {
+            if (!Ready()) return;
+            const GLuint t = NewTexture(GL_TEXTURE_2D);
+            glBindTexture(GL_TEXTURE_2D, t);
+            glTexStorage2D(GL_TEXTURE_2D, 1, GL_RGBA8, kEdge, kEdge);
+            glBindTexture(GL_TEXTURE_2D, 0);
+            const char* computeSource =
+                "#version 430 core\n"
+                "layout(local_size_x = 8, local_size_y = 8) in;\n"
+                "layout(rgba8, binding = 0) writeonly uniform image2D u_img;\n"
+                "void main() { imageStore(u_img, ivec2(gl_GlobalInvocationID.xy), vec4(0.0, 1.0, 0.0, 1.0)); }\n";
+            const GLuint shader = glCreateShader(GL_COMPUTE_SHADER);
+            glShaderSource(shader, 1, &computeSource, nullptr);
+            glCompileShader(shader);
+            const GLuint program = glCreateProgram();
+            glAttachShader(program, shader);
+            glLinkProgram(program);
+            glDeleteShader(shader);
+            m_programs.push_back(program);
+            GLint linked = GL_FALSE;
+            glGetProgramiv(program, GL_LINK_STATUS, &linked);
+            ASSERT_EQ(linked, GL_TRUE) << "compute program did not link";
+            glBindImageTexture(0, t, 0, GL_FALSE, 0, GL_WRITE_ONLY, GL_RGBA8);
+            ASSERT_EQ(FirstGLError(), 0u) << "setup";
+            const auto red = Fill(Rgba8(), kEdge * kEdge, 1.0f, 0.0f, 0.0f);
+            for (int pass = 0; pass < 3; ++pass) {
+                SCOPED_TRACE(::testing::Message() << "pass " << pass);
+                glBindTexture(GL_TEXTURE_2D, t);
+                glTexSubImage2D(GL_TEXTURE_2D, 0, 0, 0, kEdge, kEdge, GL_RGBA, GL_UNSIGNED_BYTE, red.data());
+                glBindTexture(GL_TEXTURE_2D, 0);
+                glMemoryBarrier(GL_ALL_BARRIER_BITS);
+                glUseProgram(program);
+                glDispatchCompute(1, 1, 1);
+                glUseProgram(0);
+                glMemoryBarrier(GL_ALL_BARRIER_BITS);
+                ASSERT_EQ(FirstGLError(), 0u) << "dispatch";
+                ExpectRgba8(t, false);
+            }
+            glBindImageTexture(0, 0, 0, GL_FALSE, 0, GL_READ_ONLY, GL_RGBA8);
+        }
+
         // Two boxes in opposite corners: their union box is 7x7, of which the writes are 8 texels.
         TEST_F(PartialUploadOverGpuContentScenario, TwoDistantSubImagesKeepTheGpuClearBetweenThem) {
             if (!Ready()) return;