diff --git a/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp b/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp index 9b1e9d82..a3d2c354 100644 --- a/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp +++ b/MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp @@ -1734,6 +1734,22 @@ namespace MobileGL::MG_Backend::DirectGLES { g_GLESFuncs.glResumeTransformFeedback != nullptr; } +#if MOBILEGL_BUILD_DISAGGREGATED + // How wide one OnBufferWriteback may be for a capture range of `rangeBytes`, which is + // the whole range when the transport imposes no limit (monolith, where the callback is + // a direct call with no ring under it) or when the range already fits one. + // + // BOTH XFB WRITEBACK PRODUCERS ASK THE SAME FUNCTION. They post from different sources + // - one from a live glMapBufferRange mapping, one from a staged Vector - but + // they post the same KIND of record into the same ring, and a second spelling of the + // width is how one of them ends up being the producer that still kills the server. + SizeT XfbWritebackSliceStep(SizeT rangeBytes) { + const Uint64 slice = MG_Pipe::MGPipeBufferWritebackSliceBytes(); + if (slice == 0 || slice >= static_cast(rangeBytes)) return rangeBytes; + return static_cast(slice); + } +#endif + // Mirrors one capture span's results into the frontend CPU shadows. The GPU wrote // the capture buffers behind the frontend's back, so the shadows that back // MapBuffer/GetBufferSubData still hold the pre-draw bytes. Buffers whose storage @@ -1779,11 +1795,27 @@ namespace MobileGL::MG_Backend::DirectGLES { 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}); + // ONE EVENT PER SLICE, NOT ONE PER CAPTURE. The bytes travel + // INLINE in a SEG_EVENT record and the ring refuses any record + // above half its capacity outright, so a whole-range post of a + // capture wider than ~128 KiB was Fatal{EventRingOverflow} - the + // SERVER dying on a large buffer. The slicing shape is + // BufferObject.cpp's one ring over: in-order delivery makes the + // last slice's landing imply every earlier one, so the client + // needs nothing new to reassemble them. + // + // INSIDE THE MAPPING, deliberately: the map is the expensive part + // and the slices are reads of one live pointer, so this unmaps + // once below however many events it posts. + const SizeT step = XfbWritebackSliceStep(size); + for (SizeT off = 0; off < size; off += step) { + const SizeT bytes = std::min(step, size - off); + MG_Pipe::gMGPipeCallbacks.OnBufferWriteback( + res, target.start + off, + MG_Pipe::MGPBlobRef{ + reinterpret_cast(static_cast(mapped) + off), + static_cast(bytes), 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", @@ -1992,11 +2024,23 @@ 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}); + // Sliced for the reason the readback path above is sliced: the bytes + // are INLINE in a SEG_EVENT record and the ring refuses a record wider + // than half its capacity outright, so one whole-range post of a + // capture over ~128 KiB was Fatal{EventRingOverflow} and the server + // died. Only the EVENT is cut; the glBufferSubData below stays one + // call over the whole `staged` vector, because the ES store has no + // ring and cutting it would change how many backend calls one + // application call makes. + const SizeT step = XfbWritebackSliceStep(rangeBytes); + for (SizeT off = 0; off < rangeBytes; off += step) { + const SizeT bytes = std::min(step, rangeBytes - off); + MG_Pipe::gMGPipeCallbacks.OnBufferWriteback( + splitRes, target.start + off, + MG_Pipe::MGPBlobRef{reinterpret_cast(staged.data() + off), + static_cast(bytes), + 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", diff --git a/MobileGL/MG_IntegrationTest/CMakeLists.txt b/MobileGL/MG_IntegrationTest/CMakeLists.txt index d92f44c7..3a6c4ceb 100644 --- a/MobileGL/MG_IntegrationTest/CMakeLists.txt +++ b/MobileGL/MG_IntegrationTest/CMakeLists.txt @@ -2284,11 +2284,18 @@ if (MOBILEGL_BUILD_DISAGGREGATED) # this target": silently for a wholly orphaned store, and as # Fatal{StageSnapshotTooNarrow, "xfb_scatter_pre_capture"} for a partly uploaded one. # - # NARROW FILTER ON PURPOSE. The rest of XfbAfterClipDistanceScenario is a DirectVulkan - # allocation-pattern regression (its own header says so) and has never been run under a - # transport; slice 2 registers the whole scenario once the writeback event it posts is - # sliced. Registering it wholesale here would mix that slice's reds into this one's. - mgl_itest_register_split_arms("XfbAfterClipDistanceScenario.OrphanedCaptureTarget*" "Xfb.") + # ---- P3b/P4b espryt D1 slice 2: THE WHOLE XFB FAMILY, once the writeback is sliced ------- + # + # Slice 1 registered only OrphanedCaptureTarget* here, because the rest of these two files + # could not survive a transport: both writeback producers posted the WHOLE bound range as one + # SEG_EVENT record, and a record above half the ring (128 KiB at the 256 KiB default) is + # refused outright, which the server answers with Fatal{EventRingOverflow}. With the event + # sliced, the two scenarios go on in full - including the megabyte-range pair whose whole + # subject is that slicing, and XfbRepeatedCaptureScenario's gl_NextBuffer compacted-target + # shape, which is the first thing outside the census to exercise the archive-served stride + # and varyings (ProgramArchive->Link's xfb fields) on the server. + mgl_itest_register_split_arms("XfbAfterClipDistanceScenario.*" "Xfb.") + mgl_itest_register_split_arms("XfbRepeatedCaptureScenario.*" "Xfb.") # ---- P5e fix1: THE PUSH-MONOLITH ARM, INSIDE THE GATED SET (ID-107) ---------------------- diff --git a/MobileGL/MG_IntegrationTest/Scenarios/XfbAfterClipDistanceScenario.cpp b/MobileGL/MG_IntegrationTest/Scenarios/XfbAfterClipDistanceScenario.cpp index c94080fe..10691187 100644 --- a/MobileGL/MG_IntegrationTest/Scenarios/XfbAfterClipDistanceScenario.cpp +++ b/MobileGL/MG_IntegrationTest/Scenarios/XfbAfterClipDistanceScenario.cpp @@ -364,10 +364,23 @@ void main (void) return false; } + // The capture RANGE the writeback has to cross when the point of a case is that range's + // SIZE rather than its contents (P3b/P4b espryt D1 slice 2). One SEG_EVENT record may be + // at most half the event ring, which defaults to 256 KiB - so 1 MiB is about eight times + // what a whole-range post can ever carry, and no reasonable retuning of the ring makes it + // fit. + constexpr GLsizeiptr kLargeCaptureRangeBytes = 1024 * 1024; + // Runs skip_components and reports what came back. `outCaptured` is the raw // readback so a failure can say whether anything was written at all. + // + // `storeBytes` widens the capture BUFFER without widening the capture: the records are the + // same six, but glBindBufferBase binds the whole store, so the range the backend mirrors + // back is `storeBytes` wide. That is the only way to reach a writeback of a given size + // from a test without drawing a proportional number of primitives. void RunSkipComponentsCapture(std::vector& outCaptured, std::string* buildLog, - CaptureStoreShape shape = CaptureStoreShape::Declared) { + CaptureStoreShape shape = CaptureStoreShape::Declared, + GLsizeiptr storeBytes = 0) { outCaptured.clear(); const GLuint program = BuildProgram(kXfbVertexSource, kXfbFragmentSource, SkipComponentsVaryings(), @@ -396,33 +409,37 @@ void main (void) const unsigned floatCount = kSkipVertexCount * kSkipComponentCount; const GLsizeiptr byteSize = static_cast(sizeof(float) * floatCount); + // The STORE, which is what glBindBufferBase binds and therefore what the backend + // mirrors back; the capture itself is `byteSize` of it either way. + const GLsizeiptr storeSize = storeBytes > byteSize ? storeBytes : byteSize; + const unsigned storeFloats = static_cast(storeSize / sizeof(float)); GLuint captureBuffer = 0; glGenBuffers(1, &captureBuffer); glBindBuffer(GL_ARRAY_BUFFER, captureBuffer); - glBufferData(GL_ARRAY_BUFFER, byteSize, nullptr, GL_STATIC_READ); + glBufferData(GL_ARRAY_BUFFER, storeSize, nullptr, GL_STATIC_READ); glBindBufferBase(GL_TRANSFORM_FEEDBACK_BUFFER, 0, captureBuffer); glBindBuffer(GL_ARRAY_BUFFER, 0); // The pre-fill that makes "nothing was captured" recognisable. - std::vector prefill(floatCount); - for (unsigned i = 0; i < floatCount; ++i) { + std::vector prefill(storeFloats); + for (unsigned i = 0; i < storeFloats; ++i) { prefill[i] = -1.0f - static_cast(i); } glBindBuffer(GL_ARRAY_BUFFER, captureBuffer); switch (shape) { case CaptureStoreShape::Declared: - glBufferData(GL_ARRAY_BUFFER, byteSize, prefill.data(), GL_STATIC_DRAW); + glBufferData(GL_ARRAY_BUFFER, storeSize, prefill.data(), GL_STATIC_DRAW); break; case CaptureStoreShape::OrphanedPartial: // The streaming idiom: orphan the store, then upload only the part the // application cares about keeping. - glBufferData(GL_ARRAY_BUFFER, byteSize, nullptr, GL_STATIC_DRAW); + glBufferData(GL_ARRAY_BUFFER, storeSize, nullptr, GL_STATIC_DRAW); glBufferSubData(GL_ARRAY_BUFFER, 0, static_cast(sizeof(float) * kOrphanStagedFloats), prefill.data()); break; case CaptureStoreShape::OrphanedWhole: - glBufferData(GL_ARRAY_BUFFER, byteSize, nullptr, GL_STATIC_DRAW); + glBufferData(GL_ARRAY_BUFFER, storeSize, nullptr, GL_STATIC_DRAW); break; } glBindBuffer(GL_ARRAY_BUFFER, 0); @@ -553,6 +570,104 @@ void main (void) return ::testing::AssertionSuccess(); } + // THE OTHER WRITEBACK PRODUCER (P3b/P4b espryt D1 slice 2). + // + // A capture list with no gl_SkipComponents and one buffer is expressible on ES directly, + // so the driver writes the application's buffer itself and the mirror-back is + // ReadbackCapturedRanges' rather than ScatterCapturedRecords'. That is a DIFFERENT + // OnBufferWriteback call site - it posts from a live glMapBufferRange mapping instead of a + // staged vector - with the same unbounded record behind it, so it needs its own case or + // half the fix is untested. `value1` alone is a 16-byte record; the bound range is the + // whole store, which is what the writeback crosses. + void RunPlainCaptureIntoLargeRange(std::vector& outCaptured, std::string* buildLog) { + outCaptured.clear(); + const GLuint program = BuildProgram(kXfbVertexSource, kXfbFragmentSource, {"value1"}, + GL_INTERLEAVED_ATTRIBS, buildLog); + ASSERT_NE(program, 0u) << "plain capture program failed to link: " << (buildLog ? *buildLog : ""); + glUseProgram(program); + + const std::vector vertices = { + -1.0f, -1.0f, -1.0f, 1.0f, 1.0f, -1.0f, -2.0f, 1.0f, -1.0f, 1.0f, -3.0f, 1.0f, + 1.0f, 1.0f, 4.0f, 1.0f, -1.0f, 1.0f, 5.0f, 1.0f, 1.0f, -1.0f, 6.0f, 1.0f, + }; + + GLuint vao = 0; + GLuint vbo = 0; + glGenVertexArrays(1, &vao); + glBindVertexArray(vao); + glGenBuffers(1, &vbo); + glBindBuffer(GL_ARRAY_BUFFER, vbo); + glBufferData(GL_ARRAY_BUFFER, static_cast(sizeof(float) * vertices.size()), vertices.data(), + GL_STATIC_DRAW); + const GLint location = glGetAttribLocation(program, "vertex"); + if (location >= 0) { + glEnableVertexAttribArray(static_cast(location)); + glVertexAttribPointer(static_cast(location), 4, GL_FLOAT, GL_FALSE, 0, nullptr); + } + + const unsigned capturedFloats = kSkipVertexCount * 4; + const unsigned storeFloats = static_cast(kLargeCaptureRangeBytes / sizeof(float)); + std::vector prefill(storeFloats); + for (unsigned i = 0; i < storeFloats; ++i) prefill[i] = -1.0f - static_cast(i); + + GLuint captureBuffer = 0; + glGenBuffers(1, &captureBuffer); + glBindBuffer(GL_ARRAY_BUFFER, captureBuffer); + glBufferData(GL_ARRAY_BUFFER, kLargeCaptureRangeBytes, prefill.data(), GL_STATIC_READ); + glBindBuffer(GL_ARRAY_BUFFER, 0); + + glEnable(GL_RASTERIZER_DISCARD); + glBindBufferBase(GL_TRANSFORM_FEEDBACK_BUFFER, 0, captureBuffer); + glBeginTransformFeedback(GL_TRIANGLES); + glDrawArrays(GL_TRIANGLES, 0, static_cast(kSkipVertexCount)); + glEndTransformFeedback(); + glDisable(GL_RASTERIZER_DISCARD); + + outCaptured.resize(capturedFloats); + const GLsizeiptr capturedBytes = static_cast(sizeof(float) * capturedFloats); + glBindBufferRange(GL_TRANSFORM_FEEDBACK_BUFFER, 0, captureBuffer, 0, capturedBytes); + const void* mapped = glMapBufferRange(GL_TRANSFORM_FEEDBACK_BUFFER, 0, capturedBytes, GL_MAP_READ_BIT); + if (mapped != nullptr) { + std::memcpy(outCaptured.data(), mapped, static_cast(capturedBytes)); + glUnmapBuffer(GL_TRANSFORM_FEEDBACK_BUFFER); + } + + glDisableVertexAttribArray(0); + glBindBuffer(GL_ARRAY_BUFFER, 0); + glDeleteBuffers(1, &vbo); + glDeleteBuffers(1, &captureBuffer); + glBindVertexArray(0); + glDeleteVertexArrays(1, &vao); + glUseProgram(0); + glDeleteProgram(program); + glBindBufferBase(GL_TRANSFORM_FEEDBACK_BUFFER, 0, 0); + } + + // value1 == |vertex| * 1, one tight vec4 record per vertex. + ::testing::AssertionResult CheckPlainCapture(const std::vector& captured) { + const std::vector vertices = { + -1.0f, -1.0f, -1.0f, 1.0f, 1.0f, -1.0f, -2.0f, 1.0f, -1.0f, 1.0f, -3.0f, 1.0f, + 1.0f, 1.0f, 4.0f, 1.0f, -1.0f, 1.0f, 5.0f, 1.0f, 1.0f, -1.0f, 6.0f, 1.0f, + }; + if (captured.size() != vertices.size()) { + return ::testing::AssertionFailure() + << "readback size " << captured.size() << " != " << vertices.size(); + } + for (std::size_t i = 0; i < vertices.size(); ++i) { + const float expected = std::fabs(vertices[i]); + if (std::fabs(captured[i] - expected) > 0.0125f) { + ::testing::AssertionResult failure = ::testing::AssertionFailure(); + failure << "capture mismatch at index " << i << ": got " << captured[i] << ", expected " + << expected; + if (std::fabs(captured[i] - (-1.0f - static_cast(i))) <= 0.0125f) { + failure << " - still the pre-fill, so nothing was mirrored back"; + } + return failure; + } + } + return ::testing::AssertionSuccess(); + } + // The harness turns "no context came up" into a clean skip, and a skip is // indistinguishable from a pass in a ctest summary. For this scenario that // is a hole rather than a courtesy: the defect it pins is DirectVulkan's @@ -627,6 +742,45 @@ void main (void) EXPECT_EQ(glGetError(), static_cast(GL_NO_ERROR)); } + // ------------------------------------- P3b/P4b espryt D1 slice 2: a capture range that + // does not fit one reverse-channel record + // + // Under a transport the captured bytes go back to the client INLINE in a SEG_EVENT + // record, and a ring producer refuses any record above half its capacity OUTRIGHT - + // always, not "when full", because above half a capacity a record is placeable at some + // head offsets and not at others and waiting for room would be a hang. SEG_EVENT defaults + // to 256 KiB. Both XFB writeback producers posted the WHOLE bound range as one record, so + // a capture target wider than ~128 KiB was Fatal{EventRingOverflow} and the server + // process died - on a buffer size, with nothing else wrong. + // + // A megabyte is about eight times what one record can ever be. The capture is still six + // records; it is glBindBufferBase over the whole store that makes the MIRRORED RANGE a + // megabyte, which is what a real capture buffer looks like and what the producer has to + // survive. + // + // TWO CASES BECAUSE THERE ARE TWO PRODUCERS. The skip_components layout is not + // expressible on ES, so it goes through ScatterCapturedRecords and posts from a staged + // vector; the plain single-varying layout is, so the driver writes the application's + // buffer and ReadbackCapturedRanges posts from a live glMapBufferRange mapping. They are + // different call sites with the same defect behind them. + TEST_F(XfbAfterClipDistanceScenario, AScatteredCaptureIntoAMegabyteRangeReachesTheApplication) { + if (!Ready()) return; + std::vector captured; + std::string log; + RunSkipComponentsCapture(captured, &log, CaptureStoreShape::Declared, kLargeCaptureRangeBytes); + EXPECT_TRUE(CheckSkipComponents(captured)); + EXPECT_EQ(glGetError(), static_cast(GL_NO_ERROR)); + } + + TEST_F(XfbAfterClipDistanceScenario, APlainCaptureIntoAMegabyteRangeReachesTheApplication) { + if (!Ready()) return; + std::vector captured; + std::string log; + RunPlainCaptureIntoLargeRange(captured, &log); + EXPECT_TRUE(CheckPlainCapture(captured)); + EXPECT_EQ(glGetError(), static_cast(GL_NO_ERROR)); + } + // Control: the capture on its own must work. TEST_F(XfbAfterClipDistanceScenario, SkipComponentsCaptureAlone) { if (!Ready()) return; diff --git a/MobileGL/MG_Pipe/MGPipeCallbacks.h b/MobileGL/MG_Pipe/MGPipeCallbacks.h index 0ebe2db4..d54c5688 100644 --- a/MobileGL/MG_Pipe/MGPipeCallbacks.h +++ b/MobileGL/MG_Pipe/MGPipeCallbacks.h @@ -59,4 +59,43 @@ namespace MobileGL::MG_Pipe { // Null-initialized: a backend that installs nothing sends nothing. inline MGPipeCallbacks gMGPipeCallbacks{}; + +#if MOBILEGL_BUILD_DISAGGREGATED + // HOW MUCH OF ITSELF ONE REVERSE-CHANNEL RECORD MAY BE, and it is not a tuning knob: a + // producer that ignores it kills the server process. + // + // OnBufferWriteback's bytes travel INLINE in one SEG_EVENT record, and a ring producer + // refuses any record above Capacity()/2 OUTRIGHT (Transport/Ring.h) - not "when full", but + // always, because above half a capacity a record is placeable at some head offsets and not + // at others, so waiting for room would be a hang. The server's answer to a refusal is + // Fatal{EventRingOverflow}, and SEG_EVENT defaults to 256 KiB, so ANY producer that posts a + // whole range wider than ~128 KiB is a server that dies on a large enough buffer. + // + // THE PRODUCERS ARE IN MG_Backend, which cannot reach MG_Remote::Client's + // BufferWritebackSliceBytes (that one is the CLIENT-linkage answer to the same question, + // for requests going the other way) and must not reach ServerSession either. So the + // transport publishes the one number through this hook, exactly as it publishes segment + // resolution through gMGPipeSegmentResolver: the SERVER role installs it at Accept and + // releases it at Close, and a monolith build leaves it null. + using MGPipeEventRingCapacityQuery = Uint64 (*)(); + inline MGPipeEventRingCapacityQuery gMGPipeEventRingCapacityBytes = nullptr; + + // 0 means "do not slice": no transport is installed, so the callback is a direct call with + // no ring under it and cutting the range would only multiply the calls. + // + // A QUARTER of the ring rather than the MaxRecordBytes half, which is the same number and + // the same reasoning the client side uses (MG_Remote/Client/GpuWritePending.cpp): the record + // carries its own header and an EventBufferWritebackHead beside the payload, and the ring may + // still hold small events posted earlier in the same verb's apply. The 4 KiB floor is there so + // that a pathologically small operator-supplied ring cannot make a slicing loop spin at zero + // width; such a ring is broken anyway and the producer's Fatal{EventRingOverflow} names it on + // the first post. + inline Uint64 MGPipeBufferWritebackSliceBytes() { + if (gMGPipeEventRingCapacityBytes == nullptr) return 0; + const Uint64 capacity = gMGPipeEventRingCapacityBytes(); + if (capacity == 0) return 0; + const Uint64 quarter = capacity / 4; + return quarter < 4096 ? 4096 : quarter; + } +#endif } // namespace MobileGL::MG_Pipe diff --git a/MobileGL/MG_Remote/Server/ServerSession.cpp b/MobileGL/MG_Remote/Server/ServerSession.cpp index 0350d3c8..2de331cb 100644 --- a/MobileGL/MG_Remote/Server/ServerSession.cpp +++ b/MobileGL/MG_Remote/Server/ServerSession.cpp @@ -311,6 +311,21 @@ namespace MobileGL::MG_Remote::Server { return *session; } + // THE NUMBER THE BACKEND'S WRITEBACK PRODUCERS SLICE AGAINST (P3b/P4b espryt D1 slice 2). + // + // SEG_EVENT's capacity, published to MG_Pipe so that a producer in MG_Backend can ask it + // without reaching either MG_Remote::Client (wrong linkage; that side answers the same + // question for requests going the other way) or ServerSession (wrong layer). MG_Pipe turns + // it into the per-record width - see MGPipeBufferWritebackSliceBytes - because the quarter + // and its floor are one rule, not two. A session that has not attached a data link yet + // answers 0, which reads as "do not slice" and is correct: with no ring there is nothing + // to overflow. + Uint64 ServerEventRingCapacityBytes() { + auto* session = ServerSession::Active(); + if (session == nullptr) return 0; + return session->Events().Ring().Capacity(); + } + void ServerOnBufferWriteback(MG_Pipe::MGPipeHandle res, Uint64 offset, MG_Pipe::MGPBlobRef bytes) { ServerSession& session = ReverseCallbackOwner("OnBufferWriteback"); @@ -637,6 +652,11 @@ namespace MobileGL::MG_Remote::Server { InstallReverseCallback(MG_Pipe::gMGPipeCallbacks.OnGpuWritten, &ServerOnGpuWritten); InstallReverseCallback(MG_Pipe::gMGPipeCallbacks.OnSurfaceChanged, &ServerOnSurfaceChanged); + // ... and, beside them, the one NUMBER a writeback producer has to know: how much of + // SEG_EVENT one record may be. Same ownership rule as the four entries above - installed + // here, released at Close only if it is still ours - because a producer left holding a + // dead session's ring width would slice against a ring that no longer exists. + InstallReverseCallback(MG_Pipe::gMGPipeEventRingCapacityBytes, &ServerEventRingCapacityBytes); g_active.store(this, std::memory_order_release); LogMemory("accept"); @@ -755,6 +775,9 @@ namespace MobileGL::MG_Remote::Server { if (MG_Pipe::gMGPipeCallbacks.OnSurfaceChanged == &ServerOnSurfaceChanged) { MG_Pipe::gMGPipeCallbacks.OnSurfaceChanged = nullptr; } + if (MG_Pipe::gMGPipeEventRingCapacityBytes == &ServerEventRingCapacityBytes) { + MG_Pipe::gMGPipeEventRingCapacityBytes = nullptr; + } } m_consumer.Detach(); *m_commands = Transport::RingConsumer(); diff --git a/docs/Disaggregated/notes/p34b/espryt-d1.md b/docs/Disaggregated/notes/p34b/espryt-d1.md index 31f60a5f..78ecf619 100644 --- a/docs/Disaggregated/notes/p34b/espryt-d1.md +++ b/docs/Disaggregated/notes/p34b/espryt-d1.md @@ -106,7 +106,69 @@ both byte-identical to `~/w7/p7-before/`. ## slice 2 — the unbounded writeback event -`RESULT: (pending)` +**The hole.** Both XFB writeback producers posted ONE unbounded `OnBufferWriteback` covering the +whole bound range: `ScatterCapturedRecords` from its staged vector, `ReadbackCapturedRanges` from +a live `glMapBufferRange` mapping. `ServerOnBufferWriteback` turns each into one SEG_EVENT record +of `sizeof(EventBufferWritebackHead) + bytes`, and `RingProducer::Reserve` refuses any record +above `Capacity()/2` **outright** — not "when full", but always, because above half a capacity a +record is placeable at some head offsets and not at others and waiting for room would be a hang. +`EventRingBytes` defaults to 256 KiB. So any capture target wider than ~128 KiB was +`Fatal{EventRingOverflow}` and the **server process died** — on a buffer size, with nothing else +wrong. The producers' own Fatal text already said what to do: *"Raise MOBILEGL_IPC_EVENT_KB or +slice the event at its producer, as the writeback path already does"* — which was true of the +client's request path and of nothing on this side. + +**The fix.** `MG_Pipe` grows the one number, published the way `gMGPipeSegmentResolver` is +published: `gMGPipeEventRingCapacityBytes`, installed by `ServerSession::Accept` beside the four +reverse-channel producers and released at `Close` only if still ours, and +`MGPipeBufferWritebackSliceBytes()` which turns it into a per-record width (a quarter of the ring, +4 KiB floor — the same number and the same reasoning as the client's `BufferWritebackSliceBytes`). +The backend cannot reach `MG_Remote::Client` (wrong linkage) or `ServerSession` (wrong layer), so +the hook is the seam. 0 means "do not slice": monolith has no ring under the callback. + +Both producers loop against one helper, `XfbWritebackSliceStep`, so a second spelling of the width +cannot leave one of them still killing the server. `:1782` slices INSIDE its mapping and still +unmaps once; `:1969` slices the staged vector and keeps the single `glBufferSubData` whole, +because the ES store has no ring and cutting it would change how many backend calls one +application call makes. In-order delivery makes the last slice's landing imply every earlier one, +so the client needs nothing new to reassemble them. + +**Red-once (R-16).** `XfbAfterClipDistanceScenario.{AScatteredCapture,APlainCapture}IntoAMegabyteRangeReachesTheApplication` +— a 1 MiB bound range (≈8× what one record can ever be) with the same six capture records; it is +`glBindBufferBase` over the whole store that makes the mirrored range a megabyte. Two cases +because there are two producers: the `gl_SkipComponents` layout is not expressible on ES and goes +through the scatter, the plain single-varying layout is and goes through the readback. + +With `XfbWritebackSliceStep` returning the whole range (fix reverted), all six split entries went +red and both monolith controls stayed green: + +``` +3744 DirectGLES.Split.Xfb …AScatteredCapture… (Subprocess aborted) +3745 DirectGLES.Split.Xfb …APlainCapture… (Subprocess aborted) +3753/3754 DirectGLES.Spawn.Xfb … (Failed — the server process died) +3762/3763 DirectGLES.Tcp.Xfb … (Failed — the server process died) +``` + +``` +MGPipe: Fatal{EventRingOverflow} - kEventBufferWriteback needs 1048600 bytes and SEG_EVENT caps +ONE record at 131072 (half its capacity). +``` + +inproc aborts the test process (the server is in it); spawn and tcp lose the peer instead, which +is the same defect seen from the other side of a socket. + +**Also registered here**: `XfbAfterClipDistanceScenario.*` and `XfbRepeatedCaptureScenario.*` in +full on the three split arms (slice 1 had only `OrphanedCaptureTarget*`, because the rest could +not survive the unsliced event). `XfbRepeatedCaptureScenario`'s `gl_NextBuffer` compacted-target +shape is the first thing outside the census to exercise the archive-served stride and varyings +(`ProgramArchive->Link`'s xfb fields) on the server — **it passed on all three arms with no +further change**, so there was nothing of D1's to fix behind it. + +**Gates.** `integration-split` 202/202, `integration-spawn` 118/118, `integration-tcp` 121/121, +`ctest -L unit` 2408/2408, the monolith + split XFB families 120/120, +`spawn_lane_parity.py build-split` RC 0, `fatal_census.py` RC 0 (79 sites, 0 unmarked), +`link_ratchet.py --assert-monotone` unchanged at 186. G1: `.text` `0xa52203`, both nm lists +byte-identical. ## slice 3 — the dead `OnXfbScatterReady` declaration