From ca8cbe5c3464a0d3a14da5ddacd11bedc1d261e5 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Tue, 22 Sep 2026 21:05:57 -0400 Subject: [PATCH] [MG_Pipe, MG_Remote, MG_Impl] (P7): the ID-49 neutral read_pixels pack has one spelling - MG_Pipe/MGPipeTypes.h gains inline MGPipeNeutralReadPixelsPack() next to MGPPixelPackState, whose owner it is; the applier that installs the neutral state and the comparator whose oracle it is now call the same function - MG_Remote/Server/PipeApplier.cpp read_pixels takes the constant from there instead of building it field by field - MG_Impl/Pipe/PipeFill.cpp drops ServerReadPixelsNeutralPack, the re-typed second copy: a drift between two hand-typed spellings costs a FALSE DIVERGENCE on the server read, not a build break, which is the direction that fails quietly - notes/p7/verify-split.md 2.2 says one spelling and why the old reasoning (do not reach into MG_Remote/Server from MG_Impl) did not survive the failure-direction argument --- MobileGL/MG_Impl/Pipe/PipeFill.cpp | 21 ++++++----------- MobileGL/MG_Pipe/MGPipeTypes.h | 25 +++++++++++++++++++++ MobileGL/MG_Remote/Server/PipeApplier.cpp | 10 ++++----- docs/Disaggregated/notes/p7/verify-split.md | 6 +++-- 4 files changed, 40 insertions(+), 22 deletions(-) diff --git a/MobileGL/MG_Impl/Pipe/PipeFill.cpp b/MobileGL/MG_Impl/Pipe/PipeFill.cpp index 40bbaa2d..4153cc66 100644 --- a/MobileGL/MG_Impl/Pipe/PipeFill.cpp +++ b/MobileGL/MG_Impl/Pipe/PipeFill.cpp @@ -681,19 +681,12 @@ namespace MobileGL::MG_Pipe { self.CurrentVerb() == MGPipeVerb::ReadPixels; } - // PipeApplier.cpp's `neutralPack`, field for field. A second spelling of a constant is - // the price of not reaching into MG_Remote/Server from here; if the two ever part, the - // server-side read stops matching this and the lane goes red by name, which is the - // direction that fails loudly. - PixelStoreParameters ServerReadPixelsNeutralPack() { - PixelStoreParameters neutral{}; - neutral.RowLength = 0; - neutral.SkipRows = 0; - neutral.SkipPixels = 0; - neutral.SkipImages = 0; - neutral.Alignment = 1; - return neutral; - } + // The constant itself is MG_Pipe's (MGPipeTypes.h's MGPipeNeutralReadPixelsPack), which is + // the same function MG_Remote/Server/PipeApplier.cpp's read_pixels installs. It used to be + // re-typed here, field for field, to avoid reaching into MG_Remote/Server from MG_Impl - + // but the owner of MGPPixelPackState is MG_Pipe, both sites already include it, and a + // drift between the two spellings would have cost a false divergence rather than a build + // break. #endif // MOBILEGL_BUILD_DISAGGREGATED #endif // MOBILEGL_PIPE_VERIFY } // namespace @@ -740,7 +733,7 @@ namespace MobileGL::MG_Pipe { g_verify.InHook = true; PipeInputs::VisitStorage(field, g_readScratch, g_readScratch, [](auto& live, auto&) { if constexpr (std::is_same_v, PixelStoreParameters[2]>) { - live[0] = ServerReadPixelsNeutralPack(); + live[0] = MGPipeNeutralReadPixelsPack().Pack; } return true; }); diff --git a/MobileGL/MG_Pipe/MGPipeTypes.h b/MobileGL/MG_Pipe/MGPipeTypes.h index 697c69d2..1fc23df3 100644 --- a/MobileGL/MG_Pipe/MGPipeTypes.h +++ b/MobileGL/MG_Pipe/MGPipeTypes.h @@ -1139,6 +1139,31 @@ namespace MobileGL::MG_Pipe { static_assert(sizeof(MGPPixelPackState) == 28, "MGPPixelPackState changed size; update the wire format and this assertion"); + // ID-49's NEUTRAL PACK, in one place. The server's read_pixels reads the backend with this + // state and scatters the tight reply through the application's own pack on the client side + // (MG_Remote/Server/PipeApplier.cpp), so the pack state never shapes the wire answer. + // + // ONE SPELLING, because there are two readers of the same constant and they are in different + // libraries: the applier that installs it, and (in a verify build) the compare-at-read hook in + // MG_Impl/Pipe/PipeFill.cpp, whose oracle for the pack half inside that window IS this value. + // Two hand-typed copies would drift silently in the ONE direction that matters - the hook + // would stop recognising the window and start reporting a divergence that is not one - so + // MG_Pipe, which owns MGPPixelPackState, owns the constant as well. + // + // Value-initialised and then the five fields the applier sets: RowLength / SkipRows / + // SkipPixels / SkipImages are already 0 by PixelStoreParameters' own defaults and are spelled + // out anyway, because "neutral" is a statement about every member rather than about the ones + // whose default happens to be right. + inline MGPPixelPackState MGPipeNeutralReadPixelsPack() { + MGPPixelPackState neutral{}; + neutral.Pack.RowLength = 0; + neutral.Pack.SkipRows = 0; + neutral.Pack.SkipPixels = 0; + neutral.Pack.SkipImages = 0; + neutral.Pack.Alignment = 1; + return neutral; + } + // Also a shader-variant input: both backends bake these into the synthesized // pass-through control stage. struct MGPPatchState { diff --git a/MobileGL/MG_Remote/Server/PipeApplier.cpp b/MobileGL/MG_Remote/Server/PipeApplier.cpp index 9cd5384f..9469f399 100644 --- a/MobileGL/MG_Remote/Server/PipeApplier.cpp +++ b/MobileGL/MG_Remote/Server/PipeApplier.cpp @@ -452,12 +452,10 @@ namespace MobileGL::MG_Remote::Server { // in which the pack state is neutral does not outlive the call. const MG_Pipe::PixelStoreParameters savedPack = MG_Pipe::gPipeInputs.GetPixelStoreParameters(/*isUnpack=*/false); - MG_Pipe::MGPPixelPackState neutralPack{}; - neutralPack.Pack.RowLength = 0; - neutralPack.Pack.SkipRows = 0; - neutralPack.Pack.SkipPixels = 0; - neutralPack.Pack.SkipImages = 0; - neutralPack.Pack.Alignment = 1; + // MG_Pipe owns the constant (MGPipeTypes.h): in a verify build the compare-at-read hook's + // oracle for the pack half inside this window is the same value, and a second hand-typed + // copy would drift without a build break. + const MG_Pipe::MGPPixelPackState neutralPack = MG_Pipe::MGPipeNeutralReadPixelsPack(); MG_Pipe::MGPipeApplySetPixelPackState(neutralPack); table->GL.ReadPixels(info.Box.X, info.Box.Y, static_cast(info.Box.W), diff --git a/docs/Disaggregated/notes/p7/verify-split.md b/docs/Disaggregated/notes/p7/verify-split.md index 006b7bdf..922b572e 100644 --- a/docs/Disaggregated/notes/p7/verify-split.md +++ b/docs/Disaggregated/notes/p7/verify-split.md @@ -128,8 +128,10 @@ upload，所以描述符宣告 `(target, level)` 而调用递空指针，会走 `#if MOBILEGL_BUILD_DISAGGREGATED`）。** 只在这个窗口里换预言：server stamp 之下、`ReadPixels` 动词上，pack 半边 的期望值是 applier 读用的中性 pack；unpack 半边和其它每个字段仍以活上下文为预言。整字段比较失败后，把 scratch 里的 pack 半边换成中性值**再比一次整字段**——pack 既不是应用的值、也不恰为中性值的 server 读，仍是分歧、仍 Fatal。 -中性常量是 `PipeApplier.cpp` 的 `neutralPack` 的第二种拼法（不从这里伸手进 `MG_Remote/Server`）；两者一旦分叉， -server 侧读就不再等于它，车道按名变红——失败方向是响的。 +中性常量**只有一种拼法**：`MG_Pipe/MGPipeTypes.h` 的 `MGPipeNeutralReadPixelsPack()`，applier 的 `read_pixels` +与这里的预言是同一个函数的两个调用点。它一开始是逐字段重打的第二份（为了不从 `MG_Impl` 伸手进 `MG_Remote/Server`）， +但 `MGPPixelPackState` 的属主本来就是 `MG_Pipe`、两边本来就 include 它，而两份拼法分叉的代价是**一次假分歧**而不是 +一次编译错误——失败方向是哑的，所以改成一处。 **放弃了什么，写明。** 在 server 的 `ReadPixels` 上，比对器看不见一个推送了**错误** pack 状态的 client，因为 applier 在 backend 读之前把它覆盖了；消费真实 pack 的 client 侧 scatter 由 readback 矩阵用例按字节覆盖，不按字段。