fix(render): S4 chunk 1 round 1 — the ±12 guard with retail's quantifier, seal count order, the PM/PC depth-event transcript gate
Three-lens review of c7ab5b6d8 found the round-0 port of
DrawPortalPolyInternal's degenerate-input guard inverted retail's
quantifier: it rejected a polygon when ANY vertex sat on ANY of the
four +/-12 planes. The PDB-paired bytes (VA 0x59BCD6-0x59BD66: four
per-plane fld/fcomp 12.0/fnstsw/test ah,0x44/jnp sequences, each
accumulating a "every vertex on this plane" boolean, cleared by the
first vertex NOT on it) and the named decomp's own nested-if reading
of the post-loop branch (0x59bd42-0x59bd6c) agree: retail rejects only
when EVERY local vertex lies on the SAME one of the four planes — a
polygon degenerate onto a plane, not one that merely touches a plane
at one vertex.
F1 — the quantifier. WalkVisibilityMath.IsRejectedByPortalPolygonBoundaryGuard
now tracks four booleans (everyVertexOnPlusX/MinusX/PlusY/MinusY), each
starting true and cleared by the first vertex that fails its own plane
test; the guard rejects iff any of the four survives the whole vertex
loop. T2 (WalkVisibilityMathTests.cs) is rewritten to the five §7
cases: every vertex on x=+12 -> rejected; every vertex on y=-12 ->
rejected; one vertex on x=+12, rest inside -> KEPT; every vertex at
11.999 -> kept; vertices split across x=+12 and y=+12 (no common
plane) -> kept. WalkFrameDriverTests' punch-side pin
(OnPunchGeometry_RejectsOnlyWhenEveryVertexSharesOnePlane_ButPunchesAnyOtherShape)
carries the same correction — its old "one vertex on the plane"
polygon was wrongly rejected under round 0; it punches now, and a new
all-on-plane polygon is added as the actual rejected case. The doc
comment's "essentially never" sentence is deleted and replaced with a
fresh DAT-scan citation (see datScan below).
MUTATION (verified): restore the any-vertex form -> the one-vertex and
split-plane admit cases fail (Assert.False Expected:False Actual:True,
x2; the WalkFrameDriverTests punch pin's Assert.Equal Expected:2
Actual:1). Delete the guard (always return false) -> the all-on-plane
reject cases fail (Assert.True Expected:True Actual:False, x3 —
plusX-plane, minusY-plane, and the vacuous-empty-polygon pin; the
punch pin's Assert.Equal Expected:2 Actual:3).
datScan (DAT counts backing F1's doc comment): a throwaway xunit test
(deleted after this commit — not part of the diff) walked the full
0x0000-0xFFFF landblock-prefix space (LandBlockInfo/EnvCell carry no
usable DatCollection.GetAllIdsOfType range — both are landblock-
relative — matching the method CellStructSurfaceConstructionInstalledDatTests
already established), resolved each EnvCell's Environment/CellStruct,
and for every CellPortal's polygon tested each vertex's local x/y
against +/-12. Against the installed client_cell_1.dat: 3,405
landblocks with cells, 1,854,237 total portal polygons. 2,889 polygons
have at least one vertex on a +/-12 plane; 2,163 have EVERY vertex on
the SAME plane — all 2,163 are EXIT portals (OtherCellId==0xFFFF, out
of 16,939 exit portals total); 0 interior (non-exit) portals ever
qualify (interior: 1,837,298 total, 340 any-vertex, 0 all-on-plane).
Duration ~15s. This exactly reproduces the review's cited numbers.
F2 — seal-path count order. RetailPViewPassExecutor's exit-seal loop
dropped the `localVertices.Length < 3` pre-filter that ran BEFORE both
the boundary guard and the `submitted` increment (round 0's order:
length check -> guard -> transform -> draw -> count). Retail's real
order is guard -> transform -> clip -> count, with the counter
incremented BEFORE polyClipFinish runs (0x59BD70-0x59BD74 precedes
0x59BDB0) — the counter records accepted ATTEMPTS, not successful GPU
fans. `submitted++` now runs immediately after the guard-gated
transform+EmitSeamMask, before DrawDepthFan (whose own `< 3` guard is
this port's stand-in for retail's post-clip `var_4 >= 3` check). A new
pin (RetailPViewPassExecutorTests.DrawExitPortalMask_CountsAnUnclippableTwoVertexPolygon_ButDrawsNothing)
constructs a real PortalDepthMaskRenderer (RecordingGpuDevice-backed)
and a constructor-bypassed executor (RuntimeHelpers.GetUninitializedObject,
the same pattern WalkOutsideViewReassemblyTests already uses) with a
2-vertex ordinary-coordinate portal polygon: DrawExitPortalMask returns
submitted=1 while zero draw calls reach the device.
MUTATION (verified): restore the `< 3` pre-filter ahead of the guard
-> the pin's Assert.Equal fails, Expected:1 Actual:0.
F3 — the depth-event transcript gate, automated. New
WalkAlphaDepthTrace.cs (beside WalkOracleTrace.cs) parses the four
`PM poly=<ptr> mode=<0|1> counterBefore=<hex>` / `PC ov=<n>
counter=<hex> fc=<0|1>` lines per oh-capture/*.alphadepth.log frame,
using the same F-marker frame delimiting as WalkOracleTrace. New
WalkTraceConformanceTests.AlphaDepthTranscript.cs (beside
WalkTraceConformanceTests.cs, which is now `partial`) replays each
pose's frame 2 through the real WalkFrameDriver/RetailFrameWalk over
WalkLandscapeDatBuilder-built installed-DAT world data, with:
- AlphaDepthCollectSink: an IWalkEventSink decorator forwarding every
hook to the real driver unchanged, while recording mode=1 punch
events (OnPunchGeometry, counterBefore = driver.PortalsDrawnCount
at COLLECT time — Collect finishes entirely before Replay starts,
so reading the counter at Replay time would already see the SAME
turn's post-reset value; holtburg-doorway-still's own capture
proves this: its punches show counterBefore=2 immediately followed
by its seals' counterBefore=0/1 in the identical turn) and every
PC tuple (Emit, WalkEventKind.DrawCells — fires for the root's own
turn AND every building look-in's re-entrant call, matching
retail's breakpoint not distinguishing them).
- AlphaDepthLeaf: an IWalkFrameLeafRenderer whose DrawExitSeals
reproduces DrawPortalDepthWrite's real per-portal loop (F1's guard,
F2's count-before-clip order) directly over
driver.InteriorFloodCells / WalkCell.Portals / .PortalPolygons —
the identical data the render-side production path reads — since
the interface gives DrawExitSeals no cell/portal parameter at all.
- Two Collect+Replay passes per pose (a fresh driver reaches the SAME
steady-state a running session already carries by the time
retail's capture began — holtburg-doorway-still and foundry-deep
both show a stable per-frame accepted-seal count that regenerates
identically every captured frame): the first pass's own output is
discarded, only the second is compared.
Five Facts (one per pose — the spec's "four" undercounts the five
*.alphadepth.log files actually present in oh-capture/):
AlphaDepthTranscript_CathedralArrival_MatchesRetailFrame2 — PASS
AlphaDepthTranscript_CathedralLeak_MatchesRetailFrame2 — PASS
AlphaDepthTranscript_FoundryDeep_MatchesRetailFrame2 — PASS
AlphaDepthTranscript_HoltburgDoorwayStill_MatchesRetailFrame2 — PASS
(proves the incrementing 0/1/2 accepted-seal sequence AND the
pre-reset counterBefore=2 punches, end to end, against real DAT
portal data)
AlphaDepthTranscript_TerraceEdge_MatchesRetailFrame2 — KnownFailure
(tagged [Trait("Status","KnownFailure")]): terrace-edge never runs
a qualifying interior turn in its own capture (every PC line reads
ov=0) — its punches' counterBefore=2 is a session value carried
over from BEFORE the capture even started (the file's own pre-"F 1"
content already reads 2; nothing inside the captured frames ever
touches the counter). A fresh two-pass replay from
PortalsDrawnCount=0 has no mechanism to derive that leftover value.
Printed divergence: expected [(mode=1,counterBefore=2) x2], actual
[(mode=1,counterBefore=0) x2]; the PC sequence (ov=0 both times)
matches exactly. An initial-condition gap in the fixture, not a
guard/count defect — the identical harness reproduces holtburg-
doorway-still's and foundry-deep's own steady states exactly from
cold starts.
MUTATION (verified): swap AlphaDepthLeaf's recorded seal mode from 0
to 1 -> AlphaDepthTranscript_HoltburgDoorwayStill_MatchesRetailFrame2
fails (Assert.Equal Collections differ at pos 5: expected
(0,0),(0,1)... actual (1,0),(1,1)...).
DispatcherFixture and DrawScope (WalkFrameDriverTests.cs) are widened
from private to internal so the new gate can drive a real
WbDrawDispatcher without duplicating that fixture.
F4 — manifest comment truth. VulkanShaderManifestTests.cs's
portal_depth.vert.spv re-pin comment had the ULP direction backward
(claimed the OLD 0x3F7FFFFE literal was NEARER the camera than
retail's real 0x3F7FFFEF constant; portal_depth.vert's own shader-
source comment already had this correct — FARTHER, larger z/w) and
cited T1 as living in WalkVisibilityMathTests when
PortalDepthVert_FarPunchConstant_MatchesRetailExactBits is in this
same file. Both corrected.
Gates: dotnet build tests/AcDream.App.Tests -c Release -> 0 warnings/0
errors. Hermetic lane (Lane!=InstalledDat&...&Status!=KnownFailure):
6829/6829 passed. InstalledDat lane (real client_cell_1.dat): 253
total, 247 passed, 5 failed — exactly the two #383 layout tests
(LayoutImporterMediaBearingChildSweepTests/LayoutImporterInvisibleSweepTests),
TowerAscentReplayTests' KnownFailure, Oh_doorway_still_first_frame_diff
(#458), and this round's own AlphaDepthTranscript_TerraceEdge
KnownFailure — no other divergence. Shader tests (VulkanShaderDescriptorContractTests
+ VulkanShaderManifestTests + RenderPackSpirvValidatorTests +
SkyVertexLayoutTests): 35/35. Register/divergence tests: 52/52.
dotnet build src/AcDream.App -c Release: 0 warnings/0 errors.
Register: no new AD row. F1/F2 are straight bug fixes toward retail
fidelity (not deviations); AD-119 (the C2 blend-vs-write-mask row)
does not describe the count order, so its neighbour needed no
correction per F2's own instruction.
Deviation from the fix-round spec's literal text: §7 F1 said "swap the
any-vertex hits instead of the four per-plane accumulators" as the
split-plane case's mutation — the code actually ported uses per-vertex
`if (x != 12f) flag = false;` clears, not a per-vertex OR; the mutation
verified above (restore the any-vertex FORM entirely) exercises the
same defect class and is the mutation actually run. §7 also names
"four" alphadepth.log fixtures where five exist on disk
(cathedral-arrival, cathedral-leak, foundry-deep,
holtburg-doorway-still, terrace-edge); all five are gated rather than
four, with cathedral-leak an unremarked addition alongside the four
the spec names.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
Erik
parent
d1e3e64f61
commit
5de42a12f3
9 changed files with 747 additions and 96 deletions
|
|
@ -1,11 +1,15 @@
|
|||
using System.Linq;
|
||||
using System.Numerics;
|
||||
using System.Reflection;
|
||||
using System.Reflection.Emit;
|
||||
using AcDream.App.Composition;
|
||||
using AcDream.App.Rendering;
|
||||
using AcDream.App.Rendering.Gpu;
|
||||
using AcDream.App.Rendering.Gpu.Vk;
|
||||
using AcDream.App.Rendering.Sky;
|
||||
using AcDream.App.Rendering.Walk;
|
||||
using AcDream.App.Tests.Architecture;
|
||||
using AcDream.App.Tests.Rendering.Gpu;
|
||||
|
||||
namespace AcDream.App.Tests.Rendering;
|
||||
|
||||
|
|
@ -479,6 +483,126 @@ public sealed class RetailPViewPassExecutorTests
|
|||
|| branch.OpCode == OpCodes.Brfalse || branch.OpCode == OpCodes.Brfalse_S));
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// S4-c1 fix round 1, F2: retail increments <c>portalsDrawnCount</c>
|
||||
/// (0x59BD70-0x59BD74) BEFORE <c>polyClipFinish</c> runs (0x59BDB0) —
|
||||
/// the counter records accepted ATTEMPTS, not successful GPU fans
|
||||
/// (<c>oh1-depth-lifecycle.md</c>'s "Far-Z punches and true-depth exit
|
||||
/// seals" section). A polygon that survives the boundary guard but has
|
||||
/// fewer than 3 vertices is still COUNTED by
|
||||
/// <see cref="RetailPViewPassExecutor.DrawExitPortalMask"/>'s returned
|
||||
/// <c>submitted</c> total, even though
|
||||
/// <see cref="PortalDepthMaskRenderer.DrawDepthFan"/> draws nothing (its
|
||||
/// own <c>< 3</c> guard stands in for retail's post-clip
|
||||
/// <c>var_4 >= 3</c> check). Round 0 dropped the count too — a
|
||||
/// <c>< 3</c> continue ahead of both the boundary guard and the
|
||||
/// count — never observed on authored dat data (every real portal
|
||||
/// polygon has >= 3 vertices) but the wrong order all the same.
|
||||
/// MUTATION: move the increment back to AFTER
|
||||
/// <see cref="PortalDepthMaskRenderer.DrawDepthFan"/> (or restore the
|
||||
/// old <c>localVertices.Length < 3</c> pre-filter) — this pin's
|
||||
/// synthetic 2-vertex polygon is no longer counted and the assertion
|
||||
/// fails (<c>submitted</c> comes back 0, not 1).
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public void DrawExitPortalMask_CountsAnUnclippableTwoVertexPolygon_ButDrawsNothing()
|
||||
{
|
||||
var cell = new LoadedCell
|
||||
{
|
||||
CellId = 0xA9B40105u,
|
||||
WorldTransform = Matrix4x4.Identity,
|
||||
};
|
||||
cell.Portals.Add(new CellPortalInfo(OtherCellId: 0xFFFF, PolygonId: 0, Flags: 0, OtherPortalId: 0));
|
||||
// Ordinary (non-degenerate) coordinates — the boundary guard admits
|
||||
// this polygon — but only TWO vertices: retail's post-clip
|
||||
// `var_4 >= 3` check (this port's DrawDepthFan `< 3` guard) drops
|
||||
// the fan submission even though the count already happened.
|
||||
cell.PortalPolygons.Add(
|
||||
[
|
||||
new Vector3(1f, 1f, 0f),
|
||||
new Vector3(2f, 1f, 0f),
|
||||
]);
|
||||
|
||||
using var device = new RecordingGpuDevice();
|
||||
var frames = new GpuDeviceFrameLifetime(device);
|
||||
var scope = new VulkanWorldPassScope(sampleCount: 1);
|
||||
using var portalDepthMask = new PortalDepthMaskRenderer(device, frames, scope);
|
||||
var diagnostics = new WorldRenderDiagnostics(new NeverCalledGlStateReader(), new NeverCalledDiagnosticLog());
|
||||
|
||||
// DrawPortalDepthWrite only reads _portalDepthMask, frame.Cells,
|
||||
// frame.RootCell.IsOutdoorNode, frame.ViewProjection, and
|
||||
// _diagnostics (short-circuited off by RenderingDiagnostics.
|
||||
// ProbeSeamDrawEnabled's default-off value) — the same
|
||||
// constructor-bypass pattern WalkOutsideViewReassemblyTests already
|
||||
// uses for exercising a real leaf without a full GL/DAT renderer
|
||||
// graph.
|
||||
var executor = (RetailPViewPassExecutor)System.Runtime.CompilerServices.RuntimeHelpers
|
||||
.GetUninitializedObject(typeof(RetailPViewPassExecutor));
|
||||
typeof(RetailPViewPassExecutor)
|
||||
.GetField("_portalDepthMask", BindingFlags.NonPublic | BindingFlags.Instance)!
|
||||
.SetValue(executor, portalDepthMask);
|
||||
typeof(RetailPViewPassExecutor)
|
||||
.GetField("_diagnostics", BindingFlags.NonPublic | BindingFlags.Instance)!
|
||||
.SetValue(executor, diagnostics);
|
||||
|
||||
var root = new LoadedCell { CellId = 0xF4180003u, IsOutdoorNode = false };
|
||||
RetailPViewFrameInput frame = new RetailPViewFrameInput().Reset(
|
||||
rootCell: root,
|
||||
nearbyBuildingCells: null,
|
||||
viewerEyePos: Vector3.Zero,
|
||||
viewProjection: Matrix4x4.Identity,
|
||||
cells: new SingleCellSource(cell),
|
||||
camera: null!,
|
||||
cameraWorldPosition: Vector3.Zero,
|
||||
frustum: null,
|
||||
playerLandblockId: null,
|
||||
animatedEntityIds: null,
|
||||
renderCenterLbX: 0,
|
||||
renderCenterLbY: 0,
|
||||
renderRadius: 0,
|
||||
landblockEntries: Array.Empty<(uint, Vector3, Vector3,
|
||||
IReadOnlyList<AcDream.Core.World.WorldEntity>,
|
||||
IReadOnlyDictionary<uint, AcDream.Core.World.WorldEntity>?)>(),
|
||||
renderSky: false,
|
||||
renderWeather: false,
|
||||
dayFraction: 0f,
|
||||
activeDayGroup: null,
|
||||
skyKeyframe: default,
|
||||
environOverrideActive: false,
|
||||
viewerCellId: 0,
|
||||
playerCellId: 0,
|
||||
playerViewPosition: Vector3.Zero,
|
||||
cameraView: Matrix4x4.Identity,
|
||||
cameraCellResolution: default);
|
||||
|
||||
int drawsBefore = device.Calls.OfType<GpuRecordedDraw>().Count();
|
||||
int submitted = executor.DrawExitPortalMask(frame, cell.CellId, ReadOnlySpan<Vector4>.Empty);
|
||||
int drawsAfter = device.Calls.OfType<GpuRecordedDraw>().Count();
|
||||
|
||||
Assert.Equal(1, submitted);
|
||||
Assert.Equal(drawsBefore, drawsAfter);
|
||||
}
|
||||
|
||||
private sealed class SingleCellSource(LoadedCell cell) : IRetailPViewCellSource
|
||||
{
|
||||
public LoadedCell? Find(uint cellId) => cellId == cell.CellId ? cell : null;
|
||||
}
|
||||
|
||||
private sealed class NeverCalledGlStateReader : IRenderGlStateReader
|
||||
{
|
||||
public RenderGlStateSnapshot CaptureState() =>
|
||||
throw new InvalidOperationException("EmitSeamMask must short-circuit before reading GL state.");
|
||||
|
||||
public RenderGlScissorSnapshot CaptureScissor() =>
|
||||
throw new InvalidOperationException("EmitSeamMask must short-circuit before reading GL state.");
|
||||
}
|
||||
|
||||
private sealed class NeverCalledDiagnosticLog : IRenderFrameDiagnosticLog
|
||||
{
|
||||
public void WriteLine(string message) =>
|
||||
throw new InvalidOperationException("EmitSeamMask must short-circuit before logging (ProbeSeamDrawEnabled defaults off).");
|
||||
}
|
||||
|
||||
private static int RequiredCallIndex(
|
||||
IReadOnlyList<CompiledCall> calls,
|
||||
Type declaringType,
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue