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

fix(render): S3 chunk 4 round 1 — loop-shape weather pin, one weather gate, dead terrain-clip stack deleted, sky drawn once

Browse source 
Campaign OVERHAUL S3 chunk 4 fix round 1 (docs/research/2026-09-01-overhaul/s3-walk-ownership-map.md
§10.4), on top of 6ba4b0b87. K1, K2, K3, K4, K6, K7, K8 (K5 was the lead's own plan-doc note, already
done).

K1 (blocking) — the Assert.Single pin over DrawLandscapeDynamicsPhase's DrawWeatherOnce call site
stayed green even wrapped in a foreach (it counts distinct call-site offsets, not loop shape). Added
a real LOOP-SHAPE pin reusing CompiledCallGraph.ReadBranches: asserts no backward branch (target
offset < its own offset) spans the DrawWeatherOnce call's IL offset — the shape every C#
for/foreach/while loop compiles to.

K2 (minor) — the Collect-time "OC" print fired on ctx.WeatherGateOpen alone while the Replay-time
draw independently re-derived clipAssembly.OutsideViewSlices.Length > 0, which could diverge on an
interior root whose landscape turn ran but whose reassembled outside-view slices ended up empty.
WalkFrameDriver now owns one WeatherTurnFired flag, set unconditionally inside OnWeatherTurn (so it
tracks "did the print's own gate hold" regardless of the transcript flag) and reset every
BeginFrame/AbortFrame; RetailPViewRenderer.DrawLandscapeDynamicsPhase gates DrawWeatherOnce on it
instead of re-deriving its own condition.

K3 (minor) — ClipFrame.SetTerrainClip (the terrain OutsideView writer) had no remaining caller after
chunk 4's original round and K4 below; deleted along with IWorldPassSurface.SetTerrainClip,
RhiWorldPassSurface.SetTerrainClip, the now-orphaned PublishTerrainClip helper, and PrepareClipFrame's
terrain-clip publish call. The terrain/sky shaders still declare the TerrainClip UBO (verified:
terrain_modern.vert, terrain_atmospheric.vert, sky.vert all read uTerrainClipCount/uTerrainClipPlanes),
so the section binding itself (WorldFrameSectionBinding.BindTerrainClip) stays — its existing
zeroed-ring fallback (for when nothing published the section) now binds that same all-zero disabled
block on every frame, identical bytes to the old permanent NoClip/Reset default.

K4 (major) — DrawWalkSky still looped the OUTDOOR case once per active landscape view under a doorway
scissor + BindTerrainClip + EnableClipDistances (the INTERIOR case already drew once unclipped, FW4
slice 6). Retail draws GameSky::Draw(sky,0) ONCE, unconditionally, before LScape::draw's block loop,
for both root kinds. Collapsed DrawWalkSky to one unconditional, unclipped draw; deleted
BeginDoorwayScissor and the RetailPViewPassExecutor.EnableClipDistances wrapper (both lost their only
caller). _surface.BeginScissor/EndScissor and IWorldPassSurface.EnableClipDistances stay:
RhiWorldPassSurface.ClearInteriorDepth still ends an active scissor, and WorldScenePassExecutor (the
separate flat-world path) still calls EnableClipDistances directly.

K6 (minor) — the punch-fan CPU/GPU equivalence pin hand-built a ClipViewSlice from ClipPlaneSet.From's
raw output, which could pass even if ClipFrameAssembler.Assemble's own packing/array-construction
diverged from that output. Rewrote it to build a real PortalVisibilityFrame and run it through
ClipFrameAssembler.Assemble, reading the planes back through assembly.OutsideViewSlices[0].Planes —
the exact outsideSlicesList.Add(new ClipViewSlice(slot, AabbOf(poly), planes)) path
ReassembleOutsideViewFromWalk (the walk's real interior-root producer) shares.

K7 (note) — restored the "a real look-in slice never reuses the reserved no-clip slot 0" assertion
the deleted VisibleClipSlotsInLookInTurn used to prove, via a new internal test-only accessor
(WalkFrameDriver.LookInSliceClipSlotAt) reading the same _lookInSlices storage
InteriorFloodViewClipPlanesAt resolves through _clipFrame.GetSlotPlanes(slice.ClipSlot).

K8 (note) — documented in DrawWeatherOnce's own comment that the weather mesh drawing before the rain
particles is this method's own call-order choice; retail's single GameSky::Draw(sky,1) imposes no
order between acdream's two substitutes.

Every new/changed pin's mutation was hand-verified this session (temporarily applied, ran the
specific test, confirmed the exact failing assertion, then reverted — see the parent task's structured
report for the four failing-assertion texts).

