From b88e848761bee086a8c3f01645b211b5461b5486 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Thu, 17 Sep 2026 13:13:41 -0400 Subject: [PATCH] [MG_Remote, MG_State, MG_Impl, MG_Backend, MG_IntegrationTest] (Disaggregated): P5c census triage - the three real regressions the merge owed The broad inproc census against a same-machine 11ac3de6 baseline found 183 newly-failing; 172 are the design-red the contract rules (the legacy-arm refusal under transport: the subsystem-off and legacy control lanes, and Magma's P7 surface - every one carries its Fatal name in ~/p5c-fatal-map.tsv), and the other eleven were three REAL regressions, fixed here: 1. The surface-changed event drained at the first verb's EmitAndWait, but a pre-verb glGetFramebufferAttachmentParameteriv on the default framebuffer was answered from the placeholder attachments, and buffers allocated from that answer are blit-incompatible with the real surface (DepthStencilReadbackAttachmentShapeScenario). The event ring gains its SECOND drain-point class: the blocking EGL lifecycle RPC returns (BackendObject_Remote's surface create/resize and make-current) - the same known-idle premise as EmitAndWait's post-barrier point. CONTRACT-P5C section 4.1 amended. 2. A whole-buffer writeback larger than the event ring could never fit one record (capacity/2), aborting the server with Fatal{EventRingOverflow} on the large-arena readbacks. The client now slices the request at a quarter of the ring (header and co-posted small events ride along), each slice round-tripping its own barrier + drain; the whole-buffer flag is cleared by hand after the last slice lands, which is exactly what WritebackFromBackend's clear says. CONTRACT-P5C section 4.4 amended. 3. ScatterCapturedRecords / ReadbackCapturedRanges read the capture buffers through the frontend MappedData() (a layer-1 surface since gt) and wrote back by direct call. The XFB capture path now reads the server's staged shadow (RequireStagedCoverage asserted, syncedChangeSerial bumped so no later draw overwrites the capture with stale shadow) and writes back through OnBufferWriteback, the Ops_H_Readback order. CtWireScenario's two death cases skip on DirectVulkan by name: object_death's producer is the Espryt-side death-notice ops and Magma installs none until P7. Evidence: unit 2187/2187; integration-split 111/111; the three families' named cases green again; census newly-failing reduced to the evidenced design-red set. --- MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp | 88 ++++++++++++++++++- MobileGL/MG_Impl/Pipe/PipeFill.cpp | 14 ++- .../Scenarios/CtWireScenario.cpp | 13 +++ MobileGL/MG_Pipe/PipeMutation.h | 5 ++ MobileGL/MG_Remote/CONTRACT-P5C.md | 60 ++++++++++--- .../MG_Remote/Client/BackendObject_Remote.cpp | 18 ++++ MobileGL/MG_Remote/Client/ClientSession.cpp | 9 ++ MobileGL/MG_Remote/Client/ClientSession.h | 17 ++++ MobileGL/MG_Remote/Client/GpuWritePending.cpp | 15 ++++ MobileGL/MG_Remote/Client/GpuWritePending.h | 8 ++ .../GLState/BufferState/BufferObject.cpp | 24 ++++- 11 files changed, 251 insertions(+), 20 deletions(-) diff --git a/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp b/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp index db93234f..4447dbcf 100644 --- a/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp +++ b/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp @@ -1128,7 +1128,50 @@ namespace MobileGL::MG_Backend::DirectGLES { } if (g_GLESFuncs.glMapBufferRange != nullptr && g_GLESFuncs.glUnmapBuffer != nullptr) { for (const auto& target : targets) { - if (!target.buffer || target.buffer->IsBackendPersistentMapped()) continue; + if (!target.buffer) continue; +#if MOBILEGL_BUILD_DISAGGREGATED + // P5c: under an active transport the frontend object is client + // memory (rule E), so the persistence question is the server + // resource's and the captured bytes go back as a writeback EVENT - + // WritebackFromBackend from the apply thread is the R1/R2 shape. + if (MG_Config::Transport != MG_Config::TransportMode::Monolith) { + const MG_Pipe::MGPipeHandle res = BufferImpl::HandleOfBuffer(target.buffer.get()); + auto* resource = BufferImpl::FindBufferResourceForHandle(res); + if (resource == nullptr || resource->persistentMapped) continue; + const SizeT size = target.end - target.start; + BufferImpl::BindBufferId(BufferImpl::TempBufferTarget, target.backendId); + void* mapped = g_GLESFuncs.glMapBufferRange(BufferImpl::TempBufferTarget, + static_cast(target.start), + static_cast(size), GL_MAP_READ_BIT); + if (mapped == nullptr) { + MGLOG_E_ONCE("EndTransformFeedback: failed to map backend buffer %u [%zu, %zu) for " + "capture readback (ES error %s); the captured data will NOT be visible to " + "the application", + target.backendId, target.start, target.end, + MG_Util::ConvertGLEnumToString(TakeXfbDriverError()).c_str()); + continue; + } + if (MG_Pipe::gMGPipeCallbacks.OnBufferWriteback != nullptr) { + MG_Pipe::gMGPipeCallbacks.OnBufferWriteback( + res, target.start, + MG_Pipe::MGPBlobRef{reinterpret_cast(mapped), + static_cast(size), + MG_Pipe::kMGHostSpanSegNone, 0}); + } else { + MGLOG_E_ONCE("EndTransformFeedback: no reverse channel is installed, so the " + "captured bytes of buffer {%u,%u} cannot reach the client shadow", + res.Slot, res.Gen); + } + g_GLESFuncs.glUnmapBuffer(BufferImpl::TempBufferTarget); + // Ops_H_Readback's order: writeback, unmap, THEN the serial + // stamp, so the next draw does not re-upload the server's stale + // staged bytes over the capture that just landed. + resource->syncedChangeSerial = BufferImpl::ResourceSerialForHandle(res); + BufferImpl::BumpBufferMutationEpoch(); + continue; + } +#endif + if (target.buffer->IsBackendPersistentMapped()) continue; const SizeT size = target.end - target.start; BufferImpl::BindBufferId(BufferImpl::TempBufferTarget, target.backendId); void* mapped = g_GLESFuncs.glMapBufferRange(BufferImpl::TempBufferTarget, @@ -1240,6 +1283,30 @@ namespace MobileGL::MG_Backend::DirectGLES { if (stride == 0) continue; const SizeT rangeBytes = target.end - target.start; Vector staged(rangeBytes); +#if MOBILEGL_BUILD_DISAGGREGATED + // P5c: under an active transport BufferObject::MappedData on the apply + // thread is Fatal{RoleViolation, "buffer-legacy-arm"} (CONTRACT-P5C §3.8) - + // the pre-capture bytes are the SERVER's staged shadow, and the reconciled + // range goes back as a writeback EVENT instead of a WritebackFromBackend + // poke into client memory. + MG_Pipe::MGPipeHandle splitRes = MG_Pipe::kMGPipeNullHandle; + BufferImpl::GLESBufferResource* splitResource = nullptr; + if (MG_Config::Transport != MG_Config::TransportMode::Monolith) { + splitRes = BufferImpl::HandleOfBuffer(target.buffer.get()); + splitResource = BufferImpl::FindBufferResourceForHandle(splitRes); + const Uint8* hostBytes = splitResource != nullptr ? splitResource->hostBytes : nullptr; + if (splitResource == nullptr || hostBytes == nullptr) { + MGLOG_E_ONCE("EndTransformFeedback: a scattered capture target (handle {%u,%u}) has " + "no server shadow to read the pre-capture bytes from; its capture is " + "discarded", + splitRes.Slot, splitRes.Gen); + continue; + } + BufferImpl::RequireStagedCoverage(*splitResource, hostBytes, target.start, target.end, + "xfb_scatter_pre_capture"); + Memcpy(staged.data(), hostBytes + target.start, rangeBytes); + } else +#endif Memcpy(staged.data(), target.buffer->MappedData() + target.start, rangeBytes); for (const auto& varying : program->GetTransformFeedbackVaryings()) { @@ -1253,6 +1320,25 @@ namespace MobileGL::MG_Backend::DirectGLES { } } +#if MOBILEGL_BUILD_DISAGGREGATED + if (MG_Config::Transport != MG_Config::TransportMode::Monolith) { + if (MG_Pipe::gMGPipeCallbacks.OnBufferWriteback != nullptr) { + MG_Pipe::gMGPipeCallbacks.OnBufferWriteback( + splitRes, target.start, + MG_Pipe::MGPBlobRef{reinterpret_cast(staged.data()), + static_cast(rangeBytes), + MG_Pipe::kMGHostSpanSegNone, 0}); + } else { + MGLOG_E_ONCE("EndTransformFeedback: no reverse channel is installed, so the " + "scattered capture of buffer {%u,%u} cannot reach the client shadow", + splitRes.Slot, splitRes.Gen); + } + // Ops_H_Readback's stamp, for its reason: the GL store now holds + // bytes the server's staged shadow does not, and the next draw must + // not re-upload the stale shadow over the capture. + splitResource->syncedChangeSerial = BufferImpl::ResourceSerialForHandle(splitRes); + } else +#endif target.buffer->WritebackFromBackend({staged.data(), rangeBytes}, target.start); // Serial bumped with no backend op (see ReadbackCapturedRanges). BufferImpl::BumpBufferMutationEpoch(); diff --git a/MobileGL/MG_Impl/Pipe/PipeFill.cpp b/MobileGL/MG_Impl/Pipe/PipeFill.cpp index 252e0153..9dbb02fe 100644 --- a/MobileGL/MG_Impl/Pipe/PipeFill.cpp +++ b/MobileGL/MG_Impl/Pipe/PipeFill.cpp @@ -905,14 +905,20 @@ namespace MobileGL::MG_Pipe { } void MGPipeEmitResourceReadback(BufferObject& buffer) { + // Whole-buffer by contract (BufferObject.h: the op pulls the backend's current + // contents for the WHOLE buffer into the shadow). The split arm's slicing lives in + // the CALLER (BufferObject::SyncGpuWrites): what "whole" costs is decided by the + // event ring's capacity, which this layer does not read. + MGPipeEmitResourceReadbackRange(buffer, 0, buffer.GetSize()); + } + + void MGPipeEmitResourceReadbackRange(BufferObject& buffer, SizeT offset, SizeT size) { const MGPipeHandle handle = ContentHandleFor(buffer, "resource_readback"); if (MGPipeHandleIsNull(handle)) return; MGPReadback record{}; record.Res = handle; - // Whole-buffer by contract (BufferObject.h: the op pulls the backend's current - // contents for the WHOLE buffer into the shadow). - record.Offset = 0; - record.Size = buffer.GetSize(); + record.Offset = offset; + record.Size = size; // The answer travels back through MGPipeClientOnBufferWriteback, and the server's // epoch bump happens AFTER that writeback, never before. MGPipeRouteResourceReadback(record); diff --git a/MobileGL/MG_IntegrationTest/Scenarios/CtWireScenario.cpp b/MobileGL/MG_IntegrationTest/Scenarios/CtWireScenario.cpp index 2f768a71..1ac051b0 100644 --- a/MobileGL/MG_IntegrationTest/Scenarios/CtWireScenario.cpp +++ b/MobileGL/MG_IntegrationTest/Scenarios/CtWireScenario.cpp @@ -117,6 +117,13 @@ namespace { TEST_F(CtWireScenario, TextureDeathCrossesAndTheRecycledSlotAnswersTheNewObject) { if (!Ready()) return; + // The object_death producer is Espryt-side (OnFrontendStateObjectDestroyed, + // CONTRACT-P5C.md §5.2); Magma installs no StateObjectDeathOps (P7), so under + // DirectVulkan there is no death record to watch and the case has nothing to prove. + if (HeadlessGL::Get().BackendName() != "DirectGLES") { + GTEST_SKIP() << "object_death is produced by the DirectGLES death-notice ops; " + "DirectVulkan has none until P7"; + } const GLubyte red[4] = {255, 0, 0, 255}; const GLubyte green[4] = {0, 255, 0, 255}; @@ -156,6 +163,12 @@ namespace { TEST_F(CtWireScenario, FramebufferDeathCrossesAndTheRecycledSlotAnswersTheNewObject) { if (!Ready()) return; + // Same producer reason as the texture case above: object_death is emitted by the + // DirectGLES death-notice ops; DirectVulkan has none until P7. + if (HeadlessGL::Get().BackendName() != "DirectGLES") { + GTEST_SKIP() << "object_death is produced by the DirectGLES death-notice ops; " + "DirectVulkan has none until P7"; + } // Framebuffer is the kind object_death EXISTS for: it has no other wire delete // opcode (CONTRACT-P5C.md §5.2). The renderbuffer goes along so the FBO has storage. GLuint fbo = 0, renderbuffer = 0; diff --git a/MobileGL/MG_Pipe/PipeMutation.h b/MobileGL/MG_Pipe/PipeMutation.h index cccaebf8..098b2a40 100644 --- a/MobileGL/MG_Pipe/PipeMutation.h +++ b/MobileGL/MG_Pipe/PipeMutation.h @@ -221,6 +221,11 @@ namespace MobileGL::MG_Pipe { void MGPipeEmitResourceFlushRange(MG_State::GLState::BufferObject& buffer, SizeT offset, SizeT size, Uint32 accessFlags); void MGPipeEmitResourceReadback(MG_State::GLState::BufferObject& buffer); + // The ranged form, for the split arm's sliced whole-buffer readback: the writeback's + // bytes travel INLINE in a SEG_EVENT record, so a buffer larger than the ring can hold + // is read back as [offset, offset+size) slices, each its own record. Monolith never + // calls it - the direct callback carries a pointer, not a copy. + void MGPipeEmitResourceReadbackRange(MG_State::GLState::BufferObject& buffer, SizeT offset, SizeT size); // Returns the coherent host pointer the resource owner donated, or null for a DECLINE - // which is a real answer. Every call, mint or decline, is one map-persistent roundtrip. void* MGPipeEmitMapPersistent(MG_State::GLState::BufferObject& buffer); diff --git a/MobileGL/MG_Remote/CONTRACT-P5C.md b/MobileGL/MG_Remote/CONTRACT-P5C.md index 8f2cbbf8..e15825db 100644 --- a/MobileGL/MG_Remote/CONTRACT-P5C.md +++ b/MobileGL/MG_Remote/CONTRACT-P5C.md @@ -260,9 +260,16 @@ layer-1 violation; the GL thread calling a server-installed producer is layer 2. `DrainEventRing` calls the client consumers BY NAME (`MGPipeClientOnBufferWriteback` / `MGPipeClientOnGpuWritten`, `ResourceTracker.h:553-601`), not through `gMGPipeCallbacks` — -the global table is a producer-side surface under split. The drain point is unchanged: -inside `EmitAndWait` after the barrier (`ClientSession.cpp:836`), the one instant the apply -thread is known to be outside the applier. +the global table is a producer-side surface under split. **The drain points are TWO classes** +(AMENDED at the regression triage): inside `EmitAndWait` after the barrier +(`ClientSession.cpp:836`), AND at the return of the blocking EGL lifecycle RPCs +(`BackendObject_Remote`'s surface creation / resize / make-current). Both satisfy the same +safety premise — the apply thread is known idle because the work it was doing (the verb, the +RPC) has completed and this thread has published nothing since. The second class exists +because a surface-changed event posted during bring-up must be applied BEFORE the first +frontend query of the default framebuffer's attachments: waiting for the first verb's drain +answered the placeholder format to `glGetFramebufferAttachmentParameteriv`, and every buffer +allocated from that answer was blit-incompatible with the real surface. ### 4.2 The three (+1) producers @@ -308,6 +315,16 @@ barrier, and a full ring under lockstep means a producer burst no measured workl defect instead. The exit gate `eventDropped == 0` at drain points (E-P5c #3) is what checks this row. +**AMENDED at the regression triage — the ring is sized for events, not buffers.** A +whole-buffer writeback larger than the ring can NEVER fit (one record is capped at +capacity/2), so the CLIENT slices the request (`BufferWritebackSliceBytes`: a quarter of the +ring, floored at 4 KiB — a quarter, not the half, because the record carries its header +beside the payload and the ring may still hold small events from the same verb). Each slice +round-trips its own barrier + drain, so the ring holds at most one slice's bytes, and the +in-order channel makes the last slice's landing imply every earlier one. This is a +client-side ruling about request SHAPE, not a drop policy; the Fatal stays for a producer +burst of real events. + ### 4.5 Round-trip red-once Each of the three P5 events gets a unit round-trip beside `SessionTest.cpp:747-823` (the @@ -409,11 +426,14 @@ The generator's four classes are unchanged. c0c edits rows; rv/tx/hd land the co enum values, and a row naming a non-field stops the generator by design. What pins them instead: (a) `GetTextureUnitObject`'s existing row is re-annotated — the tx-retired reads leave its site list, what remains is the unit-object POINTER reads (P3b/P4b/P7); (b) the - object-surface list (texel bytes, per-level extent, dirty region, `HasDefinedContent`, + object-surface list (texel bytes, dirty region, `HasDefinedContent`, `MappedData`/`IsMapped`/`GetChangeSerial`) is pinned verbatim in `FieldOwnershipTest` as - the layer-1 surface set, and the §6 guard is what enforces it. This is the honest form of - ROADMAP's "把纹理家族补进 FieldOwnership.def": the .def's mechanism covers fields, and the - family's object reads get a pinned list plus a guard that can go red. + the layer-1 surface set, and the §6 guard is what enforces it. **AMENDED at gt's + landing:** "per-level extent" leaves the guard list — the pinned object-class rows' + per-frame binding walk legally depends on `GetMipmapTexelSize`, so extent is retired AT + THE SITES (tx's sync reads the store) rather than guarded on the accessor. This is the + honest form of ROADMAP's "把纹理家族补进 FieldOwnership.def": the .def's mechanism covers + fields, and the family's object reads get a pinned list plus a guard that can go red. --- @@ -452,15 +472,26 @@ forgotten. **Red-once (R-16), per layer, mandatory:** each layer is switched off in one named integration scenario and the run MUST go red with the layer's Fatal — this is E-P5c #1's "关掉任一层守卫必须能在一条具名用例上变红", and it is what makes the guard a gate rather -than a comment. CI gains one `MOBILEGL_IPC_STRICT_ERRORS=1` + guards-armed -`integration-split` lane. +than a comment. **CI lane, AMENDED at gt's landing:** a scenario-green +`MOBILEGL_IPC_STRICT_ERRORS=1` lane is impossible while the object-class rows live (the +first Clear pulls `GetFramebufferBindingSlot@Clear` and aborts by design), so the lane is +two-sided: the unit lane under strict is the hard green gate (the RemoteGuards and +FieldOwnershipTest strict arms live there), and the integration-split scenario lane is +EXPECTED-RED with the `BARRIER-PULLED, MOBILEGL_IPC_STRICT_ERRORS=1` marker asserted in +each entry's private log — the shape `split_negative_controls.sh` already drives. It +becomes a plain green lane once P7 retires the object-class rows. **`rsp` at exit.** `ResidualPulls` enters the per-frame stats line and is MEASURED on the four A/B traces; the remaining reads are exactly the object-class rows pinned by -`FieldOwnershipTest` (§5.3), value rows = 0. The known blind spot — sticky / non-verb -forwards bypassing the stamp counter (ROADMAP debt "rsp=35 只是下界") — is closed by gt: -the sticky-forward pull path (`MGPipeStickyForwardPull`, `PipeInputs.cpp:180-186`) counts -into `rsp` under the same verb stamp, so a read that escapes the stamp escapes nothing. +`FieldOwnershipTest` (§5.3), value rows = 0. **AMENDED at gt's landing:** the sticky-forward +blind spot this paragraph assigned to gt was already closed (`MGPipeStickyForwardPull` has +counted into `ResidualPulls` since `d709ef3e`; `FieldOwnershipTest` pins it). What the +measurement actually found on the locally available traces: bsl in-world 948.5 rsp/frame +(38.7/draw, 123 frames), iris-complementary 1591.9, iris-iterationrp 286.2 up to its +pre-existing `Fatal{UnmigratedEmulation, "texture-remint-pull"}` — all object-class, zero +value-class. The residual known blind spot is narrower than planned: a record applied +OUTSIDE a verb boundary withdraws the stamp (`PipeApplier.cpp` notes it), and a sticky pull +inside one escapes the count; no measured workload produces one. --- @@ -488,7 +519,8 @@ into `rsp` under the same verb stamp, so a read that escapes the stamp escapes n ## §8 Exit gates this contract serves (E-P5c, restated for the packages) -1. Guards armed: `integration-split` (107) green, broad inproc census with zero regressions +1. Guards armed: `integration-split` (107 at plan time; 111 as landed — ct added four + `CtWireScenario` entries) green, broad inproc census with zero regressions vs `348d22a4`, 79 traces with no new first blocker; each guard layer red-once (§6). 2. `MOBILEGL_IPC_AUDIT=1`'s `0xDD` covers texture staged bytes on the four A/B traces; reverting adoption goes red (§2.4). diff --git a/MobileGL/MG_Remote/Client/BackendObject_Remote.cpp b/MobileGL/MG_Remote/Client/BackendObject_Remote.cpp index 44d09bb7..bf1193e0 100644 --- a/MobileGL/MG_Remote/Client/BackendObject_Remote.cpp +++ b/MobileGL/MG_Remote/Client/BackendObject_Remote.cpp @@ -190,16 +190,30 @@ namespace MobileGL::MG_Remote::Client { // a surface for, and ServerSetWindowHandle is the only way to tell it. Server::ServerSetWindowHandle(handle); if (!Server::ServerCreateEGLWindowSurface(surface, handle)) return false; + // The server's surface init published the default framebuffer's shape as a + // surface-changed EVENT (P5c ev): DirectGLES' depth/stencil format, Magma's + // swapchain extent. Apply it NOW - the RPC's return is a moment the apply thread + // is known idle - because the first verb's drain would otherwise let every pre-verb + // query answer from the placeholder attachments (GL_DEPTH32F_STENCIL8 for a + // depth24+stencil8 surface, and every buffer allocated from that answer is + // blit-incompatible with the real thing). + if (ClientSession* session = ClientSession::Active()) session->DrainPublishedEvents(); return MG_Backend::BackendObject::CreateEGLWindowSurface(surface, handle); } Bool BackendObject_Remote::ResizeEGLWindowSurface(EGLSurface surface, Uint32 width, Uint32 height) { if (!Server::ServerResizeEGLWindowSurface(surface, width, height)) return false; + // A resize re-creates the server's swapchain, which re-posts the surface-changed + // event - same drain, same reason as CreateEGLWindowSurface. + if (ClientSession* session = ClientSession::Active()) session->DrainPublishedEvents(); return MG_Backend::BackendObject::ResizeEGLWindowSurface(surface, width, height); } Bool BackendObject_Remote::CreateEGLPbufferSurface(EGLSurface surface, EGLint width, EGLint height) { if (!Server::ServerCreateEGLPbufferSurface(surface, width, height)) return false; + // Same drain as the window surface: InitPbufferSurface publishes the default + // framebuffer's depth/stencil format on SEG_EVENT from inside this very RPC. + if (ClientSession* session = ClientSession::Active()) session->DrainPublishedEvents(); return MG_Backend::BackendObject::CreateEGLPbufferSurface(surface, width, height); } @@ -225,6 +239,10 @@ namespace MobileGL::MG_Remote::Client { if (draw != EGL_NO_SURFACE && ctx != EGL_NO_CONTEXT) { if (ClientSession* session = ClientSession::Active()) { session->PumpControlPlane(); + // The event ring beside the caps channel: a make-current can follow a + // surface (re)creation that posted a surface-changed event, and this is + // the same known-idle instant the RPC returns at. + session->DrainPublishedEvents(); RefreshFormatCapabilities(); } } diff --git a/MobileGL/MG_Remote/Client/ClientSession.cpp b/MobileGL/MG_Remote/Client/ClientSession.cpp index ba2b9ad2..e7fedaa4 100644 --- a/MobileGL/MG_Remote/Client/ClientSession.cpp +++ b/MobileGL/MG_Remote/Client/ClientSession.cpp @@ -1051,6 +1051,15 @@ namespace MobileGL::MG_Remote::Client { Transport::EventRingConsumer& ClientSession::Events() { return m_events; } + Uint32 ClientSession::DrainPublishedEvents() { + // Not gated on m_started on purpose: PumpControlPlane's own pre-start call during + // Start() has its ring-consumer twin here, and DrainEventRing's Valid() check is + // the whole guard either case needs. + return DrainEventRing(m_events); + } + + Uint64 ClientSession::EventRingCapacityBytes() const { return m_shm.EventRingCapacity(); } + Wire::SegmentTable& ClientSession::Segments() { return m_segments; } Transport::RingControl* ClientSession::Control() { return m_shm.CmdControl(); } diff --git a/MobileGL/MG_Remote/Client/ClientSession.h b/MobileGL/MG_Remote/Client/ClientSession.h index 0c780ecb..775ed025 100644 --- a/MobileGL/MG_Remote/Client/ClientSession.h +++ b/MobileGL/MG_Remote/Client/ClientSession.h @@ -216,6 +216,23 @@ namespace MobileGL::MG_Remote::Client { // OnSurfaceChanged. Drained by the GL thread between verbs. Transport::EventRingConsumer& Events(); + // The event ring's SECOND drain point, for the blocking EGL lifecycle RPCs + // (BackendObject_Remote's surface creation and make-current). Their return is the + // same kind of instant as EmitAndWait's post-barrier one - the apply thread is + // known idle because the RPC it was serving has completed and this thread has + // published nothing since - so the consumers' frontend writes are as legal here as + // there. A surface-changed event posted during bring-up must be applied BEFORE the + // first frontend query of the default framebuffer's depth/stencil format: waiting + // for the first verb's drain answered GL_DEPTH32F_STENCIL8 (the placeholder) for a + // depth24+stencil8 surface, and every buffer allocated from that answer was + // blit-incompatible with the real thing. + Uint32 DrainPublishedEvents(); + + // SEG_EVENT's ring capacity, exposed so the readback path can slice a writeback + // request into records that always fit (RingProducer::MaxRecordBytes == + // capacity/2, Ring.h:288). + Uint64 EventRingCapacityBytes() const; + // The CLIENT's own segment table (P5c ev, CONTRACT-P5C §4.3): the writeback // consumer resolves a SEG_EVENT blobref through it. Never the process resolver - // table 3 installs that one on the server role only. diff --git a/MobileGL/MG_Remote/Client/GpuWritePending.cpp b/MobileGL/MG_Remote/Client/GpuWritePending.cpp index 112c5713..9e0497b3 100644 --- a/MobileGL/MG_Remote/Client/GpuWritePending.cpp +++ b/MobileGL/MG_Remote/Client/GpuWritePending.cpp @@ -169,6 +169,21 @@ namespace MobileGL::MG_Remote::Client { #endif } + SizeT BufferWritebackSliceBytes() { + ClientSession* session = ClientSession::Active(); + if (session == nullptr) return 0; + // A quarter of the ring, not the MaxRecordBytes half: the record carries its own + // header and the 24-byte EventBufferWritebackHead beside the payload, and the ring + // may still hold a few small events (gpu-written, gl-error) posted earlier in the + // same verb's apply. Each slice round-trips with its own barrier + drain, so the + // ring never holds more than one slice's bytes. + const Uint64 slice = session->EventRingCapacityBytes() / 4; + // A floor so a pathologically small operator-supplied ring cannot make the slicing + // loop in SyncGpuWrites spin at zero width; such a ring is broken anyway, and the + // producer's Fatal{EventRingOverflow} names it on the first post. + return static_cast(slice < 4096 ? 4096 : slice); + } + void AwaitBufferWriteback(BufferObject& buffer) { // THE WAIT IS THE BARRIER'S WAIT (R-3). The reply-slot id IS the record's seq, so // "appliedSeq reached my readback" and "my answer is back" are one condition, and diff --git a/MobileGL/MG_Remote/Client/GpuWritePending.h b/MobileGL/MG_Remote/Client/GpuWritePending.h index 873bcf18..3589b8b1 100644 --- a/MobileGL/MG_Remote/Client/GpuWritePending.h +++ b/MobileGL/MG_Remote/Client/GpuWritePending.h @@ -149,4 +149,12 @@ namespace MobileGL::MG_Remote::Client { // ever. It is the ONE case monolith's unconditional clear covers that a writeback cannot. Bool BufferWritebackIsReachable(const MG_State::GLState::BufferObject& buffer); + // The slice a whole-buffer readback is cut into so one writeback event always fits + // SEG_EVENT. The writeback's bytes travel INLINE in the event record (P5c ev, CONTRACT-P5C + // §4.2), one record must fit the ring (RingProducer::MaxRecordBytes == capacity/2, + // Ring.h:288), and a 24 MiB arena's single shot cannot (measured: Fatal{EventRingOverflow} + // on LargeArenaAdoptionScenario.GpuWriteIntoTheArenaIsReadBack). 0 when no session is + // active - the synchronous arm never slices. + SizeT BufferWritebackSliceBytes(); + } // namespace MobileGL::MG_Remote::Client diff --git a/MobileGL/MG_State/GLState/BufferState/BufferObject.cpp b/MobileGL/MG_State/GLState/BufferState/BufferObject.cpp index cc1f9f10..512ce94d 100644 --- a/MobileGL/MG_State/GLState/BufferState/BufferObject.cpp +++ b/MobileGL/MG_State/GLState/BufferState/BufferObject.cpp @@ -563,7 +563,29 @@ namespace MobileGL::MG_State::GLState { if (MG_Config::Transport != MG_Config::TransportMode::Monolith) { #if MOBILEGL_PIPE_PUSH if (m_size != 0 && MG_Pipe::MGPipeResourceSubsystemEnabled()) { - MG_Pipe::MGPipeEmitResourceReadback(*this); + const SizeT sliceBytes = MG_Remote::Client::BufferWritebackSliceBytes(); + if (sliceBytes != 0 && m_size > sliceBytes) { + // The writeback's bytes travel INLINE in a SEG_EVENT record (P5c ev), + // and one record must fit the ring (MaxRecordBytes == capacity/2) - a + // whole-buffer request for a buffer larger than that aborts the server + // on Fatal{EventRingOverflow}. Slice the request instead: every slice + // round-trips its own barrier + drain before the next is emitted, so + // the ring holds at most one slice's bytes at a time, and the in-order + // channel makes the last slice's landing imply every earlier one. + for (SizeT off = 0; off < m_size; off += sliceBytes) { + const SizeT left = m_size - off; + MG_Pipe::MGPipeEmitResourceReadbackRange(*this, off, + left < sliceBytes ? left : sliceBytes); + } + // No single writeback covered the whole buffer, so + // WritebackFromBackend's clear (above) never fired - but every slice + // has landed by here, which is exactly what the flag-clearing there + // says. Clearing by hand is what keeps AwaitBufferWriteback's + // third-state Fatal from misfiring on a sliced readback. + m_gpuWritePending = false; + } else { + MG_Pipe::MGPipeEmitResourceReadback(*this); + } // The wait is the barrier's wait: the reply slot id IS the record's seq, so // "my answer is back" and "appliedSeq reached me" are one condition. With no // session (a build-split lane running monolith, and every unit case) the