fix(render): S3 chunk 3 re-review follow-ups
- the LandCell event doc states the deferred cross-block batch rule (the same-landblock lookahead text was stale); - the AlphaSubmitMark arm's comment gives the true reason it is not a flush point (it only enqueues into the CPU alpha list; the drain leaves flush first); - the pending terrain batch is cleared with the other transient frame lists in AbortFrame/BeginFrame; - CompleteWalkTerrainFrame runs in a finally so a throwing Replay cannot leak its accumulated ticks into the next frame's sample; - CopyRenderableEmittersInCell keeps its own doc comment; - the outdoor-root LandCell pin now observes one real cell turn after its terrain; - the oh1 landscape contract's stale "whole pre-stage" prose is resolved. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
parent
651badc2b9
commit
e10765aaa0
5 changed files with 58 additions and 39 deletions
|
|
@ -311,10 +311,11 @@ weather/sky post-pass
|
|||
| Cell object draw consumes the cell's exact stable far-to-near `shadow_part_list`. | The driver materializes per-cell record buckets, classifies/batches them, and uses a scope-level ordered stream plus separate alpha collection. | OH3/OH7 must replace reconstructed membership/order with exact part entries and exact per-cell alpha barriers. |
|
||||
| Building is alpha drain -> portal pass -> shell, then cell objects. | `RetailFrameWalk.DrawBuilding` and the driver's building events model this order. | Mechanism broadly matches, but its correctness depends on the sibling OH1 built-mesh/view and depth-lifecycle findings. |
|
||||
|
||||
The architecture SSOT currently says that the walk records one retail-ordered
|
||||
stream including landscape cells, but the implementation and its own comments
|
||||
admit that terrain is one whole pre-stage. That is an architecture/code
|
||||
contradiction, not a documented adaptation. The same architecture section
|
||||
The architecture SSOT says that the walk records one retail-ordered stream
|
||||
including landscape cells; until S3 chunk 3 the implementation drew terrain as
|
||||
one whole pre-stage (an architecture/code contradiction, RESOLVED 2026-09-03:
|
||||
`RetailFrameWalk.DrawLandscape` now emits one land-cell terrain turn per
|
||||
admitted cell before that cell's own building/object turn). The same architecture section
|
||||
also states that every admitted portal slice GPU-clips the mesh; the dirty
|
||||
built-mesh candidate says the opposite. The sibling built-mesh evidence must
|
||||
arbitrate that claim before the architecture text or production path is
|
||||
|
|
|
|||
|
|
@ -588,14 +588,21 @@ internal sealed class RetailPViewRenderer
|
|||
// filter) is gone — the walk's own per-cell LandCell turns are now
|
||||
// the sole terrain-visibility authority, so there is no plumbing
|
||||
// left to feed here.
|
||||
driver.Replay(frame, encoder);
|
||||
// S3 chunk 3 fix round 1 (F3): "the end of the walk replay" —
|
||||
// exactly where the deleted whole-stage terrain leaf's own
|
||||
// Begin()/Complete() bracket used to close. Pushes this frame's ONE
|
||||
// accumulated terrain-timing sample (every DrawWalkLandCellBatch
|
||||
// call between here and the last CompleteWalkTerrainFrame) and
|
||||
// publishes the periodic [TERRAIN-DIAG] line if the cadence is due.
|
||||
passes.CompleteWalkTerrainFrame();
|
||||
try
|
||||
{
|
||||
driver.Replay(frame, encoder);
|
||||
}
|
||||
finally
|
||||
{
|
||||
// S3 chunk 3 fix round 1 (F3): "the end of the walk replay" —
|
||||
// exactly where the deleted whole-stage terrain leaf's own
|
||||
// Begin()/Complete() bracket used to close. Pushes this frame's
|
||||
// ONE accumulated terrain-timing sample (every DrawWalkLandCellBatch
|
||||
// call since the last CompleteWalkTerrainFrame) and publishes the
|
||||
// periodic [TERRAIN-DIAG] line if the cadence is due. In a finally
|
||||
// so a throwing Replay cannot leak its ticks into the next frame.
|
||||
passes.CompleteWalkTerrainFrame();
|
||||
}
|
||||
|
||||
// Landscape-stage static-owner particles (candles, the cathedral
|
||||
// falls) submit AT THEIR OWN WALK TURNS inside Replay
|
||||
|
|
|
|||
|
|
@ -282,15 +282,16 @@ internal enum WalkFrameEventKind : byte
|
|||
/// <summary><see cref="IWalkFrameLeafRenderer.DrawSky"/>.</summary>
|
||||
Sky,
|
||||
|
||||
/// <summary>S3 chunk 3: <see cref="IWalkFrameLeafRenderer.DrawLandCellBatch"/>
|
||||
/// for ONE admitted land cell — <see cref="WalkFrameEvent.CellId"/> is
|
||||
/// the owning landblock id, <see cref="WalkFrameEvent.IntArg"/> packs
|
||||
/// <c>(sideCellCount << 8) | cellIndex</c> (side ∈ {1,2,4,8},
|
||||
/// cellIndex < side², both fit comfortably below the 8-bit shift).
|
||||
/// <see cref="Replay"/> merges a run of consecutive same-landblock
|
||||
/// entries into ONE <see cref="IWalkFrameLeafRenderer.DrawLandCellBatch"/>
|
||||
/// call (B2's order-preserving batching) rather than replaying them
|
||||
/// one at a time.</summary>
|
||||
/// <summary><see cref="IWalkFrameLeafRenderer.DrawLandCellBatch"/> —
|
||||
/// one admitted land cell's terrain (retail <c>DrawLandCell</c>
|
||||
/// @0x0059f120): <see cref="WalkFrameEvent.CellId"/> is the landblock,
|
||||
/// <see cref="WalkFrameEvent.IntArg"/> packs <c>side << 8 | cellIndex</c>.
|
||||
/// <see cref="Replay"/> 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.</summary>
|
||||
LandCell,
|
||||
|
||||
/// <summary><see cref="IWalkFrameLeafRenderer.DrawCellShell"/> —
|
||||
|
|
@ -741,6 +742,7 @@ internal sealed class WalkFrameDriver : IWalkEventSink, IWalkLookInViewSource
|
|||
_events.Clear();
|
||||
_markPositions.Clear();
|
||||
_alphaSubmissions.Clear();
|
||||
_pendingTerrainBatch.Clear();
|
||||
_alphaSubmitMark = 0;
|
||||
VisitedCells.Clear();
|
||||
LookInCellTurns.Clear();
|
||||
|
|
@ -877,6 +879,7 @@ internal sealed class WalkFrameDriver : IWalkEventSink, IWalkLookInViewSource
|
|||
_events.Clear();
|
||||
_markPositions.Clear();
|
||||
_alphaSubmissions.Clear();
|
||||
_pendingTerrainBatch.Clear();
|
||||
_alphaSubmitMark = 0;
|
||||
VisitedCells.Clear();
|
||||
LookInCellTurns.Clear();
|
||||
|
|
@ -968,12 +971,14 @@ internal sealed class WalkFrameDriver : IWalkEventSink, IWalkLookInViewSource
|
|||
cursor = end;
|
||||
break;
|
||||
case WalkFrameEventKind.AlphaSubmitMark:
|
||||
// Not a flush point (F2): an AlphaSubmitMark event is
|
||||
// always recorded alongside — immediately after — a
|
||||
// StreamMark from the SAME MarkIfGrown/MarkAlphaIfGrown
|
||||
// pairing (every Collect-side call site pairs them),
|
||||
// so the StreamMark case above already flushed any
|
||||
// pending batch by the time this one runs.
|
||||
// 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
|
||||
// drained alpha still lands after every preceding
|
||||
// land cell's terrain, exactly as when unbatched.
|
||||
int alphaEnd = e.IntArg;
|
||||
for (; alphaCursor < alphaEnd; alphaCursor++)
|
||||
{
|
||||
|
|
@ -1064,6 +1069,7 @@ internal sealed class WalkFrameDriver : IWalkEventSink, IWalkLookInViewSource
|
|||
_events.Clear();
|
||||
_markPositions.Clear();
|
||||
_alphaSubmissions.Clear();
|
||||
_pendingTerrainBatch.Clear();
|
||||
_alphaSubmitMark = 0;
|
||||
_readyToReplay = false;
|
||||
_ctx = null;
|
||||
|
|
|
|||
|
|
@ -606,17 +606,6 @@ public sealed class ParticleSystem : IParticleSystem
|
|||
destination.Sort(static (left, right) => left.Handle.CompareTo(right.Handle));
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Copies the renderable emitters whose <see cref="ParticleEmitter.OwnerCellId"/>
|
||||
/// is <paramref name="cellId"/>, retaining spawn order. Retail
|
||||
/// <c>CPhysicsObj::add_particle_shadow_to_cell</c> (0x00514a70) gives an
|
||||
/// emitter exactly one shadow in its own current cell, independent of its
|
||||
/// attached owner's registry membership — a hidden/suspended owner's
|
||||
/// emitter is still enumerated here as long as it remains renderable
|
||||
/// (presentation-visible and view-eligible). No per-call allocation after
|
||||
/// warmup: the bucket's own sorted handle list is copied through the
|
||||
/// retained <see cref="_scopeHandleScratch"/> buffer.
|
||||
/// </summary>
|
||||
/// <summary>
|
||||
/// S3 chunk 3 fix round 1 (F2): whether <paramref name="cellId"/> has any
|
||||
/// renderable emitter for <paramref name="renderPass"/>, WITHOUT copying
|
||||
|
|
@ -635,6 +624,17 @@ public sealed class ParticleSystem : IParticleSystem
|
|||
&& bucket.FirstRenderableHandle != 0;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Copies the renderable emitters whose <see cref="ParticleEmitter.OwnerCellId"/>
|
||||
/// is <paramref name="cellId"/>, retaining spawn order. Retail
|
||||
/// <c>CPhysicsObj::add_particle_shadow_to_cell</c> (0x00514a70) gives an
|
||||
/// emitter exactly one shadow in its own current cell, independent of its
|
||||
/// attached owner's registry membership — a hidden/suspended owner's
|
||||
/// emitter is still enumerated here as long as it remains renderable
|
||||
/// (presentation-visible and view-eligible). No per-call allocation after
|
||||
/// warmup: the bucket's own sorted handle list is copied through the
|
||||
/// retained <see cref="_scopeHandleScratch"/> buffer.
|
||||
/// </summary>
|
||||
public void CopyRenderableEmittersInCell(
|
||||
ParticleRenderPass renderPass,
|
||||
uint cellId,
|
||||
|
|
|
|||
|
|
@ -1360,7 +1360,12 @@ public sealed class WalkFrameDriverTests
|
|||
using var fx = new DispatcherFixture();
|
||||
var log = new List<string>();
|
||||
var leaf = new RecordingLeafRenderer(log);
|
||||
// Every covered bucket but ONE reports no renderable emitter, so the
|
||||
// block's object turns submit nothing (no flush) except bucket
|
||||
// 0xF4180001, whose particle turn is a real submission — it must land
|
||||
// AFTER the coarse cell's own terrain (retail DrawLandCell → DrawSortCell).
|
||||
leaf.CellsWithoutEmitters.UnionWith(CoarseLandscapeBuckets(0xF4180000u));
|
||||
leaf.CellsWithoutEmitters.Remove(0xF4180001u);
|
||||
var ctx = new TestContext();
|
||||
var driver = new WalkFrameDriver(fx.Dispatcher, leaf, new FakeWorldData());
|
||||
var walk = new RetailFrameWalk();
|
||||
|
|
@ -1378,7 +1383,7 @@ public sealed class WalkFrameDriverTests
|
|||
driver.EndFrame();
|
||||
driver.Replay(draw.Frame, draw.Pass);
|
||||
|
||||
Assert.Equal(new[] { "SKY", "LANDCELL:f4180000:1:0" }, log);
|
||||
Assert.Equal(new[] { "SKY", "LANDCELL:f4180000:1:0", "PARTICLES:f4180001" }, log);
|
||||
Assert.Equal(new (uint LandblockId, int SideCellCount, int CellIndex)[] { (0xF4180000u, 1, 0) },
|
||||
Assert.Single(leaf.LandCellBatches));
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue