erik/acdream
1
0
Fork 0
Code Issues Pull requests Projects Releases 6 Packages Wiki Activity Actions

fix(render): S3 chunk 3 round 1 - slot key, deferred cross-block terrain batches, per-frame terrain diagnostic

Browse source 
Fixes the three-lens review blockers against 671eb3ad4 (S3 section 9.6 F1-F6).

F1 - slot key (blocking, retail). TerrainModernRenderer.DrawLandCells
normalizes every incoming landblockId to (id & 0xFFFF0000u) | 0xFFFFu
before the _idToSlot lookup: the walk hands 0xXXYY0000
(WalkLandBlock.LandblockId) but AddLandblock stores under the DAT id
0xXXYYFFFF (LandblockRenderPublisher.LandblockId) - every walk lookup
was missing and the walk path drew NO terrain. Unit-tested end-to-end
through a real RecordingGpuDevice-backed TerrainModernRenderer
(TerrainWalkSlotKeyNormalizationTests): AddLandblock(0xA9B4FFFF, ...)
is found by a 0xA9B40000 lookup, an unknown landblock is a silent
per-entry no-op, and a batch mixing a known and unknown entry submits
only the known one.

F2 - deferred cross-block batching (blocking, driver). Retail's
DrawSortCell always follows DrawLandCell (LC/SC strictly alternate,
never two LC in a row - S3 section 9 R1), so chunk 3's "merge
consecutive same-landblock LandCell events" rule never actually
merged anything; the driver review flagged batching as inert.
WalkFrameDriver.Replay now keeps ONE pending terrain batch across
landblocks ((landblockId, side, cellIndex) entries, cleared at
Replay's own start); a LandCell event only appends; every OTHER event
kind that will itself submit GPU work (StreamMark, Sky, CellShell,
PunchFan, AlphaBarrier, LandscapeFlush, ClearInteriorDepth,
ExitSeals) flushes the pending batch first; a StaticParticles/
CellParticles turn asks the new ParticleSystem.
HasRenderableEmittersInCell (an allocation-free sibling of
CopyRenderableEmittersInCell) and, when the cell has no renderable
emitter, submits nothing and does NOT flush either - the whole point
of the deferred rule. The end of Replay flushes the remainder. This
is order-preserving by construction: a flush always lands at the
exact point the unbatched draw would have, so GPU submission order -
and therefore pixels - is identical to the unbatched baseline; only
the number of small terrain draw calls shrinks.
TerrainModernRenderer.DrawLandCellRuns becomes DrawLandCells(
viewProjection, IReadOnlyList<(uint LandblockId, int SideCellCount,
int CellIndex)>) - one MultiDrawIndexedIndirect over every entry's
runs, unknown slots skipped per-entry. IWalkFrameLeafRenderer.
DrawLandCellBatch drops its separate landblockId parameter to match
(a batch can span several landblocks now) and gains
HasRenderableEmittersInCell.

Batch-count demonstration: driven through a real WalkFrameDriver
Replay (OnLandCellTurn_MergesAcrossLandblocksOverAnEmptyParticleTurn_
RealSubmissionsSplit), 4 LandCell turns across 3 distinct landblocks,
separated only by an empty particle turn, a real StreamMark, and a
building's alpha barrier, submit as exactly 3 DrawLandCellBatch calls
(2+1+1) instead of 4 - the empty particle turn's non-flush merges two
otherwise-separate cross-landblock entries. At production scale the
same mechanism is expected to cut the terrace-edge frame's ~578
individual DrawLandCell events (S3 section 9's captured transcript
count) to "tens" of submitted batches, per the contract's own
expectation: most terrain cells have no particle owner nearby, so the
strict LC/[empty-SC]/LC/[empty-SC]/... run collapses into one batch
per region bounded by real content (a building, a StreamMark-worthy
cell, or a genuine emitter) rather than per cell.

F3 - per-frame terrain diagnostic (blocking, build/test). The walk
leaf no longer brackets each batch with TerrainDrawDiagnosticsController
.Begin()/Complete() (a per-batch Stopwatch Restart/Stop pair that was
pushing one timing SAMPLE per batch, not per frame).
RetailPViewPassExecutor.DrawWalkLandCellBatch instead times its own
call with a raw Stopwatch.GetTimestamp() delta (no allocation) and
hands the ticks to the controller's new AccumulateWalkBatch;
RetailPViewRenderer.DrawWalkDrivenStatics calls the new
CompleteWalkTerrainFrame() exactly once, immediately after
driver.Replay finishes - "the end of the walk replay", where the
deleted whole-stage terrain leaf's own Begin()/Complete() bracket
used to close - which pushes ONE elapsed-time sample (even a
zero-batch frame pushes a zero sample: one sample per frame, not per
landscape turn) and publishes on the existing 5-second cadence.
TerrainRenderDiagnosticFacts gains a Draws field alongside
VisibleSlots (both were the same field before); TerrainModernRenderer
tracks its own per-frame WalkVisibleSlotCount/WalkDrawCount (a
HashSet<int>/int cleared in BeginFrame, populated by DrawLandCells),
and the diagnostics source reports those whenever the walk drew at
least one batch this frame, falling back to the non-walk Draw()
path's VisibleSlots otherwise (the two paths never both run in the
same frame). The [TERRAIN-DIAG] line's meaning (cpu_us per frame) is
unchanged, so the S3 section 9.5 before/after compare stays valid.

