From 0ee3384b229d000d7a7c5c05a04db34306de2088 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Fri, 28 Aug 2026 04:50:07 -0400 Subject: [PATCH] [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 | 82 ++++++++++++++++++- MobileGL/MG_Backend/DirectGLES/Managers.h | 10 +++ .../GLState/BufferState/BufferObject.cpp | 45 ++++++++++ .../GLState/BufferState/BufferObject.h | 11 +++ 4 files changed, 145 insertions(+), 3 deletions(-) diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.cpp b/MobileGL/MG_Backend/DirectGLES/Managers.cpp index f8b906cb..836f6f8d 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.cpp +++ b/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 writes; + { + const std::lock_guard 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 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 lock(resource->pendingMutex); + auto& write = resource->pendingResidentWrites.emplace_back(); + write.offset = offset; + const auto* bytes = static_cast(data.data); + write.bytes.assign(bytes, bytes + data.size); + } + void Ops_FlushMappedRange(BufferObject& bufferObject, Range1D range, Flags 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 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 lock(resource->pendingMutex); resource->pendingRanges.clear(); + resource->pendingResidentWrites.clear(); } resource->syncedChangeSerial = bufferObject->GetChangeSerial(); } else { diff --git a/MobileGL/MG_Backend/DirectGLES/Managers.h b/MobileGL/MG_Backend/DirectGLES/Managers.h index 73729c7d..6603f2cc 100644 --- a/MobileGL/MG_Backend/DirectGLES/Managers.h +++ b/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 bytes; + }; + Vector pendingResidentWrites; std::mutex pendingMutex; // Buffer-mutation epoch (see CurrentBufferMutationEpoch) at which this // resource last probed IsBufferDrawClean == true, 0 = never (epochs start diff --git a/MobileGL/MG_State/GLState/BufferState/BufferObject.cpp b/MobileGL/MG_State/GLState/BufferState/BufferObject.cpp index 5c0fd40c..fd43cae5 100644 --- a/MobileGL/MG_State/GLState/BufferState/BufferObject.cpp +++ b/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 expanded(size); + if (pattern.size == 1) { + Memset(expanded.data(), *static_cast(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. diff --git a/MobileGL/MG_State/GLState/BufferState/BufferObject.h b/MobileGL/MG_State/GLState/BufferState/BufferObject.h index 756aedab..583fd0dc 100644 --- a/MobileGL/MG_State/GLState/BufferState/BufferObject.h +++ b/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.