[Refactor] (MG_Impl/GLImpl): replace Flywheel dispatch sync hack with MOBILEGL_COHERENT_AS_FLUSH

MobileGL/Config.h

@@ -48,6 +48,13 @@ namespace MobileGL::MG_Config {
         // MOBILEGL_AVOID_SAMPLER_MIPMAP_MIN_FILTER: avoid mipmap min filters in samplers,
         // resolves certain rendering bugs on ANGLE + llvmpipe.
         Bool AvoidSamplerMipmapMinFilter = false;
+        // MOBILEGL_COHERENT_AS_FLUSH: app-compat for engines (e.g. Flywheel) that write
+        // GPU-read data through persistent GL_MAP_FLUSH_EXPLICIT_BIT maps they never
+        // flush. Persistent FLUSH_EXPLICIT map requests are rewritten to coherent
+        // semantics: writes reach the backend without glFlushMappedBufferRange, and
+        // flush calls on rewritten maps become error-free no-ops. Non-persistent maps
+        // keep spec FLUSH_EXPLICIT behavior.
+        Bool CoherentAsFlush = false;
         // MOBILEGL_TRACE_SKIP_AUTODESTROY: skip teardown in the ELF destructor (Init.cpp).
         Bool TraceSkipAutodestroy = false;
     };

MobileGL/ConfigLoader.cpp

@@ -119,6 +119,7 @@ namespace MobileGL::MG_ConfigLoader {
         features.MagmaFramesInFlight = QueryEnvUint32("MOBILEGL_MAGMA_FRAMESINFLIGHT", 3, 1, 64);
         features.AvoidSamplerMipmapMinFilter =
             QueryEnvFlag("MOBILEGL_AVOID_SAMPLER_MIPMAP_MIN_FILTER");
+        features.CoherentAsFlush = QueryEnvFlag("MOBILEGL_COHERENT_AS_FLUSH");
         features.TraceSkipAutodestroy = QueryEnvFlag("MOBILEGL_TRACE_SKIP_AUTODESTROY");
     }
 

MobileGL/MG_Impl/GLImpl/Buffer/GL_Buffer.cpp

@@ -8,6 +8,7 @@
 
 #include "GL_Buffer.h"
 #include "Validators.h"
+#include <Config.h>
 #include <MG_State/GLState/Core.h>
 #include <MG_State/GLState/ErrorState/Error.h>
 #include <MG_Util/Converters/GLToStr/GLEnumConverter.h>
@@ -208,6 +209,26 @@ namespace MobileGL::MG_Impl::GLImpl {
             return bufferObject;
         }
 
