[Fix] (DirectVulkan, tools/trace_replay): preserve resident buffer ordering and coherent trace copies - fix intermittent geometry and UI corruption in Minecraft 26.3-rc-3

Stage updates to adopted GPU-resident buffers on the command timeline instead of overwriting memory that earlier draws still read. Wait for submitted copies as well as pending commands before CPU readback.

Commit coherent mapped writes before Android trace buffer copies, and retire buffer shadows on deletion so copies see current vertices and name reuse cannot leave dangling dirty-shadow entries.

Validated draw ordering, submitted-copy readback, traced buffer copies, and mapped-buffer deletion/name reuse on Redmi Adreno 830. Minecraft 26.3-rc-3 Magma passed movement, HUD, and menu checks and exited normally; the new trace parsed all 64,410 frames without warnings.

MobileGL/MG_Backend/DirectVulkan/Renderer/VkBufferManager.cpp

@@ -65,6 +65,12 @@ namespace MobileGL::MG_Backend::DirectVulkan {
             }
         }
 
+        void Ops_ResidentSubData(BufferObject& bufferObject, SizeT offset, DataPtr data) {
+            if (g_activeBufferManager) {
+                g_activeBufferManager->OnResidentSubData(bufferObject, offset, data);
+            }
+        }
+
         void Ops_FlushMappedRange(BufferObject& bufferObject, Range1D range,
                                   Flags<BufferMappingAccessBit> appAccess) {
             if (g_activeBufferManager) {
@@ -76,11 +82,12 @@ namespace MobileGL::MG_Backend::DirectVulkan {
         // host-visible GPU storage (EnsureGpuResidentStorage adopts it when the buffer is
         // bound as a shader storage buffer), so nothing needs copying - but coherence only
         // says the writes are visible once they have happened, so the work has to retire
-        // first.
+        // first, including copies already submitted by a sync-point flush.
         void Ops_ReadbackFromGpu(BufferObject& bufferObject) {
             (void)bufferObject;
             if (pVulkanRenderer) {
-                pVulkanRenderer->FinishPendingGpuWork();
+                pVulkanRenderer->WaitForSubmitIndex(
+                    pVulkanRenderer->GetSyncPointSubmitIndex(), UINT64_MAX, true);
             }
         }
 
@@ -104,6 +111,7 @@ namespace MobileGL::MG_Backend::DirectVulkan {
         const BufferBackendOps g_vulkanBufferBackendOps = {
             .Respecify = Ops_Respecify,
             .SubData = Ops_SubData,
+            .ResidentSubData = Ops_ResidentSubData,
             .FlushMappedRange = Ops_FlushMappedRange,
             .OnDestroy = Ops_OnDestroy,
             .AcquirePersistentMap = Ops_AcquirePersistentMap,
@@ -343,13 +351,13 @@ namespace MobileGL::MG_Backend::DirectVulkan {
         return true;
     }
 
-    Bool VkBufferManager::StagedRangeCopy(VkBufferResource& resource, MG_State::GLState::BufferObject& bufferObject,
+    Bool VkBufferManager::StagedRangeCopy(VkBufferResource& resource, const void* data,
                                           SizeT offset, SizeT size) {
         if (!m_copyProvider) {
             return false;
         }
         BufferSlice staging{};
-        if (!m_transientUploadArena.Upload(m_currentFrameIndex, bufferObject.MappedData() + offset,
+        if (!m_transientUploadArena.Upload(m_currentFrameIndex, data,
                                            static_cast<VkDeviceSize>(size), 16, staging)) {
             return false;
         }
@@ -454,11 +462,32 @@ namespace MobileGL::MG_Backend::DirectVulkan {
         // Busy partial write: stage + GPU copy preserves GL ordering within the
         // frame and leaves bytes outside the range (possibly GPU-written, e.g.
         // SSBO) intact. Fall back to a storage swap if staging is unavailable.
-        if (!StagedRangeCopy(*resource, bufferObject, offset, size)) {
+        if (!StagedRangeCopy(*resource, bufferObject.MappedData() + offset, offset, size)) {
             SwapStorageAndUploadAll(*resource, bufferObject);
         }
     }
 
+    void VkBufferManager::OnResidentSubData(MG_State::GLState::BufferObject& bufferObject,
+                                           SizeT offset, DataPtr data) {
+        auto* resource = ResourceOf(bufferObject);
+        MOBILEGL_ASSERT(resource && resource->persistentMapped && resource->buffer.IsValid(),
+                        "OnResidentSubData requires adopted Vulkan storage");
+        // The mapping is also the GPU's storage. Copy the supplied bytes onto the
+        // command timeline before touching it: earlier draws must keep seeing the
+        // old contents, including draws recorded but not yet submitted. The buffer
+        // cannot be orphaned because the application may hold its mapped pointer.
+        if (StagedRangeCopy(*resource, data.data, offset, data.size)) {
+            return;
+        }
+        // Allocation failure: a host write is safe only after all prior work retires.
+        if (pVulkanRenderer && pVulkanRenderer->WaitForSubmitIndex(
+                pVulkanRenderer->GetSyncPointSubmitIndex(), UINT64_MAX, true)) {
+            resource->buffer.Upload(data.data, data.size, offset);
+        } else {
+            MGLOG_E_ONCE("VkBufferManager::OnResidentSubData: ordered upload failed");
+        }
+    }
+
     void VkBufferManager::OnFlushMappedRange(MG_State::GLState::BufferObject& bufferObject, Range1D range,
                                              Flags<BufferMappingAccessBit> appAccess) {
         auto* resource = ResourceOf(bufferObject);
@@ -488,7 +517,7 @@ namespace MobileGL::MG_Backend::DirectVulkan {
             return;
         }
 
-        if (!StagedRangeCopy(*resource, bufferObject, offset, size)) {
+        if (!StagedRangeCopy(*resource, bufferObject.MappedData() + offset, offset, size)) {
             SwapStorageAndUploadAll(*resource, bufferObject);
         }
     }

MobileGL/MG_Backend/DirectVulkan/Renderer/VkBufferManager.h

@@ -160,6 +160,7 @@ namespace MobileGL::MG_Backend::DirectVulkan {
         // Immediate ops, dispatched from the frontend BufferBackendOps table.
         void OnRespecify(MG_State::GLState::BufferObject& bufferObject);
         void OnSubData(MG_State::GLState::BufferObject& bufferObject, SizeT offset, SizeT size);
+        void OnResidentSubData(MG_State::GLState::BufferObject& bufferObject, SizeT offset, DataPtr data);
         void OnFlushMappedRange(MG_State::GLState::BufferObject& bufferObject, Range1D range,
                                 Flags<BufferMappingAccessBit> appAccess);
         void OnResourceDestroyed(SharedPtr<MG_State::GLState::BackendBufferResource>&& resource);
@@ -189,7 +190,7 @@ namespace MobileGL::MG_Backend::DirectVulkan {
         Bool SwapStorageAndUploadAll(VkBufferResource& resource, MG_State::GLState::BufferObject& bufferObject);
         // Record a staging-slice copy into the resident storage, ordered against
         // in-flight and already-recorded GPU work.
-        Bool StagedRangeCopy(VkBufferResource& resource, MG_State::GLState::BufferObject& bufferObject,
+        Bool StagedRangeCopy(VkBufferResource& resource, const void* data,
                              SizeT offset, SizeT size);
         void DeferRelease(VkBufferObject&& buffer);
         void CollectDeferredReleases(Uint32 frameIndex);

tools/trace_replay/skills/trace-fixture-authoring-on-android-fcl/scripts/build_android_egltrace.py

@@ -123,6 +123,41 @@ static void mobilegl_capture_record_request(void) {
     if brace < 0:
         raise SystemExit("eglSwapBuffers wrapper has no function body")
     text = text[:brace + 1] + "\n    mobilegl_capture_record_request();" + text[brace + 1:]
+
+    # A buffer copy consumes coherent mapped writes just like a draw. Upstream
+    # only commits shadows at draws, so Minecraft's staging-buffer copies could
+    # consume the previous frame's vertices before the current writes landed.
+    for name in ("glCopyBufferSubData", "glCopyNamedBufferSubData", "glNamedCopyBufferSubDataEXT"):
+        start = text.find(f"void APIENTRY {name}(")
+        if start < 0:
+            raise SystemExit(f"generated egltrace.cpp has no {name} wrapper to patch")
+        brace = text.index("{", start)
+        text = (text[:brace + 1] +
+                "\n    GLMemoryShadow::commitAllWrites(gltrace::getContext(), trace::fakeMemcpy);" +
+                text[brace + 1:])
+
+    # Deleting a mapped buffer implicitly unmaps it. Retire its shadow before
+    # the real storage is freed; a later draw must not commit to a deleted map,
+    # and reuse of the GL name must not leave a dangling dirtyShadows entry.
+    retire_shadows = r'''
+    auto *_ctx = gltrace::getContext();
+    for (GLsizei i = 0; i < n; ++i) {
+        auto it = _ctx->sharedRes->bufferToShadowMemory.find(buffers[i]);
+        if (it != _ctx->sharedRes->bufferToShadowMemory.end()) {
+            if (it->second->getMapFlags() != 0) {
+                it->second->unmap(trace::fakeMemcpy);
+            }
+            _ctx->sharedRes->bufferToShadowMemory.erase(it);
+        }
+    }
+'''
+    for name in ("glDeleteBuffers", "glDeleteBuffersARB"):
+        start = text.find(f"void APIENTRY {name}(")
+        if start < 0:
+            raise SystemExit(f"generated egltrace.cpp has no {name} wrapper to patch")
+        brace = text.index("{", start)
+        text = text[:brace + 1] + retire_shadows + text[brace + 1:]
+
     generated.write_text(text, encoding="utf-8", newline="\n")
     return generated