[Fix] (Espryt): land a SubData into an adopted store as a GPU-ordered copy - the in-place coherent write tore the frames still reading the old section bytes

MobileGL/MG_Backend/DirectGLES/Managers.cpp

@@ -783,6 +783,7 @@ namespace MobileGL::MG_Backend::DirectGLES {
                 resource.storageInitialized = true;
                 resource.pendingRespecify = false;
                 resource.pendingRanges.clear();
+                resource.pendingResidentWrites.clear();
                 resource.syncedChangeSerial = bufferObject.GetChangeSerial();
                 // A GROWN store keeps its indexed bindings, and BindBufferBaseCached skips a
                 // rebind whenever the shadow already records this id at that index - so on a
@@ -926,6 +927,49 @@ namespace MobileGL::MG_Backend::DirectGLES {
                 }
             }
 
+            // Land the app bytes queued for an ADOPTED store on the GPU timeline: staged
+            // into the upload ring and delivered by glCopyBufferSubData. The destination
+            // is the IMMUTABLE persistent store, which the driver can neither rename nor
+            // ghost, so the copy is plain job ordering - after every in-flight reader,
+            // before the next consumer - which is exactly glBufferSubData's contract.
+            // (The in-place host write these bytes replaced tore the frames still
+            // reading the old vertex data: one-frame wrong geometry during fast camera
+            // movement.) Fallback: direct glBufferSubData - the adopted store carries
+            // DYNAMIC_STORAGE, and immutability again forbids the whole-store ghost.
+            void DrainResidentWritesNow(GLESBufferResource& resource, BufferObject& bufferObject) {
+#ifdef TRACY_ENABLE
+                ZoneScopedC(TRACY_ZONECOLOR_BACKEND);
+#endif
+                Vector<GLESBufferResource::PendingResidentWrite> writes;
+                {
+                    const std::lock_guard<std::mutex> lock(resource.pendingMutex);
+                    if (resource.pendingResidentWrites.empty()) return;
+                    writes = std::move(resource.pendingResidentWrites);
+                    resource.pendingResidentWrites.clear();
+                }
+                const SizeT limit = resource.storageSize;
+                const Bool ringUsable = UploadRingUsableNow();
+                for (const auto& write : writes) {
+                    if (write.offset >= limit) continue;
+                    const SizeT size = std::min(write.bytes.size(), limit - write.offset);
+                    if (size == 0) continue;
+                    SizeT ringOffset = 0;
+                    if (ringUsable && size <= kUploadRingMaxBytes &&
+                        RingAllocate(g_uploadRing, size, ringOffset)) {
+                        Memcpy(g_uploadRing.store.mappedPtr + ringOffset, write.bytes.data(), size);
+                        BindBufferId(GL_COPY_READ_BUFFER, g_uploadRing.store.id);
+                        BindBufferId(GL_COPY_WRITE_BUFFER, resource.id);
+                        g_GLESFuncs.glCopyBufferSubData(GL_COPY_READ_BUFFER, GL_COPY_WRITE_BUFFER,
+                                                        (GLintptr)ringOffset, (GLintptr)write.offset,
+                                                        (GLsizeiptr)size);
+                    } else {
+                        BindBufferId(TempBufferTarget, resource.id);
+                        g_GLESFuncs.glBufferSubData(TempBufferTarget, (GLintptr)write.offset, (GLsizeiptr)size,
+                                                    write.bytes.data());
+                    }
+                }
+            }
+
             // EXT_buffer_storage bit values (same numeric values as the desktop ARB
             // tokens); defined locally so this compiles regardless of which GLES headers
             // expose the EXT tokens.
