From ff426da3a98025fbd41f5eafdf421a4ccf8c7820 Mon Sep 17 00:00:00 2001
From: Swung0x48 <swung0x48@outlook.com>
Date: Fri, 28 Aug 2026 03:01:15 -0400
Subject: [PATCH] [Fix, Test] (MG_State, DirectVulkan): order host writes to an
 adopted store after recorded GPU work - a SubData issued after a dispatch
 landed in coherent memory before the deferred dispatch executed, so its
 increments overwrote the newer bytes; un-skip the DirectVulkan half of the
 SubData-after-dispatch scenario

---
 .../Scenarios/AtomicCounterScenario.cpp       | 30 ++++++++-----------
 .../GLState/BufferState/BufferObject.cpp      | 18 +++++++++++
 2 files changed, 31 insertions(+), 17 deletions(-)

diff --git a/MobileGL/MG_IntegrationTest/Scenarios/AtomicCounterScenario.cpp b/MobileGL/MG_IntegrationTest/Scenarios/AtomicCounterScenario.cpp
index c2572e60..da6fc5fe 100644
--- a/MobileGL/MG_IntegrationTest/Scenarios/AtomicCounterScenario.cpp
+++ b/MobileGL/MG_IntegrationTest/Scenarios/AtomicCounterScenario.cpp
@@ -226,25 +226,21 @@ void main() {
     }
 
     // A CPU glBufferSubData issued AFTER a dispatch, read back with NO further GPU work in
-    // between. The DirectGLES backend queues app SubData ranges for the draw-time staged-copy
-    // flush (the upload ring) instead of uploading in place, and readback of a GPU-written
-    // buffer overwrites the frontend shadow with the driver copy - so if the readback path
-    // forgets to flush the queued range first, the newer CPU write is REVERTED by the readback
-    // and offset 0 reads the dispatch's value instead of the reseed. Offset 4 pins the other
-    // direction: the flush must not clobber GPU results outside the written range.
+    // between. Each backend has its own way to invert this pair, and both are pinned here.
+    // DirectGLES queues app SubData ranges for the draw-time staged-copy flush (the upload
+    // ring) instead of uploading in place, and readback of a GPU-written buffer overwrites
+    // the frontend shadow with the driver copy - so if the readback path forgets to flush the
+    // queued range first, the newer CPU write is REVERTED by the readback and offset 0 reads
+    // the dispatch's value instead of the reseed. DirectVulkan adopts the buffer into
+    // coherent GPU memory the moment the dispatch resolves its descriptor, so the SubData
+    // write lands in the very bytes the GPU reads - while the dispatch still sits recorded in
+    // the deferred frame command buffer. Unless the frontend retires that pending work before
+    // writing the adopted store (BufferObject::UploadSubData), the dispatch executes ON TOP
+    // of the reseed and offset 0 reads reseed + increments instead of the reseed. Offset 4
+    // pins the other direction for both: the upload must leave bytes outside its range - the
+    // dispatch's results - untouched.
     TEST_F(AtomicCounterScenario, SubDataAfterDispatchSurvivesAnImmediateReadback) {
         if (!Ready() || IsSkipped()) return;
-        // DirectGLES-only for now. DirectVulkan fails this case with or without the upload
-        // ring, on revisions that predate it: its buffer uploads submit immediately while the
-        // dispatch sits in the deferred frame command buffer, so the GPU increments the
-        // RESEEDED value (reads 4242 + increments instead of 4242) - a pre-existing
-        // upload-vs-recorded-work ordering gap in that backend, kept visible here rather than
-        // silently absorbed. Un-skip once DirectVulkan orders app uploads against already
-        // recorded GPU work.
-        if (Gl().BackendName() != std::string("DirectGLES")) {
-            GTEST_SKIP() << "SubData-after-dispatch ordering is a known DirectVulkan gap; this case pins the "
-                            "DirectGLES readback pre-flush only";
-        }
 
         const GLuint zero = MakeCounterBuffer(0, {0u, 0u});
         MakeCounterBuffer(1, {0u});
diff --git a/MobileGL/MG_State/GLState/BufferState/BufferObject.cpp b/MobileGL/MG_State/GLState/BufferState/BufferObject.cpp
index 276f01fa..cd4c5b51 100644
--- a/MobileGL/MG_State/GLState/BufferState/BufferObject.cpp
+++ b/MobileGL/MG_State/GLState/BufferState/BufferObject.cpp
@@ -253,6 +253,18 @@ namespace MobileGL::MG_State::GLState {
                         "UploadSubData out of bounds: atOffset (%zu) + data.size (%zu) > m_size (%zu)", atOffset,
                         data.size, m_size);
 
+        // An adopted store's Bytes() IS the memory the GPU reads, and a backend that
+        // defers work (DirectVulkan's frame command buffer) may still be holding a
+        // recorded-but-unsubmitted dispatch that GL orders this write AFTER. Writing
+        // the mapping now would land the bytes underneath that dispatch - its
+        // increments then execute on top of the newer data and invert the call order.
+        // Retire the pending GPU writes first, as FillSubData already does. Shadow-
+        // backed stores need none of this: the Memcpy below touches only the shadow,
+        // and the backend's SubData op does its own ordering against in-flight work.
+        if (m_resource.IsGpuResident()) {
+            SyncGpuWrites();
+        }
+
         Memcpy(m_resource.Bytes() + atOffset, data.data, data.size);
         NotifyContentWrite(atOffset, data.size);
     }
@@ -305,6 +317,12 @@ namespace MobileGL::MG_State::GLState {
                         size, m_size);
 
         src->SyncGpuWrites();
+        // The DESTINATION needs the same ordering as UploadSubData: an adopted store is
+        // written in place, so pending recorded GPU writes to it must retire before the
+        // copy lands or they would execute on top of it.
+        if (m_resource.IsGpuResident()) {
+            SyncGpuWrites();
+        }
         Memcpy(m_resource.Bytes() + dstOffset, src->m_resource.Bytes() + srcOffset, size);
         NotifyContentWrite(dstOffset, size);
     }