F4 - driver pins for the LandCell position (major). Three RunFrame-
level pins replace the deleted TERRAIN:0 pins: an outdoor-root
sequence (SKY, then one LANDCELL, driving RetailFrameWalk.
DrawLandscape directly with a one-view/zero-vertex WalkPortalView so
WalkLandscape.CheckBlocks' admission stays the same deterministic
"CY-only" test RetailFrameWalkTests already relies on, while still
satisfying WalkFrameDriver's real >=1-active-view fail-loud guard);
an interior-root test with one real exit view and one populated
block (SKY, LANDCELL, LFLUSH, SEALS, SHELL...) built on the existing
RunFrame_InteriorFloodWithExitView_... fixture; and the T4 batching
pin re-expressed for the F2 rule (OnLandCellTurn_
MergesAcrossLandblocksOverAnEmptyParticleTurn_RealSubmissionsSplit,
described above). The fake leaf's DrawLandCellBatch now logs
LANDCELL:<lb>:<side>:<idx>[,...] per batch and gains
HasRenderableEmittersInCell backed by an opt-out CellsWithoutEmitters
set (default true - has-emitters - so every pre-existing pin in the
file keeps its old unconditional-submission behavior unchanged).

F5 - no code change: the walk's in-view gate is unchanged; no
whole-block terrain re-added.

F6 - minor/notes: DrawLandCells' own comment now states the walk's
CheckBlocks/landcell_check admission is the sole terrain culling
authority (retail has no separate terrain frustum test); the
HandleLandscapeTurn comment's inverted claim is corrected (a FARTHER
building's punch survived because NEARER terrain was drawn BEFORE
it, not after - the interleave now draws it after, matching retail);
the "flat/directional-shadow paths" claim is corrected to the one
actual caller, WorldScenePassExecutor.DrawFlatTerrain (a directional-
shadow receiver selects its pipeline inside the SAME DrawRhi call,
not through a second caller); the cathedral order-trace token gains
the LOD side/index (":LC<lb>/<side>:<idx>"); T2's vacuous "no
TERRAIN event" assertion in RetailFrameWalkTests is replaced by a
comment pointing at the F4 driver-level pins; and the stale
"Confirmed OH5 defect" row in oh1-construction-landscape-contract.md
is retired with "FIXED by S3 chunk 3 (commit 671eb3ad4 + fix round
1)".

App hermetic lane: 6,795/6,795 (up from 671eb3ad4's 6,786 baseline -
net +9 tests: 3 F1 slot-key tests, 2 F4a/b driver RunFrame pins, 3
TerrainDrawDiagnosticsController walk-frame tests, plus the T4->F4c
rewrite and the RetailPViewPassExecutorTests split are net neutral).
InstalledDat lane: 241 passed, the same 3 accepted failures (2
pre-existing #383 layout fixture-drift tests, 1 TowerAscent
Status=KnownFailure) - unchanged from baseline. Core Vfx tests:
109/109 (108 baseline + 1 new HasRenderableEmittersInCell lifecycle
pin mirroring CopyRenderableEmittersInCell's own add/move/remove
test).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
Erik 2026-09-03 09:38:48 +02:00
parent 4ee2866a7b
commit 651badc2b9
15 changed files with 1003 additions and 131 deletions
Download patch file Download diff file Expand all files Collapse all files

272
tests/AcDream.App.Tests/Rendering/TerrainWalkSlotKeyNormalizationTests.cs Normal file
View file