No register row added or removed — every change here deletes an acdream-only rule or repairs a pin;
none introduces a new deviation.

Full solution build: 0 warnings/0 errors. App hermetic (Lane!=InstalledDat/PreparedPackage/Live/
Manual/Timing/Windows/Linux/SystemFont & Purpose!=Diagnostic & Status!=KnownFailure): 6831/6831.
InstalledDat: 244 pass/1 skip/4 known — identical to 6ba4b0b87's own baseline (2x #383 layout tests,
TowerAscent, and the pre-existing #458 WalkLandscape.CheckBlocks block-visibility divergence,
unrelated to and untouched by this round).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
Erik 2026-09-03 14:04:53 +02:00
parent 08e864ea40
commit ff607a1e04
10 changed files with 345 additions and 155 deletions
Download patch file Download diff file Expand all files Collapse all files

62
tests/AcDream.App.Tests/Rendering/ClipFrameLayoutTests.cs
View file

@ -180,20 +180,11 @@ public class ClipFrameLayoutTests
AssertPlaneAt(bytes, baseOff + ClipFrame.CellClipPlanesOffset + i * 16, cps.Planes[i]);
}
[Fact]
public void SetTerrainClip_WritesCountAndPlanes_AtStd140Offsets()
{
var frame = ClipFrame.NoClip();
var p0 = new Vector4(0.3f, -0.4f, 0f, 0.1f);
var p1 = new Vector4(-0.6f, 0.8f, 0f, -0.2f);
frame.SetTerrainClip(new[] { p0, p1 });
var t = frame.TerrainBytesForTest;
Assert.Equal(2, ReadInt(t, 0)); // int count at offset 0
AssertPlaneAt(t, ClipFrame.CellClipPlanesOffset + 0, p0); // planes start at 16 under std140 too
AssertPlaneAt(t, ClipFrame.CellClipPlanesOffset + 16, p1);
}
// S3 chunk 4 fix round 1 (K3): SetTerrainClip_WritesCountAndPlanes_AtStd140Offsets
// is deleted along with ClipFrame.SetTerrainClip itself (no production
// writer remains). NoClip_TerrainBytes_Count0_AllZeros above still pins
// the permanent all-zero state SetTerrainClip used to be the only way
// to move off of.
private static void AssertPlaneAt(System.ReadOnlySpan<byte> bytes, int offset, Vector4 expected)
{
@ -255,15 +246,21 @@ public class ClipFrameLayoutTests
/// KEEP item 2 — punch fans: <c>RetailPViewPassExecutor.DrawWalkPunchFan</c>
/// reads its clip planes through <c>clipAssembly.OutsideViewSlices
/// [activeViewIndex].Planes</c> — <see cref="ClipViewSlice"/>'s
/// <c>Planes</c> field, which <c>ClipFrameAssembler.Assemble</c> sets
/// DIRECTLY to <c>cps.PlaneArray</c> (no packed-byte round trip at
/// all). This pin is therefore the more fundamental of the two: it
/// proves <see cref="ClipPlaneSet.From(CellView)"/>'s own output is
/// geometrically correct for the polygon it was built from — the
/// property BOTH keep items ultimately depend on. Same synthetic-view
/// method as the exit-seal pin, a different (non-axis-aligned)
/// synthetic polygon so the two pins are not testing the identical
/// input.
/// <c>Planes</c> field. S3 chunk 4 fix round 1 (K6): this pin now builds
/// that slice through the REAL production assembly —
/// <c>ClipFrameAssembler.Assemble</c>'s own
/// <c>outsideSlicesList.Add(new ClipViewSlice(slot, AabbOf(poly),
/// planes))</c> line, the exact construction
/// <c>ReassembleOutsideViewFromWalk</c> (the walk's real interior-root
/// producer) shares — instead of hand-constructing a
/// <see cref="ClipViewSlice"/> directly from <see
/// cref="ClipPlaneSet.From(CellView)"/>'s raw output: a hand-built slice
/// could pass even if Assemble's own packing/array-construction diverged
/// from that raw output, which is exactly the gap a prior round's
/// three-lens review found (a hand-built <c>ClipViewSlice</c> is not
/// proof the production path builds the same one). Same synthetic-view
/// helper as the exit-seal pin, a different (non-axis-aligned) synthetic
/// polygon so the two pins are not testing the identical input.
/// </summary>
[Fact]
public void ClipViewSlicePlanes_PunchFanPath_EqualsCpuViewPolygonEdgePlanes_ForASyntheticView()
@ -272,16 +269,19 @@ public class ClipFrameLayoutTests
[
new(0f, 0.6f), new(-0.6f, -0.4f), new(0.5f, -0.5f), new(0.7f, 0.2f),
];
var cv = new CellView();
cv.Add(new ViewPolygon(verts));
ClipPlaneSet cps = ClipPlaneSet.From(cv);
Assert.True(cps.Count >= 3);
// ClipFrameAssembler.Assemble: `planes = cps.PlaneArray; slices.Add(
// new ClipViewSlice(slot, AabbOf(poly), planes));` — the SAME array
// reference DrawWalkPunchFan reads through
// The EXACT production assembly path: ClipFrameAssembler.Assemble
// packs the outside_view polygon into a slot and constructs the
// ClipViewSlice DrawWalkPunchFan reads back through
// clipAssembly.OutsideViewSlices[activeViewIndex].Planes.
var slice = new ClipViewSlice(0, default, cps.PlaneArray);
var pvFrame = new PortalVisibilityFrame();
pvFrame.OutsideView.Add(new ViewPolygon(verts));
var frame = ClipFrame.NoClip();
ClipFrameAssembly assembly = ClipFrameAssembler.Assemble(frame, pvFrame);
ClipViewSlice slice = Assert.Single(assembly.OutsideViewSlices);
Assert.True(slice.Planes.Length >= 3);
AssertEveryEdgeMidpointLiesOnSomeGpuPlane(verts, slice.Planes);
}

