mirror of
https://github.com/MobileGL-Dev/MobileGL
synced 2026-09-07 19:58:32 +09:00
- docs/Disaggregated/PLAN.md is now the one plan: the MGPipe design with the transport, control-plane, present, threading, EGL/process, monolith/build chapters inlined as real chapters (7-13) instead of references, sections renumbered 0-17 + appendices, every internal citation updated - the replica-GLContext plan and its review record are removed at the user's request; REVIEW.md is the MGPipe design-competition and adversarial-review record only, with the comparison verdicts dropped and the remaining finding text reworded to the new section numbers - day-43 GO/NO-GO now names its two outcomes (continue / shrink to headless tooling or re-evaluate) without any rollback path
132 KiB
132 KiB
拆分设计评审记录(MGPipe)
生成于 2026-09-05,配合同目录
PLAN.md阅读。这一轮的前提是用户的方向修正:backend 应拥有贴近后端 API 的状态机并暴露 gallium 式显式接口;memo/SharedPtr/版本计数器无 wire 对应物是要解决的工程问题,不是否定薄后端的理由。
1. 候选方案与评分
三个独立方案,三位评审按 5 项加权打分(边界清晰度/架构价值 0.25、改造成本与风险 0.20、性能 0.15、语义完整性 0.20、可增量/monolith 保留/可测试 0.20)。
| 方案 | 角度 | 三位评审加权分 |
|---|---|---|
| MGPipe: a split-first explicit backend interface (server owns its state machine, no MG_State replica) | SPLIT-FIRST PRAGMATIC. Keep PLAN.md's transport/data-plane/sync/present/threading/platform/build design essentially verbatim, and replace on | 8.2 / 8.8 / 8.4 |
| MGPipe: a gallium-faithful explicit interface for MobileGL | GALLIUM-FAITHFUL. Introduce MGPipe — an MGPipeScreen/MGPipeContext pair modelled directly on pipe_screen/pipe_context (CSOs with create/bind | 7.3 / 7.65 / 7.7 |
| MGPipe: a twin-derived explicit backend interface for MobileGL | Backend-native state machine first. The interface is not designed top-down from gallium; it is read off the memo/snapshot/twin structures Di | 8.45 / 8.6 / 8.25 |
评审指出的致命缺陷(已在综合稿中处理)
- Design 1 — internal schedule contradiction, and it is the axis this review weighs hardest. Its comparison section claims 'the earliest honest IPC frame on a trivial workload is day ~45-55, and a Minecraft frame ~day 120+'. Its own phase list places the first IPC frame in P11, which follows P0-P10 (8-11 + 10-14 + 8-11 + 12-16 + 12-16 + 9-12 + 35-44 + 24-30 + 26-33 + 8-12 + 10-14 = 217-283 days). The phase list is the binding artifact, so the real first frame is ~day 220. A plan that asks for 260-340 engineer-days with zero IPC value for ten months, against a verified 77-day alternative (PLAN.md P0..P9 sums to exactly 77), will be rejected on schedule regardless of its architectural merit — and its own comparison text obscures that rather than confronting it.
- Design 1 — it takes the one gallium deviation the tree argues against, and takes it on the hottest path. Decomposing RenderStateParameters into blend/depth_stencil/rasterizer CSOs discards a documented layout invariant (ScissorBoxWrittenMask at RenderState.h:363 and ClipDistanceEnabledMask at :369 were deliberately placed in the tail span after LogicOp so DirectGLES's three-span memcmp at :2035-2046 catches them) and turns one 8-byte version compare into three hash computations plus three lookups per state transition. Content-addressing answers the correctness half but not the cost half, and DirectGLES still needs the blob per CSO anyway to diff against the driver and emit only changed GL calls — so the decomposition buys the server a handle compare while the client pays three hashes. Not fatal to the architecture; fatal to the claim that this is the cheapest shape.
- Design 3 — the residual value block is a live semantic hole during the P5-P8 split window with only half a guard. The poison mask catches UNFILLED fields; it does not catch a block whose layout differs between the emitting client and the applying server, which is exactly the failure a union of heterogeneous PODs invites across a compiler/ABI boundary. The design specifies static_assert on sizeof but not on member offsets. Without per-member offsetof asserts (or serializing the block field-wise rather than memcpying it), a padding difference produces silently wrong render state in split mode that the monolith verify harness cannot see, because in monolith mode both sides are the same translation unit.
- Design 3 — P7 (DirectVulkan, 48 days) is roughly half the independent 85-111 estimate for the same work, and it sits on the critical path for the second backend's split support. The design names this honestly and makes P3a the falsification point, which is the right response, but the 192-day total should be read as 192-260 and the plan should state that a P3a overrun by more than 50% re-baselines the whole schedule before P4a starts — which it says, but only in the risk list, not in the headline number.
- All three — the central performance claim is unfalsified and cannot be settled from the tree. Every design argues the per-draw reachability traversal MOVES to the client rather than doubling (as the replica plan's does), and therefore that net CPU is <= monolith. Nothing in the tree measures per-frame bytes or calls: MG_Util/Metrics is format arithmetic and Tracy has zones but no plots. All three correctly put TracyPlot counters in P0, and all three correctly nominate per-thread CPU time rather than wall-clock frame time as the metric. But until those land, every ring size, every batching threshold, the render-state wire granularity decision and the headline CPU argument are estimates. Any adopted plan must treat the P0 counters as a hard prerequisite, not a nice-to-have.
- All three — the server-initiated texture re-mint pull is a genuinely new stall class that the replica plan does not have, and its rate on the real corpus is unmeasured by all three. imageBindableHint pre-empts RequireImageBindableStorage (Managers.cpp:2813), but full format regeneration (:3950-4195) fires on ordinary glTexImage format changes and is not pre-emptible. All three ship the same three mitigations (hint, asynchronous park-and-re-emit so the stall lands on the apply thread, bounded retention LRU) and all three gate it with a scenario plus a published per-case pull counter, which is the right shape. The residual risk is identical across designs and should be tracked as a portfolio risk, not scored against any one of them.
- Design 1 — the render-state CSO decomposition is wrong and its justification is internally inconsistent. I verified both halves of the counter-evidence: DirectGLES.cpp:2025-2050 does a three-span head/blend/tail memcmp guarded by static_assert(is_trivially_copyable_v), and RenderState.h:355-370 states verbatim that ScissorBoxWrittenMask and ClipDistanceEnabledMask were placed 'Deliberately beside ScissorBoxes so it shares their tail span (after LogicOp) and DirectGLES' span memcmp picks a transition up like any other state.' Design 1 §5.4 then proposes hashing 'the three spans DirectGLES already memcmps' to obtain three CSO handles — but head/blend/tail is not the blend/depth-stencil/rasterizer partition, so the proposed mechanism cannot produce the proposed handles. Beyond the inconsistency, decomposition introduces a hand-maintained field→CSO partition over a ~150-field struct with no completeness tripwire: a field added to RenderStateParameters and not assigned to a CSO is silently never pushed, whereas under the blob it rides along and a sizeof static_assert catches schema drift. Not fatal to the design as a whole — replace this one entry with Design 3's create/bind_render_state and Design 1 becomes competitive.
- Design 2 — handle/data-structure mismatch. MGHandle is defined as the monotone, never-reused GetLifetimeId() (8 B), and the design then claims the six StateBackendObjectRegistry instances and thirteen Magma caches become 'arrays indexed by handle' and that this is what deletes TwinLookupMemo/OwnerEquals/g_fbSlotCache. A sparse monotone u64 cannot index an array; without a dense per-kind slot allocator the server keeps a hash map and retains most of the lookup cost the design books as deleted. The fix is Design 3's PipeHandle{slot, gen} with per-kind dense slots plus reserved bands — same 8 bytes, same ABA guarantee, and it actually delivers the array.
- Design 2 — an asserted factual correction that is itself wrong. It opens by 'correcting' the evidence to 'exactly 71 function pointers plus one capability bool, GLFunctionsTable BackendObject.h:117-278 … not 67, not 73.' Measured: 67 function pointers in that range. Minor in substance, non-trivial in credibility for a design whose entire method is 'I re-measured the tree where the reports disagree.'
- Design 3 — the day-62 milestone is narrower than it reads. Emulations (client vertex/index arrays, primitive-restart rewrite, indirect-count resolve, CopyImage mirror) are deliberately Fatal in split mode until P8, so 'first cross-process frame' means OpenRA on a reduced path. That is a legitimate engineering choice but it must be labelled at the go/no-go, or a stakeholder will read it as 'the split works' when the answer is 'the transport and five object classes work.'
- Design 3 — the 192-day total is the least defensible number in the set, against a refactor-cost evidence range of 202-266 days for the backend work alone plus ~68 for IPC. The design concedes this and names a falsification (P3a overrun >50% ⇒ re-baseline before P4a), which is the right response, but the headline figure should be presented as a range with the P3a checkpoint attached.
- All three — the central performance claim (the per-draw reachability traversal MOVES to the client and gets cheaper rather than doubling) is unmeasured, because the tree has no per-frame byte or call metric at all (MG_Util/Metrics is format arithmetic; Tracy has zones and no plots). All three correctly schedule TracyPlot counters in P0/M0 and all three correctly insist the metric be per-thread CPU time rather than wall clock. No design should be believed on CPU until that lands, and the first real datapoint (render state on both backends) must be a hard go/no-go, not a report.
- All three — loss of PLAN.md's byte-identity monolith gate (nm --defined-only plus stripped .text equality) is unavoidable and all three say so explicitly. This is a shared cost, not a flaw of any one design, and the five-part replacement (purity grep + nm, per-draw field-wise MOBILEGL_PIPE_VERIFY, behavioural A/B across {monolith-pull, monolith-push, split}, per-thread CPU non-regression, coverage/poison/no-raw-pointer-memo asserts) is stronger semantically than what it replaces. It must be written down as a cost in the final doc, not buried.
- DESIGN 1 — MAJOR, not strictly fatal but must be reversed before P0 freezes the header: decomposing RenderStateParameters into blend/depth_stencil/rasterizer CSOs (§1.2 D3, §3.2). Its own evidence contradicts it — RenderState.h:359-368 records that ScissorBoxWrittenMask and ClipDistanceEnabledMask were deliberately placed in the tail span so DirectGLES' three-span memcmp (DirectGLES.cpp:2035-2046, guarded by a static_assert(is_trivially_copyable_v) at :2033) picks a transition up like any other state. Espryt keeps a byte-for-byte value mirror precisely so it can emit only the changed GL calls, so the server must retain the blob per CSO regardless; the decomposition therefore buys a handle compare the versioned blob already provides and adds a span re-hash plus three cache lookups on every GetPipelineStateVersion move. Fix: adopt Design 2/3's versioned blob with a dirty-span mask (Design 3's client LRU makes a repeat cost 12 bytes), and let the server derive whatever CSOs it wants internally.
- DESIGN 2 — CREDIBILITY, not architecture: the opening Verification note asserts 'GLFunctionsTable has exactly 71 function pointers plus one capability bool ... with Present/SetSwapInterval that is 74 members — not 67, not 73' and explicitly overrides the other reports. Measured at dev@81b17c0b: 67 function pointers + 1 Bool = 68 members, 70 with GlobalBackendFunctionsTable. It also states '50 include lines over 18 distinct MG_State headers' where I measure 50 lines over 15 distinct MG_State paths, and carries 169 DirectVulkan pGLContext reads where the actual count is 166 (VulkanRenderer 126 + DirectVulkan 18 + UniformManager 14 + VkRenderPassManager 3 + VkTextureManager 2 + BackendObject_DirectVulkan 2 + VkClearManager 1). A design whose central methodological claim is 'I re-derived this from the tree rather than copying the brief' cannot afford to be wrong in the one place it says so loudest. None of this invalidates the design, but every other unverified number in it now needs an independent check before it is used for sizing.
- DESIGN 3 — SCHEDULE, acknowledged but under-absorbed: P7 (DirectVulkan, all subsystems) is priced at 48 days against the refactor-cost reader's 85-111 for the same scope, and the 192-day total sits below the reader's 202-266 for the backend refactor ALONE. Design 3 names this as a risk and supplies a falsification trigger (re-baseline if P3a overruns >50%), which is the right instinct, but the trigger fires on Espryt's wave-1 and cannot detect a Magma-specific overrun until P7 is already the critical path. Fix: add a second explicit re-baseline gate at P7 midpoint, and price the CTS turnaround (gl44to46 is ~56,271 cases) as a separate line rather than folding it into the phase estimates.
- ALL THREE — completeness gap in the migration mechanism, shared and unaddressed: MG_Backend has 348 pGLContext mentions of which only 290 are arrow uses. All three designs propose a mechanical sed of 'MG_State::pGLContext->' to a macro/alias over '293 sites' and none accounts for the 58 non-arrow uses — the null-guards (Managers.cpp:3608, 3737, 3808, 4663, 8678; BackendObject_DirectVulkan.cpp:388, 788), the MOBILEGL_ASSERT truth tests, the raw-pointer capture at DirectGLES.cpp:146 (MG_State::GLState::GLContext* ctx = MG_State::pGLContext.get()), and the patch-parameter ternaries at Managers.cpp:7120-7131 that sit inside the transpile path. The patch reads are semantically covered by set_patch_state in all three catalogues, but the mechanical step is under-specified and the raw .get() capture defeats an accessor-shaped alias entirely. Whichever design is chosen must enumerate and convert those 58 sites explicitly, and the interface-purity gate must grep for 'pGLContext' (not 'pGLContext->').
- NONE OF THE THREE is fatally incomplete on semantics. Each satisfies all 290 backend reads, both texture-byte channels, the 26 reverse pulls, XFB (CPU accounting client-side, capture writeback as a reply), queries and fences (client-minted, two-valued contract preserved), persistent maps (explicitly quarantined from the refactor, decided by a POST-probed tier), GPU-written buffer reads (conservative client pending set narrowed by an EvGpuWritten reply), share groups (one flat handle space in v1, screen/context split declared in the header from day one), and the composite pipeline program (never crosses; resolved by Core.cpp:592-744 as today). All three correctly identify the server-initiated texture re-mint pull as the one genuinely NEW stall class and mitigate it three ways with a dedicated gate and a per-trace-case counter.
评审建议嫁接的要点
- From Design 3 — the Track V / Track H accessor split. Roughly 55% of the class-B reads are value-typed (RenderStateParameters, PixelStoreParameters, IsCapabilityEnabled, GetStencilState, GetColorMaskIndexed, the ~22 Magma singletons) and need no reshaping whatsoever: the client memcpys, the server hands the backend a reference to its own copy. Only the 167 SharedPtr<MG_State...> points need real work. This is the decomposition that makes migration granularity one accessor rather than one subsystem, and it is the load-bearing premise under any split-first schedule. Neither Design 1 nor Design 2 states it.
- From Design 3 — the residual value block with a compile-error retirement. One temporary set_residual_value_state carrying the union of not-yet-migrated value accessors, guarded by static_assert(sizeof(ResidualValueBlock) == MGL_RESIDUAL_BLOCK_SIZE) with the constant bumped DOWN each phase, ending at static_assert(sizeof(...) == 0). This is what lets the split run subsystem by subsystem instead of after a finished refactor, and it is the only temporary in any of the three designs with a mechanical (not procedural) retirement. Add the layout static_assert it omits: the block must be byte-identically laid out on both sides, so assert offsetof for every member, not only sizeof.
- From Design 3 — the PipeInputs::m_filledMask poison. In debug and disaggregated builds, reading a field the tracker never pushed is Fatal{UnmigratedPipeInput, "GetStencilState"} on the first draw. Design 2's G5 written-once bitmask is the same idea, but Design 3's runtime-fatal formulation is the one that cannot be rendered past, and it works during the split window where Design 2's generated comparer needs both models live in one address space.
- From Design 3 — the ordering rule that identity handle-ification precedes the first frame while memo re-keying follows it (P3a/P4a before P5/P6; P3b/P4b after). The wire needs handles; the 28 days of memo re-keying, dirty-flag inversion and program-staleness rework are optimizations that can land behind a working split. This single reordering is worth ~5 weeks of time-to-first-frame and neither other design exploits it.
- From Design 3 — the explicit day-21 hedge: run PLAN.md's P0 verbatim (its hygiene, skeleton, spikes and byte counters are state-model-independent), then MGPipe P1+P2 (15 days), then decide. At day 21 you hold the verify harness proving push works, render state pushed on both backends, a measured monolith per-thread CPU delta on two devices, and the per-accessor cost of Track H sampled. That is a genuine, cheap decision point, and it is the only one offered in the set.
- From Design 1 — the client-side content-addressed CSO cache modelled on Mesa's cso_context/cso_cache, with per-kind caps and LRU eviction issuing delete_*_state. Design 2's render-state LRU is the same idea applied to one blob; Design 1 generalizes it to vertex-elements, samplers and sampler views, and the property that two different programs setting identical state produce ZERO server-side transitions is a real per-draw win worth keeping even while shipping the render-state blob rather than three CSOs.
- From Design 1 — the framing that inproc IS u_threaded_context: a push-only interface recorded into batches and applied on the server thread. Mesa proved this shape can be transparently threaded, and it reframes the monolith render-thread deliverable from 'an IPC side effect' to 'the interface's second consumer'. Worth stating explicitly in whatever plan is adopted, because it is the argument that the interface pays for itself even if the process split never ships.
- From Design 1 — homing each emulation by gallium's own rule (state-tracker side when caps say the driver cannot, driver side when it is a driver lowering) with a named cap bit per decision: kCapPrimitiveRestart, kCapMultiDrawIndirectCount, kCapFloat64VertexAttrib, kCapNeedsHostIndexBytes. That turns the per-backend asymmetry (Magma's deliberately null ResidentSubData, the 8 null slots, PrefersCpuXfbPrimitiveAccounting) from a wart into the mechanism, and it replaces today's implicit slot-nullness capability probes at GL_Query.cpp:471/545/768.
- From Design 2 — PipeCalls.def as one X-macro consumed by five generators (function table, monolith thunks, wire records with per-kind static_assert plus generated runtime bounds checks, the shadow-compare comparer, the written-once mask). Design 3 has the coverage generator but not the comparer/mask generators; generating the semantic gate from the same source as the call table is what stops the gate going stale as the catalogue grows.
- From Design 2 — the D18 exception. Its D-class table is the only one that marks VkRenderPassManager::m_renderbufferResources / VkTextureManager::m_textureResources as UNCHANGED, with the reason (callers cache Resource* across further lookups; a table grow once relocated a cached &layout and BlitFramebuffer silently bailed at 'source image layout undefined'; ska's erase-shift makes it worse, not historical). Whichever plan is adopted must carry that postmortem verbatim into the review checklist, because converting those to slot arrays is exactly the change a refactor makes without reading the comment.
- From Design 2 — the DERIVATION METHOD, adopted as the doc's opening chapter: build the call catalogue by inverting the backends' own key structures (SetupDrawSnapshot VulkanRenderer.h:948-1042, BackendTextureObject::IsDrawSyncClean Managers.h:1003-1020, ResolvedDrawBuffers Managers.h:697-717, ResolvedVertexBindings VulkanRenderer.h:1153-1218, g_syncedRenderStateParameters DirectGLES.cpp:1956, BufferBackendOps BufferObject.h:76-120), not top-down from gallium. This is both the honest justification for every entry and the reason the interface is complete: the inputs to those structures ARE the interface.
- From Design 2 — PipeCalls.def as single source of truth with FIVE generators: function tables, monolith thunks, wire records with per-kind static_assert plus generated runtime bounds checks, the MOBILEGL_PIPE_VERIFY field-wise comparer, and the written-once bitmask. Generating both tripwires removes the hand-maintenance risk that is the design's own biggest exposure. Graft over Design 3's hand-written verify.
- From Design 2 — the explicit two-kinds-of-generation statement: client-owned identity vs the twelve server-only epochs (g_bufferMutationEpoch, g_bufferBackendIdGeneration, g_attachmentBackendIdGeneration, g_backendContextGeneration, m_textureImageEpoch, m_resourceEraseEpoch, m_renderbufferImageEpoch, m_sliceEpochCounter, m_cacheStructureEpoch, m_evictionEpoch, m_recordingGeneration, m_frameSerial) that the client must never be asked about. Write this as a normative interface rule, not prose.
- From Design 2 — D18 marked UNCHANGED with a review-checklist note: VkRenderPassManager::m_renderbufferResources and VkTextureManager::m_textureResources are deliberately node-based std::unordered_map, not the project's open-addressed UnorderedMap, because callers cache Resource* across further lookups (postmortem at VkRenderPassManager.h:375-397, a BlitFramebuffer silently bailing at 'source image layout undefined' after a table grow relocated a cached &layout). It is the only design that explicitly flags 'do not optimise this container back during the refactor.'
- From Design 2 — the dirtySpanMask on the render-state wire. Compose with Design 3's CSO: on a CSO cache MISS ship only the changed spans of the blob plus the previous CSO handle as a base, rather than the full ~1.1 KiB. Cheapest of all three encodings.
- From Design 1 — CAPS-GATED emulation homing, replacing fixed client/server assignment. MGPipeCaps carries kCapPrimitiveRestart, kCapPrimitiveRestartFixedIndex, kCapMultiDraw, kCapMultiDrawIndirectCount, kCapFloat64VertexAttrib, kCapResidentSubData, kCapCpuXfbPrimitiveAccounting, kCapNeedsHostIndexBytes, and each lowering (u_primconvert-style restart rewrite, indirect-count fallback, client-array upload) runs client-side only when the cap says the server cannot. This replaces today's implicit null-slot capability probes at GL_Query.cpp:471/545/768 and makes per-backend asymmetry (Magma's deliberately absent ResidentSubData, VkBufferManager.cpp:104-111) the mechanism rather than a wart.
- From Design 1 — kCapNeedsHostIndexBytes specifically: it prices the monolith/split asymmetry of MGHostSpan honestly (a free pointer in-process, a copy on the wire) so a backend that never needs host index bytes does not pay.
- From Design 1 — the explicit deviations-from-gallium table with a tree citation per row. Keep the format; replace only the render-state row with Design 3's blob-CSO.
- From Design 3 — the render-state shape itself: create_render_state(cso, blob) + bind_render_state(cso, v, pipeV) with a client LRU. Graft into whichever design wins.
- From Design 3 — PipeFramebufferState with a CLIENT-RESOLVED readSurface and inline attachment internalFormats. Two defect classes and one lookup deleted by struct shape alone.
- From Design 3 — Track V / Track H accessor split, per-accessor migration granularity, and MOBILEGL_PIPE_PUSH as a per-subsystem bitmask latched at init like MOBILEGL_BACKEND_TYPE (ConfigLoader.cpp:212-225), so every commit has a same-binary A/B on either backend.
- From Design 3 — every temporary gets a compile-error retirement: PipeInputs::m_filledMask poison giving Fatal{UnmigratedPipeInput, fieldName}, and static_assert(sizeof(ResidualValueBlock) == 0) before the pull path may be deleted. Adopt this rule wholesale; it is the difference between a strangler that finishes and one that ossifies.
- From all three, unchanged — the EvLogLine severity split (level <= WARN lossy, level >= ERROR lossless plus a per-second rate limiter emitting 'N suppressed'), because backend program link failure is surfaced ONLY as MGLOG_E plus a bind-program-0 no-op (Managers.cpp:8091-8126, 8357-8372) and PLAN.md §7.4's uniform lossy policy would silently drop the system's most valuable diagnostic.
- FROM DESIGN 2 — derive the interface from the backends' own key structures, not from gallium top-down. SetupDrawSnapshot (VulkanRenderer.h:948-1042) is a 40-field enumeration of everything Magma must have pinned for a draw; DrawTextureSyncKeys + IsDrawSyncClean (Managers.h:1003-1020) is the same for Espryt's textures; ResolvedDrawBuffers/ResolvedVertexBindings are the vertex-input statement; g_syncedRenderStateParameters is the render-state statement verbatim. This is a stronger completeness argument than any coverage table, and it is what produces the correct blob-not-CSO answer on render state. Design 3 should adopt this as the explicit derivation rationale for its call catalogue.
- FROM DESIGN 2 — PipeCalls.def with five generators from one file: function table, monolith thunks, wire records + per-kind static_assert + generated runtime bounds checks, the MOBILEGL_PIPE_VERIFY field-wise comparer, and the written-once bitmask. Generating the verify comparer and the completeness tripwire from the same declaration as the call list means the gates cannot drift from the interface. Design 3 hand-writes both; it should generate them.
- FROM DESIGN 2 — keying PipeInputs on MEMO KEYS rather than read sites. That is why the pushed block stays ~20 KB with a field set stable across the migration, and it is the reason per-accessor granularity actually works. Design 3's PipeInputs is described per-accessor, which is a larger and less stable field set.
- FROM DESIGN 2 — D18 explicitly marked UNCHANGED with the VkRenderPassManager.h:375-397 postmortem carried verbatim into the review checklist, so nobody 'optimises' m_renderbufferResources/m_textureResources back to the project's open-addressed UnorderedMap. The ska erase-shift behaviour makes that hazard worse, not historical. Neither other design guards this.
- FROM DESIGN 2 — MGHostSpan: one 32-byte accessor for the four host-byte classes (client vertex arrays, client index arrays, indirect/parameter command blocks, index bytes) whose fill policy differs by build. Zero monolith cost (one pointer load), and it is the abstraction that makes the disappearance of the 26 SyncPersistentMappedRange/SyncGpuWrites reverse pulls a mechanical consequence rather than a per-site argument.
- FROM DESIGN 1 — the emulation-homing RULE (gallium's own: state-tracker lowering when a cap says the driver cannot, driver lowering when the driver forces it), with each emulation gated on a named capability bit — kCapPrimitiveRestart, kCapMultiDrawIndirectCount, kCapFloat64VertexAttrib, kCapNeedsHostIndexBytes. Designs 2 and 3 assign emulation ownership case by case; Design 1's rule generalises to a third backend and makes the assignment auditable.
- FROM DESIGN 1 — kCapNeedsHostIndexBytes specifically: it prices the monolith-vs-split asymmetry (a shadow pointer costs nothing in-process, a copy in split) into the interface as a capability, so a backend that never needs host index bytes never pays.
- FROM DESIGN 1 — the explicit 8-deviation ledger (each deviation from gallium named, justified by a file:line or a measured cliff, and numbered). This is the right way to document an interface that will outlive its authors; Designs 2 and 3 justify their deviations inline and less traceably.
- FROM DESIGN 1 — MGPipeCallbacks as a single named struct of 8 reply/event kinds installed at context_create, rather than an ad-hoc event list. In the monolith they are direct calls; in split they are records. This makes the reverse channel a first-class part of the interface rather than an appendix.
- FROM DESIGN 3 (keep) — dense per-kind slots in an 8-byte PipeHandle{slot, gen}. Designs 1 and 2 use sparse 64-bit lifetime ids as the wire handle, which keeps the server on a hash table; dense slots make the server's object tables literal arrays, which is what actually deletes the hashing/ABA layer rather than merely re-keying it. The lifetime id stays client-side as the tracker's own identity.
- FROM DESIGN 3 (keep) — client-resolved readSurface in the framebuffer payload, and static_assert(sizeof(ResidualValueBlock)==0) as the retirement device for a deliberate temporary.
2. 对抗性审查(三个视角)
GL 语义正确性(refuted=False,12 条)
- [major] The headline per-draw cost comparison (§10.2, §5.1) is a static-site-count vs dynamic-call-count category error; the baseline is overstated by roughly an order of magnitude
- 问题:§10.2's table and §5.1 price today's per-draw state acquisition as "Espryt 124 / Magma 169 accessor calls + version compares + a ~1.2KB three-span memcmp + CurrentUnitBindingsEpoch's per-unit owner walk + Magma's two lossy version sums + ~40 payload accessor walks". 124/169 are STATIC
pGLContext->call sites (§2.1's own definition), not dynamic per-draw calls. Every one of those costs is already memo-gated in the tree: -SyncRenderStatereturns at the top on a single Uint16 compare (MobileGL/MG_Backend/DirectGLES/DirectGLES.cpp:2016-2018:if (!forceFullPush && !colorMaskWidenDirty && g_hasSyncedRenderState && currentRenderStateVersion == g_syncedRenderStateVersion) return;). The three memcmps run only when the version moved. -SyncNeccessaryTexturessteady state is a 6-value key compare plusPairingsIntactand a per-entryIsDrawSyncCleanword compare (DirectGLES.cpp:1537-1560); the unit walk runs only on a miss. -CurrentUnitBindingsEpochhas a three-value fast gate and only walks owners when the bind generation moved (DirectGLES.cpp:1421-1426). - Magma'sTrySetupDrawFastPathsteady state is ~10 accessor calls and ~20 word compares (MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp:6002-6300), not 169. -GetOrCreatePipelinerecomputes the pipeline-state hash only whenGetPipelineStateVersion()moved (VulkanRenderer.cpp:4982-4993), and the "~40 payload accessor walk" at :5155-5200 runs only on a pipeline memo MISS. -ApplyDynamicDrawStateTailhas a two-level gate: one version compare, then a value key built from one bulk fetch (VulkanRenderer.cpp:5888-5893). So the real steady-state pull cost is on the order of 10-25 accessor calls and a few dozen word compares per draw per backend. Comparing that against "1 dirty word test + N set_*" is a much narrower margin than the plan's table implies, and the plan's entire business case (B-R2, the day-24 GO/NO-GO in §0.6/P2, the "traversal is moved, not doubled" claim) is built on the inflated figure. - 修法:Restate §10.2's table in DYNAMIC terms and stop citing 124/169 as a per-draw cost anywhere in the document (they belong only in §2.1's coupling-surface argument). Add a per-draw dynamic counter (accessor calls executed, memo hit/miss per gate) to P0's TracyPlot deliverable list alongside the byte counters — the plan currently lands byte counters but no call counters, so it will still be guessing at P2. Then make the day-24 GO/NO-GO threshold an ABSOLUTE number (ns/draw of tracker cost measured on both devices) rather than "within the noise of monolith-pull", because relative-to-noise passes trivially when the true baseline is 20 calls, not 124.
- 问题:§10.2's table and §5.1 price today's per-draw state acquisition as "Espryt 124 / Magma 169 accessor calls + version compares + a ~1.2KB three-span memcmp + CurrentUnitBindingsEpoch's per-unit owner walk + Magma's two lossy version sums + ~40 payload accessor walks". 124/169 are STATIC
- [major] The tracker is specified as a poll of existing counters, which is the same traversal it claims to eliminate — §5.2 and §10.2 are mutually inconsistent
- 问题:§1.1/§5.2 state "MG_State 零新增记账" and map every dirty bit onto an existing version counter; §5.4-2 explicitly requires the two high-water-mark walks (
TouchBindPoint/GetTouchedBindPointCount,NoteUnitTouched/GetMaxTouchedUnit) to stay "in the tracker's walk". That meansm_dirtyis COMPUTED by polling, not SET by the mutators. But §10.2 and §5.1 price the steady state as "one 64-bit dirty word test + N set_* calls". These cannot both be true.MGPIPE_NEW_SAMPLER_VIEWSalone is mapped in §5.2 ontoGetContentVersion+GetShapeVersion+GetTextureParamsVersion+GetTextureBindGeneration+GetSamplingResolutionGeneration. The first three are PER-TEXTURE, so computing that one bit requires walking the touched units and reading three counters per bound texture — which is exactlySetupDrawSnapshot'ssampledContentSum/sampledParamsSumwalk (VulkanRenderer.cpp:6253-6254) that §4.7.3-D14 claims collapses to "one compare", and exactly Espryt's unit list walk. Same forNEW_VERTEX_BUFFERS(per-attributeVertexAttributeVersiontriples) andNEW_FRAMEBUFFER(Array<Uint16,40>attachment versions). Gallium does not work this way:st_invalidate_*sets dirty bits from the GL entry points;st_validate_statenever polls object versions. The plan adopts gallium's validate-time push but not gallium's dirty-marking, and then quotes gallium's cost. - 修法:Choose explicitly, in the design document, and price the choice. The correct answer is dirty-MARKING: have MG_Impl's mutating entry points call
MGPipeTracker::MarkDirty(group)so validate is genuinely O(dirty groups). Then delete the "zero new bookkeeping in MG_State" claim, add the marking-site audit to B-R6 (it is the same completeness obligation as the reconciler, on a larger surface — every GL setter, not every backend read), and let the G5 written-once bitmask plus MOBILEGL_PIPE_VERIFY cover it. If instead polling is kept, §10.2 and §5.1 must be rewritten to say the tracker performs the same per-object walk as today's backend, and the net win reduces to the server-side memo deletions only.
- 问题:§1.1/§5.2 state "MG_State 零新增记账" and map every dirty bit onto an existing version counter; §5.4-2 explicitly requires the two high-water-mark walks (
- [major] The ~115-line unit-bindings epoch machinery is booked as deleted, but it cannot be deleted — only moved to the client
- 问题:§2.5, §4.7.3-D3 ("结构性删除") and §10.4-1 count
UnitBindingsSnapshot/CaptureUnitBindings/UnitBindingsUnchanged/CurrentUnitBindingsEpoch/UnitTextureSyncEntry/PairingsIntact(~115 lines,DirectGLES.cpp:1372-1489) as a structural deletion, on the ground that "the push call IS the change signal". That is only true if the client can cheaply decide WHETHER to push. It cannot, for exactly the reason the machinery exists:GetTextureBindGeneration()bumps on REDUNDANT rebinds — the comment atDirectGLES.cpp:1414-1420records that MC 26.2 rebinds the same sampler around every texture-unit switch. If the tracker keysset_sampler_viewson the bind generation it will push a full resolved view array on every redundantglBindSampler, which in the workload that motivated the machinery is per-batch. To avoid that it must do the same owner-comparison walk — i.e. the code moves toMG_Impl/Pipe/Tracker.cpp, it does not disappear. Worse, in split mode a spurious push is not just CPU:set_sampler_viewsis akVarTailrecord carrying anMGPSamplerView-shaped entry per sampled unit, so a redundant push costs hundreds of ring bytes per draw. The same argument applies tog_fboTextureSyncList(D8) and, in weaker form, toResolvedTextureBindingMemo(D9): the client needs its own memo keyed on the same epoch to avoid re-resolving completeness (IsMipmapCompleteForFilter/SamplesAsIncompleteTexture/IsUndefinedDefaultTexture) per draw, since §5.5 puts view resolution on the client. - 修法:Move these rows from "deleted" to "relocated" in §2.5, §4.7.3 and §10.4-1, and subtract them from the "~550 lines deleted" ledger (which then drops to roughly 350-400, of which the genuinely-deleted parts are TwinLookupMemo×3 + OwnerEquals, the six registry GC sweeps,
sourcePin, and the placeholder-texture puppetry). Add the client-side epoch memo and its key to §5.5 as an explicit deliverable of P3b/P4b, and add aset_sampler_viewspush-count-per-frame counter to the P0 counter list so a regression to per-batch pushing is visible immediately.
- 问题:§2.5, §4.7.3-D3 ("结构性删除") and §10.4-1 count
- [major] D-B1's whole-block RenderStateCso re-creates the exact regression the two version counters exist to prevent
- 问题:
RenderState.h:519-528documents why there are two counters: "Viewport, scissor, depth range, blend colour, line width, polygon offset, stencil write mask, the clear values, hints and the point-size family are all either dynamic pipeline state or not pipeline state at all, so changing one of them must not evict a cached pipeline. Keeping one counter for both made a glViewport call knock the next draw off the pipeline memo AND the draw fast path." Verified:RenderState.cpp:639-640, 702-735and neighbours bump only++m_versionfor those setters, neverBumpVersions(). D-B1 makes the CSO identity the CONTENT of the wholeRenderStateParametersblock. ThereforeglViewport,glScissor,glBlendColor,glClearColor,glLineWidth,glStencilMaskandglPolygonOffseteach produce a different content hash, hence a different CSO handle. Consequences: (a) a 64-entry client LRU (§4.5.2/§4.1) keyed on a block containing 16 viewports + 16 scissor boxes + 16 depth ranges + clear values will thrash under Iris shader packs and shadow-cascade rendering, which change viewport/scissor many times per frame; (b) each LRU miss re-sends a ~1.2 KBcreate_render_stateblob; (c) a new CSO handle invalidates any per-CSO pipeline-hash memo the server keeps, which is the very thing §4.5.2 promises ("Magma 每 CSO 算一次 pipeline hash"). D-B1 and D3 ("CSO 边界跟 Vulkan 动态状态走") therefore contradict each other inside the same document. - 修法:Key the CSO on the pipeline-relevant subset only — the same field set
ComputePipelineStateHashalready enumerates (VulkanRenderer.cpp:4826-4906) and the same subsetm_pipelineStateVersionguards — and carry viewport/scissor/depth-range/blend-colour/line-width/polygon-offset/stencil-ref-and-write-mask as a separateset_dynamic_statepayload, mirroringDynamicStateShadowandApplyDynamicDrawStateTail. Accept and state that this breaks the "reuse the existing head/blend/tail span division" argument (the head span starts withViewportsand also containsLineWidth/PointSize/PolygonOffset*, so the existing spans do not align with the pipeline/dynamic split); the span-memcmp layout invariant then applies inside the pipeline-subset blob and must be re-derived, which is cheaper than paying a CSO per glViewport.
- 问题:
- [major] Content-addressed CSOs make the single path the code names as hottest more expensive, not cheaper
- 问题:
DirectGLES.cpp:2029-2032names the target: "a per-draw blend toggle used to re-diff all ~40 pieces of state field by field on every draw (Blaze3D brackets every batch with glEnable/glDisable(GL_BLEND), making this the hottest thing mc_state_toggle did)". Verified that a real toggle does move the version —SET_CAPABILITYshort-circuits only on a REDUNDANT set (RenderState.cpp:311-313), and enable/disable pairs are not redundant. Today's cost on that path: three memcmps over ~1.2 KB, server-side, once per draw whose version moved. Under the plan the client must find the CSO by hashing, and it cannot shortcut via the version:m_versionis monotonic (++m_version), so a version value never repeats and no version→CSO memo can ever hit on the alternating-content pattern. So the client pays an xxHash over the same ~1.2 KB plus aska::flat_hash_mapprobe on every such draw. Then, because the handle changed, Espryt's 693-line body still runs its span memcmp — P2's deliverable explicitly keeps it "一行不动". Net: a full-block hash and a map probe ADDED, nothing removed. For Magma it is worse in a subtler way:ComputePipelineStateHashfolds roughly 25-30 words out of one bulk fetch (VulkanRenderer.cpp:4826-4906) — far cheaper than an xxHash of the full 1.2 KB block. Moving pipeline-hash computation behind a CSO handle therefore trades a cheap server-side hash for an expensive client-side one on precisely the toggle pattern §4.5.2 cites as the justification. - 修法:Do not content-address on the full block. Derive the CSO key from the pipeline-subset field list (reuse
ComputePipelineStateHash's enumeration verbatim so the two can never disagree) plus the two version counters, and let the CSO cache hold the small key. Alternatively drop content addressing on the hot path entirely: mint a CSO per distinctm_pipelineStateVersionvalue and run a dedupe/coalesce pass off the draw path at frame boundaries. Either way, P2's acceptance must include a dedicated microbenchmark of the Blaze3D toggle pattern (enable/draw/disable/draw at MC batch rates) on both devices, because that single pattern decides whether §10.2's central claim survives.
- 问题:
- *[major] §5.8.1's blanket reconcile rule adds a per-frame round trip on the IndirectCount path that the monolith does not pay, on a named trace fixture
- 问题:§5.8.1 asserts that "every client-side scan/rewrite in the table above immediately follows
SyncPersistentMappedRange()+SyncGpuWrites()in the monolith" and mandates "publish → wait for appliedSeq → drain events" at each. That is true for the restart rewrite and multi-draw flattening (DirectGLES.cpp:4412-4413,MultiDraw.cpp:498-499,VulkanRenderer.cpp:3431, 4159), but it is NOT true for the*IndirectCountCPU fallback, which §5.8's table also assigns to the client. Verified:MultiDrawElementsIndirectCount(DirectGLES.cpp:4667-4668) calls onlydrawBuffer->SyncPersistentMappedRange(); parameterBuffer->SyncPersistentMappedRange();and then reads the count and the command block straight out ofMappedData()(:4690-4694). There is noSyncGpuWrites()and therefore no stall today.SyncGpuWritesis what triggersReadbackFromGpu(BufferObject.cpp:265-274). If the plan applies its blanket rule here, everyglMultiDrawElementsIndirectCountacquires a publish-and-wait round trip. The trace corpus containsminecraft-1.21.1-neoforge-create-indirect-in-world— a Create/Flywheel fixture whose indirect and parameter buffers are compute-written each frame — so this would be a per-frame, per-batch synchronous round trip on a named acceptance fixture, and the plan's §9.2 #10 dismisses it as "常见情况不 pending,代价为零". - 修法:Replace the blanket rule with a per-site table that reproduces the monolith's reconcile set exactly:
SyncPersistentMappedRangeonly where the monolith calls only that,SyncPersistentMappedRange + SyncGpuWriteswhere the monolith calls both. Add the round-trip counter for the indirect-count path to the P8 acceptance and require it to read zero oncreate-indirect. Separately, note that the monolith's omission ofSyncGpuWritesthere may itself be a latent correctness gap — but that is adevquestion, not something the split should silently fix by adding a stall.
- 问题:§5.8.1 asserts that "every client-side scan/rewrite in the table above immediately follows
- [major] The day-24 GO/NO-GO measures the one subsystem where push's benefit is smallest and its overhead is largest
- 问题:§0.5 and P2's acceptance make the day-24 decision on "monolith-push within monolith-pull's noise on p50 and p99 per-thread CPU" after converting only render state. But render state is the subsystem where push helps LEAST and the plan's CSO design costs MOST: - Espryt already holds a byte-exact value mirror with a version early-out and a span memcmp (
DirectGLES.cpp:2016-2047) — there is almost nothing to save. - Magma already caches the pipeline-state hash under the version (VulkanRenderer.cpp:4982-4993) and gates the dynamic tail twice (:5888-5893). - The CSO overheads identified above (full-block hash on the client, CSO churn on glViewport) land squarely and only on this subsystem. So a GREEN P2 does not validate the claim it gates (that Track H handle-ization pays for itself across 200+ days), and a RED P2 is more likely to indict the CSO design than the push model. Either way the decision the gate is supposed to inform is not the decision it measures. §0.6 also asserts the fallback cost is "only 16 of the 24 days", which understates it: P1's 293-site sed plus the 58 hand-converted non-arrow sites plus the G4/G5 generators are not reusable by the earlier (since-dropped) design. - 修法:Extend the day-24 gate to require both (a) the render-state conversion and (b) one Track H slice — the plan already prices the cheapest ones: 0d handle infrastructure (5-7 days, §6.4) and Magma's
VertexInputStateFactory/VaoDrawMemore-key (2-3 days, §6.5-4, explicitly "低(纯结构性收益)"). That yields a real Track H unit cost, which is what B-R14's re-baselining actually needs. Add an explicit exit criterion that separates "push is slower" from "the CSO design is slower" by running P2 with content addressing disabled (aMOBILEGL_PIPE_PUSHsub-bit) as a negative control.
- 问题:§0.5 and P2's acceptance make the day-24 decision on "monolith-push within monolith-pull's noise on p50 and p99 per-thread CPU" after converting only render state. But render state is the subsystem where push helps LEAST and the plan's CSO design costs MOST: - Espryt already holds a byte-exact value mirror with a version early-out and a span memcmp (
- [major] The interface-purity gate's shared-value-header allowlist is not achievable as written, and the nm gate cannot detect the failure
- 问题:§4.7.2 and §10.3-① define the purity gate as:
MG_Backendmay include only "a shared VALUE header allowlist (RenderStateParametersfrom RenderState.h,SamplerParametersfrom SamplerObject.h,PixelStoreParameters,VertexAttribute, texture/format enums)", plusnm --undefined-only libMobileGLServer.so | grep -E 'MG_State::GLState::|glslang'empty. Verified that the allowlist is not a leaf set:MobileGL/MG_State/GLState/RenderState/RenderState.h:12includesMG_State/GLState/FramebufferState/FramebufferObject.h, which at:12-13includesMG_State/GLState/TextureState/TextureObject.handMG_State/GLState/RenderbufferState/RenderbufferObject.h. The dependency is structural:RenderStateParameterssizes two of its arrays withMG_State::GLState::FramebufferObject::MAX_DRAW_BUFFERS(RenderState.h:263, 273). So shippingRenderStateParametersto a "pure" MG_Backend drags the entire framebuffer/texture/renderbuffer class graph in with it. And the nm gate is blind to this: header inclusion of classes whose members are never called emits no undefined symbols, sonm --undefined-only | grep MG_State::GLState::can be empty while the include graph is fully coupled. The plan prices this cleanup inside P13's 6 days ("MG_Backend 的 MG_State include 收缩到共享值头白名单") as if it were a mechanical trim. - 修法:Make header extraction an explicit P0/P1 deliverable, not a P13 trim: move
MAX_DRAW_BUFFERS,PerBufferBlendState,StencilFaceState,PixelStoreParametersandRenderStateParametersinto a dependency-freeMG_Pipe/MGPipeValueTypes.hthat includes nothing fromMG_State/GLState, and haveRenderState.hinclude that instead. Then replace the nm gate with an INCLUDE-GRAPH gate — compileMG_Backendin the disaggregated configuration withMG_State/GLStateremoved from the include search path (or assert on-Houtput), which is the only check that can actually go red for the reason the gate exists.
- 问题:§4.7.2 and §10.3-① define the purity gate as:
- [minor] draw_vbo's payload construction is priced at parity with today's 3-scalar call, and mandates fields that are currently computed only where needed
- 问题:§10.2's first table row reads "每 verb 的分发: 1 次间接调用 (已经在付) → 1 次间接调用", implying parity. But today's entry is
DrawArrays(GLenum mode, GLint first, GLsizei count)— three scalars in registers (MG_Backend/BackendObject.h:117). The replacement isdraw_vbo(const MGPDrawInfo*, Uint32, const MGPDrawIndirect*, const MGPDrawRange*, Uint), andMGPDrawInfoas specified in §4.5.7 is ~80 bytes (mode, indexSize, flags, pad, instanceCount, startInstance, restartIndex, minIndex, maxIndex, an 8-byte handle, a 32-byteMGHostSpan, and an 8-bytexfbCpuCapturedVertices) plus a 12-byteMGPDrawRange. That is ~90 bytes of stores constructed per draw where there were three register moves. Two of those fields are new work, not just new stores:minIndex/maxIndexcome from an index scan that today runs only for client-memory arrays (TryComputeMaxIndexFromHostBytes,VulkanRenderer.cpp:3407-3470, used at:3599), andxfbCpuCapturedVerticesis aGetTransformFeedbackCapturedVertices()read that today happens only inside the XFB scatter path (DirectGLES.cpp:~900). At MC draw rates this is small but not nothing, and §10.2 accounts for none of it. - 修法:State the payload cost explicitly in §10.2, gate
minIndex/maxIndexandxfbCpuCapturedVerticesbehindMGPDrawInfo::flagsso they are only computed when a consumer asked for them, and add per-draw payload bytes to the P0 counter set (cmd-recordsis per-frame; a per-draw histogram is what sizes SEG_CMD).
- 问题:§10.2's first table row reads "每 verb 的分发: 1 次间接调用 (已经在付) → 1 次间接调用", implying parity. But today's entry is
- [minor] The +50-60 MiB memory figure omits the retention LRU the same document introduces, and that LRU is probably unnecessary
- 问题:§7.11 (formerly the removed comparison table) gives the plan's memory as "transport segments (~48MiB) + POD slot records + an optional bounded ≤32MiB texel-retention LRU ≈ +50-60MiB". The arithmetic does not include the LRU it just described: §8.1's segment defaults are SEG_CMD 8 + SEG_STAGE 32 + SEG_REPLY 8 + SEG_EVENT 0.25 = 48.25 MiB, and
MOBILEGL_PIPE_TEXEL_RETAIN_MBdefaults to 32 (附 B). That is 80 MiB before §8.2's mandated SEG_STAGE growth for the four new byte classes. Separately, the retention LRU appears to be unnecessary.MipmapStoragekeepsVector<Vector<Uint8>> m_data— a complete CPU shadow of every level (MobileGL/MG_State/GLState/TextureState/MipmapStorage.h:117) — so a server-initiated pull (§7.5) can always be serviced from bytes the client already holds. The LRU therefore buys latency, not correctness, and its cost lands on the metric (memory) that §0.4 uses as the plan's strongest argument against the earlier (since-dropped) design in a project whose headline result was saving ~400 MB. - 修法:Correct the arithmetic to 48 MiB + SEG_STAGE headroom + POD records, and default
MOBILEGL_PIPE_TEXEL_RETAIN_MB=0. Turn it on only if §7.5(d)'s measured per-trace pull rate justifies it — which is exactly the discipline §7.5 already commits to for the pull count itself.
- 问题:§7.11 (formerly the removed comparison table) gives the plan's memory as "transport segments (~48MiB) + POD slot records + an optional bounded ≤32MiB texel-retention LRU ≈ +50-60MiB". The arithmetic does not include the LRU it just described: §8.1's segment defaults are SEG_CMD 8 + SEG_STAGE 32 + SEG_REPLY 8 + SEG_EVENT 0.25 = 48.25 MiB, and
- [minor] §9.1's "glGetTexImage = 0 round trips on DirectGLES" does not survive the plan's own generated-mipmap ownership split
- 问题:§9.1 claims zero round trips for
glGetTexImage/glGetTextureImageon DirectGLES because the client shadow answers. Verified that MG_Impl routes to the backend only when the backend is DirectVulkan (MobileGL/MG_Impl/GLImpl/Texture/GL_Texture.cpp:6453-6459), otherwise callingCopyTextureImageToClientOrPBO_State. But §5.8's row for generated mipmaps splits ownership: "client 分配 level 存储 … server 生成". A GPU-generated mip level therefore has allocated-but-empty client storage.CopyTextureImageToClientOrPBO_Statewill happily answer from that empty shadow. The plan's answer ison_mip_levels_generated(§7.1), but that callback as specified carries only{res, base, count}— no texels — so it can only mark the levels as needing a pull, which converts the query into a blocking round trip (the same class as §9.2 #9), or the design must instead eagerly write back every generated level (potentially megabytes perglGenerateMipmapon an atlas). The plan never says which, and §9.1 books it as zero. - 修法:Decide explicitly in §5.8/§7.2 between eager
on_texture_writebackof generated levels and lazy pull-on-query, and move the DirectGLESglGetTexImagerow from §9.1 (zero) to §9.2 (conditional blocking) with the condition named. Add the generated-level case toTextureRemintPullScenarioso the chosen path has a gate.
- 问题:§9.1 claims zero round trips for
- [minor] Two smaller round-trip accountings are optimistic: map_persistent is per-respecify not per-object-lifetime, and MGHostSpan is not free
- 问题:(a) §9.2 #8 prices
map_persistentunder tier T1 as "每 store 生命桥期一次,不是每次使用". But storage respecification re-mints the store, and the plan's own P3a acceptance listsStorageBufferRegrowScenario.TryAdoptLargeStoragefires at storage-definition time, so a buffer that grows N times costs N blocking round trips, not one. For a workload that grows chunk arenas during world load this is a burst of stalls at exactly the moment the user perceives them. (b) §4.5.7 states "monolith 代价为零(一次指针加载)" forMGHostSpan. It is a 32-byte struct embedded in everyMGPDrawInfoand read throughMGPipeHostByteswhich the same section describes as "一次分支,每次使用解析一次". That is a branch plus 32 bytes of payload on every draw record, whether or not the draw uses host bytes — which for VBO-based workloads (all of MC/Sodium) is every draw. - 修法:(a) Reword §9.2 #8 to "once per storage definition" and add a
map-persistent-roundtripscounter to the P0/P11 counter set, withStorageBufferRegrowScenariopublishing it. (b) Reword §4.5.7's cost line to "one predictable branch plus 32 bytes on the draw record", and consider movinguserIndicesout ofMGPDrawInfointo thekHostSpanvar-tail so draws that carry no host bytes do not pay for the field.
- 问题:(a) §9.2 #8 prices
已验证的优点:
- Push at draw-validate time rather than at GL-setter time (推论 1 / §5.1) is the right call and is directly supported by the tree:
RenderState::SetCapabilityshort-circuits redundant sets (RenderState.cpp:311-313) but a real enable/disable pair does bump the version, andDirectGLES.cpp:2029-2032names the Blaze3D per-batch blend toggle as the hottest path. A per-setter push would have turned that into an interface call plus a server CSO lookup per toggle. The plan identifies this as its most-likely-to-be-implemented-wrong decision and writes it as a spec clause (B-R15). - The A/B/C/D/E read classification (§2.3) and the conclusion that the interface must push VALUES not invalidation is correct and load-bearing. Verified: Magma keeps no render-state mirror and rebuilds its payload from ~40 direct field reads on a pipeline miss (
VulkanRenderer.cpp:5155-5200region) while Espryt keeps a byte mirror and diffs it (DirectGLES.cpp:1956,:2035-2047). A bump-a-version-and-let-the-server-pull interface would indeed regress to today's model. MOBILEGL_PIPE_VERIFY(§10.3-②) is a genuine semantic gate that exists only because the interface lands in the monolith first, and the plan is right to require FIELD-WISE comparison rather than memcmp —DirectGLES.cpp:2029-2032documents that aRenderStateParametersmemcmp can false-DIFFER on padding but never false-match, so a byte comparer would produce false positives in the verify harness. This is the specific defect prior candidate designs were judged on, and it is answered.- D-B5 is honest about the cost: the plan states plainly that the earlier byte-identity monolith gate dies by construction and puts the loss in the design document rather than hiding it. Verified that no configuration can preserve it — the backend stops reading
pGLContext, memos re-key, and MG_Impl gains validate calls. - Keeping
resource_subdatacarrying BOTH the union box and the rect list with the shape decision server-side (§4.5.6, §7.3) correctly preserves a measured hardware cliff.MipmapStorage.h:60-83documents the 96-slot rationale and the ~100-sprites/frame Minecraft pattern that motivated it; putting the decision on the side that pays the GPU cost is the right call. - PBO readback becoming fire-and-forget (§9.1) is strictly better than the monolith, verified:
DirectGLES.cpp:9191-9204maps the pack PBO withGL_MAP_READ_BITand copies back synchronously insideReadPixels, which stalls on the read regardless of whether the application ever touches the PBO. LikewiseglFinish/glFlushare genuine no-ops today (MG_Impl/GLImpl/Exporting/Definitions.cpp:111-112), so the requirement that they stay free is achievable rather than aspirational. - Per-backend optionality as a first-class interface property (§4.4.4, B-R9) is faithful to the existing contract:
BackendObject.h:212-215and:265-269already document null table entries as "not implemented, frontend falls back", DirectVulkan already leaves 8 entries null, and Magma's deliberate omission ofResidentSubData(VkBufferManager.cpp:104-111) is preserved rather than papered over. Choosing a function-pointer struct over a virtual base is correctly justified by this, not by dispatch cost. - The composite pipeline-program answer (§5.6.3) is correct and cost-free:
GLContext::GetProgramForDraw(Core.cpp:592) already resolves and links the composite entirely frontend-side, so the client pushes one handle and the blockingJoinLinkAndSpirv()leaves the server draw path. This closes the objection that killed the prior thin-server design without adding machinery. - P0 landing per-frame byte and call counters BEFORE any migration, and clearing the uncommitted per-draw
fprintfinstrumentation first, is the right sequencing — the tree genuinely has no per-frame byte or call metrics today, so every ring size, batching threshold and wire-granularity decision would otherwise be a guess. - The identity model is sound where it matters: verified that the ABA hazards the re-key table addresses are real and documented in-tree (
TwinLookupMemo's owner-equality atDirectGLES.cpp:83-90exists precisely because a recycled heap address would otherwise hit a memo slot), and that a dense{slot, gen}array index genuinely replaces a Fibonacci-hashed probe plus twoowner_beforecalls that touch a control block — a real per-draw win on three lookups per draw.
改造可行性与估时(refuted=False,13 条)
- [major] Stage-A snapshot is filled at 2 sites, but 48 of 70 backend entry points read pGLContext outside them
- 问题:§6.2.1 and §11 P1 place
SnapshotFromGLContext()at exactly two points: the top ofPrepareForDraw(DirectGLES.cpp:2916) andSetupDraw(VulkanRenderer.cpp:6371). §5.1's tracker has exactly four validate entry points (ValidateForDraw/Dispatch/Clear/BlitOrCopy). Both are far too few. Of the 70 distinctgBackendFunctionsTable.GL.*entries reached from MG_Impl (89 call sites), 48 are neither draw nor dispatch, and many read pGLContext on their own:UpdateTextureBindingAtTargetreadsGetActiveTextureUnit()/GetTextureUnitObject()at DirectGLES.cpp:6051-6052 and is reached from CopyTexImage2D/CopyTexSubImage2D;GenerateMipmapreads them at :6876-6877;GetTexImageat :9254-9257;BlitFramebufferreads both FBO slots at :5988-5989;ClearreadsGetRenderStateParameters().ClearColorat :4106 and the draw FBO at :4165; the readback family reads pack state at :6129/:7614/:9101/:9480 and the pack PBO at :7622/:8604/:8834/:9144/:9570; DSA-by-name reads at :4038-4043 and :7417-7418. The code says so explicitly: the comment at DirectGLES.cpp:1501-1502 states the no-argCaptureDrawTextureSyncKeyswrappers exist "for every non-draw call site (Clear, readbacks)". The G5 poison mask does not save this: it fires only on a field that was NEVER filled; a field filled by an earlier draw reads STALE, not poisoned. - 修法:Enumerate a validate/fill hook per non-draw backend entry class (texture-op, readback, blit, clear, xfb-span, query, DSA-by-name) in
PipeCalls.defalongside the verbs, and make G5's written-once bitmask assert per CALL rather than per draw (a field written by draw N must not satisfy the read in the glTexSubImage that follows it). Alternatively makePipeInputsaccessors lazily filled with a per-call fill generation. Until this is fixed P1's acceptance criterion ("40 traces green under MOBILEGL_PIPE_VERIFY") is unreachable, and §11's day-16 milestone should not be scheduled against the two-site design.
- 问题:§6.2.1 and §11 P1 place
- [major] Pushing texture resource_subdata at GL-call time destroys the dirty-rect coalescing the plan's own +6 ms/frame evidence rests on
- 问题:§5.1 states the rule "only resource mutations push at GL-call time — which is exactly what BufferBackendOps does today". That is true for buffers and false for textures.
glTexSubImage*never calls the backend table at all: MG_Impl/GLImpl/Texture/GL_Texture.cpp:1817, :1937, :2004 only callMarkStorageDirtyRegion. Espryt coalesces the ACCUMULATED region at sync time (Managers.cpp:4274-4311), where MipmapStorage's 96-rect cascade merge and thesummedArea*4 >= unionArea*3union-box fallback run, and then deliberately collapses the rect list to one box when the unpack ring is live (if (BufferImpl::UnpackRingAvailable()) dirtyRectCount = 0;, :4321) with the in-tree measurement "~100 sprite rects become ~100 jobs ... measured +6 ms/frame of GPU time in MC's animated-atlas ticks. One box, one job." Emitting oneresource_subdataper glTexSubImage call reproduces exactly the ~100-job shape. §7.3 gestures at a deferred "emission cursor" but never resolves the contradiction with §5.1, and §5.1 is the section an implementer will follow because it is written as the design's most emphatic rule. - 修法:Amend §5.1 to say the GL-call-time rule applies only to the ops that already dispatch at GL-call time today (the seven BufferBackendOps hooks). State that texture subdata is accumulated in the client's existing MipmapStorage rect model and emitted at the next validate/flush point, so the merge heuristic keeps running before anything crosses the interface. Add a MOBILEGL_PIPE_STATS counter for
resource_subdataemits per frame with an explicit ceiling on the MC animated-atlas fixture.
- 问题:§5.1 states the rule "only resource mutations push at GL-call time — which is exactly what BufferBackendOps does today". That is true for buffers and false for textures.
- [major] Sub-rect texture upload is gated on pointer identity and whole-level stride arithmetic that no MGPBlobRef can satisfy in split mode
- 问题:§5.4 prices subsystem 5's repack family as "unchanged in place, only the input changes from a pulled shadow pointer to an MGPBlobRef (the same pointer in monolith)". The code does not permit that. Managers.cpp:4278-4283 gates the whole sub-rect path on
uploadData == mipData— literally "the upload source IS the whole level shadow" — and :4288-4293 computesregionPtr = uploadData + z*levelSliceBytes + y*levelRowBytes + x*bpp, striding into the FULL level with UNPACK_ROW_LENGTH;rectShadowPtr(:4321-4326) does the same per rect. The comment at :4270-4273 says conversion fallbacks "rewrite the whole level into a fresh buffer, so they stay on the full-level path" — i.e. the moment the source is not the level shadow, sub-rect upload is disabled by design. In split mode the client can stage (a) the whole level every time, which destroys the bandwidth benefit and contradicts §0.4's "零副本 / +50-60MiB" headline claim, (b) tightly-packed regions, which makesuploadData == mipDatafalse and silently forces full-level uploads, or (c) nothing — requiring a server-side whole-level mirror, which IS the duplicated MipmapStorage the plan's strongest argument against the earlier (since-dropped) design says it avoids. §4.5.6's "carry both box and rect list, server picks the shape" does not address the stride source at all. - 修法:Redefine MGPSubData so each region carries {dstBox, srcRowStride, srcSliceStride, blob} and rework Managers.cpp:4274-4326 to take a strided-source descriptor instead of comparing pointers, so the server can set UNPACK_ROW_LENGTH from the descriptor over a tightly-packed staged region. Move this out of "原地不动" and into subsystem 5's day estimate, and add a Mali-device gate that publishes the box-vs-rect job count and frame-time delta at P3b/P4b exit — the plan already names this as B-R5's cliff but assigns it no work.
- 问题:§5.4 prices subsystem 5's repack family as "unchanged in place, only the input changes from a pulled shadow pointer to an MGPBlobRef (the same pointer in monolith)". The code does not permit that. Managers.cpp:4278-4283 gates the whole sub-rect path on
- [major] The XFB scatter path is a read-modify-write of the client's buffer shadow, and MGPipeCallbacks has no buffer pull
- 问题:§7.2 assigns all 8
WritebackFromBackendsites toMGPReplySlot(readback) pluson_buffer_writeback(XFB capture, PBO readback) — all one-way server→client. ButScatterCapturedRecords(DirectGLES.cpp:928) doesMemcpy(staged.data(), target.buffer->MappedData() + target.start, rangeBytes): it STARTS from the application's existing bytes so that the holesgl_SkipComponentsasks for keep whatever the application had put there (the comment at :891-895 says this is "the whole point of the feature"), patches only the captured varyings in, then writes back and re-uploads. The server has noMappedData(), and §7.1's callback table hason_texture_pull_requestbut no buffer equivalent. As specified the scatter either zero-fills the skip holes — a conformance break; DirectGLES.cpp:882-883 namesKHR-GL46.transform_feedback.capture_special_interleaved_testas the case that reaches this path — or needs an unnamed synchronous reverse buffer read at glEndTransformFeedback, a stall class the plan's §9.2 roundtrip table does not list. - 修法:Move the scatter to the client: the server pushes the packed scratch bytes via
on_buffer_writeback, and the client — which owns the destination shadow and already hasGetTransformFeedbackVaryings()/GetTransformFeedbackStride()/GetTransformFeedbackPackedStride()from the reflection archive — performs the patch and re-emits the range as an ordinaryresource_subdata. If the scatter must stay server-side, add an explicitresource_read_host(res, off, size)reverse request to §7.1 and price its stall in §9.2 next to the texture pull.
- 问题:§7.2 assigns all 8
- [major] The unit-bindings debouncer is deleted while its dirty signal is replaced by the very counter it exists to filter
- 问题:§2.5, §10.4-1, and §4.7.3 D3/D9 book ~115 lines at DirectGLES.cpp:1372-1489 as deleted because "the push call IS the change signal". But the comment at DirectGLES.cpp:1412-1421 states why
CurrentUnitBindingsEpochexists:GetTextureBindGeneration()bumps on REDUNDANT re-binds (26.2 re-binds the same sampler around every texture-unit switch), so the counter is untrustworthy and the epoch is built to "move exactly when WHAT is bound changes, never on a redundant re-bind". §5.2 then namesGetTextureBindGeneration()as a dirty-bit input for NEW_SAMPLER_VIEWS. The tracker therefore re-emitsset_sampler_viewson every redundant re-bind, and D9's replacement (viewSetSerialbumped by the server insideset_sampler_views) invalidates the server's resolved-binding and sampler-pass memos on every batch — a per-batch regression on the exact workload the project optimises for, concealed inside a claimed 115-line deletion.set_sampler_viewsis a kVarTailset_*, not a CSO, so §4.2.3's "content addressing gives N=0 for repeated state" does not cover it; the same holds forset_shader_imagesandset_shader_buffers. - 修法:State that the debounce MOVES to the client rather than disappearing: the tracker must hash the resolved view/image/buffer sets and suppress the emit on an unchanged hash (
MGPFramebufferState::contentHashalready demonstrates the pattern — extend it to the other var-tail set_* calls and use it client-side as an emit suppressor, not only as the server's memo key). Re-charge ~115 lines to MG_Impl/Pipe/Tracker.cpp and correct §10.2's per-draw arithmetic and §10.4's deletion count accordingly.
- 问题:§2.5, §10.4-1, and §4.7.3 D3/D9 book ~115 lines at DirectGLES.cpp:1372-1489 as deleted because "the push call IS the change signal". But the comment at DirectGLES.cpp:1412-1421 states why
- [major] Multi-draw cannot be split by a static screen cap: tier selection is per-batch and depends on backend-only program facts
- 问题:§5.8 assigns "CPU tier on the client (!kCapMultiDraw); compute tier stays server-side".
ResolveTierForBatch(MultiDraw.cpp:282-320) chooses among five tiers PER BATCH usingprogramReadsDrawID— a property of the transpiled ESSL, which exists only on the server — plusperSubDrawBaseVertexand the batch's index totals againstkMaxFlattenedIndices(MultiDraw.cpp:72, 1<<24) andkMaxComputeFlattenedIndices(:82). The auto ladder is Ext → BaseVertex → MultiIndirect → Indirect → DrawElements (:241-243), so the CPU-flattenDrawElementstier is a FALLBACK reached only after the batched tiers decline for reasons the client cannot evaluate. A client that flattens whenever!kCapMultiDrawbypasses the BaseVertex and compute tiers; a client that does not flatten leaves the server-side fallback with no index bytes in split mode.kCapMultiDraw*as a lowering-ownership switch is therefore not expressible. - 修法:Keep all five tiers server-side. Carry what they need through the interface instead:
draw_vbo(info, indirect, MGPDrawRange[], numDraws)plus akCapNeedsHostIndexBytes-gatedMGHostSpanfor the index data, with the server deciding the tier. DeletekCapMultiDraw/kCapMultiDrawIndirect/kCapMultiDrawIndirectCountfrom §5.8's ownership table and replace them with a single rule: the server always owns multi-draw tiering; the client supplies index bytes when the caps say the server may need them.
- 问题:§5.8 assigns "CPU tier on the client (!kCapMultiDraw); compute tier stays server-side".
- [major] on_texture_pull_request can park a twin forever: there is no negative completion
- 问题:§7.5(b) says the server marks the twin not-ready and the client re-emits on its next publish, and §9.2-9 says the resulting stall lands on mgl-srv-apply. But the client may have nothing to send.
RequireImageBindableStorage(Managers.cpp:2789-2822) re-dirties every level of every upload target, and the replay reads the shadow — while :2810-2812 already skips levels whoseGetMipmapByteSize(...)is 0, and a level whose content came from rendering, from aglCopyTexSubImageinto a shapeCanMirrorCopyImageShadowdeclines (DirectGLES.cpp:7068-7073), or from a GPU-side mip generation has no client bytes at all. With no negative completion the apply thread blocks on a twin that never becomes ready. B-R4 and theTextureRemintPullScenariogate address the RATE of pulls, never the unanswerable pull. - 修法:Make the pull a request/response pair terminated by an explicit
resource_subdata_complete(res, target, firstLevel, levelCount)that may carry zero regions, and specify that the server proceeds with allocated-and-empty storage on an empty answer (matching today's monolith behaviour) with a logged diagnostic. Add the unanswerable case — a texture whose only content came from rendering, then image-bound — to TextureRemintPullScenario, and require the scenario to be red before the terminator lands.
- 问题:§7.5(b) says the server marks the twin not-ready and the client re-emits on its next publish, and §9.2-9 says the resulting stall lands on mgl-srv-apply. But the client may have nothing to send.
- [major] MOBILEGL_PIPE_VERIFY is the plan's only semantic gate, and P13 deletes the code that produces its reference
- 问题:§13.3-② calls the per-draw per-field shadow compare "the decisive one" and §0.4 D-B5 makes it the whole justification for abandoning the earlier byte-identity monolith gate. Verify computes its reference by calling
SnapshotFromGLContext()(§6.2.1 stage B). §6.7 and §11 P13 then say: "delete SnapshotFromGLContext(), the MGB_CTX macro, MOBILEGL_PIPE_PUSH ... KEEP the MOBILEGL_PIPE_VERIFY harness for later work." With the snapshot gone, verify has nothing to compare against; after P13 the design has no semantic tripwire at all. Open question 11 half-acknowledges the same hole for split-only diagnosis ("the plan's server has no MG_Impl, so a split-only rendering bug has no second opinion") without connecting it to the loss of verify. - 修法:Decide this before P0 freezes the gate list, because it changes what P13's purity gate may assert. Either keep SnapshotFromGLContext() compiled only under MOBILEGL_PIPE_VERIFY past P13 and scope the purity gate's
grep -c 'pGLContext' MG_Backend/to the non-verify build, or replace it at P13 with the recorded-golden mode the plan already sketches at §10.4-9: turn MG_Test's mock backend into an MGPipe recorder, capture pushed state per draw on a set of fixtures, and diff future builds against the stored trace.
- 问题:§13.3-② calls the per-draw per-field shadow compare "the decisive one" and §0.4 D-B5 makes it the whole justification for abandoning the earlier byte-identity monolith gate. Verify computes its reference by calling
- [minor] Texture parameters are modelled only on sampler-view CSOs, but they are per-texture-object state that non-sampled textures still need
- 问题:§4.7.1 maps the "TexParam / SamplerParam" delta class (9 read points) entirely onto
create_sampler_view(base/max level, swizzle, dsMode) pluscreate_sampler_state. But Espryt callsSyncTextureParamsToBackendfor every touched unit binding AND every draw-FBO attachment texture (DirectGLES.cpp:1548-1560 for the unit list, :1580-1601 for the attachment list), andRequireImageBindableStoragesetsm_forceTextureParamsResyncprecisely because a channel-widened carrier needs a swizzle override the frontend params version never moves (Managers.cpp:2815-2821). A texture that is only an FBO attachment, only an image-unit binding, or only aglCopyImageSubDataendpoint has no sampler view, so under §4.7.1 itsglTexParameterstate has no carrier across the interface. - 修法:Put base/max level, swizzle, depth-stencil mode and the LOD clamps on
MGPResourceDescor a dedicatedset_texture_params(res, ...)call, and letMGPSamplerViewcarry only the view restriction (min/num level, min/num layer, alias format). This also keepsglTextureViewmodellable as what it actually is — a real texture object with its own parameters that can itself be an FBO attachment and a glTexSubImage destination (TextureObjectView.cpp:281, :290) — rather than the "ordinary view CSO" §4.5.4 reduces it to.
- 问题:§4.7.1 maps the "TexParam / SamplerParam" delta class (9 read points) entirely onto
- [minor] The client's per-(texture, uploadTarget, level) emission cursor aliases across glTextureView and its storage owner
- 问题:§7.3 inverts dirty ownership and gives the client a cursor keyed on
(texture, uploadTarget, level)that it clears on emit. ButTextureObjectViewforwardsIsStorageDirty,MapMipmapDataandGetStorageDirtyRegionto the storage OWNER's mipmap with index remapping (TextureObjectView.cpp:290-322, and :281 writes into the owner's data). A view and its owner therefore share one underlying dirty state while carrying two independent cursors: whichever emits first clears the flag the other still needed, or both emit the same texels. The plan's own §4.7.3-D18 discipline about not "optimising" a documented hazard away applies here too, but the aliasing is never mentioned. - 修法:Key the emission cursor on
(storageOwner, ownerUploadTarget, ownerLevel)— resolve throughGetViewStorageOwner()and the view'sToOwnerUploadTarget()/ToOwnerLevel()mapping before consulting or clearing. Add a scenario that uploads through a view and samples through the owner (and the reverse) across a draw boundary.
- 问题:§7.3 inverts dirty ownership and gives the client a cursor keyed on
- [minor] The OOM-ack story names entry points that never reach the backend
- 问题:§7.4 and §9.2-7 mark "glRenderbufferStorage*, the failure-capable forms of glTexImage*/glTexStorage*/glCopyTexImage*, and glBufferStorage" as kNeedsAck so the OOM-probe idiom works. The texture family never calls the backend table at all: MG_Impl/GLImpl/Texture/GL_Texture.cpp only calls
MarkStorageDirty(..., true)at :2515, :2671, :2755, and Espryt allocates lazily at sync time.RecordGLError(DirectGLES.cpp:6309-6324) — the texture-side error reporter — has exactly one caller, glGenerateMipmap at :6916. Even the one genuine synchronous allocation,glRenderbufferStorage*, runs its OOM check insideBackendRenderbufferObject::SyncToBackend(Managers.cpp:8674-8684), i.e. also lazily. So kNeedsAck as specified has no producer for the texture family, and the renderbuffer case would need a forced sync at the GL call to be ackable at all. - 修法:Enumerate the actual synchronous allocation points rather than the GL entry points that look like them. State plainly that texture allocation OOM is already deferred to sync time in the monolith so the split changes nothing observable, and restrict kNeedsAck to the one case that can be made synchronous (renderbuffer storage, if forced to sync at the GL call) plus glBufferStorage. Otherwise §9.2-7's "rare and already expensive, so the ack is nearly free" is pricing a mechanism that does not fire.
- 问题:§7.4 and §9.2-7 mark "glRenderbufferStorage*, the failure-capable forms of glTexImage*/glTexStorage*/glCopyTexImage*, and glBufferStorage" as kNeedsAck so the OOM-probe idiom works. The texture family never calls the backend table at all: MG_Impl/GLImpl/Texture/GL_Texture.cpp only calls
- [minor] SEG_STAGE sizing omits the largest single-call payload the plan itself moves to the client
- 问题:§8.2 lists four new byte classes for SEG_STAGE (client vertex arrays, client index arrays, multi-draw argument blocks, client-resolved indirect command blocks) and claims "byte volume unchanged — they are re-uploaded per draw today". The whole-EBO primitive-restart rewrite that §5.8 moves to the client is not among them, and it is bounded at
kMaxRestartRewriteBytes = SizeT{1} << 26— 64 MiB (DirectGLES.cpp:4218) — twice the defaultMOBILEGL_IPC_STAGE_MB=32in Appendix B. Unlike client vertex arrays these bytes are not re-uploaded per draw today: the rewrite lands in a backend scratch buffer the driver keeps. The multi-draw flattened index stream (kMaxFlattenedIndices = 1<<24 indices, MultiDraw.cpp:72) is in the same class. - 修法:Add the restart-rewrite blob and the multi-draw flattened index stream to §8.2's list, size SEG_STAGE against them or specify the grow/decline path for a single record larger than the segment, and keep the ceiling check with its
m_valid=falsedecline and MGLOG_E_ONCE on the client (DirectGLES.cpp:4401-4409) so the diagnostic still fires on the thread that issued the draw.
- 问题:§8.2 lists four new byte classes for SEG_STAGE (client vertex arrays, client index arrays, multi-draw argument blocks, client-resolved indirect command blocks) and claims "byte volume unchanged — they are re-uploaded per draw today". The whole-EBO primitive-restart rewrite that §5.8 moves to the client is not among them, and it is bounded at
- [minor] The fixed validate order puts set_shader_images after set_draw_program, contradicting D-B3's own argument
- 问题:§5.3's order is 1 framebuffer, 2 program, 3 sampler views / images / buffers / global constants, 4 render state, 5 vertex. D-B3 (§0.5) and §5.3 both claim the fixed order is what retires
ImageUnitFormatsStillMatch(Managers.cpp:6545-6573, whose comment says it is "not expressible as a monotone version") by telling the server the image formats before the program build — but images are pushed at step 3, after the program at step 2. It only works because D-B2 defers specialization to draw time. And once specialization is deferred todraw_vbo, the framebuffer-before-program ordering argument carries no weight either: what actually retires the fragColor-broadcast workaround at DirectGLES.cpp:2712-2732 is LATE specialization, not call order. An implementer who takes §5.3 literally will build ordering assumptions the design does not need and does not honour. - 修法:Replace the numbered order with the invariant that actually holds: all set_* for a command complete before the verb, and the server specializes the shader at the verb from whatever has been pushed. Then §5.3's list is a convenience, and D-B3's claim should be restated as "late specialization plus complete state at the verb" rather than "framebuffer strictly first".
- 问题:§5.3's order is 1 framebuffer, 2 program, 3 sampler views / images / buffers / global constants, 4 render state, 5 vertex. D-B3 (§0.5) and §5.3 both claim the fixed order is what retires
已验证的优点:
- The dead-capability finding is real and independently verified: CapabilityInput::FramebufferSrgb and DepthClamp exist as enum values (RenderState.h:165, :168) but SetCapability falls to
default: // not supported currently(RenderState.cpp:380) and IsCapabilityEnabled returns false at thedefault:arm (:428-429). All six backend consumers therefore read a constant false today. §10.4-6 is right to demand an answer before the render-state blob is frozen; writing the interface down genuinely surfaced this. - The dirty-ownership inversion (§7.3) is sound and rests on a fact I verified:
grep -rn 'IsStorageDirty|GetStorageDirtyRects|GetStorageDirtyRegion' MG_Impl/returns exactly 0 hits — the frontend never reads its own texture dirty state, only sets and clears it. Deleting PLAN.md §5.6a's ack protocol and risk R6 is therefore justified. - The backend-memo-writeback asymmetry is exactly as claimed: DirectGLES writes zero Set*Memo calls into frontend objects (0 grep hits under MG_Backend/DirectGLES/), while DirectVulkan writes four — ProgramFactory.cpp:3448 and VertexInputStateFactory.cpp:60/78/83, with :78 storing a raw backend-heap pointer (
vao.SetBackendStateMemo(&entry, m_evictionEpoch)). D12's verdict of "delete outright, do not translate" is the right call and the D13 VaoDrawMemo replacement really does already exist. - D21 is a genuine latent bug, verified:
VulkanRenderer::CurrentXfbCounterSlot(VulkanRenderer.cpp:11136-11146) keysm_xfbCounterSlotByObjectonGetBoundTransformFeedbackName()— a raw, LIFO-recycled GL name with no generation — so a deleted-and-regenerated XFB object inherits the predecessor's counter slot. Landing this ondevindependently at P0 is correct sequencing. - The composite-pipeline-program answer ("nothing to do") is correct. GLContext::GetProgramForDraw (Core.cpp:592-660) already performs the whole flattening frontend-side, including both J1 join sites,
ComputeDrawProgramSignature(), andMakeShared<ProgramObject>(0u)at :644 with the in-code rationale "deliberately not a named program ... backend registries key on the object, not the name". Deleting PLAN.md's proposedSetReplicaResolvedDrawProgramhook is justified, and this answers the prior judges' "unpriced composite" objection. - Moving the CopyImage shadow mirror to the client is correct and does delete a whole reverse byte channel.
MirrorCopyImageIntoDestinationShadow(DirectGLES.cpp:7085-7148) is a pure shadow→shadow row memcpy whose eligibility (CanMirrorCopyImageShadow, :7068-7073 — single upload target, not 1D-array) and whose bounds/texel-size checks are all decidable from frontend data alone, and it deliberately does not mark dirty. RecProgramLinkOpreally is impossible, not merely undesirable: ProgramObject.h:11 includes ShaderObject.h, which at :12 includes ShaderCompileTask.h and at :145 returnsconst SharedPtr<glslang::TShader>&; ProgramObject.h:14 pulls SpvcSession.h. Collapsing PLAN.md's two program tiers to one, deleting phase P5, and promotingnm -D | grep glslangto a P7 acceptance criterion all follow correctly.- §2.4's catalogue of the 58 non-arrow
pGLContextuses is a real gap no prior design caught, and DirectGLES.cpp:146 (MG_State::GLState::GLContext* ctx = MG_State::pGLContext.get();) is verified as sed-invisible. The adjacentusing FbBindingSlot = std::remove_reference_t<decltype(MG_State::pGLContext->GetFramebufferBindingSlot(...))>at :142 is a second wrinkle in the same family. Making the purity gate greppGLContextrather thanpGLContext->is the right response. - The interface-purity gate (§4.7.2) is a genuinely stronger completeness argument than the prior branch's 477-row read inventory: making
MG_State::pGLContextundeclared in the MGPipe build turns every unsatisfied read into a named compile error rather than a catalogue entry that can go stale. Keeping the inventory only as a G6 coverage checklist is the right demotion. - Carrying the CPU-modelled XFB vertex count on MGPDrawInfo is correct on the point I expected to be wrong:
AccountTransformFeedbackPrimitives(mode, count)runs BEFORE the backend draw call (GL_Drawing.cpp:1132-1133, :1140-1141), so the value pushed with a draw already includes that draw's contribution. - The function-pointer-struct-not-vtable decision (§4.1) is well grounded in this codebase: the boundary already is a function-pointer struct installed at one hook point, null entries already mean "not implemented, frontend falls back", and that is the natural expression of a partially migrated subsystem during the strangler. A pure-virtual class would need stub overrides that lie.
- D18 being the single identity row marked UNCHANGED — the deliberate node-based
std::unordered_mapfor VkTextureManager/VkRenderPassManager resources, with the BlitFramebuffer "layout undefined" postmortem carried verbatim into the review checklist — is exactly the right instinct for a refactor of this size, and B-R8 names the failure mode (someone "optimising" it back) correctly. - The plan is honest about the two things that most threaten it: D-B5 states in the open that the earlier byte-identity monolith gate dies by construction and is a cost of this design, and B-R2 states that the central performance claim (the reachability traversal moves rather than doubles) is unmeasured and that the tree has no per-frame byte or call metric today. Landing TracyPlot counters and clearing the working-tree per-draw fprintf in P0, before any migration, is the correct ordering.
性能(refuted=False,14 条)
- [major] Program reflection payload cannot be decoded without linking glslang — the plan's own enforcement gate is unreachable and the fix is unbudgeted
- 问题:§4.5.5 defines MGPProgramDesc.reflection as "Visit() 归档的 LinkArtifacts + SpirvArtifacts(全结构体)", and §5.7/§11-P7 make
nm -D libMobileGLServer.so | grep glslangempty the "整个论点的强制执行点". But all five payload types are declared INSIDE ProgramObject.h: TypeFacts at MG_State/GLState/ProgramState/ProgramObject.h:44, ResourceReflection :76, XfbVarying :1146, LinkArtifacts :1210, SpirvArtifacts :1409. ProgramObject.h:11 includes ShaderObject.h (which exposesSharedPtr<glslang::TShader>at ShaderObject.h:146 and at :12 includes ShaderCompileTask.h, which itself pulls MG_Util/Async/JobNode.h, MG_Util/ShaderTranspiler/CompileEnv.h and MG_State/GLState/BufferState/BufferState.h), and ProgramObject.h:14 includes MG_Util/ShaderTranspiler/SpvcSession.h, which at :11 includes spirv_reflect.h. The server must have the definitions of LinkArtifacts/SpirvArtifacts to deserialize into, so it must include the exact header the gate forbids. ProgramObject.h is 1803 lines with 10 in-tree includers. The plan never budgets this extraction in any phase, and open question 5 concedes the MG_Util/MG_State seam "没有审计过" — while P7 acceptance depends on it. - 修法:Insert an explicit phase (before P4a, ~5-8 days) that extracts TypeFacts/ResourceReflection/XfbVarying/LinkArtifacts/SpirvArtifacts into a standalone MG_State/GLState/ProgramState/ProgramArtifacts.h with no ShaderObject.h/SpvcSession.h dependency, update the 10 includers, and add a CI assert that ProgramArtifacts.h's transitive include closure contains no glslang, no SPIRV-Cross and no spirv_reflect header. Only then is
nm -D | grep glslanga gate rather than a wish.
- 问题:§4.5.5 defines MGPProgramDesc.reflection as "Visit() 归档的 LinkArtifacts + SpirvArtifacts(全结构体)", and §5.7/§11-P7 make
- [major] Per-draw named-uniform-block bytes have no MGPipe call — the "all 26 reverse pulls disappear" claim is false and SEG_STAGE is under-sized
- 问题:§7.2 asserts the 20 SyncPersistentMappedRange sites "作为反向调用彻底消失" because "每一处都紧挨着一次对客户端字节的 CPU 读,而那些读全部搬到了 client(§5.8)". Verified counter-example: UniformManager::ResolveUniformBufferPayload calls bufferObject->SyncPersistentMappedRange() at MG_Backend/DirectVulkan/Renderer/UniformManager.cpp:2022 and then reads
outData = bufferObject->MappedData() + rangeStartat :2052 (with a zero-padding copy at :2053-2057) to pack the block into Magma's own UBO ring — a per-draw read whose consumer is server-side, so it cannot move to the client. §5.8's ownership table does not list it; §4.4.3 and 附A define set_shader_buffers(cls, start, count, const MGPBufferRange*, writableMask) with flags V only, no kHasBlob and no MGHostSpan. §5.7/D6's set_global_constants covers only the DEFAULT uniform block (SpirvArtifacts::globalUboScratch), not named blocks. So every Iris/MC draw with a named UBO has an uncarried data dependency, and §8.2's SEG_STAGE sizing list (client vertex arrays, client index arrays, multi-draw args, resolved indirect blocks) omits it. - 修法:Either (a) add kHasBlob/MGHostSpan to set_shader_buffers for cls==Uniform and price the per-draw byte volume with the P0 counters before freezing the payload, or (b) land a separate dev PR making Magma descriptor-bind the resident VkBuffer range instead of ring-packing it, with its own perf gate on the Iris traces. Then re-audit all 26 sites individually (they are 20+6 and enumerable) and publish the per-site disposition rather than a blanket claim.
- 问题:§7.2 asserts the 20 SyncPersistentMappedRange sites "作为反向调用彻底消失" because "每一处都紧挨着一次对客户端字节的 CPU 读,而那些读全部搬到了 client(§5.8)". Verified counter-example: UniformManager::ResolveUniformBufferPayload calls bufferObject->SyncPersistentMappedRange() at MG_Backend/DirectVulkan/Renderer/UniformManager.cpp:2022 and then reads
- [major] Phase days contradict the plan's own per-subsystem tables; P3a's re-baseline checkpoint fires by construction
- 问题:§11-P3a is "slot 基建、buffer、VAO(12 天)" and its deliverable list is exactly §6.4 rows 0b (handle infra, 5-7 d), 2 (buffer + 7 BufferBackendOps, 10-13 d) and 3 (VAO/vertex elements, 7-9 d) = 22-29 days. The phase then declares "⚠ 再基线检查点 1:若 P3a 超期 >50%(>18 天)… 必须重定基线" — i.e. the plan's own subsystem table already predicts the checkpoint trips. Same shape at P4a: 16 days for §6.4 row 4 (7-9) plus the identity halves of rows 5 (20-26) and 6 (14-18). P7 is stated 48-85 against §6.5's own total of 85-111, and B-R14 admits "P7 的 48 天下界明显低于同口径的 85-111" yet the headline 199-236/200-260 still uses 48. Espryt subsystem 7 (XFB, 5-7 d) has no phase home at all — it appears only in P9's split acceptance list. Summing §6.4 (89-120) + §6.5 (85-111) + shared infra + the 51 days of IPC phases (P5 12 + P6 5 + P9 10 + P10 6 + P11 8 + P12 10) gives ~245-310 excluding CTS, versus the advertised 200-260 including IPC.
- 修法:Rebuild §11's day column by summing §6.4/§6.5 rows per phase rather than assigning budgets independently; publish the arithmetic. Set P3a's checkpoint at the subsystem-derived number (e.g. >36 days) and give Espryt XFB an explicit phase. Restate the headline as ~245-310 person-days excluding CTS turnaround, or split P3a into P3a-i (handle infra) / P3a-ii (buffer) / P3a-iii (VAO) so each has a checkpoint that can actually fire early.
- [major] The verify harness — the plan's decisive replacement for the byte gate — is structurally blind in the subsystem the plan calls most dangerous
- 问题:§10.3-② and §6.2.1 stage B make MOBILEGL_PIPE_VERIFY (tracker fills a second PipeInputs via SnapshotFromGLContext, G4 compares field-wise per draw) the mechanism that "在语义上严格强于任何符号 diff" and the answer to every prior review. But §7.3 inverts texture dirty ownership: the client keeps the MipmapStorage rect model, maintains a per-(texture, uploadTarget, level) emission cursor, and "在发射后清自己的标志". Once the client has cleared the flags, a from-scratch snapshot recompute cannot reconstruct the dirty rect set, so the comparator has no independent second opinion for resource_subdata payloads — precisely subsystem 5, which §6.4 and B-R5 both single out as "全表最危险" because of the measured +6 ms/frame box-vs-rects cliff (Managers.cpp:4311-4319) and the 7 fallback-repack paths whose eligibility test requires uploadData == mipData. The same blindness applies to any group where the push path consumes-and-clears rather than reads.
- 修法:Add a verify-only retention mode: under MOBILEGL_PIPE_VERIFY the tracker keeps the pre-clear dirty set for the draw and G4 compares emitted (box, rectCount, rects[]) against a snapshot recompute. Additionally record the pull-mode upload shape per texture per frame into a golden and compare it in a TextureUploadShapeScenario, so the +6 ms cliff is gated by shape equality, not only by SSIM.
- [major] After stage C the MOBILEGL_PIPE_PUSH knob is no longer an A/B against the old backend, and the plan claims otherwise
- 问题:§6.7 states "任何一次提交都能在同一份二进制上按子系统 A/B" and "设备回归可以二分到'哪个子系统'", and §12-B-R1/B-R3 lean on this as the migration-risk mitigation. But stage C (§6.2.1) changes the PipeInputs field TYPE from SharedPtr to MGPipeHandle + POD descriptor, rekeys the backend memos to {slot,gen}, and (P3a) replaces the six StateBackendObjectRegistry hash tables (Managers.h:270-390, instances at :806/:1123/:1216/:1731/:1830/:1858) with slot arrays while deleting TwinLookupMemo x3 and OwnerEquals. With the bit cleared, SnapshotFromGLContext must still synthesise the handle from the client slot map and the backend still executes the rekeyed memo code — so both arms run the same new code. A rekeying bug (exactly the D1/D2/D3/D11/D13 hazard class the plan is trying to close) is present in both arms and cannot be bisected by the knob. The plan never states this narrowing.
- 修法:State in §6.7 that the bitmask A/B is scoped to stage-B value fields. For P3a and P4a add a second, compile-time switch (e.g. MOBILEGL_PIPE_LEGACY_MEMOS) that keeps the registry/TwinLookupMemo implementations alive behind the same PipeInputs surface, so the first two handle waves retain a true old-vs-new arm on device; retire it at P13 with the pull path.
- [major] P2's day-24 GO/NO-GO measures the one face where the pull model is already nearly free, so a green result does not de-risk the central claim
- 问题:§0.6 and §11-P2 make day 24 the GO/NO-GO for "可达性遍历是搬走了而不是翻倍", on monolith-push per-thread CPU after only render state, pack state, patch state and attrib defaults have moved. But Espryt's render-state pull already early-outs on a single Uint16 compare before ever touching the block: DirectGLES.cpp:2007 reads GetRenderStateParametersVersion(), :2016-2018 returns when it matches g_syncedRenderStateVersion, and only then is GetRenderStateParameters() read at :2021 and the three-span memcmp run at :2042-2047. The tracker replaces that with an xxHash over the same ~1.2 KB plus a 64-entry CSO LRU probe — roughly neutral for Espryt, a clear win for Magma (~55 reads), and in neither case representative. The costs the claim actually rests on are the ones P2 does not move and that become NEW client work at P3a/P4a: the touched-unit sampler walk over Array<TextureUnit,192> (TextureState.h:41,128), the 84-per-target buffer binding-point walk, the 32-attribute VAO walk, and the per-texture content/params version reads. §3's own table concedes "这是主张,不是测量".
- 修法:Move one object-valued group into the GO/NO-GO — set_sampler_views over the GetMaxTouchedUnit prefix is the cheapest honest candidate — and measure that. Otherwise relabel day 24 as "mechanism proven, zero product risk" and place the real GO/NO-GO at the P3a exit, where the first Track-H walk exists; adjust B-R1's "退回the earlier (since-dropped) design 只损失 16 天" accordingly (it becomes ~36 days).
- [major] "Zero new bookkeeping in MG_State" and "one 64-bit dirty word test" cannot both hold for object-valued groups; the mutator-enumeration obligation plan A had is not deleted, only renamed
- 问题:§5.2 promises the dirty bits come entirely from existing counters with "MG_State 零新增记账"; §5.1 and §10.2 price steady state at "一次 64 位 dirty word 测试 + N 次 set_". For NEW_SAMPLER_VIEWS the listed sources are per-object and per-slot — ITextureObject::GetContentVersion/GetShapeVersion/GetTextureParamsVersion plus GetTextureBindGeneration()/GetSamplingResolutionGeneration() — and there is no aggregate covering "did any bound texture's content move". That is exactly why Magma resorts to the lossy sampledContentSum/sampledParamsSum (VulkanRenderer.h:975-1000). So the tracker must either walk the touched units at every validate (not O(1), and it is new client work the backend's ResolvedTextureBindingMemo currently skips), or add aggregate generations to TextureState (new bookkeeping), or set dirty bits from every MG_Impl mutator entry point — MobileGL implements desktop GL 4.6 and MG_Impl/GLImpl alone references 181 distinct gl names. §0.4-4 claims plan A's "第七个面" and gen_impl_mutation_surface.py vanish because there is no replica to replay into; but plan A enumerated MG_Impl mutations to REPLAY them and plan B must enumerate them to MARK them dirty. The generator is deleted; the enumeration is not, and no phase budgets it. B-R6 names the risk but its three mitigations (written-once bitmap, poison, verify) all detect omissions, none enumerate the surface.
- 修法:Decide per group and write it down: for value groups use the existing counter; for object groups either add an explicit aggregate generation to TextureState/BufferState/VertexArrayState (and price it as MG_State work), or keep gen_impl_mutation_surface.py in a repurposed form that enumerates the MG_Impl mutators which must set each MGPIPE_NEW_* bit and fails CI on an unmapped mutator. Then correct §10.2's steady-state cost row to show the per-group walk that survives.
- [minor] P1's byte-identity acceptance is contradicted by P1's own deliverables
- 问题:§11-P1 acceptance: "pull 构建里 nm --defined-only + 剥调试信息 .text size 与替换前完全一致——本阶段可证明是一次替换(这是最后一次这条等式成立)". But P1's deliverables include the §2.4 conversion list, of which the ~22 real null guards generate code: 7
if (MG_State::pGLContext)(e.g. Managers.cpp:3608, verified: the guard wraps three assignments in BackendTextureObject::StampViewSyncKeys), 14!= nullptrand 1== nullptr. Deleting or unconditionalising those changes .text in RelWithDebInfo. Only the 34 MOBILEGL_ASSERT sites are genuinely free — Defines.h:114 defines the macro as empty outside debug builds (verified). P1 also installs SnapshotFromGLContext() at the top of PrepareForDraw (DirectGLES.cpp:2916) and SetupDraw (VulkanRenderer.cpp:6371) with no stated #if guard, which adds a call in the pull build. - 修法:Guard SnapshotFromGLContext and the G4/G5 machinery behind MOBILEGL_PIPE_PUSH/_VERIFY/debug, defer the null-guard and ternary rewrites to P2 (where the fields are genuinely always-valid), and restate P1's acceptance as "nm --defined-only unchanged; .text within N bytes with the delta attributable line-by-line" rather than exact equality.
- 问题:§11-P1 acceptance: "pull 构建里 nm --defined-only + 剥调试信息 .text size 与替换前完全一致——本阶段可证明是一次替换(这是最后一次这条等式成立)". But P1's deliverables include the §2.4 conversion list, of which the ~22 real null guards generate code: 7
- [minor] P1 snapshots only at the two draw-prepare sites, but a large share of the pull reads are in non-draw verbs — the poison mask will Fatal on the first glGenerateMipmap/glReadPixels
- 问题:§11-P1 places SnapshotFromGLContext() at PrepareForDraw and SetupDraw only, while arming G5's poison mask so that reading an unfilled field is Fatal{UnmigratedPipeInput} "发生在第一个 draw 上", and then requires "全部 40 个 trace 与 367 个集成测试在 MOBILEGL_PIPE_VERIFY=1 下零分歧". Verified non-draw reads that would be unfilled: DirectGLES.cpp:6051-6052 (GetActiveTextureUnit + GetTextureUnitObject inside the GenerateMipmap path), :6129 and :7614 (GetPixelStoreParameters(false) in readback paths), :6643-6644, :6738-6739, :6876-6877 (texture verbs resolving the active unit), :6319 (RecordError). §5.1 does declare ValidateForClear/ValidateForBlitOrCopy/ValidateForDispatch, but P1's deliverable list does not enumerate them or the texture/readback verbs.
- 修法:Make the per-verb snapshot points an explicit P1 deliverable derived from PipeCalls.def: generate, per kCtxVerb/kCtxObject call, the set of PipeInputs fields it may read, and emit the snapshot/validate call at each of the ~89 MG_Impl boundary sites accordingly. This also converts G5 from "catches an omission at some draw" into "catches it at the specific verb that needed it".
- [minor] §4.5.7 and §5.8 disagree on where primitive-restart rewrite and indirect-count resolve live; either answer moves the A/B baseline a second time
- 问题:§4.5.7's MGHostSpan consumer table says for restart rewrite / multi-draw flattening: "monolith 填法: ptr 指向 shadow" (server does it) / "split 填法: 暂存,或 client 已重写". §5.8's ownership table says client, gated on !kCapPrimitiveRestart. Both backends actually perform the rewrite — DirectGLES.cpp:4283 RewriteRestartIndices, :4377 ScopedRestartIndexSubstitution, whole-EBO bounded by kMaxRestartRewriteBytes = 1<<26 at :4218; VulkanRenderer.cpp:3990/:4089/:4161 — so the cap is false on both and the client always does it, i.e. a monolith behaviour change scheduled at P8 (day ~97-111), long after §10.3-③'s name-for-name integration baseline was taken at P2. If instead it is split-only, monolith and split run different implementations of a whole-buffer correctness-critical transform and the name-for-name gate compares two different programs. Open question 12 flags the diagnostic-thread change but not the baseline problem.
- 修法:Choose client-side unconditionally, land it as an independent dev PR before P2 together with the decline-diagnostic relocation (resolving open question 12), so the monolith baseline moves exactly once and before any comparison is taken. Delete the conflicting row from §4.5.7's table.
- [minor] set_sampler_views/bind_sampler_states import a per-stage slot space that MobileGL's state model does not have
- 问题:§4.4.3 defines set_sampler_views(stage, start, count, const MGPBoundView*) and bind_sampler_states(stage, start, count, const MGPipeHandle*). Verified model: TextureState::m_textureUnits is Array<TextureUnit, MAX_TEXTURE_IMAGE_UNITS> with MAX_TEXTURE_IMAGE_UNITS = 192 (TextureState.h:41, :128) — one COMBINED unit space, with the per-stage limit only an advertised number (:42). TextureUnit holds Array<BindingSlot, TextureTargetCount> plus a single sampler (TextureUnit.h:20, :24-25). The same combined unit can be sampled by two stages, and both backends bind by combined unit (g_boundTexturesCache[192][TargetCount]). A stage parameter forces the client either to duplicate views under each stage or to invent a stage attribution GL does not define, and it adds a dimension the server must collapse again.
- 修法:Drop the stage parameter from both calls and address the combined unit space directly — which is also what LinkArtifacts::uniformSamplerOrImageUnitIndex already yields for the client-side resolution described in §5.5. Keep stage only where the target API genuinely needs it (Magma's descriptor stage flags), derived server-side from the reflection archive.
- [minor] The monolith benefit is argued on ~550 deleted lines with no accounting of the code added
- 问题:§2.5, §3's comparison table and §10.4-1 lead the monolith case with "~550 行 per-draw 失效发现机制删除". Nowhere does the plan estimate the permanent additions: PipeCalls.def plus six generators (G1-G6), MG_Impl/Pipe/{Tracker, SlotAllocator, CsoCache, HostResolve, CompositeResolver}, MG_Pipe/{MGPipeTypes, MGPipeHandles, MGPipeCallbacks, MGPipeHostSpan}, MG_Backend/MGPipe/{PipeInputs, two impl files}, plus MG_Remote's emitter and PipeApplier/PipeObjectTables. For a ~72-call interface with ~14 POD payloads across two backends that is plainly an order of magnitude more than 550 lines, all permanently maintained, and it is added to a codebase where MG_Backend is already 68k lines and MG_Impl 37k.
- 修法:Publish a net-LOC estimate and, more importantly, a net per-draw instruction/cache-line estimate next to the deletion list, and make §10.3-④'s per-thread CPU number — not the deletion count — the stated monolith case. This also gives B-R2 a falsifiable prediction rather than a qualitative claim.
- [minor] A block of SamplerObject.h citations point at lines that do not exist in the file
- 问题:The document header asserts "全部 file:line 引用针对工作树 dev@81b17c0b". MG_State/GLState/SamplerState/SamplerObject.h is 160 lines at
81b17c0b(identical at HEAD): BorderColorForm is at :66-70 and struct SamplerParameters at :72-96. But §4.5.4 cites ":468-492" for SamplerParameters, ":462-466" for BorderColorForm and ":455-461" for its rationale; §5.2 cites ":532, 551" for GetVersion/m_version; §4.2.1 cites ":533-537" for GetLifetimeId. All are past end-of-file. The substance is correct and is in the file (borderColorForm is mandatory because all three representations are always populated, :60-66; BumpVersion also bumps the context-wide sampling-resolution generation, :152-158), so this is an inherited transcription error rather than an invented fact — but the plan is meant to be an implementation spec, and every other citation I sampled was exact (293 arrow / 58 non-arrow pGLContext, 89 gBackendFunctionsTable.GL. sites, 40 pActiveBackendObject-> sites, 354/709 MG_State:: mentions, 50 include lines over 18 headers, DirectGLES.cpp:2035 static_assert, :2042-2047 three-span memcmp, RenderState.h:363/:369/:522/:529 all verified). - 修法:Re-verify the SamplerObject.h block and anything else inherited from the same reader report before P0 freezes MGPipeTypes.h, and add a cheap CI lint that every file:line in docs/Disaggregated/*.md resolves to a line that exists at the referenced baseline.
- 问题:The document header asserts "全部 file:line 引用针对工作树 dev@81b17c0b". MG_State/GLState/SamplerState/SamplerObject.h is 160 lines at
- [minor] The day-64 "first inproc IPC frame" milestone is unfalsifiable as specified
- 问题:§11-P5 delivers InProcessTransport and claims the milestone "★ 第 64 天 — 首个 IPC 帧(inproc)", honestly flagged as a reduced path. But nothing in §11-P5 or §8.1 says whether inproc goes through the same G3-generated encode/decode as spawn or short-circuits it. If it passes PipeInputs by pointer inside one address space, the subsystems not yet handle-ified at P5 (Espryt XFB, which has no phase at all; readback beyond the single blocking read_pixels) keep working via SharedPtr and the milestone proves nothing about wire completeness — while P6 (spawn, day 69) would then discover the gap five days later, on the critical path.
- 修法:Specify that InProcessTransport uses the identical G3 serialization and differs only in the doorbell/copy mechanism, and add a debug assertion in PipeApplier that no SharedPtr or raw frontend pointer crosses the applier boundary in any transport. Then day 64 and day 69 differ only by process boundary, which is what the milestone is meant to assert.
已验证的优点:
- The pull-surface accounting is exact and better than every prior design's. Verified at dev@81b17c0b: 293
pGLContext->occurrences and 58 lines using pGLContext without the arrow, with the plan's §2.4 breakdown reproducing precisely (34 MOBILEGL_ASSERT truth tests, 14!= nullptr, 7if (, 1== nullptr, 1.get()at DirectGLES.cpp:146, 1 comment at VertexInputStateFactory.h:133). Identifying the.get()capture as invisible to sed, and specifying that the purity gate grepspGLContextrather thanpGLContext->, closes a real hole the three earlier candidate designs all left open. - The function-pointer-table-over-vtable decision is correctly argued from this codebase rather than from gallium. Verified: GLFunctionsTable + GlobalBackendFunctionsTable contain 69 function pointers (BackendObject.h:117-285), reached from 89
gBackendFunctionsTable.GL.sites and 40pActiveBackendObject->sites in MG_Impl, installed at the single hook point MG_Backend/Init.cpp, and null entries already mean "not implemented, frontend falls back" (documented at BackendObject.h:212-215, 265-269). A nullset_*is a native expression of "this subsystem is not migrated"; a pure-virtual class would need stub overrides that lie. - D-B1 (ship RenderStateParameters as one blob, not three gallium CSOs) is grounded in verified in-tree evidence rather than preference:
static_assert(std::is_trivially_copyable_v<RenderStateParameters>)at DirectGLES.cpp:2035, the head/blend/tail memcmp at :2042-2047 keyed on offsetof(...,BlendStates)/offsetof(...,LogicOp), and the load-bearing field placement of ScissorBoxWrittenMask (RenderState.h:363) and ClipDistanceEnabledMask (:369). Carrying both m_version (:522) and m_pipelineStateVersion (:529) on the wire is likewise correct and correctly justified by the glViewport-evicts-pipeline-memo regression recorded at :523-528. - The texture dirty-ownership inversion rests on a fact I confirmed independently: MG_Impl contains zero
IsStorageDirty(,GetStorageDirtyRects(andGetStorageDirtyRegion(call sites while callingMarkStorageDirty(14 times. Deleting plan A's §5.6a ack protocol and risk R6 on that basis is sound, and keeping the box-vs-rects upload-shape decision server-side (MGPSubData carrying both payloads) correctly leaves the choice on the side that paid for the +6 ms/frame measurement at Managers.cpp:4311-4319. - D-B4 — leave AcquirePersistentMap completely untouched through the entire monolith refactor and isolate it to the IPC step behind a week-one POST spike — is the right structural call. It is already an explicit call returning a pointer (BufferObject.h), so it genuinely passes through unchanged, and refusing to let one platform unknown gate ~200 days of interface work is exactly the right sequencing judgement.
- The two backend-internal MG_State usages that the previous review round priced at zero are correctly identified and costed. Verified: UniformManager::MakePlaceholderTextureObject at UniformManager.cpp:161-181 with the real construction at :1417-1424, :1479-1496 (including SetSamples(2) for VUID-RuntimeSpirv-samples-08726 and TruncateMipmapLevels at :1496) and :1620; and the two internal shaders at VulkanRenderer.cpp:4211 and :4287 building MakeShared<MG_State::GLState::ShaderObject> (:4214, :4222, :4290, :4300), a ProgramObject (:4230) and calling Link(false) (:4233). Preferring checked-in SPIR-V guarded by an in-tree-glslang byte-compare MG_Test over a host-tool build step is the right trade for this repo's four build lanes.
- VertexInputStateFactory's backend-heap-pointer write-back into the frontend VAO is correctly classified D12 "delete, do not translate", and D18 (VkRenderPassManager/VkTextureManager's deliberate node-based std::unordered_map) is correctly the single UNCHANGED row with a mandate to carry its postmortem comment verbatim into the P7 review checklist. Naming the one thing a large refactor must not "optimise back" is exactly the discipline these reviews usually find missing.
- The milestone labelling is honest where a weaker plan would have overclaimed: P5/P6 are explicitly marked 缩减路径 with emulation Fatal in split until P8; §3 concedes plan A wins first-frame time by 4-5x; D-B5 states outright that the byte-identity gate dies by construction and calls it a cost that must be written down rather than hidden; and §9.3 refuses a blanket zero-round-trip claim in favour of published per-trace-case round-trip and texture-pull counters.
- The design surfaced two genuine in-tree defects as by-products and routed them correctly: D21, m_xfbCounterSlotByObject keyed on the raw GL name (VulkanRenderer.cpp:11136-11146), so a deleted-and-regenerated XFB object resumes a capture that should restart — scheduled as an independent dev PR in P0; and the dead CapabilityInput::FramebufferSrgb/DepthClamp with no storage (RenderState.cpp:380, :428-429) feeding six constant-false backend reads, correctly made a blocking question before the render-state blob is frozen.
- Ordering the strangler so framebuffer precedes textures and programs (D-B3, §6.6 step 4) is right and well-evidenced: the four cross-object masks are derived from attachment formats at Managers.cpp:5616-5619 and consumed by the render-state push (DirectGLES.cpp:2014) and the program staleness test (:2769-2770), and inlining internalFormat into MGPSurface lets them be derived at push time with no lookup — which genuinely retires the fragColor re-derivation workaround at :2712-2732 rather than porting it.
3. 综合稿的关键决定
- Wrote 5 files (part2 split into 2a/2b): part1=§0-3, part2a=§4, part2b=§5-6, part3=§7-10, part4=§11-14+附. Single title in part1 only; §0-§14+附 headings in required order; each file ~35-49KB UTF-8 ≈ 12-16K Chinese chars, well under the cap.
- Base = winning Design 3 (split-first) phase plan, grafted with Design 2's twin-derived interface derivation (SetupDrawSnapshot / IsDrawSyncClean / ResolvedDrawBuffers / g_syncedRenderStateParameters / BufferBackendOps as the source of the call catalogue), its PipeCalls.def six-generator toolchain, its two-kinds-of-generation split (client identity vs 12 server-only MGGen epochs), its D18-UNCHANGED node-container discipline, and its MGHostSpan; plus Design 1's caps-gated emulation-homing rule, its numbered gallium-deviation ledger, and MGPipeCallbacks as a named struct.
- Resolved Design 1's fatal flaw: render state ships as ONE versioned blob behind a content-addressed CSO handle (create_render_state(blob) + bind_render_state 12B, client 64-entry LRU keyed on the three existing memcmp spans), never decomposed into blend/depth-stencil/rasterizer CSOs — cited RenderState.h:359-368 (field order load-bearing), DirectGLES.cpp:2035 static_assert + :2042-2047 three-span memcmp, and the :523-528 two-counter regression.
- Resolved Design 2's fatal flaw: MGPipeHandle is {slot:Uint32, gen:Uint32} with CLIENT-ALLOCATED DENSE PER-KIND SLOTS (not a sparse 64-bit lifetimeId), which is what actually turns the 6 StateBackendObjectRegistry hash tables and 13 Magma caches into arrays; GetLifetimeId() stays client-side as the tracker's own identity; 2^32 slot-reuse wrap documented and asserted.
- Re-measured every contested count against the working tree rather than inheriting any report: GLFunctionsTable = 67 function pointers + 1 Bool (BackendObject.h:117-278), 69 fps with GlobalBackendFunctionsTable (not 73 or 71); 293 pGLContext-> occurrences over 290 lines + 58 non-arrow lines; 50 MG_State include lines over 18 distinct headers; 95 backend->frontend mutator sites over 17 methods; 7 BufferBackendOps hooks; 89 MG_Impl table sites + 40 pActiveBackendObject->; 1494 MG_Impl pGLContext->; 367 TEST_F / 428 TEST( / 40 trace cases at SSIM 0.99; PLAN.md phases sum to exactly 77 days.
- Closed the shared migration gap all three designs missed: the 58 non-arrow pGLContext uses (≈40 MOBILEGL_ASSERT truth tests, ~10 null guards, 3 patch-param ternaries, the DirectGLES.cpp:146 .get() raw capture that sed cannot catch, 2 != nullptr conditions, 1 comment) are enumerated by form in §2.4, made an explicit P1 deliverable, and the purity gate greps 'pGLContext' not 'pGLContext->'.
- Hardened the residual value block (the split-first accelerant): per-member offsetof static_asserts in addition to sizeof, AND field-wise serialization in split mode instead of a bulk memcpy — because the monolith verify harness cannot see a layout mismatch when both sides are the same TU; retirement is a compile error via static_assert(sizeof(ResidualValueBlock)==0) at P13.
- Priced the schedule honestly: 200-260 engineer-days (single track 199-236, P7/Magma 48-85), first inproc IPC frame day 64 and first cross-process frame day 69 — both explicitly labelled REDUCED PATH (emulations Fatal in split until P8, full function at day 111) — against PLAN.md's verified 77 days and day-15 cross-process frame; added TWO re-baseline checkpoints (P3a overrun >50%, P7 midpoint <40% complete) and priced CTS turnaround (~56,271 cases) as a separate tiered-gating line, not folded into phase estimates.
- Stated D-B5 as an explicit cost in the TL;DR: PLAN.md's byte-identity monolith gate dies by construction, replaced by a five-part gate (purity grep+nm, per-draw field-wise MOBILEGL_PIPE_VERIFY shadow-compare, behavioural A/B across {monolith-pull, monolith-push, split}, per-thread-CPU non-regression, coverage+poison+handle-recycle asserts) with two surviving nm equalities kept as assertions and .text drift published as informational.
- Kept the texture re-mint pull as a named NEW stall class with all three mitigations shipping together (imageBindableHint pre-emption, asynchronous park-and-re-emit so the stall lands on mgl-srv-apply not the app thread, bounded 32MiB retention LRU), a dedicated TextureRemintPullScenario, and a per-trace-case pull counter that is PUBLISHED rather than asserted to zero.
- Corrected PLAN.md §7.4 with evidence: backend program link/compile failure is surfaced ONLY as MGLOG_E plus a bind-program-0 no-op (Managers.cpp:8091-8126, 8357-8372, rationale at :7098/:7247-7249/:6478/:7827), so on_log must split by severity — <=WARN lossy, >=ERROR lossless with a per-second rate limiter emitting 'N errors suppressed' — with a log-flood fault-injection gate.
- Quarantined AcquirePersistentMap from the refactor entirely (it is already an explicit pointer-returning call and survives P0-P13 untouched; only IPC breaks it), deferring it to PLAN.md §6.8's three POST-probed tiers with spike B in week one, so no platform unknown blocks 200 days of interface work.
- Inherited PLAN.md §6-§13 essentially verbatim with a per-section table in §8.1 (no re-derivation), and listed every delete/change/add against it in §8.2 and §14.1 — including that the copy account drops to 3/2 (PLAN.md's own 'the plan' target) and inproc isolation drops from four process globals to two, which makes PLAN.md's earliest falsification gate cheap.
4. 修订记录(综合稿 v1 → 定稿 v2)
- [stage-A fill sites] Verified only ~22 of the 70 table entries MG_Impl uses are draw/dispatch; confirmed non-draw entries read pGLContext themselves (DirectGLES.cpp:6051-6052 GenerateMipmap path, :6129 pack state, :4106/:4165 Clear, :5988-5989 Blit, :1501-1502 comment). Replaced the 2-site SnapshotFromGLContext with G5-generated per-verb-class fill/validate points at the ~93 MG_Impl boundary sites; Tracker grows from 4 to 8 validate entries (§5.1, §6.2.1, P1).
- [poison granularity] Upgraded G5's written-once bitmask to a per-verb generation (m_filledGen[f] == m_currentVerbSerial, sticky fields listed explicitly), so a field filled by draw N no longer satisfies the read in the following glTexSubImage; poison now fires on the verb that needed it (§6.2.2).
- [texture push timing] Verified glTexSubImage* never calls the backend table (GL_Texture.cpp has 3 MarkStorageDirtyRegion sites only) and that Espryt coalesces at sync time with the union-box collapse at Managers.cpp:4386-4390 (+6 ms/frame). Rewrote 推论 1 and added §5.1.1: the GL-call-time push rule applies only to the seven BufferBackendOps hooks; texture subdata accumulates in the client's rect model and is emitted as one resource_subdata at the next validate/flush point, with a per-frame emit counter and an MC animated-atlas ceiling.
- [sub-rect upload] Verified the
uploadData == mipDatagate (Managers.cpp:4278-4283) and whole-level stride arithmetic (:4288-4293, :4321-4326), and that the unpack-ring path already uses a strided source descriptor (UnpackStagingBlock, :4340-4390, tightly repacked). Redefined MGPSubData to carry MGPSubRegion{dstBox, srcRowStride, srcSliceStride, srcOffset} plus sourceIsVerbatimLevelShadow, reworked Managers.cpp:4274-4326 to read strides from the descriptor, moved this out of 原地不动 and priced it into Espryt subsystem 5 (+3-4 days). - [XFB scatter] Verified ScatterCapturedRecords does a read-modify-write of the client shadow (DirectGLES.cpp:928, rationale :889-892, case KHR-GL46.transform_feedback.capture_special_interleaved_test). Moved the scatter to the client: server pushes packed scratch bytes via on_buffer_writeback + new on_xfb_scatter_ready{packedStride, vertices}; client patches and re-emits an ordinary resource_subdata. No new reverse read is introduced (§7.2.1).
- [unit-bindings debouncer] Confirmed GetTextureBindGeneration bumps on redundant re-binds (DirectGLES.cpp:1414-1420). Reclassified the ~115 lines from 'deleted' to 'relocated': the debounce becomes a client-side resolved-set xxHash emit suppressor (m_lastSetHash[]) covering every kVarTail set_*, and D9's viewSetSerial now has that as an explicit precondition. §2.5 split into ~372 lines truly deleted vs ~175 relocated; §3, §10.2 and §10.4 ledgers corrected.
- [multi-draw / restart ownership] Verified ResolveTierForBatch (MultiDraw.cpp:282-320) selects per batch using programReadsDrawID (a server-only ESSL fact) and that both backends perform the restart rewrite. Deleted kCapPrimitiveRestart/kCapPrimitiveRestartFixedIndex/kCapMultiDraw/kCapMultiDrawIndirect/kCapMultiDrawIndirectCount as ownership switches (D-B7); all five tiers and the restart rewrite stay server-side, fed in split mode by a new incrementally-maintained Server/IndexHostMirror gated on kCapNeedsHostIndexBytes (budgeted, counted, with a per-draw shipping fallback). Resolves the §4.5.7-vs-§5.8 contradiction and closes open question 12.
- [texture pull terminator] Added resource_subdata_complete(res, target, firstLevel, levelCount, pullSerial) which may carry zero regions; server proceeds with allocated-and-empty storage (matching monolith EnsureGenerateMipmapStorageAllocated at DirectGLES.cpp:6270-6271) plus a logged diagnostic. TextureRemintPullScenario must include the unanswerable case (render-only texture later image-bound) and be red before the terminator lands (§7.5e, P9).
- [verify survives P13] SnapshotFromGLContext and its MG_State includes are now kept behind #if MOBILEGL_PIPE_VERIFY past P13; the three purity gates run only on the non-verify build; P13 additionally delivers the MGPipe recorder golden mode as a long-term MG_State-free semantic gate and as the answer to open question 11 (D-B5, B-R17).
- [texture params] Verified SyncTextureParamsToBackend runs for FBO attachment textures (DirectGLES.cpp:1580-1601) and that RequireImageBindableStorage sets m_forceTextureParamsResync (Managers.cpp:2815-2821). Added set_texture_params(res, ...) carrying base/max level, swizzle, depth-stencil mode, LOD clamps and forceResync; MGPSamplerView reduced to view restriction only (new gallium deviation D10, plus a gate for attachment-only / image-only / CopyImage-endpoint textures).
- [emission cursor aliasing] Verified TextureObjectView forwards IsStorageDirty/MapMipmapData/MarkStorageDirty(Region)/GetStorageDirtyRegion to the storage owner with index remapping (TextureObjectView.cpp:281, 290-322). Keyed the client emission cursor on (storageOwnerHandle, ownerUploadTarget, ownerLevel) and added a view/owner aliasing scenario.
- [OOM ack] Verified the texture family never reaches the backend table and that even glRenderbufferStorage allocates lazily in SyncToBackend (Managers.cpp:8674-8684). Narrowed kNeedsAck to glBufferStorage plus, conditionally, glRenderbufferStorage*; P0 must answer whether the corpus actually contains a glRenderbufferStorage OOM probe. Stated plainly that texture allocation OOM is already deferred in the monolith so the split changes nothing observable (§7.4, §9.2-7).
- [SEG_STAGE sizing] Rewrote the new-byte-class list to six items including named-UBO host payloads and tightly repacked texture regions; removed the 64 MiB restart rewrite and the multi-draw flattened stream from SEG_STAGE entirely (they are served by the index host mirror), and required G3 to define a chunking/degradation path for a single record larger than the segment (§8.2, open question 9).
- [validate order] Replaced the numbered order contract with the invariant 'all set_* for a command complete before the verb; the server specializes at the verb'. D-B3 restated: what retires the fragColor workaround and ImageUnitFormatsStillMatch is late specialization, not framebuffer-first ordering (§5.3, D-B3).
- [reflection payload / glslang gate] Verified TypeFacts/ResourceReflection/XfbVarying/LinkArtifacts/SpirvArtifacts all live in ProgramObject.h, which includes ShaderObject.h (glslang) and SpvcSession.h (spirv_reflect), with 7 in-tree includers. Added a new prerequisite phase P0.5 that extracts them into ProgramArtifacts.h with a CI include-closure assertion, without which P7's
nm -D | grep glslangcriterion is unreachable (§0.4, §4.5.5, P0.5). - [named UBO bytes] Verified UniformManager::ResolveUniformBufferPayload syncs at UniformManager.cpp:2022 and reads MappedData()+rangeStart at :2052 into Magma's own UBO ring - a server-side consumer that cannot move. Added an optional MGHostSpan payload to set_shader_buffers(cls==Uniform) gated by a new kCapNeedsHostUboBytes, plus a stage-ubo-named counter, and forbade freezing the payload shape before P0 gives byte volumes (D-B8, §5.7, §7.2).
- [phase arithmetic] Rebuilt every phase day count as the sum of the §6.4/§6.5 rows it contains and published the arithmetic; total changed from 200-260 to 267-337 person-days excluding CTS turnaround; milestones moved to days 25 / 43 / 99 / 104 / 145 / 187 / 267; re-baseline checkpoints set at the summed upper bound +50% (P3a >27d, P4a >39d); Espryt XFB given an explicit phase home in P3b/P4b (§11.5, B-R14).
- [verify blind spot] Added a verify-only retention mode: under MOBILEGL_PIPE_VERIFY the tracker keeps the pre-clear dirty set and G4 compares the emitted (unionBox, regionCount, regions[]) against a snapshot recompute; added TextureUploadShapeScenario recording upload shape and job count as a golden, because SSIM is insensitive to the +6 ms/frame box-vs-rect cliff (§7.3, §10.3-②, P3b/P4b).
- [stage-C A/B narrowing] Stated in §6.7 that MOBILEGL_PIPE_PUSH stops being an old-vs-new arm after stage C (both arms run the rekeyed memo code), and added a compile-time MOBILEGL_PIPE_LEGACY_MEMOS switch keeping the registry/TwinLookupMemo implementations alive through P3a/P4a, retired with the pull path at P13 (+1 day per phase, costed; new risk B-R16).
- [GO/NO-GO scope] Extended P2 to include one Track H slice per backend (Espryt 0b handle infrastructure, Magma subsystem 4) plus a Blaze3D blend-toggle microbenchmark and a CSO-content-addressing negative control, so day 43 measures the decision it gates; fallback cost restated honestly as 28-39 days rather than 16 (§0.6, P2, B-R1).
- [dirty marking vs polling] Verified no aggregate exists for 'did any bound texture's content move' (which is why Magma uses lossy sampledContentSum/sampledParamsSum). Added 推论 4: value groups keep the polling model with zero new bookkeeping; object groups get 5 new aggregate generations in MG_State (~20 lines at existing bump points), and gen_impl_mutation_surface.py is repurposed as gen_pipe_dirty_surface.py enumerating MG_Impl mutators to aggregate generations with a CI failure on any unmapped mutator (§0.3, §5.2, §10.3-⑤, B-R6 layer 4).
- [P1 byte identity] Verified MOBILEGL_ASSERT compiles away outside debug (Defines.h:114) but that the 7 null guards, 14 != nullptr conditions and 3 ternaries do generate code. Deferred those rewrites to P2, guarded SnapshotFromGLContext/G4/G5 behind build switches, and restated P1's acceptance as 'nm unchanged; .text delta attributable line by line' (P1).
- [restart/indirect ownership conflict] Resolved the §4.5.7-vs-§5.8 contradiction by keeping restart rewrite and multi-draw tiering server-side (D-B7), which also means the monolith's behaviour and diagnostic thread do not change and the name-for-name baseline moves only once (open question 12 closed).
- [stage parameter] Verified MobileGL has one combined 192-unit texture space (TextureState.h:41,128; TextureUnit.h:20,24-25) with the per-stage 32 being an advertised number only. Dropped the stage parameter from set_sampler_views and bind_sampler_states; stage flags are derived server-side from the reflection archive where the target API needs them (§4.4.3).
- [net LOC honesty] Added §2.7 estimating MGPipe's permanent additions (~6,650 hand-written + ~4,000 generated in the monolith, excluding MG_Remote) against ~372 lines truly deleted, demoted the deletion ledger to supporting evidence, and made §10.3-④'s per-thread CPU number the primary monolith argument (new risk B-R18).
- [citations] Verified SamplerObject.h is 160 lines and corrected every reference (BorderColorForm :60-70, SamplerParameters :72-96, GetLifetimeId :141, BumpVersion :151, m_version :155); added scripts/check_doc_citations.py as a P0 CI lint that every file:line in the docs resolves at the baseline commit.
- [per-draw cost口径] Verified the dynamic early-outs (SyncRenderState :2016-2018, SyncNeccessaryTextures, CurrentUnitBindingsEpoch :1418-1436, TrySetupDrawFastPath, GetOrCreatePipeline :4982-4993, ApplyDynamicDrawStateTail :5888-5893) and added §2.3.1: the real steady-state pull is ~10-25 accessor calls per backend per draw, not 124/169. Rewrote §10.2 in dynamic terms, added dynamic call/memo-hit counters to P0's deliverables, and required an absolute ns/draw threshold at the GO/NO-GO instead of a relative-to-noise one.
- [render-state CSO] Verified the two-counter rationale (RenderState.h:519-528) and that viewport/scissor/line-width setters bump only ++m_version while SET_CAPABILITY bumps BumpVersions (RenderState.cpp:312). Rewrote D-B1: the blob still travels whole for Espryt's span memcmp, but the CSO identity is the pipeline subset only (MGPipeComputePipelineSubsetHash moved verbatim out of VulkanRenderer.cpp:4826-4906 into MG_Pipe/), the dynamic subset goes through a new set_dynamic_state, the server keeps one working RenderStateParameters, and G7 generates a setter-consistency test asserting pipelineSubsetHash changes iff m_pipelineStateVersion changes. Client gates the hash on m_pipelineStateVersion so glViewport costs zero hashing and never evicts Magma's pipeline memo.
- [reconcile discipline] Verified MultiDrawElementsIndirectCount calls only SyncPersistentMappedRange (DirectGLES.cpp:4666-4667), never SyncGpuWrites. Replaced §5.8.1's blanket publish/wait/drain rule with a per-site table reproducing the monolith's set exactly, and added a P8 acceptance requiring roundtrips-per-frame to read zero on the create-indirect fixture; flagged the monolith's own omission as a separate dev question the split must not silently fix (open question 15).
- [purity gate] Verified RenderState.h:12 includes FramebufferObject.h which includes TextureObject.h/RenderbufferObject.h, and that RenderStateParameters sizes arrays with FramebufferObject::MAX_DRAW_BUFFERS (:263, :273), so the value-header allowlist is not a leaf set and nm --undefined-only is blind to include coupling. Split the purity gate into three: an include-graph gate (compile MG_Backend with MG_State/GLState off the search path) backed by a new MGPipeValueTypes.h extracted in P0.5, the symbol gate, and the undeclared gate - all run only on the non-verify build.
- [draw payload cost] Stated MGPDrawInfo's real cost against today's three-register DrawArrays, flag-gated minIndex/maxIndex and xfbCpuCapturedVertices (computed only where a consumer asked), moved the 32-byte MGHostSpan out of the fixed header into the var-tail, and added a per-draw payload-byte histogram to P0's counters (§4.5.7, §10.2).
- [memory arithmetic] Corrected §0.4-1 to a full table: 48.25 MiB transport + 0-32 MiB SEG_STAGE headroom + 0-64 MiB index host mirror (split only) + ~1-2 MiB records, with MOBILEGL_PIPE_TEXEL_RETAIN_MB defaulted to 0 because MipmapStorage keeps a complete CPU shadow so retention buys latency, not correctness. Typical +50-60 MiB, worst case ~+145 MiB.
- [generated mipmaps] Verified EnsureGenerateMipmapStorageAllocated does AllocateStorage + MarkStorageDirty(false) with no content (DirectGLES.cpp:6270-6271), so GPU-generated levels are allocated-and-zero in the monolith too. Decided explicitly that on_mip_levels_generated carries shape only, glGetTexImage stays 0 round trips on DirectGLES, and only the CPU fallback path produces texels via on_texture_writeback (§9.1).
- [map_persistent frequency] Corrected 'once per store lifetime' to 'once per storage definition' (TryAdoptLargeStorage fires at storage-definition time, so a regrowing arena pays N times) and required StorageBufferRegrowScenario to publish a map-persistent-roundtrips counter (D-B4, §8.3, §9.2-8).
- [MGHostSpan cost] Restated the monolith cost as one predictable branch plus 32 bytes carried only when kHasUserIndices is set, rather than 'zero'.
- [P5 inproc honesty] Added a specification clause that InProcessTransport uses the identical G3 serialization and differs only in doorbell/copy mechanism, plus a PipeApplier debug assertion that no SharedPtr or raw frontend pointer crosses the applier boundary in any transport, so the day-99 milestone actually proves wire completeness (P5).
- [P2 baseline definition] Defined the name-for-name functional baseline as 'the refactored monolith at P1 exit' (itself proven equivalent to
81b17c0bby verify), with81b17c0bretained only as the performance anchor (§10.3-③, B-R3). - [gate list] Added HandleRecycleScenario / TextureRemintPullScenario (with the unanswerable case) / TextureUploadShapeScenario / view-owner cursor aliasing scenario / attachment-only glTexParameter scenario / ClientArrayAfterComputeWriteScenario, each with an explicit statement of what must make it red before the corresponding fix lands.
- [callbacks] MGPipeCallbacks grew from 9 to 10 (added on_xfb_scatter_ready) plus the forward terminator resource_subdata_complete; set_* grew from 14 to 17 (set_dynamic_state, set_texture_params, and set_shader_buffers gaining kHostSpan); appendix A and the call-count totals updated throughout.
5. 被驳回或部分驳回的审查意见
- [performance #11, partial] 'glGetTexImage = 0 round trips does not survive the generated-mipmap ownership split' - the demand for an explicit decision was accepted, but the implied conclusion (it must become a blocking round trip or an eager multi-megabyte writeback) is refuted. EnsureGenerateMipmapStorageAllocated (DirectGLES.cpp:6270-6271) does AllocateStorage + MarkStorageDirty(false) with no content, so a GPU-generated level's shadow is allocated-and-zero in the monolith too; CopyTextureImageToClientOrPBO_State answers from it identically in both modes. on_mip_levels_generated therefore carries shape only and the row stays in §9.1 at zero round trips; only the CPU fallback path (RGB16F/RGB32F, :6811-6861) needs on_texture_writeback. Documented as an explicit decision in §9.1 rather than a fix.
- [skeptic framing on §0.4-4] The claim that gen_impl_mutation_surface.py 'vanishes' was corrected rather than accepted as-is: the replay obligation genuinely disappears (there is no replica), but the enumeration obligation reappears as dirty-marking, so the generator is repurposed (gen_pipe_dirty_surface.py) rather than deleted. Listing it as a pure deletion in §0.4-4 was the error; listing the enumeration obligation as unbudgeted was also inaccurate once the generator is repurposed - it is now a P2 deliverable.
- [correctness #6, partial] The proposed fix 'delete kCapMultiDraw* and let the client supply index bytes when caps say the server may need them' was accepted for tiering ownership but rejected in its transport form: shipping index bytes per draw through MGHostSpan would put up to 1<<24 indices on the ring per batch. Replaced with an incrementally-maintained server-side index host mirror (D-B7) that costs zero per-draw wire traffic, at the price of a budgeted, counted memory duplication limited to element-array-bound buffers in split mode only - stated openly in the §0.4-1 memory table as the design's one data copy.