@ -0,0 +1,272 @@
using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.Diagnostics.CodeAnalysis;
using System.Numerics;
using AcDream.App.Rendering;
using AcDream.App.Rendering.Gpu;
using AcDream.App.Rendering.Gpu.Vk;
using AcDream.App.Tests.Rendering.Gpu;
using AcDream.Content;
using AcDream.Core.Terrain;
using DatReaderWriter;
using DatReaderWriter.DBObjs;
using DatReaderWriter.Lib.IO;
using Xunit;
namespace AcDream.App.Tests.Rendering;
/// <summary>
/// S3 chunk 3 fix round 1 (§9.6 F1): <c>TerrainModernRenderer._idToSlot</c>
/// is keyed by the DAT landblock id <c>0xXXYYFFFF</c>
/// (<c>LandblockRenderPublisher.LandblockId =&gt; Build.Landblock.LandblockId</c>,
/// which <see cref="TerrainModernRenderer.AddLandblock"/> stores under
/// verbatim), while the walk hands <c>0xXXYY0000</c>
/// (<c>WalkLandBlock.LandblockId = bx&lt;&lt;24 | by&lt;&lt;16</c>). Before
/// this fix, <see cref="TerrainModernRenderer.DrawLandCells"/> normalized
/// nothing, so every walk-path lookup missed and the walk drew NO terrain.
/// This pins the fix end-to-end through the REAL RHI submission path (a
/// <see cref="RecordingGpuDevice"/>, not a mock of the lookup alone) — an
/// entry keyed by the walk's <c>0xA9B40000</c> convention must resolve the
/// SAME slot <see cref="TerrainModernRenderer.AddLandblock"/> published
/// under the DAT's <c>0xA9B4FFFF</c> convention, and an entry for a
/// genuinely unknown landblock must be a silent per-entry no-op rather than
/// a thrown exception or a spurious draw.
/// </summary>
public sealed class TerrainWalkSlotKeyNormalizationTests : IDisposable
{
private readonly RecordingGpuDevice _device = new();
private readonly GpuDeviceFrameLifetime _frameLifetime;
private readonly VulkanWorldPassScope _scope = new(sampleCount: 1);
private readonly TerrainAtlas _atlas;
private readonly TerrainModernRenderer _terrain;
public TerrainWalkSlotKeyNormalizationTests()
{
_frameLifetime = new GpuDeviceFrameLifetime(_device);
// A Region with no TerrainInfo takes BuildBackendNeutral's own
// documented single-white-fallback-layer branch — no installed DAT
// needed, and this suite stays hermetic.
_atlas = TerrainAtlas.BuildBackendNeutral(_device, new EmptyRegionDats());
_terrain = new TerrainModernRenderer(_device, _frameLifetime, _scope, _atlas, _device.Retirement);
}
private DrawScope BeginDraw()
{
_frameLifetime.BeginFrame();
IGpuFrame frame = _frameLifetime.CurrentFrame!;
IGpuPassEncoder pass = frame.BeginPass(
GpuPassDescription.BackbufferClear(
"s3-chunk3-fix-round-1-f1", Vector4.Zero, sampleCount: 1));
IDisposable publication = _scope.Publish(pass);
_device.Clear();
return new DrawScope(frame, pass, publication);
}
private readonly struct DrawScope(
IGpuFrame frame, IGpuPassEncoder pass, IDisposable publication) : IDisposable
{
public IGpuFrame Frame { get; } = frame;
public IGpuPassEncoder Pass { get; } = pass;
public void Dispose() => publication.Dispose();
}
private static LandblockMeshData MakeFullSizeMesh()
{
var vertices = new TerrainVertex[LandblockMesh.VerticesPerLandblock];
var indices = new uint[LandblockMesh.VerticesPerLandblock];
for (int i = 0; i < indices.Length; i++)
indices[i] = (uint)i;
return new LandblockMeshData(vertices, indices);
}
[Fact]
public void DrawLandCells_ResolvesTheDatSlotFromTheWalksLowWordZeroLandblockId()
{
// Stored under the DAT id (LandblockRenderPublisher's own
// convention, low word 0xFFFF).
_terrain.AddLandblock(0xA9B4FFFFu, MakeFullSizeMesh(), Vector3.Zero);
using DrawScope draw = BeginDraw();
_terrain.BeginFrame(frameSlot: 0);
// The walk's own convention (WalkLandBlock.LandblockId), low word
// 0x0000 — F1's normalization must still find the slot above.
_terrain.DrawLandCells(
Matrix4x4.Identity,
new (uint LandblockId, int SideCellCount, int CellIndex)[] { (0xA9B40000u, 8, 0) });
GpuRecordedMultiDrawIndirect call = Assert.Single(
_device.Calls.OfType<GpuRecordedMultiDrawIndirect>());
Assert.Equal(1u, call.DrawCount);
}
[Fact]
public void DrawLandCells_UnknownLandblockIsASilentNoOp()
{
using DrawScope draw = BeginDraw();
_terrain.BeginFrame(frameSlot: 0);
_terrain.DrawLandCells(
Matrix4x4.Identity,
new (uint LandblockId, int SideCellCount, int CellIndex)[] { (0xDEAD0000u, 8, 0) });
Assert.Empty(_device.Calls.OfType<GpuRecordedMultiDrawIndirect>());
Assert.Equal(0, _terrain.WalkDrawCount);
Assert.Equal(0, _terrain.WalkVisibleSlotCount);
}
[Fact]
public void DrawLandCells_KnownAndUnknownLandblocksInOneBatch_SubmitsOnlyTheKnownEntry()
{
_terrain.AddLandblock(0xA9B4FFFFu, MakeFullSizeMesh(), Vector3.Zero);
using DrawScope draw = BeginDraw();
_terrain.BeginFrame(frameSlot: 0);
_terrain.DrawLandCells(
Matrix4x4.Identity,
new (uint LandblockId, int SideCellCount, int CellIndex)[]
{
(0xDEAD0000u, 8, 0), (0xA9B40000u, 8, 1),
});
GpuRecordedMultiDrawIndirect call = Assert.Single(
_device.Calls.OfType<GpuRecordedMultiDrawIndirect>());
Assert.Equal(1u, call.DrawCount);
Assert.Equal(1, _terrain.WalkDrawCount);
Assert.Equal(1, _terrain.WalkVisibleSlotCount);
}
public void Dispose()
{
_terrain.Dispose();
_atlas.Dispose();
_device.Dispose();
}
/// <summary>Minimal hermetic <see cref="IDatReaderWriter"/> — every read
/// misses except <c>Get&lt;Region&gt;(0x13000000)</c>, which returns a
/// Region with no <c>TerrainInfo</c> so <see
/// cref="TerrainAtlas.BuildBackendNeutral"/> takes its documented
/// single-white-fallback-layer branch instead of throwing.</summary>
private sealed class EmptyRegionDats : IDatReaderWriter
{
private readonly Region _region = new();
private readonly StubDatabase _portal = new();
private readonly StubDatabase _highRes = new();
private readonly StubDatabase _language = new();
private readonly StubDatabase _cell = new();
public string SourceDirectory => string.Empty;
public IDatDatabase Portal => _portal;
public IDatDatabase Cell => _cell;
public ReadOnlyDictionary<uint, IDatDatabase> CellRegions { get; } =
new(new Dictionary<uint, IDatDatabase>());
public IDatDatabase HighRes => _highRes;
public IDatDatabase Language => _language;
public IDatDatabase Local => _language;
public ReadOnlyDictionary<uint, uint> RegionFileMap { get; } =
new(new Dictionary<uint, uint>());
public int PortalIteration => 0;
public int CellIteration => 0;
public int HighResIteration => 0;
public int LanguageIteration => 0;
public bool TryGetFileBytes(
uint regionId,
uint fileId,
ref byte[] bytes,
out int bytesRead)
{
bytesRead = 0;
return false;
}
public IEnumerable<uint> GetAllIdsOfType<T>() where T : IDBObj =>
Array.Empty<uint>();
public IEnumerable<IDatReaderWriter.IdResolution> ResolveId(uint id) =>
Array.Empty<IDatReaderWriter.IdResolution>();
public bool TrySave<T>(T obj, int iteration = 0) where T : IDBObj =>
throw new NotSupportedException();
public bool TrySave<T>(
uint regionId,
T obj,
int iteration = 0) where T : IDBObj =>
throw new NotSupportedException();
[return: MaybeNull]
public T Get<T>(uint fileId) where T : IDBObj
{
if (typeof(T) == typeof(Region) && fileId == 0x13000000u)
return (T)(object)_region;
return default;
}
public bool TryGet<T>(
uint fileId,
[MaybeNullWhen(false)] out T value) where T : IDBObj
{
value = Get<T>(fileId);
return value is not null;
}
public void Dispose()
{
}
private sealed class StubDatabase : IDatDatabase
{
public DatDatabase Db => throw new NotSupportedException();
public int Iteration => 0;
public IEnumerable<uint> GetAllIdsOfType<T>() where T : IDBObj =>
Array.Empty<uint>();
public bool TryGet<T>(
uint fileId,
[MaybeNullWhen(false)] out T value) where T : IDBObj
{
value = default;
return false;
}
public bool TryGetFileBytes(
uint fileId,
[MaybeNullWhen(false)] out byte[] value)
{
value = null;
return false;
}
public bool TryGetFileBytes(
uint fileId,
ref byte[] bytes,
out int bytesRead)
{
bytesRead = 0;
return false;
}
public bool TrySave<T>(T obj, int iteration = 0) where T : IDBObj =>
throw new NotSupportedException();
public void Dispose()
{
}
}
}
}