@@ -1011,6 +1055,7 @@ namespace MobileGL::MG_Backend::DirectGLES {
                 {
                     const std::lock_guard<std::mutex> lock(resource->pendingMutex);
                     resource->pendingRanges.clear();
+                    resource->pendingResidentWrites.clear();
                 }
                 resource->syncedChangeSerial = bufferObject.GetChangeSerial();
                 return ptr;
@@ -1048,12 +1093,14 @@ namespace MobileGL::MG_Backend::DirectGLES {
                     resource->storageSize = 0;
                     resource->pendingRespecify = true;
                     resource->pendingRanges.clear();
+                    resource->pendingResidentWrites.clear();
                     return;
                 }
                 if (!CanTouchGLNow() || resource->id == 0 ||
                     resource->contextGeneration != g_bufferContextGeneration) {
                     resource->pendingRespecify = true;
                     resource->pendingRanges.clear();
+                    resource->pendingResidentWrites.clear();
                     return;
                 }
                 if (bufferObject.GetSize() == 0) {
@@ -1061,6 +1108,7 @@ namespace MobileGL::MG_Backend::DirectGLES {
                     resource->storageSize = 0;
                     resource->pendingRespecify = false;
                     resource->pendingRanges.clear();
+                    resource->pendingResidentWrites.clear();
                     return;
                 }
                 RespecifyStorageNow(*resource, bufferObject);
@@ -1101,6 +1149,19 @@ namespace MobileGL::MG_Backend::DirectGLES {
                 resource->pendingRanges.Add({offset, offset + size});
             }
 
+            // App bytes for an ADOPTED store: queue them untouched-by-the-mapping; the
+            // draw-time sync (or a readback) lands them GPU-ordered through
+            // DrainResidentWritesNow. No GL here, so the op is thread-agnostic.
+            void Ops_ResidentSubData(BufferObject& bufferObject, SizeT offset, DataPtr data) {
+                auto* resource = ResourceOf(bufferObject);
+                if (!resource || data.size == 0) return;
+                const std::lock_guard<std::mutex> lock(resource->pendingMutex);
+                auto& write = resource->pendingResidentWrites.emplace_back();
+                write.offset = offset;
+                const auto* bytes = static_cast<const Uint8*>(data.data);
+                write.bytes.assign(bytes, bytes + data.size);
+            }
+
             void Ops_FlushMappedRange(BufferObject& bufferObject, Range1D range,
                                       Flags<BufferMappingAccessBit> appAccess) {
                 auto* resource = ResourceOf(bufferObject);
@@ -1174,8 +1235,11 @@ namespace MobileGL::MG_Backend::DirectGLES {
                 if (!resource || resource->id == 0 || !resource->storageInitialized) return;
                 if (!CanTouchGLNow() || resource->contextGeneration != g_bufferContextGeneration) return;
                 if (resource->persistentMapped) {
-                    // Host writes to a persistent map must not race shader writes already queued
-                    // on this context. There is no backend copy to read back in this case.
+                    // Queued resident SubData bytes land first (GPU-ordered), then the
+                    // finish makes them - and any shader writes already queued on this
+                    // context - visible through the coherent mapping the reads use.
+                    // There is no backend copy to read back in this case.
+                    DrainResidentWritesNow(*resource, bufferObject);
                     if (g_GLESFuncs.glFinish) g_GLESFuncs.glFinish();
                     return;
                 }
@@ -1241,6 +1305,10 @@ namespace MobileGL::MG_Backend::DirectGLES {
                 Ops_SubData(bufferObject, offset, size);
                 BumpBufferMutationEpoch();
             }
+            void Ops_ResidentSubDataTracked(BufferObject& bufferObject, SizeT offset, DataPtr data) {
+                Ops_ResidentSubData(bufferObject, offset, data);
+                BumpBufferMutationEpoch();
+            }
             void Ops_FlushMappedRangeTracked(BufferObject& bufferObject, Range1D range,
                                              Flags<BufferMappingAccessBit> appAccess) {
                 Ops_FlushMappedRange(bufferObject, range, appAccess);
@@ -1265,6 +1333,7 @@ namespace MobileGL::MG_Backend::DirectGLES {
             const BufferBackendOps g_glesBufferBackendOps = {
                 .Respecify = Ops_RespecifyTracked,
                 .SubData = Ops_SubDataTracked,
+                .ResidentSubData = Ops_ResidentSubDataTracked,
                 .FlushMappedRange = Ops_FlushMappedRangeTracked,
                 .OnDestroy = Ops_OnDestroyTracked,
                 .AcquirePersistentMap = Ops_AcquirePersistentMapTracked,
@@ -1366,7 +1435,11 @@ namespace MobileGL::MG_Backend::DirectGLES {
             if (resource->id == 0) return false;
             // Zero-copy coherent persistent store: EnsureBufferResource's own early-out —
             // the app writes straight into the mapped GPU storage, nothing to sync.
-            if (resource->persistentMapped) return resource->persistentPtr != nullptr;
+            // Except queued resident SubData bytes, which land through the sync path
+            // (same unlocked emptiness probe as pendingRanges below).
+            if (resource->persistentMapped) {
+                return resource->persistentPtr != nullptr && resource->pendingResidentWrites.empty();
+            }
             // A live non-zero-copy map may owe a per-draw SyncPersistentMappedRange push
             // (persistent maps mutate the shadow without bumping the change serial).
             if (frontend->IsMapped()) return false;
@@ -1398,6 +1471,7 @@ namespace MobileGL::MG_Backend::DirectGLES {
                 resource->storageSize = 0;
                 resource->pendingRespecify = true;
                 resource->pendingRanges.clear();
+                    resource->pendingResidentWrites.clear();
                 resource->contextGeneration = g_bufferContextGeneration;
                 // The persistent map (and its pointer) died with the old context; the
                 // frontend re-acquires a fresh one on its next map.
@@ -1427,6 +1501,7 @@ namespace MobileGL::MG_Backend::DirectGLES {
             // persistently mapped immutable store, so there is nothing to (re)upload at
             // draw time. This is where the per-draw whole-buffer glBufferSubData used to run.
             if (resource->persistentMapped && resource->persistentPtr && resource->id != 0) {
+                DrainResidentWritesNow(*resource, *bufferObject);
                 return resource;
             }
 
@@ -1448,6 +1523,7 @@ namespace MobileGL::MG_Backend::DirectGLES {
                     {
                         const std::lock_guard<std::mutex> lock(resource->pendingMutex);
                         resource->pendingRanges.clear();
+                    resource->pendingResidentWrites.clear();
                     }
                     resource->syncedChangeSerial = bufferObject->GetChangeSerial();
                 } else {

MobileGL/MG_Backend/DirectGLES/Managers.h

@@ -461,6 +461,16 @@ namespace MobileGL::MG_Backend::DirectGLES {
             // the owning thread replaying them: guard both fields with pendingMutex.
             Bool pendingRespecify = false;
             VecRange1D pendingRanges;
+            // App bytes for an ADOPTED store, awaiting their GPU-ordered landing (ring
+            // stage + glCopyBufferSubData at the next sync; see
+            // BufferBackendOps::ResidentSubData). The frontend keeps such writes out of
+            // the coherent mapping - an in-place host write tears the in-flight frames
+            // still reading the old bytes. Guarded by pendingMutex like pendingRanges.
+            struct PendingResidentWrite {
+                SizeT offset = 0;
+                Vector<Uint8> bytes;
+            };
+            Vector<PendingResidentWrite> pendingResidentWrites;
             std::mutex pendingMutex;
             // Buffer-mutation epoch (see CurrentBufferMutationEpoch) at which this
             // resource last probed IsBufferDrawClean == true, 0 = never (epochs start

MobileGL/MG_State/GLState/BufferState/BufferObject.cpp

@@ -283,6 +283,22 @@ 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 in-flight frames are reading, and
+        // GL orders a glBufferSubData after those already-submitted reads. A backend
+        // that can land the bytes on the GPU timeline takes them here, untouched by
+        // the mapping - the in-place host write below tore the frames still reading
+        // the old bytes. The bytes are not current in the mapping until the backend's
+        // ordered copy executes, so reads reconcile through the same gate GPU-written
+        // buffers use.
+        if (m_resource.IsGpuResident() && data.size > 0 && g_bufferBackendOps &&
+            g_bufferBackendOps->ResidentSubData) {
+            g_bufferBackendOps->ResidentSubData(*this, atOffset, data);
+            m_hasDefinedContent = true;
+            ++m_changeSerial;
+            m_gpuWritePending = true;
+            return;
+        }
+
         // 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
@@ -311,6 +327,24 @@ namespace MobileGL::MG_State::GLState {
                         "Cannot fill data while buffer is non-persistently mapped.");
         if (size == 0) return;
 
+        // An adopted store takes the same GPU-timeline landing as UploadSubData: the
+        // in-place write below would tear in-flight readers of the mapping.
+        if (m_resource.IsGpuResident() && g_bufferBackendOps && g_bufferBackendOps->ResidentSubData) {
+            Vector<Uint8> expanded(size);
+            if (pattern.size == 1) {
+                Memset(expanded.data(), *static_cast<const Uint8*>(pattern.data), size);
+            } else {
+                for (SizeT at = 0; at < size; at += pattern.size) {
+                    Memcpy(expanded.data() + at, pattern.data, pattern.size);
+                }
+            }
+            g_bufferBackendOps->ResidentSubData(*this, atOffset, {expanded.data(), size});
+            m_hasDefinedContent = true;
+            ++m_changeSerial;
+            m_gpuWritePending = true;
+            return;
+        }
+
         // A clear is ordered after all earlier GPU writes. Partial clears additionally need the
         // retained shadow bytes; whole-store clears need the same synchronization before writing
         // an adopted persistent mapping that the GPU may still be accessing.
@@ -347,6 +381,17 @@ namespace MobileGL::MG_State::GLState {
                         size, m_size);
 
         src->SyncGpuWrites();
+        // An adopted DESTINATION takes the same GPU-timeline landing as UploadSubData;
+        // the in-place write below would tear in-flight readers of the mapping.
+        if (m_resource.IsGpuResident() && size > 0 && g_bufferBackendOps &&
+            g_bufferBackendOps->ResidentSubData) {
+            g_bufferBackendOps->ResidentSubData(*this, dstOffset,
+                                                {src->m_resource.Bytes() + srcOffset, size});
+            m_hasDefinedContent = true;
+            ++m_changeSerial;
+            m_gpuWritePending = true;
+            return;
+        }
         // 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.

MobileGL/MG_State/GLState/BufferState/BufferObject.h

@@ -80,6 +80,17 @@ namespace MobileGL {
             void (*Respecify)(BufferObject& bufferObject) = nullptr;
             // Contents update of [offset, offset + size) from the shadow.
             void (*SubData)(BufferObject& bufferObject, SizeT offset, SizeT size) = nullptr;
+            // Contents update of an ADOPTED (GPU-resident) store. `data` holds the app's
+            // bytes; the frontend has NOT touched the resident mapping. GL orders a
+            // glBufferSubData after already-submitted GPU reads of the store, and an
+            // in-place host write into the coherent mapping tears the frames still
+            // reading the old bytes (Minecraft patches LIVE chunk sections this way -
+            // the tear shows as one-frame wrong geometry/UVs during fast movement). The
+            // backend lands the bytes on the GPU timeline instead: after in-flight
+            // readers, before the next consumer. The frontend marks the buffer
+            // gpu-write-pending so reads reconcile through ReadbackFromGpu. Backends
+            // without this op keep the legacy ordered in-place host write.
+            void (*ResidentSubData)(BufferObject& bufferObject, SizeT offset, DataPtr data) = nullptr;
             // Write-map flush (glUnmapBuffer / glFlushMappedBufferRange). Carries the
             // app's real mapping flags so the backend can honour INVALIDATE_* /
             // UNSYNCHRONIZED semantics per call instead of merging them.