From 896c63fe9af378584e22100a79298fe0197059ec Mon Sep 17 00:00:00 2001 From: Erik Date: Thu, 3 Sep 2026 10:28:59 +0200 Subject: [PATCH] =?UTF-8?q?fix(render):=20S3=20chunk=203=20round=202=20?= =?UTF-8?q?=E2=80=94=20opaque=20stream=20marks=20no=20longer=20split=20ter?= =?UTF-8?q?rain=20batches?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Measured (§9.7): the connected R6 soak at e10765aa showed the round-1 batching fix (§9.6 F2) was inert in practice — CPU p50 flat at 31.2 ms and FPS flat at 32 across every outdoor destination (baseline 2.0-15.5 ms / 51-452 FPS), GPU p50 2-4x baseline. Root cause: nearly every admitted land cell carries scenery statics, so a StreamMark (recorded whenever the walk's ordered stream grows) follows almost every LandCell event and was itself treated as a flush point, splitting the pending terrain batch back down to ~500 submissions/frame — each paying TerrainModernRenderer's full bind sequence plus a ring allocation and an MDI. F10: a StreamMark no longer flushes the pending terrain batch. Citation — the walk's ordered stream holds ONLY opaque batches (WalkStaticStreamPopulator.cs:179, `if (batch.IsOpaque)` routes translucent batches to the alpha list instead of the stream) drawn through the Opaque/OpaqueAlphaToCoverage pipelines, both created with depth test AND write ON and GpuBlendMode.None (WbDrawDispatcher.Rhi.cs:240-248,343: `Depth = new GpuDepthState(Test: true, Write: depthWrite, depthCompare)` with depthWrite: true); DrawOrderedRange resolves every walk-stream command's bucket to PipelineBucket.Opaque because every command it holds is opaque (WbDrawDispatcher.OrderedStream.cs:569-571). Opaque terrain and opaque statics are therefore depth-resolved identically in either submission order, so deferring the terrain batch across a StreamMark is pixel-identical (except exact z-ties, which retail itself leaves order-independent). Flush points that REMAIN, because their GPU order against terrain IS observable: PunchFan (DEPTHTEST_ALWAYS + write — the far-Z stamp this chunk's interleave exists to order correctly), AlphaBarrier and LandscapeFlush (translucent drains), ClearInteriorDepth, ExitSeals, Sky, CellShell, a particle turn whose cell has a renderable emitter, and the end of Replay. AlphaSubmitMark stays a non-flush point (unchanged — it only enqueues into the CPU alpha list). No ordered-stream stage found that blends or disables depth write, so no amendment to the contract was needed; the Replay doc comment records the citation so a future stage that does must become a flush point again. F11: no per-batch bind latch added (tens of batches/frame is acceptable). F12: the F4(c) driver pin is re-expressed to prove a StreamMark from a cell with real content (not just an empty particle turn) no longer splits, while PunchFan/AlphaBarrier/a has-emitter particle turn still do; a new punch-order pin (LandCell(far), building turn with a PunchFan, LandCell(near)) proves the far terrain flushes before the punch and the near terrain starts a fresh batch after it. Hermetic App lane: 6,796 passed (6,795 base + 1 new fact), 0 failed. Co-Authored-By: Claude Fable 5.1 --- .../Rendering/Walk/WalkFrameDriver.cs | 151 +++++++++++++----- .../Rendering/Walk/WalkFrameDriverTests.cs | 111 ++++++++++--- 2 files changed, 206 insertions(+), 56 deletions(-) diff --git a/src/AcDream.App/Rendering/Walk/WalkFrameDriver.cs b/src/AcDream.App/Rendering/Walk/WalkFrameDriver.cs index e10b5148..76192c47 100644 --- a/src/AcDream.App/Rendering/Walk/WalkFrameDriver.cs +++ b/src/AcDream.App/Rendering/Walk/WalkFrameDriver.cs @@ -102,15 +102,18 @@ internal interface IWalkFrameLeafRenderer /// calls this exactly once per frame's Landscape turn). void DrawSky(); - /// S3 chunk 3 (§9.2 B1/B2), fix round 1 (F2): retail - /// RenderDeviceD3D::DrawLandCell @0x0059f120 — one or more + /// S3 chunk 3 (§9.2 B1/B2), fix round 1 (F2), round 2 (F10): + /// retail RenderDeviceD3D::DrawLandCell @0x0059f120 — one or more /// admitted LOD cells, possibly spanning several landblocks, submitted /// together as ONE indirect draw. /// keeps a PENDING batch that /// events append to and flushes (calls this once) immediately before any - /// OTHER event that will itself submit GPU work — see that method's own - /// doc comment for the exact flush points, including the F2 particle-turn - /// exception (a genuinely empty / + /// is NOT one of them (F10: the ordered stream it draws is opaque-only, + /// depth test AND write on, no blending); the F2 particle-turn exception + /// still holds (a genuinely empty / turn neither submits nor flushes). This is /// order-preserving by construction: the batch's GPU submission point is /// always the SAME point the unbatched terrain draws would have occupied @@ -287,11 +290,21 @@ internal enum WalkFrameEventKind : byte /// @0x0059f120): is the landblock, /// packs side << 8 | cellIndex. /// does NOT draw it immediately: it appends the - /// cell to ONE pending terrain batch that may span landblocks and flushes - /// that batch only before a leaf that will submit GPU work (S3 chunk 3 - /// fix round 1, §9.6 F2) — a particle turn whose cell has no renderable - /// emitter submits nothing and keeps the batch open. The GPU submission - /// order is therefore identical to replaying each cell alone. + /// cell to ONE pending terrain batch that may span landblocks and + /// flushes only at a flush point whose GPU order against terrain is + /// OBSERVABLE — , , + /// , , + /// , , , + /// a particle turn whose cell has a renderable emitter, or the end of + /// Replay (S3 chunk 3 fix round 2, §9.7 F10; see 's + /// own doc comment for the full citation). A is + /// deliberately NOT a flush point: the ordered stream it draws holds + /// ONLY opaque batches, depth test AND write on, no blending, so opaque + /// terrain and opaque statics resolve identically in either submission + /// order. A particle turn whose cell has no renderable emitter submits + /// nothing and keeps the batch open (fix round 1, F2). The GPU + /// submission order is therefore identical to replaying each cell + /// alone. LandCell, /// — @@ -580,12 +593,14 @@ internal sealed class WalkFrameDriver : IWalkEventSink, IWalkLookInViewSource private readonly List _floodViewRouteScratch = new(); private WalkPlane _lookInCyPlane; - /// S3 chunk 3 (§9.2 B2), fix round 1 (F2): 's - /// PENDING terrain batch — every - /// event since the last flush, across however many landblocks, in event - /// order. Cleared at the start of each call ("frame - /// start") and by every flush; NOT cleared between events, since it is - /// the whole point of the deferred-batching rule. + /// S3 chunk 3 (§9.2 B2), fix round 1 (F2), round 2 (F10): + /// 's PENDING terrain batch — every + /// event since the last flush, + /// across however many landblocks and however many + /// events (F10: not a flush point), in event order. Cleared at the start + /// of each call ("frame start") and by every flush; + /// NOT cleared between events, since it is the whole point of the + /// deferred-batching rule. private readonly List<(uint LandblockId, int SideCellCount, int CellIndex)> _pendingTerrainBatch = new(); private readonly HashSet _cellShellsDrawnThisFrame = new(); @@ -932,6 +947,49 @@ internal sealed class WalkFrameDriver : IWalkEventSink, IWalkLookInViewSource /// these events at EXACTLY the points the pre-FW3.4a immediate driver /// used to flush/draw, this reproduces the SAME interleaved GPU command /// order — the campaign invariant — from one walk instead of two. + /// + /// S3 chunk 3 fix round 2 (§9.7 F10) — the pending terrain + /// batch's flush points, authoritative list. A + /// is NOT one of them: the + /// ordered stream it draws holds ONLY opaque batches — WalkStaticStreamPopulator's + /// per-entity classify step appends to the stream iff batch.IsOpaque + /// and otherwise routes translucent batches to the alpha list + /// (WalkStaticStreamPopulator.cs:179) — drawn through the Opaque/ + /// OpaqueAlphaToCoverage pipelines, both built with depth test AND + /// write ON and NO blending (WbDrawDispatcher.CreateMeshPipeline: + /// Depth = new GpuDepthState(Test: true, Write: depthWrite, + /// depthCompare) called with depthWrite: true, + /// GpuBlendMode.None for both, WbDrawDispatcher.Rhi.cs:240-248,343); + /// DrawOrderedRange resolves every walk-stream command's bucket to + /// PipelineBucket.Opaque because every command it holds is opaque + /// (WbDrawDispatcher.OrderedStream.cs:569-571). Opaque terrain and opaque + /// statics are therefore depth-resolved correctly in EITHER submission + /// order — pixel-identical except exact z-ties — so deferring the pending + /// terrain batch across a StreamMark changes nothing observable. + /// The flush points that DO remain, because their GPU order against + /// terrain IS observable: + /// (DEPTHTEST_ALWAYS + write — the far-Z stamp this chunk's + /// interleave exists to order correctly against nearer terrain, S3 §9.1 + /// R4), and + /// (both drain the + /// translucent alpha list, which must see every opaque surface — terrain + /// included — already resolved in the depth buffer), + /// , + /// , + /// , , + /// a / + /// turn whose cell has a + /// renderable emitter (a genuinely empty turn still submits nothing and + /// still does not flush, unchanged from fix round 1's F2), and the end of + /// this method. This list's premise is the opaque-only, depth-test-and- + /// write, no-blend pipeline pair the walk's ordered stream actually uses + /// TODAY — not StreamMark itself: if any future ordered-stream + /// stage draws with blending or a disabled depth write (for example the + /// deferred building-detail overlay replay, + /// WbDrawDispatcher.DrawBuildingDetailRangeRhi's + /// RetailDetailTransparent pass — not wired into this walk path + /// today, OrderedStream.cs:46-52), that stage's StreamMark MUST + /// become a flush point again; the contract gets amended, not bent. /// internal void Replay(IGpuFrame frame, IGpuPassEncoder encoder) { @@ -962,7 +1020,18 @@ internal sealed class WalkFrameDriver : IWalkEventSink, IWalkLookInViewSource switch (e.Kind) { case WalkFrameEventKind.StreamMark: - FlushPendingTerrainBatch(); + // S3 chunk 3 fix round 2 (F10): NOT a terrain flush + // point. The ordered stream this draws holds ONLY + // opaque batches (WalkStaticStreamPopulator.cs:179) + // drawn with the Opaque/OpaqueAlphaToCoverage + // pipelines — depth test AND write on, no blending + // (WbDrawDispatcher.Rhi.cs:240-248,343; bucket + // resolution WbDrawDispatcher.OrderedStream.cs:569-571) + // — so opaque terrain and opaque statics are + // depth-resolved correctly in either submission + // order. See this method's own doc comment for the + // full flush-point list and the citation this rests + // on. int end = e.IntArg; int count = end - cursor; if (_trace is not null) @@ -971,12 +1040,12 @@ internal sealed class WalkFrameDriver : IWalkEventSink, IWalkLookInViewSource cursor = end; break; case WalkFrameEventKind.AlphaSubmitMark: - // Not a flush point (F2): this arm only ENQUEUES the - // translucent instances into the CPU alpha list - // (SubmitWalkAlphaInstance); nothing reaches the GPU - // until the list drains at an AlphaBarrier / - // LandscapeFlush leaf or the pass end — each of which - // flushes the pending terrain batch first, so the + // Not a flush point (unchanged by fix round 2, F10): + // this arm only ENQUEUES the translucent instances + // into the CPU alpha list (SubmitWalkAlphaInstance); + // nothing reaches the GPU until the list drains at an + // AlphaBarrier / LandscapeFlush leaf or the pass end + // — both remain terrain flush points (F10) — so the // drained alpha still lands after every preceding // land cell's terrain, exactly as when unbatched. int alphaEnd = e.IntArg; @@ -995,11 +1064,16 @@ internal sealed class WalkFrameDriver : IWalkEventSink, IWalkLookInViewSource _leafRenderer.DrawSky(); break; case WalkFrameEventKind.LandCell: - // S3 chunk 3 fix round 1 (F2): append-only — no - // lookahead, no same-landblock restriction, no - // flush. The batch may span several landblocks; it - // flushes only when a later event actually needs the - // GPU (or at Replay's own end). + // S3 chunk 3 fix round 1 (F2), round 2 (F10): + // append-only — no lookahead, no same-landblock + // restriction, no flush. The batch may span several + // landblocks; it flushes only at one of the F10 + // flush points (PunchFan, AlphaBarrier, + // LandscapeFlush, ClearInteriorDepth, ExitSeals, + // Sky, CellShell, a has-emitter particle turn) or at + // Replay's own end — NOT at a StreamMark, since the + // opaque ordered stream's GPU order against terrain + // is unobservable (see Replay's own doc comment). _pendingTerrainBatch.Add((e.CellId, e.IntArg >> 8, e.IntArg & 0xFF)); break; case WalkFrameEventKind.CellShell: @@ -1059,8 +1133,9 @@ internal sealed class WalkFrameDriver : IWalkEventSink, IWalkLookInViewSource break; } } - // S3 chunk 3 fix round 1 (F2): the end of Replay flushes the - // remainder — a batch never crosses into the next frame. + // S3 chunk 3 fix round 1 (F2), round 2 (F10): the end of Replay + // is itself a flush point — the remainder never crosses into + // the next frame. FlushPendingTerrainBatch(); } finally @@ -1076,13 +1151,17 @@ internal sealed class WalkFrameDriver : IWalkEventSink, IWalkLookInViewSource } } - /// S3 chunk 3 fix round 1 (F2): submits the pending terrain - /// batch as ONE + /// S3 chunk 3 fix round 1 (F2), round 2 (F10): submits the + /// pending terrain batch as ONE /// call, if it holds anything, and clears it. Called immediately before - /// every OTHER event kind that will itself submit GPU work — see - /// 's own doc comment for the full flush-point - /// list — so the batch's actual submission point is always the exact - /// point the unbatched per-cell draws would have occupied. + /// every event kind whose GPU order against terrain is OBSERVABLE — see + /// 's own doc comment for the full flush-point list + /// and its citation — so the batch's actual submission point is always + /// the exact point the unbatched per-cell draws would have occupied + /// relative to those events. A + /// is deliberately NOT one of the callers (F10): the opaque ordered + /// stream it draws is depth-resolved identically regardless of + /// submission order relative to terrain. private void FlushPendingTerrainBatch() { if (_pendingTerrainBatch.Count == 0) diff --git a/tests/AcDream.App.Tests/Rendering/Walk/WalkFrameDriverTests.cs b/tests/AcDream.App.Tests/Rendering/Walk/WalkFrameDriverTests.cs index 2e7c933a..0ff7f4dc 100644 --- a/tests/AcDream.App.Tests/Rendering/Walk/WalkFrameDriverTests.cs +++ b/tests/AcDream.App.Tests/Rendering/Walk/WalkFrameDriverTests.cs @@ -1483,18 +1483,23 @@ public sealed class WalkFrameDriverTests } // ── T4 (S3 chunk 3 §9.3), re-expressed for fix round 1's F2 rule - // (§9.6 F4c): the batch is now a single PENDING list Replay keeps - // across the WHOLE frame, not a same-landblock lookahead merge — two - // LandCell turns of DIFFERENT landblocks with only an empty particle - // turn between them still merge into ONE DrawLandCellBatch call; a - // StreamMark (a real cell with statics) OR a building's own alpha - // barrier between two LandCell turns splits the batch; the fake leaf's - // HasRenderableEmittersInCell reports "no emitters" for one cell (via - // CellsWithoutEmitters) and "has emitters" (the default) for another, - // so both arms of the F2 particle-turn gate are covered in one test. ── + // (§9.6 F4c), then again for fix round 2's F10 rule (§9.7 F12): the + // batch is a single PENDING list Replay keeps across the WHOLE frame. + // Two LandCell turns of DIFFERENT landblocks merge into ONE + // DrawLandCellBatch call across EITHER of two things that used to (or, + // for a real StreamMark, still might look like they should) split it: + // an intervening EMPTY particle turn (F2, unchanged) AND an intervening + // StreamMark from a cell that DOES grow the stream but has no + // renderable emitter of its own (F10, new — proves the split doesn't + // come back just because content was appended). A cell with BOTH real + // stream content AND a renderable emitter still splits, because its + // StaticParticles turn itself is a flush point (F10) — the split just + // no longer comes from the StreamMark that precedes it. A building's + // own alpha barrier and its portal pass's punch fan each split too + // (F10: both remain flush points). ───────────────────────────────────── [Fact] - public void OnLandCellTurn_MergesAcrossLandblocksOverAnEmptyParticleTurn_RealSubmissionsSplit() + public void OnLandCellTurn_MergesAcrossLandblocksOverStreamMarksAndEmptyParticleTurns_RealFlushPointsSplit() { using var fx = new DispatcherFixture(); const ulong gfxObj = 0x0200_0024UL; @@ -1502,6 +1507,15 @@ public sealed class WalkFrameDriverTests MakeBatch(0x08100024u, TranslucencyKind.Opaque, 0, 0, 3, 1))); var worldData = new FakeWorldData(); + // Has REAL stream content (so its own turn records a genuine + // StreamMark event) but NO renderable emitter — the F10 case: the + // StreamMark must not split the pending terrain batch even though + // the ordered stream visibly grew. + worldData.OutdoorStaticsByCell[0xAAAA0002u] = new WalkFrameStaticRecords( + new[] { MakeRecord(311, 0, Vector3.Zero, [new MeshRef((uint)gfxObj, Matrix4x4.Identity)]) }, + 0xAAAAu); + // Has real stream content AND a renderable emitter (default) — its + // StaticParticles turn is itself a flush point. worldData.OutdoorStaticsByCell[0xBBBB0002u] = new WalkFrameStaticRecords( new[] { MakeRecord(310, 0, Vector3.Zero, [new MeshRef((uint)gfxObj, Matrix4x4.Identity)]) }, 0xBBBBu); @@ -1509,9 +1523,12 @@ public sealed class WalkFrameDriverTests var log = new List(); var leaf = new RecordingLeafRenderer(log); // The 0xAAAA0001 particle turn has no world-data records AND is - // marked without emitters -> a genuinely empty turn (F2's other - // arm: 0xBBBB0002 below keeps the default "has emitters"). + // marked without emitters -> a genuinely empty turn (F2, unchanged). + // 0xAAAA0002 has world-data records (grows the stream, so Collect + // records a real StreamMark) but is ALSO marked without emitters -> + // the F10 case. 0xBBBB0002 keeps the default "has emitters". leaf.CellsWithoutEmitters.Add(0xAAAA0001u); + leaf.CellsWithoutEmitters.Add(0xAAAA0002u); var ctx = new TestContext(); var driver = new WalkFrameDriver(fx.Dispatcher, leaf, worldData); IWalkEventSink sink = driver; @@ -1521,35 +1538,45 @@ public sealed class WalkFrameDriverTests sink.OnLandscapeViews(new WalkPortalView()); sink.OnLandCellTurn(0xF4180000u, 8, 0); // landblock A - sink.OnLandscapeCellTurn(0xAAAA0001u); // empty particle turn: no emitters -> no submit, no flush + sink.OnLandscapeCellTurn(0xAAAA0001u); // empty particle turn: no growth, no emitters -> no submit, no flush (F2) sink.OnLandCellTurn(0xF3180000u, 8, 0); // landblock B, DIFFERENT -> still merges (F2) - sink.OnLandscapeCellTurn(0xBBBB0002u); // real content -> StreamMark splits; has emitters -> submits + sink.OnLandscapeCellTurn(0xAAAA0002u); // grows the stream (a real StreamMark fires) but no emitters -> the StreamMark does NOT flush (F10), and the particle turn doesn't either + sink.OnLandCellTurn(0xF2180001u, 8, 0); // landblock C -> still merges straight across that StreamMark (F10) - sink.OnLandCellTurn(0xF3180000u, 8, 1); // new batch, started after the StreamMark split + sink.OnLandscapeCellTurn(0xBBBB0002u); // grows the stream too, but HAS emitters (default) -> its StaticParticles turn flushes (F10), not the StreamMark ahead of it - sink.OnBuildingTurn(new WalkBuilding()); // the building's own alpha barrier splits again + sink.OnLandCellTurn(0xF3180000u, 8, 1); // new batch, started after that flush - sink.OnLandCellTurn(0xF2180000u, 8, 0); // final batch, flushed at Replay's own end + sink.OnBuildingTurn(new WalkBuilding()); // the building's own alpha barrier flushes (F10) + + sink.OnLandCellTurn(0xF2180000u, 8, 0); // new batch + + sink.OnPunchGeometry(new WalkBuilding(), Quad(0f), 0); // the portal pass's punch fan flushes (F10) + + sink.OnLandCellTurn(0xF1180000u, 8, 0); // final batch, flushed at Replay's own end driver.EndFrame(); driver.Replay(draw.Frame, draw.Pass); Assert.Equal( new[] { - "LANDCELL:f4180000:8:0,f3180000:8:0", + "LANDCELL:f4180000:8:0,f3180000:8:0,f2180001:8:0", "PARTICLES:bbbb0002", "LANDCELL:f3180000:8:1", "ALPHA", "LANDCELL:f2180000:8:0", + "PUNCH:4@v0", + "LANDCELL:f1180000:8:0", }, log); Assert.DoesNotContain("PARTICLES:aaaa0001", log); - Assert.Equal(3, leaf.LandCellBatches.Count); + Assert.DoesNotContain("PARTICLES:aaaa0002", log); + Assert.Equal(4, leaf.LandCellBatches.Count); Assert.Equal( new (uint LandblockId, int SideCellCount, int CellIndex)[] { - (0xF4180000u, 8, 0), (0xF3180000u, 8, 0), + (0xF4180000u, 8, 0), (0xF3180000u, 8, 0), (0xF2180001u, 8, 0), }, leaf.LandCellBatches[0]); Assert.Equal( @@ -1558,6 +1585,50 @@ public sealed class WalkFrameDriverTests Assert.Equal( new (uint LandblockId, int SideCellCount, int CellIndex)[] { (0xF2180000u, 8, 0) }, leaf.LandCellBatches[2]); + Assert.Equal( + new (uint LandblockId, int SideCellCount, int CellIndex)[] { (0xF1180000u, 8, 0) }, + leaf.LandCellBatches[3]); + } + + // ── New punch-order pin (S3 chunk 3 fix round 2, §9.7 F12): a + // PunchFan flushes the pending terrain batch (F10 — PunchFan's + // DEPTHTEST_ALWAYS + write is exactly the ordering this chunk exists to + // preserve, §9.1 R4). LandCell(far), a building turn whose portal pass + // emits a PunchFan, LandCell(near) -> the leaf log reads + // LANDCELL(far batch), PUNCH, LANDCELL(near batch): the far terrain is + // flushed and drawn BEFORE the punch stamps its far-Z, and the near + // terrain starts a NEW batch drawn strictly AFTER it — so the nearer + // terrain still overwrites the punched depth, exactly as retail's own + // interleave requires. ────────────────────────────────────────────────── + + [Fact] + public void OnPunchGeometry_FlushesPendingTerrainBatch_FarTerrainBeforePunchNearTerrainAfter() + { + using var fx = new DispatcherFixture(); + var log = new List(); + var leaf = new RecordingLeafRenderer(log); + var ctx = new TestContext(); + var driver = new WalkFrameDriver(fx.Dispatcher, leaf, new FakeWorldData()); + IWalkEventSink sink = driver; + + using DrawScope draw = fx.BeginDraw(); + driver.BeginFrame(ctx, Matrix4x4.Identity, Vector3.Zero); + sink.OnLandscapeViews(new WalkPortalView()); + + sink.OnLandCellTurn(0xF4180000u, 8, 0); // far land cell + sink.OnPunchGeometry(new WalkBuilding(), Quad(0f), 0); // the building's far-Z punch + sink.OnLandCellTurn(0xF4180000u, 8, 1); // near land cell (same landblock — the point is ORDER, not the landblock key) + driver.EndFrame(); + driver.Replay(draw.Frame, draw.Pass); + + Assert.Equal( + new[] + { + "LANDCELL:f4180000:8:0", + "PUNCH:4@v0", + "LANDCELL:f4180000:8:1", + }, + log); } // ── Fixture (mirrors WalkStaticStreamPopulatorTests' DispatcherFixture —