[Feat] (GLImpl): mark the pack PBO after a glReadPixels and the capture targets at glEndTransformFeedback, in place of a stall and an unbounded fence wait

This commit is contained in:
2026-09-11 14:31:46 -04:00
parent d9ade1e7cd
commit 0b847d462a
2 changed files with 43 additions and 1 deletions
+28 -1
View File
@@ -12,6 +12,9 @@
#include <MG_State/EGLState/Core.h> #include <MG_State/EGLState/Core.h>
#include <MG_Backend/BackendObjects.h> #include <MG_Backend/BackendObjects.h>
#include <MG_Impl/Pipe/PipeFill.h> #include <MG_Impl/Pipe/PipeFill.h>
#if MOBILEGL_BUILD_DISAGGREGATED
#include <MG_Remote/Client/GpuWritePending.h>
#endif
#include "../Getter/GL_Getter.h" #include "../Getter/GL_Getter.h"
namespace MobileGL::MG_Impl::GLImpl { namespace MobileGL::MG_Impl::GLImpl {
@@ -1315,6 +1318,14 @@ namespace MobileGL::MG_Impl::GLImpl {
static_cast<Uint>(bufferIndex)); static_cast<Uint>(bufferIndex));
const auto& buffer = bindingPoint.GetBoundObject(); const auto& buffer = bindingPoint.GetBoundObject();
if (buffer == nullptr) continue; if (buffer == nullptr) continue;
#if MOBILEGL_BUILD_DISAGGREGATED
// This fixup READS the captured bytes back through the shadow, so it is the one
// consumer that cannot simply inherit the deferral EndTransformFeedback's dropped
// fence introduces. Under split it pays the reconciliation itself, which is the
// same cost the fence used to charge every caller - here charged only to the
// capture shapes that actually need reordering.
buffer->SyncGpuWrites();
#endif
const Range1D range = bindingPoint.GetRange(); const Range1D range = bindingPoint.GetRange();
const Uint8* mapped = buffer->MappedData(); const Uint8* mapped = buffer->MappedData();
if (mapped == nullptr) continue; if (mapped == nullptr) continue;
@@ -1355,12 +1366,28 @@ namespace MobileGL::MG_Impl::GLImpl {
MGP_FILL(EndTransformFeedback); MGP_FILL(EndTransformFeedback);
endXfb(); endXfb();
} }
#if MOBILEGL_BUILD_DISAGGREGATED
// P5 (b1), the second producer the client-side GPU-write set ADDS, and it has to be
// taken HERE - before GLContext::EndTransformFeedback clears the live bindings, since
// a mark taken after it marks nothing.
MG_Remote::Client::MarkEndTransformFeedbackCaptureTargets();
#endif
MG_State::pGLContext->EndTransformFeedback(); MG_State::pGLContext->EndTransformFeedback();
// Captured results must be visible to MapBuffer/GetBufferSubData after // Captured results must be visible to MapBuffer/GetBufferSubData after
// End; the capture targets are host-coherent GPU memory, so completing // End; the capture targets are host-coherent GPU memory, so completing
// the GPU work is all that is required. // the GPU work is all that is required.
//
// P5 (b1): UNDER SPLIT THE UNBOUNDED WAIT GOES AND THE MARK ABOVE REPLACES IT. The
// wait exists for one reason - so that a later MapBuffer sees real captured results -
// and that is precisely what m_gpuWritePending says; SyncGpuWrites then pays for it
// once, on the first read that actually wants the bytes, instead of on every
// glEndTransformFeedback. A ~0ull ClientWaitSync on the GL thread is also the one
// shape a verb barrier cannot make cheap, because it is the driver's wait and not the
// barrier's.
auto& backendGL = MG_Backend::gBackendFunctionsTable.GL; auto& backendGL = MG_Backend::gBackendFunctionsTable.GL;
if (backendGL.FenceSync && backendGL.ClientWaitSync) { const Bool waitForTheCapture =
MG_Config::Transport == MG_Config::TransportMode::Monolith;
if (waitForTheCapture && backendGL.FenceSync && backendGL.ClientWaitSync) {
MGP_FILL(FenceSync); MGP_FILL(FenceSync);
if (auto sync = backendGL.FenceSync()) { if (auto sync = backendGL.FenceSync()) {
MGP_FILL(ClientWaitSync); MGP_FILL(ClientWaitSync);
@@ -16,6 +16,9 @@
#include <MG_Impl/GLImpl/Getter/GL_Getter.h> #include <MG_Impl/GLImpl/Getter/GL_Getter.h>
#include <MG_State/GLState/ErrorState/Error.h> #include <MG_State/GLState/ErrorState/Error.h>
#include <MG_Impl/Pipe/PipeFill.h> #include <MG_Impl/Pipe/PipeFill.h>
#if MOBILEGL_BUILD_DISAGGREGATED
#include <MG_Remote/Client/GpuWritePending.h>
#endif
#if MOBILEGL_PIPE_PUSH #if MOBILEGL_PIPE_PUSH
// P4a, ID-19(c). This file is the ONLY place every DSA framebuffer entry point lives, and the // P4a, ID-19(c). This file is the ONLY place every DSA framebuffer entry point lives, and the
// emitter it reaches is this package's own header rather than a declaration in one of the // emitter it reaches is this package's own header rather than a declaration in one of the
@@ -3093,6 +3096,18 @@ namespace MobileGL::MG_Impl::GLImpl {
void ReadPixels_Backend(GLint x, GLint y, GLsizei width, GLsizei height, GLenum format, GLenum type, void* pixels) { void ReadPixels_Backend(GLint x, GLint y, GLsizei width, GLsizei height, GLenum format, GLenum type, void* pixels) {
MGP_FILL(ReadPixels); MGP_FILL(ReadPixels);
MG_Backend::gBackendFunctionsTable.GL.ReadPixels(x, y, width, height, format, type, pixels); MG_Backend::gBackendFunctionsTable.GL.ReadPixels(x, y, width, height, format, type, pixels);
#if MOBILEGL_BUILD_DISAGGREGATED
// P5 (b1), one of the two producers the client-side GPU-write set ADDS. A read into a
// bound GL_PIXEL_PACK_BUFFER is a GPU write to that buffer exactly as a shader's store
// is, and marking it is what makes the next glMapBuffer / glGetBufferSubData of the
// PBO reconcile. It is a no-op on the monolith path, where the backend still maps the
// PBO and copies it into the shadow inside the call
// (DirectGLES.cpp:10983-10993) - an unconditional stall on every glReadPixels whether
// or not anything ever reads the shadow. Deferring that to the first read that wants
// it is STRICTLY BETTER, which is the only reason a split build is allowed to differ
// here at all.
MG_Remote::Client::MarkReadPixelsPackBuffer();
#endif
} }
/* @INSERTION_POINT:FUNCTION_IMPLEMENTATION@ */ /* @INSERTION_POINT:FUNCTION_IMPLEMENTATION@ */