55
tests/AcDream.App.Tests/Rendering/RetailPViewPassExecutorTests.cs
View file

@ -157,11 +157,15 @@ public sealed class RetailPViewPassExecutorTests
/// <c>DrawLandscapeSliceLate</c> leaf, one call per active landscape
/// view) is deleted — <c>DrawLandscapeDynamicsPhase</c> now calls
/// <see cref="RetailPViewPassExecutor.DrawWeatherOnce"/> exactly once,
/// unconditionally, with no loop of any kind around it (a deleted
/// symbol cannot be re-introduced without a compile error, so this pin
/// only needs to rule out a NEW multi-call path). MUTATION: adding a
/// second call site (e.g. reintroducing a per-slice loop around a new
/// leaf) makes <c>Assert.Single</c> fail.
/// unconditionally, with no loop of any kind around it. This
/// <c>Assert.Single</c> alone proved insufficient at fix round 1 (K1):
/// it counts DISTINCT call-site offsets, so it stays green even with a
/// <c>foreach</c> wrapped around the one call site (the exact round-1
/// regression this file's review caught) — see
/// <see cref="DrawLandscapeDynamicsPhase_DrawWeatherOnceCallSiteHasNoEnclosingBackwardBranch"/>
/// for the pin that actually rules that out. Kept as a cheap first-line
/// check: MUTATION: adding a second, textually distinct call site (e.g.
/// a duplicated call, not a loop) makes <c>Assert.Single</c> fail.
/// </summary>
[Fact]
public void DrawLandscapeDynamicsPhase_CallsDrawWeatherOnceExactlyOnce()
@ -177,6 +181,47 @@ public sealed class RetailPViewPassExecutorTests
&& call.Target.Name == nameof(RetailPViewPassExecutor.DrawWeatherOnce));
}
/// <summary>
/// S3 chunk 4 fix round 1 (K1, blocking): the real loop-shape pin.
/// <see cref="DrawLandscapeDynamicsPhase_CallsDrawWeatherOnceExactlyOnce"/>'s
/// <c>Assert.Single</c> over call-site offsets still passes when the ONE
/// call site sits inside a <c>foreach</c> — an IL-offset ORDER pin has
/// now failed three times to be discriminating for this exact class of
/// regression, so this asks the LOOP-SHAPE question directly: does any
/// BACKWARD branch (a branch whose target offset is lower than its own
/// offset — the shape every C# loop compiles to, whether
/// <c>for</c>/<c>foreach</c>/<c>while</c>) enclose the
/// <c>DrawWeatherOnce</c> call's own IL offset? A call sitting strictly
/// between a backward branch's target and its own offset is inside that
/// loop's body and can run more than once per method invocation; a call
/// outside every backward branch's span cannot. MUTATION: wrap the call
/// in <c>foreach (var slice in clipAssembly.OutsideViewSlices)</c> —
/// the compiled <c>foreach</c> emits a backward branch (the
/// condition-check jump back to the loop body) whose span now contains
/// the call's offset, so this test fails; restore the single
/// unconditional call to make it pass again.
/// </summary>
[Fact]
public void DrawLandscapeDynamicsPhase_DrawWeatherOnceCallSiteHasNoEnclosingBackwardBranch()
{
MethodInfo method = typeof(RetailPViewRenderer).GetMethod(
"DrawLandscapeDynamicsPhase",
BindingFlags.Instance | BindingFlags.NonPublic)!;
IReadOnlyList<CompiledCall> calls = CompiledCallGraph.Read(method);
int callIndex = RequiredCallIndex(
calls,
typeof(RetailPViewPassExecutor),
nameof(RetailPViewPassExecutor.DrawWeatherOnce));
int callOffset = calls[callIndex].Offset;
IReadOnlyList<CompiledBranch> branches = CompiledCallGraph.ReadBranches(method);
Assert.DoesNotContain(
branches,
branch => branch.TargetOffset < branch.Offset
&& branch.TargetOffset <= callOffset
&& callOffset < branch.Offset);
}
/// <summary>
/// S3 chunk 1 fix round 2 (§11.6 H1): <see
/// cref="RetailPViewPassExecutor.ShouldDrawWeatherOnce"/> is