+        // MOBILEGL_COHERENT_AS_FLUSH: rewrite a validated persistent FLUSH_EXPLICIT mapping
+        // request to coherent semantics: the map becomes coherent-persistent (eligible for
+        // the zero-copy backend map, otherwise synced wholesale at draw time), so its writes
+        // reach the GPU without glFlushMappedBufferRange; flush calls on rewritten maps are
+        // tolerated as no-ops by the FlushMappedBufferRange entry points. Non-persistent
+        // maps keep spec FLUSH_EXPLICIT behavior on purpose: the GPU cannot read them while
+        // mapped, so they gain nothing from the rewrite, and honoring only the app's flushed
+        // subranges avoids clobbering GPU-written bytes elsewhere in the mapped range.
+        // Runs after validation so the app's original access combination is what gets
+        // validated (Coherent is injected without requiring GL_MAP_COHERENT_BIT storage).
+        Flags<BufferMappingAccessBit> ApplyCoherentAsFlush(Flags<BufferMappingAccessBit> accessBits) {
+            if (!MG_Config::Features.CoherentAsFlush) return accessBits;
+            if (!(accessBits & BufferMappingAccessBit::FlushExplicit)) return accessBits;
+            if (!(accessBits & BufferMappingAccessBit::Persistent)) return accessBits;
+            accessBits = Flags<BufferMappingAccessBit>(
+                accessBits.GetRaw() & ~static_cast<Uint>(BufferMappingAccessBit::FlushExplicit));
+            accessBits |= BufferMappingAccessBit::Coherent;
+            return accessBits;
+        }
+
         Bool ValidateStorageFlags(GLbitfield flags, BufferOp op) {
             constexpr GLbitfield validFlags = GL_MAP_READ_BIT | GL_MAP_WRITE_BIT | GL_MAP_PERSISTENT_BIT |
                                               GL_MAP_COHERENT_BIT | GL_DYNAMIC_STORAGE_BIT | GL_CLIENT_STORAGE_BIT;
@@ -465,6 +486,15 @@ namespace MobileGL::MG_Impl::GLImpl {
 
         auto mappingAccess = bufferObject->GetMappingAccess();
         if (!(mappingAccess & BufferMappingAccessBit::FlushExplicit)) {
+            // MOBILEGL_COHERENT_AS_FLUSH strips FLUSH_EXPLICIT from persistent maps at map
+            // time (leaving Persistent|Coherent), so honor the app's flush on such a map as
+            // a no-op: its writes reach the backend without explicit flushes. Other maps
+            // keep the spec error.
+            if (MG_Config::Features.CoherentAsFlush &&
+                (mappingAccess & BufferMappingAccessBit::Persistent) &&
+                (mappingAccess & BufferMappingAccessBit::Coherent)) {
+                return;
+            }
             MG_State::pGLContext->RecordError(
                 ErrorCode::InvalidOperation,
                 MakeUnique<GenericErrorInfo>(
@@ -605,7 +635,7 @@ namespace MobileGL::MG_Impl::GLImpl {
         }
 
         void* result = bufferObject->AcquireMemoryRange(
-            {static_cast<SizeT>(offset), static_cast<SizeT>(offset + length)}, accessBits);
+            {static_cast<SizeT>(offset), static_cast<SizeT>(offset + length)}, ApplyCoherentAsFlush(accessBits));
         if (!result) {
             MG_State::pGLContext->RecordError(
                 ErrorCode::OutOfMemory,
@@ -1197,7 +1227,7 @@ namespace MobileGL::MG_Impl::GLImpl {
         }
 
         return bufferObject->AcquireMemoryRange({static_cast<SizeT>(offset), static_cast<SizeT>(offset + length)},
-                                                accessBits);
+                                                ApplyCoherentAsFlush(accessBits));
     }
 
     GLboolean UnmapNamedBuffer_State(GLuint buffer) {
@@ -1239,7 +1269,15 @@ namespace MobileGL::MG_Impl::GLImpl {
                                              "Offset and length exceed mapped range."));
             return;
         }
-        if (!(bufferObject->GetMappingAccess() & BufferMappingAccessBit::FlushExplicit)) {
+        const auto namedMappingAccess = bufferObject->GetMappingAccess();
+        if (!(namedMappingAccess & BufferMappingAccessBit::FlushExplicit)) {
+            // See FlushMappedBufferRange_State: rewritten coherent-as-flush persistent maps
+            // tolerate app flushes as no-ops.
+            if (MG_Config::Features.CoherentAsFlush &&
+                (namedMappingAccess & BufferMappingAccessBit::Persistent) &&
+                (namedMappingAccess & BufferMappingAccessBit::Coherent)) {
+                return;
+            }
             MG_State::pGLContext->RecordError(
                 ErrorCode::InvalidOperation,
                 MakeUnique<GenericErrorInfo>("MG_Impl/GLImpl", "FlushMappedNamedBufferRange_State",

MobileGL/MG_Impl/GLImpl/Drawing/GL_Drawing.cpp

@@ -246,26 +246,6 @@ namespace MobileGL::MG_Impl::GLImpl {
         MG_Backend::gBackendFunctionsTable.GL.DrawArraysIndirect(mode, indirect);
     }
 
-    // Flywheel-style engines write GPU-copy descriptors into a FLUSH_EXPLICIT
-    // persistent map and glBindBufferRange that span as an SSBO for a compute
-    // dispatch WITHOUT ever flushing it (undefined per spec, but real drivers'
-    // persistent maps alias GPU-visible memory, so it works there). MobileGL's
-    // persistent maps alias the CPU shadow, so those bytes would never reach the
-    // GPU: push every explicitly-ranged SSBO binding of such maps down right
-    // before each dispatch. Whole-buffer (BindBufferBase) bindings are excluded
-    // on purpose — ranges the app DID flush already arrived, and re-uploading a
-    // 16MB staging ring per dispatch would be prohibitive.
-    static void SyncUnflushedMappedSsboRangesForDispatch() {
-        const auto pointCount = MG_State::pGLContext->GetBufferBindingPointCount(BufferTarget::ShaderStorage);
-        for (SizeT i = 0; i < pointCount; ++i) {
-            auto& point = MG_State::pGLContext->GetBufferBindingPoint(BufferTarget::ShaderStorage, i);
-            if (!point.HasExplicitRange()) continue;
-            const auto& bufferObject = point.GetBoundObject();
-            if (!bufferObject) continue;
-            bufferObject->SyncMappedRangeForGpuRead(point.GetRange());
-        }
-    }
-
     /* @INSERTION_POINT:FUNCTION_IMPLEMENTATION@ */
     void DispatchCompute(GLuint numGroupsX, GLuint numGroupsY, GLuint numGroupsZ) {
         auto dispatchCompute = MG_Backend::gBackendFunctionsTable.GL.DispatchCompute;
@@ -276,7 +256,6 @@ namespace MobileGL::MG_Impl::GLImpl {
             return;
         }
         if (!ValidateCurrentProgramForCompute(__func__)) return;
-        SyncUnflushedMappedSsboRangesForDispatch();
         dispatchCompute(numGroupsX, numGroupsY, numGroupsZ);
     }
 
@@ -290,7 +269,6 @@ namespace MobileGL::MG_Impl::GLImpl {
             return;
         }
         if (!ValidateCurrentProgramForCompute(__func__)) return;
-        SyncUnflushedMappedSsboRangesForDispatch();
         dispatchComputeIndirect(indirect);
     }
 

MobileGL/MG_State/GLState/BufferState/BufferObject.cpp

@@ -7,7 +7,6 @@
 // End of Source File Header
 
 #include "BufferObject.h"
-#include <algorithm>
 
 namespace MobileGL::MG_State::GLState {
     namespace {
@@ -167,23 +166,6 @@ namespace MobileGL::MG_State::GLState {
         NotifySubData(m_mappedRange.start, m_mappedRange.end - m_mappedRange.start);
     }
 
-    void BufferObject::SyncMappedRangeForGpuRead(Range1D range) {
-        if (!m_isMapped) return;
-        // GPU-resident maps already alias GPU-visible memory; nothing to push.
-        if (m_resource.IsGpuResident()) return;
-        if (!(m_mappingAccess & BufferMappingAccessBit::Persistent)) return;
-        if (!(m_mappingAccess & BufferMappingAccessBit::Write)) return;
-        // Non-FLUSH_EXPLICIT persistent maps are already covered wholesale by
-        // SyncPersistentMappedRange.
-        if (!(m_mappingAccess & BufferMappingAccessBit::FlushExplicit)) return;
-
-        const SizeT start = std::max(range.start, m_mappedRange.start);
-        const SizeT end = std::min({range.end, m_mappedRange.end, m_size});
-        if (start >= end) return;
-
-        NotifySubData(start, end - start);
-    }
-
     void BufferObject::WritebackFromBackend(DataPtr data, SizeT atOffset) {
         MOBILEGL_ASSERT(atOffset + data.size <= m_size,
                         "WritebackFromBackend out of bounds: atOffset (%zu) + data.size (%zu) > m_size (%zu)", atOffset,

MobileGL/MG_State/GLState/BufferState/BufferObject.h

@@ -139,12 +139,6 @@ namespace MobileGL {
             // Pushes the persistently-mapped write range to the backend; called by
             // backends at draw time (persistent maps mutate the shadow without API calls).
             void SyncPersistentMappedRange();
-            // App-compat for FLUSH_EXPLICIT persistent maps: engines like Flywheel
-            // write copy descriptors into the mapping and bind that range as an SSBO
-            // without ever flushing it. Undefined per spec, but works on drivers whose
-            // persistent maps alias GPU-visible memory. Ours alias the CPU shadow, so
-            // callers push the GPU-read range down right before it is consumed.
-            void SyncMappedRangeForGpuRead(Range1D range);
             // Shadow-only write used when the backend copies GPU results (e.g. ReadPixels
             // into a pixel-pack buffer) back into the frontend mirror. Does not issue a
             // backend op: the backend storage already holds these bytes.

MobileGL/MG_Test/Buffer/BufferTest.cpp

@@ -10,6 +10,7 @@
 
 #include "Includes.h"
 #include "Init.h"
+#include <Config.h>
 #include <MG_State/GLState/Core.h>
 
 #include <MG_Impl/GLImpl/Buffer/GL_Buffer.h>
@@ -1009,3 +1010,173 @@ TEST_F(GeneralBufferTest, General_PersistentCoherentFallbackSyncsPerDrawWhenBack
     EXPECT_TRUE(UnmapBuffer(GL_ARRAY_BUFFER));
     g_zeroCopyMock = nullptr;
 }
+
+// ---------------------------------------------------------------------------
+// MOBILEGL_COHERENT_AS_FLUSH: persistent FLUSH_EXPLICIT mapping requests are
+// rewritten to coherent semantics, so apps that bind mapped ranges for GPU reads
+// without ever calling glFlushMappedBufferRange (e.g. Flywheel's copy descriptors)
+// still get their writes, and their flush calls stay error-free no-ops.
+// Non-persistent maps keep spec FLUSH_EXPLICIT behavior.
+namespace {
+    struct ScopedCoherentAsFlush {
+        ScopedCoherentAsFlush() { MG_Config::Features.CoherentAsFlush = true; }
+        ~ScopedCoherentAsFlush() { MG_Config::Features.CoherentAsFlush = false; }
+    };
+} // namespace
+
+TEST_F(GeneralBufferTest, General_CoherentAsFlush_NonPersistentMapKeepsExplicitFlushSemantics) {
+    ScopedCoherentAsFlush scopedFeature;
+
+    GLuint buffer = CreateBoundBuffer(GL_ARRAY_BUFFER, 64, GL_STATIC_DRAW);
+    const char initial[16] = "0123456789ABCDE";
+    BufferSubData(GL_ARRAY_BUFFER, 20, sizeof(initial), initial);
+
+    auto bufferObject = MG_State::pGLContext->GetBufferObject(buffer);
+    ASSERT_NE(bufferObject, nullptr);
+
+    auto* mapped =
+        static_cast<char*>(MapBufferRange(GL_ARRAY_BUFFER, 20, 16, GL_MAP_WRITE_BIT | GL_MAP_FLUSH_EXPLICIT_BIT));
+    ASSERT_NE(mapped, nullptr);
+    // Non-persistent maps are not rewritten: the FLUSH_EXPLICIT contract stays.
+    EXPECT_TRUE(bufferObject->GetMappingAccess() & BufferMappingAccessBit::FlushExplicit);
+
+    memcpy(mapped, "PARTIAL", 8);
+    memcpy(mapped + 8, "WRITTEN", 8);
+    FlushMappedBufferRange(GL_ARRAY_BUFFER, 0, 8); // flush only the first half
+    EXPECT_EQ(GetError(), GL_NO_ERROR);
+    EXPECT_TRUE(UnmapBuffer(GL_ARRAY_BUFFER));
+
+    // Only the flushed subrange reaches the shadow; the un-flushed half keeps its
+    // previous contents (spec behavior, unchanged by the feature).
+    char readBack[16] = {};
+    GetBufferSubData(GL_ARRAY_BUFFER, 20, sizeof(readBack), readBack);
+    EXPECT_EQ(memcmp(readBack, "PARTIAL", 8), 0);
+    EXPECT_EQ(memcmp(readBack + 8, "89ABCDE", 8), 0);
+    EXPECT_EQ(GetError(), GL_NO_ERROR);
+}
+
+TEST_F(GeneralBufferTest, General_FlushWithoutExplicitBitStillErrorsWhenFeatureOff) {
+    GLuint buffer = CreateBoundBuffer(GL_ARRAY_BUFFER, 64, GL_STATIC_DRAW);
+    (void)buffer;
+    void* mapped = MapBufferRange(GL_ARRAY_BUFFER, 0, 16, GL_MAP_WRITE_BIT);
+    ASSERT_NE(mapped, nullptr);
+    FlushMappedBufferRange(GL_ARRAY_BUFFER, 0, 8);
+    EXPECT_EQ(GetError(), GL_INVALID_OPERATION);
+    EXPECT_TRUE(UnmapBuffer(GL_ARRAY_BUFFER));
+}
+
+TEST_F(GeneralBufferTest, General_CoherentAsFlush_PersistentMapSyncsWithoutExplicitFlush) {
+    ScopedCoherentAsFlush scopedFeature;
+
+    GLuint buffer = 0;
+    GenBuffers(1, &buffer);
+    BindBuffer(GL_ARRAY_BUFFER, buffer);
+
+    GLint initial[] = {10, 20, 30, 40};
+    BufferStorage(GL_ARRAY_BUFFER, sizeof(initial), initial, GL_MAP_WRITE_BIT | GL_MAP_PERSISTENT_BIT);
+    EXPECT_EQ(GetError(), GL_NO_ERROR);
+
+    auto bufferObject = MG_State::pGLContext->GetBufferObject(buffer);
+    ASSERT_NE(bufferObject, nullptr);
+    const Uint64 baseSerial = bufferObject->GetChangeSerial();
+
+    auto* mapped = static_cast<GLint*>(MapBufferRange(
+        GL_ARRAY_BUFFER, 0, sizeof(initial), GL_MAP_WRITE_BIT | GL_MAP_PERSISTENT_BIT | GL_MAP_FLUSH_EXPLICIT_BIT));
+    ASSERT_NE(mapped, nullptr);
+    const auto access = bufferObject->GetMappingAccess();
+    EXPECT_FALSE(access & BufferMappingAccessBit::FlushExplicit);
+    EXPECT_TRUE(access & BufferMappingAccessBit::Coherent);
+
+    mapped[1] = 200;
+    // Un-flushed writes are picked up by the draw-time persistent sync - the coverage
+    // the removed FLUSH_EXPLICIT dispatch hack (SyncMappedRangeForGpuRead) used to add.
+    bufferObject->SyncPersistentMappedRange();
+    EXPECT_GT(bufferObject->GetChangeSerial(), baseSerial);
+    EXPECT_EQ(reinterpret_cast<const GLint*>(bufferObject->MappedData())[1], 200);
+
+    FlushMappedBufferRange(GL_ARRAY_BUFFER, 0, sizeof(GLint));
+    EXPECT_EQ(GetError(), GL_NO_ERROR);
+
+    EXPECT_TRUE(UnmapBuffer(GL_ARRAY_BUFFER));
+    EXPECT_EQ(GetError(), GL_NO_ERROR);
+}
+
+TEST_F(GeneralBufferTest, General_CoherentAsFlush_DsaPersistentMapRewritesAndToleratesFlush) {
+    ScopedCoherentAsFlush scopedFeature;
+
+    GLuint buffer = 0;
+    GenBuffers(1, &buffer);
+
+    GLint initial[] = {1, 2, 3, 4};
+    NamedBufferStorage(buffer, sizeof(initial), initial, GL_MAP_WRITE_BIT | GL_MAP_PERSISTENT_BIT);
+    EXPECT_EQ(GetError(), GL_NO_ERROR);
+
+    auto bufferObject = MG_State::pGLContext->GetBufferObject(buffer);
+    ASSERT_NE(bufferObject, nullptr);
+    const Uint64 baseSerial = bufferObject->GetChangeSerial();
+
+    // The DSA map entry point applies the same rewrite as the bound-target one.
+    auto* mapped = static_cast<GLint*>(MapNamedBufferRange(
+        buffer, 0, sizeof(initial), GL_MAP_WRITE_BIT | GL_MAP_PERSISTENT_BIT | GL_MAP_FLUSH_EXPLICIT_BIT));
+    ASSERT_NE(mapped, nullptr);
+    const auto access = bufferObject->GetMappingAccess();
+    EXPECT_FALSE(access & BufferMappingAccessBit::FlushExplicit);
+    EXPECT_TRUE(access & BufferMappingAccessBit::Coherent);
+
+    mapped[2] = 300;
+    bufferObject->SyncPersistentMappedRange();
+    EXPECT_GT(bufferObject->GetChangeSerial(), baseSerial);
+    EXPECT_EQ(reinterpret_cast<const GLint*>(bufferObject->MappedData())[2], 300);
+
+    // The DSA flush entry point tolerates the app's flush as a no-op too.
+    FlushMappedNamedBufferRange(buffer, 0, sizeof(GLint));
+    EXPECT_EQ(GetError(), GL_NO_ERROR);
+
+    EXPECT_TRUE(UnmapNamedBuffer(buffer));
+    EXPECT_EQ(GetError(), GL_NO_ERROR);
+}
+
+TEST_F(GeneralBufferTest, General_CoherentAsFlush_PersistentMapAdoptsZeroCopyBackendStorage) {
+    ScopedCoherentAsFlush scopedFeature;
+    ZeroCopyMockBackend mock;
+    g_zeroCopyMock = &mock;
+    ScopedBackendOps scopedOps(&kZeroCopyMockOps);
+
+    GLuint buffer = 0;
+    GenBuffers(1, &buffer);
+    BindBuffer(GL_ARRAY_BUFFER, buffer);
+
+    constexpr SizeT kCount = 256;
+    Vector<GLint> initial(kCount, 0);
+    BufferStorage(GL_ARRAY_BUFFER, static_cast<GLsizeiptr>(kCount * sizeof(GLint)), initial.data(),
+                  GL_MAP_WRITE_BIT | GL_MAP_PERSISTENT_BIT);
+    ASSERT_EQ(GetError(), GL_NO_ERROR);
+
+    // Flywheel-style map: FLUSH_EXPLICIT and never flushed. Under the feature it becomes
+    // coherent-persistent and takes the zero-copy path: the app writes straight into the
+    // backend's GPU storage, so nothing depends on flush calls.
+    auto* mapped = static_cast<GLint*>(
+        MapBufferRange(GL_ARRAY_BUFFER, 0, static_cast<GLsizeiptr>(kCount * sizeof(GLint)),
+                       GL_MAP_WRITE_BIT | GL_MAP_PERSISTENT_BIT | GL_MAP_FLUSH_EXPLICIT_BIT));
+    ASSERT_NE(mapped, nullptr);
+    EXPECT_EQ(mock.acquireMapCalls, 1);
+
+    auto bufferObject = MG_State::pGLContext->GetBufferObject(buffer);
+    ASSERT_NE(bufferObject, nullptr);
+    EXPECT_TRUE(bufferObject->IsBackendPersistentMapped());
+    EXPECT_EQ(static_cast<void*>(mapped), static_cast<void*>(mock.gpu.data()));
+
+    mock.subDataCalls = 0;
+    mock.flushCalls = 0;
+
+    mapped[7] = 1234;
+    bufferObject->SyncPersistentMappedRange(); // draw-time hook: nothing to transfer
+    EXPECT_EQ(reinterpret_cast<const GLint*>(mock.gpu.data())[7], 1234);
+    EXPECT_EQ(mock.subDataCalls, 0);
+    EXPECT_EQ(mock.flushCalls, 0);
+
+    EXPECT_TRUE(UnmapBuffer(GL_ARRAY_BUFFER));
+    EXPECT_EQ(mock.flushCalls, 0);
+    EXPECT_EQ(GetError(), GL_NO_ERROR);
+    g_zeroCopyMock = nullptr;
+}