mirror of
https://github.com/MobileGL-Dev/MobileGL
synced 2026-09-08 12:18:30 +09:00
[Fix] (MG_State, DirectGLES): read a shader-written storage buffer back before mapping it
Buffer contents live in a CPU shadow that every read - MapBuffer, MapBufferRange, GetBufferSubData, CopyBufferSubData - resolves against, and backend transfer ops only ever push the shadow outwards. Two paths already knew the GPU can write a buffer on its own and mirrored the result back by hand (ReadPixels into a pixel-pack buffer, the transform feedback capture at EndTransformFeedback); a shader storage buffer written by a draw or a dispatch had no such path at all, so the map handed the application the bytes from before the dispatch. Nothing exercised it until now because GL 3.3 has no compute stage. Every KHR-GL40.texture_gather case ends by dispatching a compute shader that writes its sampled texel into an SSBO and comparing the mapped result, and all 71 read back the zero-filled shadow. Adds the missing direction as a backend op: BufferObject::MarkGpuWritten flags a buffer the GPU may have moved ahead of the shadow, SyncGpuWrites pulls it back at every read point, and DirectGLES implements the readback with a plain read map of the ES buffer. The flag is raised where the storage-buffer points are bound for the upcoming draw or dispatch, which is the last moment the set of exposed buffers is known, and cleared by the readback - so a buffer nothing writes costs one bool test per map. Backends that cannot read their storage back leave the op null and keep today's behaviour; a GPU-resident (coherent persistent) buffer needs nothing, since its reads already resolve against the memory the shader wrote. Drops the texture_gather failures from 71/75 to 25/75 with no crashes left.
This commit is contained in:
@@ -295,6 +295,20 @@ namespace MobileGL::MG_Backend::DirectGLES {
|
||||
}
|
||||
}
|
||||
|
||||
// Called once the storage-buffer points are bound and the draw/dispatch is about to
|
||||
// go out: whatever the shader writes there lands in the ES driver's buffers, behind
|
||||
// the frontend's CPU shadow. Flagging them makes the next MapBuffer/GetBufferSubData
|
||||
// pull the real contents back (BufferObject::SyncGpuWrites).
|
||||
void MarkShaderStorageBuffersGpuWritten() {
|
||||
const SizeT bindingPointCnt =
|
||||
MG_State::pGLContext->GetTouchedBufferBindingPointCount(BufferTarget::ShaderStorage);
|
||||
for (SizeT i = 0; i < bindingPointCnt; ++i) {
|
||||
const auto& obj =
|
||||
MG_State::pGLContext->GetBufferBindingPoint(BufferTarget::ShaderStorage, i).GetBoundObject();
|
||||
if (obj) obj->MarkGpuWritten();
|
||||
}
|
||||
}
|
||||
|
||||
void SyncBoundBuffer(BufferTarget target, GLenum glTarget) {
|
||||
#ifdef TRACY_ENABLE
|
||||
ZoneScopedC(TRACY_ZONECOLOR_BACKEND);
|
||||
@@ -367,6 +381,7 @@ namespace MobileGL::MG_Backend::DirectGLES {
|
||||
// BindCurrentProgramWithResources binds no SSBO points, so this is their sole draw-path
|
||||
// binder (e.g. Flywheel's indirect vertex shaders pull instance data from storage buffers).
|
||||
SyncBufferBindingPoints(BufferTarget::ShaderStorage, GL_SHADER_STORAGE_BUFFER);
|
||||
MarkShaderStorageBuffersGpuWritten();
|
||||
}
|
||||
|
||||
void SyncComputeBuffers(Bool includeDispatchIndirectBuffer) {
|
||||
@@ -376,6 +391,7 @@ namespace MobileGL::MG_Backend::DirectGLES {
|
||||
ProcessDeferredBufferReleases();
|
||||
SyncBufferBindingPoints(BufferTarget::Uniform, GL_UNIFORM_BUFFER);
|
||||
SyncBufferBindingPoints(BufferTarget::ShaderStorage, GL_SHADER_STORAGE_BUFFER);
|
||||
MarkShaderStorageBuffersGpuWritten();
|
||||
if (includeDispatchIndirectBuffer) {
|
||||
SyncBoundBuffer(BufferTarget::DispatchIndirect, GL_DISPATCH_INDIRECT_BUFFER);
|
||||
}
|
||||
|
||||
@@ -630,6 +630,32 @@ namespace MobileGL::MG_Backend::DirectGLES {
|
||||
resource->syncedChangeSerial = bufferObject.GetChangeSerial();
|
||||
}
|
||||
|
||||
// A shader wrote this buffer through a storage/atomic-counter binding, so the ES
|
||||
// driver's copy is ahead of the frontend shadow. Pull the whole thing back so
|
||||
// MapBuffer/GetBufferSubData/CopyBufferSubData see the real results.
|
||||
void Ops_ReadbackFromGpu(BufferObject& bufferObject) {
|
||||
auto* resource = ResourceOf(bufferObject);
|
||||
if (!resource || resource->id == 0 || !resource->storageInitialized) return;
|
||||
if (resource->persistentMapped) return; // shadow already IS the GPU storage
|
||||
if (!CanTouchGLNow() || resource->contextGeneration != g_bufferContextGeneration) return;
|
||||
if (!g_GLESFuncs.glMapBufferRange || !g_GLESFuncs.glUnmapBuffer) return;
|
||||
const SizeT size = std::min<SizeT>(bufferObject.GetSize(), resource->storageSize);
|
||||
if (size == 0) return;
|
||||
|
||||
BindBufferId(TempBufferTarget, resource->id);
|
||||
void* mapped = g_GLESFuncs.glMapBufferRange(TempBufferTarget, 0, static_cast<GLsizeiptr>(size),
|
||||
GL_MAP_READ_BIT);
|
||||
if (mapped == nullptr) {
|
||||
MGLOG_E("Ops_ReadbackFromGpu: glMapBufferRange(read) failed for buffer %u", resource->id);
|
||||
return;
|
||||
}
|
||||
bufferObject.WritebackFromBackend({mapped, size}, 0);
|
||||
g_GLESFuncs.glUnmapBuffer(TempBufferTarget);
|
||||
// The shadow now matches the backend byte for byte; without this the next
|
||||
// draw would see a newer change serial and re-upload the readback over it.
|
||||
resource->syncedChangeSerial = bufferObject.GetChangeSerial();
|
||||
}
|
||||
|
||||
void Ops_OnDestroy(SharedPtr<BackendBufferResource>&& resource) {
|
||||
if (!resource) return;
|
||||
auto* glesResource = static_cast<GLESBufferResource*>(resource.get());
|
||||
@@ -662,6 +688,7 @@ namespace MobileGL::MG_Backend::DirectGLES {
|
||||
.FlushMappedRange = Ops_FlushMappedRange,
|
||||
.OnDestroy = Ops_OnDestroy,
|
||||
.AcquirePersistentMap = Ops_AcquirePersistentMap,
|
||||
.ReadbackFromGpu = Ops_ReadbackFromGpu,
|
||||
};
|
||||
} // namespace
|
||||
|
||||
|
||||
@@ -878,6 +878,7 @@ namespace MobileGL::MG_Impl::GLImpl {
|
||||
return;
|
||||
}
|
||||
|
||||
bufferObject->SyncGpuWrites();
|
||||
bufferObject->DownloadSubData(data, static_cast<SizeT>(offset), static_cast<SizeT>(size));
|
||||
}
|
||||
|
||||
|
||||
@@ -174,6 +174,24 @@ namespace MobileGL::MG_State::GLState {
|
||||
++m_changeSerial;
|
||||
}
|
||||
|
||||
void BufferObject::MarkGpuWritten() {
|
||||
// A GPU-resident buffer has no separate shadow to refresh: reads already resolve
|
||||
// against the coherent map the shader wrote into.
|
||||
if (m_resource.IsGpuResident()) return;
|
||||
m_gpuWritePending = true;
|
||||
}
|
||||
|
||||
void BufferObject::SyncGpuWrites() {
|
||||
if (!m_gpuWritePending) return;
|
||||
// Cleared unconditionally: without a readback op the shadow can never catch up,
|
||||
// and retrying on every subsequent read would only repeat the same no-op.
|
||||
m_gpuWritePending = false;
|
||||
if (m_size == 0 || g_bufferBackendOps == nullptr || g_bufferBackendOps->ReadbackFromGpu == nullptr) {
|
||||
return;
|
||||
}
|
||||
g_bufferBackendOps->ReadbackFromGpu(*this);
|
||||
}
|
||||
|
||||
void BufferObject::UploadSubData(DataPtr data, SizeT atOffset) {
|
||||
MOBILEGL_ASSERT(!m_isMapped || (m_mappingAccess & BufferMappingAccessBit::Persistent),
|
||||
"Cannot upload sub data while buffer is non-persistently mapped.");
|
||||
@@ -204,11 +222,13 @@ namespace MobileGL::MG_State::GLState {
|
||||
"Destination buffer copy out of bounds: dstOffset (%zu) + size (%zu) > m_size (%zu)", dstOffset,
|
||||
size, m_size);
|
||||
|
||||
src->SyncGpuWrites();
|
||||
Memcpy(m_resource.Bytes() + dstOffset, src->m_resource.Bytes() + srcOffset, size);
|
||||
NotifyContentWrite(dstOffset, size);
|
||||
}
|
||||
|
||||
void* BufferObject::AcquireMemory(Bool markMapped, Bool read, Bool write) {
|
||||
SyncGpuWrites();
|
||||
if (markMapped) {
|
||||
m_isMapped = true;
|
||||
m_mappingAccess = (read ? BufferMappingAccessBit::Read : BufferMappingAccessBit::Null) |
|
||||
@@ -250,6 +270,10 @@ namespace MobileGL::MG_State::GLState {
|
||||
MOBILEGL_ASSERT(range.end <= m_size && range.start <= range.end,
|
||||
"AcquireMemoryRange out of bounds: range (%zu, %zu) exceeds m_size (%zu)", range.start,
|
||||
range.end, m_size);
|
||||
// The app is about to look at the bytes; a shader may have rewritten them since
|
||||
// the shadow was last authoritative. Also needed for a write map without an
|
||||
// invalidate bit, whose staging copy is seeded from the shadow.
|
||||
SyncGpuWrites();
|
||||
m_isMapped = true;
|
||||
m_mappingAccess = access;
|
||||
m_mappedRange = range;
|
||||
|
||||
@@ -99,6 +99,13 @@ namespace MobileGL {
|
||||
// Must be idempotent: a second call for an already-backed buffer returns the
|
||||
// same base pointer.
|
||||
void* (*AcquirePersistentMap)(BufferObject& bufferObject) = nullptr;
|
||||
// Pulls the backend's current contents for the whole buffer into the shadow
|
||||
// (through WritebackFromBackend). Only ever called for a buffer the GPU may
|
||||
// have written behind the frontend's back - a shader storage or atomic counter
|
||||
// binding of a draw or dispatch - because nothing else can desynchronise the
|
||||
// shadow. Backends that cannot read their storage back leave this null; the
|
||||
// shadow then keeps its pre-dispatch bytes, which is the old behaviour.
|
||||
void (*ReadbackFromGpu)(BufferObject& bufferObject) = nullptr;
|
||||
};
|
||||
|
||||
// Registered by the active backend at init, cleared at shutdown.
|
||||
@@ -149,6 +156,14 @@ namespace MobileGL {
|
||||
// backend op: the backend storage already holds these bytes.
|
||||
void WritebackFromBackend(DataPtr data, SizeT atOffset);
|
||||
|
||||
// A draw or dispatch just ran with this buffer bound where a shader can write
|
||||
// it (shader storage / atomic counter): the GPU copy may now differ from the
|
||||
// shadow, so the next read has to pull it back.
|
||||
void MarkGpuWritten();
|
||||
// Refreshes the shadow from the backend when a GPU write is outstanding. Called
|
||||
// from every path that reads the shadow on the app's behalf.
|
||||
void SyncGpuWrites();
|
||||
|
||||
Bool IsMapped() const;
|
||||
Bool IsImmutableStorage() const;
|
||||
SizeT GetSize() const;
|
||||
@@ -196,6 +211,8 @@ namespace MobileGL {
|
||||
Bool m_isImmutableStorage = false;
|
||||
GLbitfield m_storageFlags = 0;
|
||||
Uint64 m_changeSerial = 0;
|
||||
// Set by MarkGpuWritten, cleared by SyncGpuWrites once the shadow is refreshed.
|
||||
Bool m_gpuWritePending = false;
|
||||
Range1D m_mappedRange;
|
||||
Vector<Uint8> m_stagingData;
|
||||
Bool m_ownsStagingData;
|
||||
|
||||
Reference in New Issue
Block a user