7
tests/AcDream.App.Tests/Rendering/Walk/WalkFrameDriverTests.cs
View file

@ -791,6 +791,13 @@ public sealed partial class WalkFrameDriverTests
0, in insideCone, 0.1f));
Assert.False(driver.SphereVisibleInLookInTurn(
0, new Vector3(10_000f, 0f, 10f), 0.1f));
// S3 chunk 4 fix round 1 (K7): restores the pin the deleted
// VisibleClipSlotsInLookInTurn used to prove — a real look-in slice
// is packed into its own GPU clip slot, never left on the reserved
// no-clip slot 0 (the exit-seal KEEP reads planes through the SAME
// slot mechanism, _clipFrame.GetSlotPlanes(slice.ClipSlot)).
uint clipSlot = driver.LookInSliceClipSlotAt(0);
Assert.NotEqual(0u, clipSlot);
Assert.Equal(2, clipFrame.SlotCount);
// The punch polygon reached the leaf renderer in WORLD space: the

75
tests/AcDream.App.Tests/Rendering/Walk/WalkFrameDriverTranscriptTests.cs
View file

@ -711,4 +711,79 @@ public sealed partial class WalkFrameDriverTests
RenderingDiagnostics.DumpWalkTranscriptEnabled = previous;
}
}
/// <summary>
/// Campaign OVERHAUL S3 chunk 4 fix round 1 (K2): proves the print's own
/// gate and the Replay-time draw's gate are now the SAME predicate —
/// <see cref="WalkFrameDriver.WeatherTurnFired"/> — instead of two
/// independently re-derived conditions that could diverge. Before this
/// fix, the print fired on <c>ctx.WeatherGateOpen</c> alone while the
/// Replay-time draw additionally required
/// <c>clipAssembly.OutsideViewSlices.Length &gt; 0</c>, re-derived
/// separately at Replay — on an interior root whose landscape turn ran
/// with the gate open but whose reassembled outside-view slices ended
/// up empty, the transcript could report a weather turn the frame never
/// actually drew. <see cref="WalkFrameDriver.WeatherTurnFired"/> is exactly the flag
/// <c>RetailPViewRenderer.DrawLandscapeDynamicsPhase</c> now gates
/// <c>DrawWeatherOnce</c> on, so this test's flag assertion doubles as
/// the draw-side pin: gate closed -&gt; the flag stays false (so
/// <c>DrawWeatherOnce</c> would not fire either) AND no "OC" line
/// prints; gate open -&gt; the flag becomes true (so
/// <c>DrawWeatherOnce</c> would fire) AND exactly one "OC" line prints.
/// </summary>
[Theory]
[InlineData(false)]
[InlineData(true)]
public void Collect_WeatherTurnFiredMatchesThePrintedOcLineExactly(bool weatherGateOpen)
{
bool previous = RenderingDiagnostics.DumpWalkTranscriptEnabled;
RenderingDiagnostics.DumpWalkTranscriptEnabled = true;
TextWriter originalOut = Console.Out;
var capture = new StringWriter();
try
{
Console.SetOut(capture);
using var fx = new DispatcherFixture();
const uint viewerCellId = 0xF4180003u; // low word < 0x100 -> outdoor root
var ctx = new TestContext
{
WeatherGateOpen = weatherGateOpen,
ViewerCellId = viewerCellId,
};
var landscape = new WalkLandscape { MidWidth = 1, Blocks = new WalkLandBlock?[1] };
var leaf = new RecordingLeafRenderer(new List<string>());
var driver = new WalkFrameDriver(fx.Dispatcher, leaf, new FakeWorldData());
var walk = new RetailFrameWalk();
driver.Collect(
walk, viewerCellId, null, landscape, ctx, Matrix4x4.Identity, Vector3.Zero);
// The one predicate the Replay-time draw now reads instead of
// re-deriving its own condition — checked before the second
// Collect call resets it for the next frame.
Assert.Equal(weatherGateOpen, driver.WeatherTurnFired);
// A second Collect call — WalkOracleTrace.Parse (like every
// real capture) discards the final in-progress frame.
driver.Collect(
walk, viewerCellId, null, landscape, ctx, Matrix4x4.Identity, Vector3.Zero);
Console.Out.Flush();
string[] lines = capture.ToString()
.Split('\n', StringSplitOptions.RemoveEmptyEntries)
.Select(l => l.TrimEnd('\r'))
.ToArray();
IReadOnlyList<WalkOracleFrame> frames = WalkOracleTrace.Parse(lines);
WalkOracleFrame frame = Assert.Single(frames);
bool printed = frame.Events.Any(e => e.Kind == WalkOracleEventKind.ObjectCellTurn);
Assert.Equal(weatherGateOpen, printed);
}
finally
{
Console.SetOut(originalOut);
RenderingDiagnostics.DumpWalkTranscriptEnabled = previous;
}
}
}