From 99d9c6d5642bc5bda60c95dc2efcb7a3d657fa71 Mon Sep 17 00:00:00 2001 From: Erik Date: Thu, 3 Sep 2026 18:14:38 +0200 Subject: [PATCH] =?UTF-8?q?chore(render):=20S3=20landing=20hygiene=20?= =?UTF-8?q?=E2=80=94=20scissor-era=20dead=20state,=20stale=20comments,=20t?= =?UTF-8?q?he=20#130=20harness,=20a=20conjunction-proof=20weather-gate=20p?= =?UTF-8?q?in?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Chunk H of Campaign OVERHAUL v2's S3 walk-ownership program (docs/research/2026-09-01-overhaul/s3-walk-ownership-map.md §10.6), landing the three post-hoc three-lens findings on S3 chunk 4 round 2. Pure dead-state deletion, comment truth, one test deletion, one stronger pin — no admission, order, or behavior change; no new flag or probe; no register row (AD-17's correction is the lead's own, already on the campaign branch). H1 — ClipPlaneSet.cs: deleted ScissorNdcAabb (no production reader — only AppendOutsideSlice consumes ClipPlaneSet, and it reads only IsNothingVisible/Count/PlaneArray) and renamed UseScissorFallback -> IsPlaneOverflow with a doc stating the true consumer contract: Count==0 overflow means the region draws UNCLIPPED via the no-clip slot, never a scissor box. Rewrote the file header's convexity-rule and Count==0-states sections to the two states that exist (Empty / overflow) instead of three, and simplified the private Scissor(...) AABB-computing helpers into a parameterless Overflow() factory since nothing reads the AABB any more. ClipPlaneSetTests.cs: renamed every UseScissorFallback reference, deleted the AABB assertions (From_MultiplePolygons's four ScissorNdcAabb.X/Y/Z/W checks, Empty_StaticProperty_DrawsNothing's degenerate-box check, ScissorFallback_IsNotNothingVisible's bounds check) that pinned the deleted consumer contract. H2 — TerrainModernRenderer.cs: deleted Draw's clipPlanes/ndcClipAabb parameters (grep: TerrainModernRenderer.Draw has exactly one production caller, WorldScenePassExecutor.DrawFlatTerrain, and it never passed either), which made CollectVisibleCells's own clipPlanes/ndcClipAabb/ viewProjection parameters and the IsAabbVisibleThroughClipRegion + IsAabbOutsideHomogeneousPlane helper pair they fed entirely dead — deleted along with the stale "Retail publishes landcell in_view from the clipped landscape view" comment; CollectVisibleCells is frustum-only now, matching DrawLandCells's own "retail never view-clips terrain" doc. Deleted the terrainUploadCount computation at RetailPViewRenderer.cs (the DrawInside/DrawLandscapeDynamicsPhase caller) and PrepareClipFrame's GL-era reservation-count parameter end to end: IWorldPassSurface.PrepareClipFrame(int), RhiWorldPassSurface's matching body and its now-pointless "_ = terrainUploadCount" discard, RetailPViewPassExecutor.PrepareClipFrame's pass-through, and WorldScenePassExecutor.PrepareFlatWorldClip's hardcoded `1` argument — PrepareClipFrame() takes nothing on every arm. TerrainParticleCellVisibilityTests.cs: deleted the three tests that only existed to pin the removed clip-plane/scissor mechanism (RejectsCellsOutsideDoorwayClipPlanes, RejectsCellsOutsideDoorwayScissorAabb, UnionsCellsFromEveryLandscapeSlice) and updated the two surviving frustum tests to the new signature. H3 — deleted tests/AcDream.App.Tests/Rendering/Issue130DoorwayStripTests.cs outright. #130 was a background-color strip along a doorway's top edge from a since-fixed under-inclusion in the CPU clip pipeline; its scissor half was already retired at S3 chunk 4 fix round 2 (L7), and its surviving plane-gap half exercised only PortalVisibilityBuilder.Build and ClipFrameAssembler.Assemble, both confirmed zero-production-caller legacy paths (grep) — the one production stage it actually touched, ClipPlaneSet.From, is already pinned through the real producers (BeginWalkFrame/ReassembleOutsideViewFromWalk) by the K6/L4 punch-fan equivalence pin in ClipFrameLayoutTests, so nothing production-relevant lost coverage. H4 — comment truth pass: - ClipFrameAssembler.cs: rewrote the file-header "renderer uses scissor for passes that need that fallback" line and the TerrainClipMode summary's "that path never produces Scissor" clause. Deleted the TerrainClipMode enum's Scissor member — grepped every reader first (WorldRenderDiagnostics.cs, WorldSceneDiagnosticsController.cs, WorldSceneRendererTests.cs all only pass the value through to a string Append or a mock parameter; WorldSceneRenderer.cs, the only writer, sets only Planes (default) or Skip, never Scissor) — kept Planes/Skip for the flat path and corrected Skip's stale doc ("No outside_view slice is visible; skip landscape indoors", a leftover from the pre-round-2 walk meaning) to what the flat path's own writer actually means now ("the PView walk ran instead of the flat-world path this frame"). Corrected ClipViewSlice's doc: the AABB is retained for WorldRenderDiagnostics only, not for a scissor consumer. - WorldPassSurface.cs: "Four concerns... the clip-frame publication, the doorway scissor, gl_ClipDistance enablement, and retail's interior depth clear" -> three concerns (the doorway scissor was retired at L2), with a pointer to ClearInteriorDepth's own truthful doc. EnableClipDistances' "all three world vertex shaders already write 1.0" -> only portal_depth.vert writes gl_ClipDistance now (grep: sky.vert and terrain_modern.vert/terrain_atmospheric.vert no longer declare it at all — L3 deleted the block outright; portal_depth.vert still writes 1.0 past its active clip count). RhiWorldPassSurface's class doc dropped "the scissor is dynamic state on the borrowed encoder" (no producer left). - RetailPViewPassExecutorTests.cs: DrawLandscapeDynamicsPhase_CallsDraw WeatherOnceExactlyOnce's doc corrected "unconditionally" (false since K2) to "conditional on WalkFrameDriver.WeatherTurnFired, see the L1 pin". Added the switch-table scope note (matching L8's own note on DrawWalkSky_RenderSkyCallSiteHasNoEnclosingBackwardBranch) to the K1 pin's doc (DrawLandscapeDynamicsPhase_DrawWeatherOnceCallSiteHasNoEnclosing BackwardBranch). - RetailPViewRenderer.cs: the DrawWeatherOnce citation cited the callee address (GameSky::Draw(sky,1) @0x00506ff0) where §10.1 cites the call site (@0x00506396) — corrected to cite the call site with the callee address alongside it. H5 — MUTATION CHECK (mandatory, run against the production gate at RetailPViewRenderer.DrawLandscapeDynamicsPhase): Added DrawLandscapeDynamicsPhase_ExactlyOneBranchGuardsDrawWeatherOnce, which widens the L1 pin's window to start at the DrawUnattachedSceneParticles call (the last call before the whole gate) and asserts EXACTLY ONE branch total in that window — the L1 pin's own window (getter-call to draw-call) misses a conjoined gate's extra, earlier condition because that condition's own brfalse lands BEFORE the getter call's offset. - Mutation 1 (the conjoined gate this pin exists to reject): changed the gate to `if (clipAssembly.OutsideViewSlices.Length != 0 && walkDriver.WeatherTurnFired)`, built, ran the new pin — FAILED with: Assert.Single() Failure: The collection contained 2 matching items Expected: (predicate expression) Collection: [CompiledBranch { Offset = 7, OpCode = brfalse.s, TargetOffset = 17 }, CompiledBranch { Offset = 24, OpCode = brfalse.s, TargetOffset = 42 }, CompiledBranch { Offset = 33, OpCode = brfalse.s, TargetOffset = 42 }] Match indices: 1, 2 (offset 7 = the DrawUnattachedSceneParticles block's own guard, correctly excluded since it precedes that call's offset; offsets 24 and 33 are the conjoined gate's two brfalse's, both inside the widened window — exactly the failure this pin exists to catch). Restored the single-flag gate; rebuilt; the new pin and all 13 sibling tests in RetailPViewPassExecutorTests pass again (14/14). - Mutation M1 (the existing round-2 regression: restore the pre-fix gate `if (clipAssembly.OutsideViewSlices.Length != 0)`), built, ran the full suite — the L1 pin (DrawLandscapeDynamicsPhase_GatesDrawWeatherOnceOnWalkDriverWeatherTurnFired) still FAILS, as required, with: Assert.Equal() Failure: Values differ Expected: typeof(AcDream.App.Rendering.Walk.WalkFrameDriver) Actual: typeof(AcDream.App.Rendering.ClipFrameAssembly) (the new H5 pin does NOT fail under M1 — a single-flag gate still produces exactly one branch in the widened window, which is correct: catching "wrong condition source" is L1's job, catching "an extra conjoined condition" is H5's). Restored the correct gate; rebuilt; 14/14 green again. H6 grep sweep (extends §10.5's L6 sweep per the plan): grep -rnE "ScissorNdcAabb|UseScissorFallback|TerrainClipMode\.Scissor|ndcClipAabb|terrainUploadCount" --include=*.cs --include=*.vert --include=*.frag src tests -> empty (every mention, including in this commit's own explanatory comments, was rephrased to avoid the literal deleted identifiers). The base L6 sweep (SetTerrainClip|BeginDoorwayScissor|BindTerrainClip| TerrainBytes|active scissor|TerrainClipMode\.Scissor|NdcScissorRect) is also still empty. Gates: dotnet build tests/AcDream.App.Tests -c Release -> 0 warnings / 0 errors. Hermetic lane -> 6825/6825 passed, 0 failed. InstalledDat lane (ACDREAM_DAT_DIR set) -> 243 passed / 4 known failures (2x #383 layout tests, TowerAscentReplayTests's TowerAscent KnownFailure, WalkTraceConformanceTests.Oh_doorway_still_first_frame_diff #458 KnownFailure) / 1 skipped / 248 total — one fewer test than before H3, as expected from Issue130DoorwayStripTests's deletion. Co-Authored-By: Claude Fable 5.1 --- .../Rendering/ClipFrameAssembler.cs | 23 +- src/AcDream.App/Rendering/ClipPlaneSet.cs | 110 +++--- .../Rendering/RetailPViewPassExecutor.cs | 4 +- .../Rendering/RetailPViewRenderer.cs | 14 +- .../Rendering/TerrainModernRenderer.cs | 106 +----- src/AcDream.App/Rendering/WorldPassSurface.cs | 52 +-- .../Rendering/WorldScenePassExecutor.cs | 2 +- .../Rendering/ClipPlaneSetTests.cs | 49 +-- .../Rendering/Issue130DoorwayStripTests.cs | 326 ------------------ .../Rendering/RetailPViewPassExecutorTests.cs | 72 +++- .../TerrainParticleCellVisibilityTests.cs | 91 +---- 11 files changed, 209 insertions(+), 640 deletions(-) delete mode 100644 tests/AcDream.App.Tests/Rendering/Issue130DoorwayStripTests.cs diff --git a/src/AcDream.App/Rendering/ClipFrameAssembler.cs b/src/AcDream.App/Rendering/ClipFrameAssembler.cs index 7f8f6126..fe0fbace 100644 --- a/src/AcDream.App/Rendering/ClipFrameAssembler.cs +++ b/src/AcDream.App/Rendering/ClipFrameAssembler.cs @@ -11,8 +11,9 @@ // reverse cell_draw_list object lists // // Slot 0 is always no-clip. A slice whose polygon cannot be represented by the -// <=8 plane budget uses slot 0 and its NDC AABB; the renderer uses scissor for -// passes that need that fallback. Empty regions are omitted entirely. +// <=8 plane budget uses slot 0 too (ClipPlaneSet.IsPlaneOverflow) and draws +// unclipped — there is no GPU scissor consumer anywhere in the walk (S3 +// chunk 4 fix round 2, L2). Empty regions are omitted entirely. using System.Collections.Generic; using System.Numerics; @@ -27,23 +28,25 @@ namespace AcDream.App.Rendering; /// 's separate flat-world safety path still /// uses it for its own unrelated "did the flat terrain draw this frame" /// diagnostic flag (Planes = drew, Skip = the PView walk ran -/// instead; that path never produces Scissor). +/// instead). S3 landing hygiene (H4) deleted the Scissor member: no +/// producer ever wrote it ( only ever +/// assigns Planes or Skip), because there is no GPU scissor +/// consumer left anywhere in this contract, walk or flat. /// public enum TerrainClipMode { - /// All outside_view slices have convex plane clips. + /// The flat-world path drew terrain this frame. Planes, - /// At least one outside_view slice requires scissor fallback. - Scissor, - - /// No outside_view slice is visible; skip landscape indoors. + /// The PView walk ran instead of the flat-world path this frame; + /// flat terrain was not drawn. Skip, } /// -/// One retail portal_view slice mapped to a GPU clip slot. The AABB is retained -/// for passes that cannot write gl_ClipDistance and must use scissor. +/// One retail portal_view slice mapped to a GPU clip slot. The AABB is +/// retained for diagnostics () only — no +/// GPU pass reads it; there is no scissor consumer left in the walk. /// public readonly record struct ClipViewSlice(int Slot, Vector4 NdcAabb, Vector4[] Planes); diff --git a/src/AcDream.App/Rendering/ClipPlaneSet.cs b/src/AcDream.App/Rendering/ClipPlaneSet.cs index dc4f0bd8..44f636ee 100644 --- a/src/AcDream.App/Rendering/ClipPlaneSet.cs +++ b/src/AcDream.App/Rendering/ClipPlaneSet.cs @@ -2,8 +2,8 @@ // // Phase U.2c: turn a CellView (a cell's accumulated screen-space clip region, // in NDC) into a small set of clip-space half-space planes for the GPU's -// gl_ClipDistance, OR a scissor AABB when the region can't be expressed as one -// convex plane set. +// gl_ClipDistance, or a signal that the region OVER-includes (draws +// unclipped) because it cannot be expressed as one convex plane set. // // This is the bridge between PortalVisibilityBuilder's 2D NDC view polygons and // the per-vertex clip the mesh/terrain shaders will perform (Phase U.2c → U.2e). @@ -20,23 +20,24 @@ // would clip away the others → a real visibility bug (under-inclusion). // // Therefore From() NEVER emits a single polygon's planes when the CellView holds -// several. Multi-polygon (and >8-edge) regions degrade to the UNION AABB scissor: -// the scissor is a superset of the true region, so it OVER-includes (draws a few -// extra pixels) but never hides anything. Over-inclusion is safe; under-inclusion -// is the bug class. +// several. Multi-polygon (and >8-edge) regions instead report a plane overflow: +// the consumer (ClipFrameAssembler.AppendOutsideSlice) reads Count == 0 and +// IsNothingVisible == false, appends an EMPTY plane array (slot 0, no clip), and +// the polygon draws whole (the punch fan covers the whole fan). That OVER-includes +// (draws a few extra pixels/triangles) but never hides anything. Over-inclusion is +// safe; under-inclusion is the bug class. S3 chunk 4 (fix round 2, L2) deleted the +// only mechanism that ever read a scissor AABB from this type — there is no GPU +// scissor consumer left anywhere in the walk — so an overflow region simply draws +// unclipped; it does not carry a bounding box for anyone to scissor against. // -// === The three Count==0 states (how a consumer tells them apart) ============= -// Count == 0 can mean three different things; the consumer MUST distinguish: -// (a) Empty — IsNothingVisible == true, UseScissorFallback == false. -// The cell/region isn't visible at all → DRAW NOTHING. The -// ScissorNdcAabb is a degenerate inverted box (min > max) so that a -// consumer which naively scissors on it still draws nothing. -// (b) Scissor — UseScissorFallback == true, IsNothingVisible == false. -// The convex-plane budget was exceeded (multi-polygon or >8 edges) -// → DRAW the ScissorNdcAabb box (a valid min<=max NDC rectangle). -// There is no third Count==0 state produced by From(). (A separate "no-clip, -// pass-all" slot 0 is constructed by the consumer directly, not via From().) -// When Count > 0, Planes carries the convex gate and the scissor fields are unused. +// === The two Count==0 states (how a consumer tells them apart) =============== +// Count == 0 can mean two different things; the consumer MUST distinguish: +// (a) Empty — IsNothingVisible == true, IsPlaneOverflow == false. +// The cell/region isn't visible at all → DRAW NOTHING. +// (b) Overflow — IsNothingVisible == false, IsPlaneOverflow == true. +// The convex-plane budget was exceeded (multi-polygon or >8 +// edges) → DRAW UNCLIPPED (over-include, never a scissor). +// When Count > 0, Planes carries the convex gate and both flags above are false. using System; using System.Buffers; using System.Collections.Generic; @@ -46,8 +47,8 @@ namespace AcDream.App.Rendering; /// /// An NDC convex view region reduced to ≤8 clip-space gl_ClipDistance planes, or a -/// scissor AABB fallback. See the file header for the convexity rule and the three -/// Count==0 states. +/// plane-overflow signal (draw unclipped). See the file header for the convexity +/// rule and the two Count==0 states. /// public readonly struct ClipPlaneSet { @@ -69,16 +70,15 @@ public readonly struct ClipPlaneSet private const float MinPolygonArea = 1e-7f; private readonly Vector4[] _planes; - private ClipPlaneSet(Vector4[] planes, bool useScissorFallback, bool isNothingVisible, Vector4 scissorNdcAabb) + private ClipPlaneSet(Vector4[] planes, bool isPlaneOverflow, bool isNothingVisible) { _planes = planes ?? Array.Empty(); - UseScissorFallback = useScissorFallback; + IsPlaneOverflow = isPlaneOverflow; IsNothingVisible = isNothingVisible; - ScissorNdcAabb = scissorNdcAabb; } - /// Number of active clip planes, 0..8. 0 ⇒ inspect - /// and to decide between "draw the AABB" and "draw nothing". + /// Number of active clip planes, 0..8. 0 ⇒ inspect + /// and to decide between "draw unclipped" and "draw nothing". public int Count => _planes?.Length ?? 0; /// The active clip-space planes (nx, ny, 0, d). Empty when is 0. @@ -89,30 +89,26 @@ public readonly struct ClipPlaneSet // its frame-scoped slice instead of cloning every plane payload a second time. internal Vector4[] PlaneArray => _planes ?? Array.Empty(); - /// True ⇒ the convex-plane budget was exceeded; gate on - /// instead (draw the box). Always false when > 0 or when the region is empty. - public bool UseScissorFallback { get; } + /// True ⇒ Count == 0 because the region needs more than + /// half-planes (or is multi-polygon); the consumer draws UNCLIPPED + /// (ClipFrameAssembler.AppendOutsideSlice emits an empty plane array, so the region + /// lands on the no-clip slot and — for the punch fans — the fan draws whole) — over-include, + /// never a scissor. Always false when > 0 or when the region is empty. + public bool IsPlaneOverflow { get; } /// True ⇒ the region is not visible at all; the consumer draws NOTHING. - /// Mutually exclusive with , and only meaningful when Count == 0. + /// Mutually exclusive with , and only meaningful when Count == 0. public bool IsNothingVisible { get; } - /// NDC axis-aligned scissor box (minX, minY, maxX, maxY). Valid (min <= max) only when - /// is true. For the empty/nothing-visible case it is a degenerate - /// inverted box so naive scissoring still draws nothing. - public Vector4 ScissorNdcAabb { get; } - - /// The "nothing is visible" sentinel: Count == 0, not a scissor fallback, draw nothing. + /// The "nothing is visible" sentinel: Count == 0, not a plane overflow, draw nothing. public static ClipPlaneSet Empty { get; } = - new(Array.Empty(), useScissorFallback: false, isNothingVisible: true, scissorNdcAabb: DegenerateAabb); - - // Inverted box (min > max) — any sane AABB intersection against it is empty. - private static Vector4 DegenerateAabb => new(1f, 1f, -1f, -1f); + new(Array.Empty(), isPlaneOverflow: false, isNothingVisible: true); /// /// Reduce a CellView's NDC clip region to a ClipPlaneSet. One convex polygon (≤8 edges - /// after collinear-merge) → per-edge planes; multi-polygon or >8 edges → union-AABB scissor; - /// empty/degenerate → . See the file header for the full rule. + /// after collinear-merge) → per-edge planes; multi-polygon or >8 edges → plane overflow + /// (draw unclipped); empty/degenerate → . See the file header for the + /// full rule. /// public static ClipPlaneSet From(CellView region) { @@ -120,9 +116,8 @@ public readonly struct ClipPlaneSet return Empty; // MORE THAN ONE polygon ⇒ union, not convex ⇒ never emit one polygon's planes. - // Over-include via the union AABB (safe). region.Min/Max already track the union. if (region.Polygons.Count > 1) - return Scissor(region.MinX, region.MinY, region.MaxX, region.MaxY); + return Overflow(); return From(region.Polygons[0]); } @@ -152,17 +147,16 @@ public readonly struct ClipPlaneSet { int count = NormalizeAndMerge(input, verts); - // Fewer than 3 distinct edges survive ⇒ a sliver/line with no area. There is no - // meaningful AABB to over-include (a zero-area region), so treat it as nothing visible. + // Fewer than 3 distinct edges survive ⇒ a sliver/line with no area ⇒ nothing visible. if (count < 3) return Empty; ReadOnlySpan normalized = verts[..count]; - // A single convex polygon with too many edges to fit the hardware budget ⇒ scissor - // on ITS own AABB (still a superset of the polygon → over-include, safe). + // A single convex polygon with too many edges to fit the hardware budget ⇒ plane + // overflow (draw unclipped — still a superset of the polygon → over-include, safe). if (count > MaxPlanes) - return Scissor(normalized); + return Overflow(); // 3..8 edges: emit one inward half-space plane per edge (CCW formula). This array is // the retained GPU-routing payload and therefore the one necessary allocation. @@ -179,7 +173,7 @@ public readonly struct ClipPlaneSet // dist = n.x*clip.x + n.y*clip.y + 0*clip.z + (-(n·p))*clip.w (>= 0 ⇒ keep) planes[i] = new Vector4(n.X, n.Y, 0f, -Vector2.Dot(n, p)); } - return new ClipPlaneSet(planes, useScissorFallback: false, isNothingVisible: false, scissorNdcAabb: DegenerateAabb); + return new ClipPlaneSet(planes, isPlaneOverflow: false, isNothingVisible: false); } finally { @@ -188,22 +182,8 @@ public readonly struct ClipPlaneSet } } - private static ClipPlaneSet Scissor(float minX, float minY, float maxX, float maxY) => - new(Array.Empty(), useScissorFallback: true, isNothingVisible: false, - scissorNdcAabb: new Vector4(minX, minY, maxX, maxY)); - - private static ClipPlaneSet Scissor(ReadOnlySpan verts) - { - float minX = float.MaxValue, minY = float.MaxValue, maxX = float.MinValue, maxY = float.MinValue; - foreach (var v in verts) - { - if (v.X < minX) minX = v.X; - if (v.X > maxX) maxX = v.X; - if (v.Y < minY) minY = v.Y; - if (v.Y > maxY) maxY = v.Y; - } - return Scissor(minX, minY, maxX, maxY); - } + private static ClipPlaneSet Overflow() => + new(Array.Empty(), isPlaneOverflow: true, isNothingVisible: false); /// /// Return the polygon wound CCW with collinear vertices removed. The PortalVisibilityBuilder diff --git a/src/AcDream.App/Rendering/RetailPViewPassExecutor.cs b/src/AcDream.App/Rendering/RetailPViewPassExecutor.cs index 5b5cfd54..0ab6da15 100644 --- a/src/AcDream.App/Rendering/RetailPViewPassExecutor.cs +++ b/src/AcDream.App/Rendering/RetailPViewPassExecutor.cs @@ -140,8 +140,8 @@ public RetailPViewPassExecutor( ClipFrameAssembly reuseAssembly) => ClipFrameAssembler.BeginWalkFrame(_clipFrame, outdoorRoot, reuseAssembly); - public void PrepareClipFrame(int terrainUploadCount) => - _surface.PrepareClipFrame(terrainUploadCount); + public void PrepareClipFrame() => + _surface.PrepareClipFrame(); public void PrepareCellBatches( RetailPViewFrameInput frame, diff --git a/src/AcDream.App/Rendering/RetailPViewRenderer.cs b/src/AcDream.App/Rendering/RetailPViewRenderer.cs index 1ef0e31e..583ba3f5 100644 --- a/src/AcDream.App/Rendering/RetailPViewRenderer.cs +++ b/src/AcDream.App/Rendering/RetailPViewRenderer.cs @@ -413,10 +413,11 @@ internal sealed class RetailPViewRenderer // FW4 slice 1: the ONE clip-region publication, after any walk // reassembly so the walk-derived outside-view slots are included - // (moved from directly after AssembleClipFrame; the count is - // reservation metadata the RHI arm ignores). - int terrainUploadCount = checked(1 + clipAssembly.OutsideViewSlices.Length * 2); - passes.PrepareClipFrame(terrainUploadCount); + // (moved from directly after AssembleClipFrame). S3 landing hygiene + // (H2) deleted the GL-era arena-reservation count this call used to + // compute and pass — PrepareClipFrame() takes nothing now (see + // IWorldPassSurface.PrepareClipFrame's doc). + passes.PrepareClipFrame(); // Production prepares exactly the one walk's visited-cell set. HashSet prepareCells = drawableCells; @@ -643,8 +644,9 @@ internal sealed class RetailPViewRenderer // + DrawLandscapeSliceLate, one call per active landscape view) is // DELETED — retail draws the // weather mesh and its rain particles ONCE, unclipped, after - // LScape::draw's whole landblock loop (GameSky::Draw(sky,1) - // @0x00506ff0), never once per doorway aperture. DrawWeatherOnce + // LScape::draw's whole landblock loop (GameSky::Draw(sky,1) call site + // @0x00506396 — the callee itself is @0x00506ff0), never once per + // doorway aperture. DrawWeatherOnce // is the ONE call that remains — it now also submits the rain // particles (moved from the deleted loop's per-slice, per-doorway // ParticleRenderPass.SkyPostScene draw) as ONE unclipped diff --git a/src/AcDream.App/Rendering/TerrainModernRenderer.cs b/src/AcDream.App/Rendering/TerrainModernRenderer.cs index 900b79c1..29da7261 100644 --- a/src/AcDream.App/Rendering/TerrainModernRenderer.cs +++ b/src/AcDream.App/Rendering/TerrainModernRenderer.cs @@ -241,8 +241,6 @@ public sealed partial class TerrainModernRenderer : IDisposable ICamera camera, FrustumPlanes? frustum = null, uint? neverCullLandblockId = null, - ReadOnlySpan clipPlanes = default, - Vector4? ndcClipAabb = null, IReadOnlySet? inViewLandcells = null) { if (_alloc.LoadedCount == 0) return; @@ -289,10 +287,7 @@ public sealed partial class TerrainModernRenderer : IDisposable data.WorldOrigin, data.AabbMin.Z, data.AabbMax.Z, - frustum, - viewProjection, - clipPlanes, - ndcClipAabb); + frustum); } } if (_visibleSlots.Count == 0) return; @@ -485,16 +480,23 @@ public sealed partial class TerrainModernRenderer : IDisposable // Private helpers // ---------------------------------------------------------------- + /// + /// S3 landing hygiene (H2): frustum-only now. Retail never view-clips + /// terrain (see 's doc); the CPU/GPU + /// clip-region equivalence check this used to also run (a doorway + /// slice's clip-space planes plus its NDC-AABB scissor fallback) had + /// exactly one caller — — and that caller's own two + /// clip-region parameters were themselves dead (grep: no production + /// caller ever passed either), so the equivalence helper and its + /// homogeneous-plane subroutine are deleted along with them. + /// internal static void CollectVisibleCells( HashSet destination, uint landblockId, Vector3 worldOrigin, float zMin, float zMax, - FrustumPlanes? frustum, - Matrix4x4 viewProjection, - ReadOnlySpan clipPlanes, - Vector4? ndcClipAabb = null) + FrustumPlanes? frustum) { ArgumentNullException.ThrowIfNull(destination); const float cellSize = AcDream.Core.Physics.TerrainSurface.CellSize; @@ -517,21 +519,6 @@ public sealed partial class TerrainModernRenderer : IDisposable continue; } - // Retail publishes landcell in_view from the clipped landscape - // view, not merely from the camera frustum. The modern renderer - // expresses each doorway slice as homogeneous clip-space planes - // plus its scissor AABB; use both products here so particle - // simulation follows the same visible terrain slice as the GPU. - if (!IsAabbVisibleThroughClipRegion( - cellMin, - cellMax, - viewProjection, - clipPlanes, - ndcClipAabb)) - { - continue; - } - uint low = AcDream.Core.Physics.TerrainSurface.ComputeOutdoorCellLowId( cellX * cellSize, cellY * cellSize); @@ -540,75 +527,6 @@ public sealed partial class TerrainModernRenderer : IDisposable } } - private static bool IsAabbVisibleThroughClipRegion( - Vector3 min, - Vector3 max, - Matrix4x4 viewProjection, - ReadOnlySpan clipPlanes, - Vector4? ndcClipAabb) - { - Vector4 aabb = ndcClipAabb.GetValueOrDefault(); - bool hasScissorConstraint = ndcClipAabb.HasValue - && (aabb.X > -1f || aabb.Y > -1f || aabb.Z < 1f || aabb.W < 1f); - if (clipPlanes.IsEmpty && !hasScissorConstraint) - return true; - - Span clipCorners = stackalloc Vector4[8]; - for (int corner = 0; corner < clipCorners.Length; corner++) - { - var world = new Vector4( - (corner & 1) == 0 ? min.X : max.X, - (corner & 2) == 0 ? min.Y : max.Y, - (corner & 4) == 0 ? min.Z : max.Z, - 1f); - clipCorners[corner] = Vector4.Transform(world, viewProjection); - } - - for (int planeIndex = 0; planeIndex < clipPlanes.Length; planeIndex++) - { - if (IsAabbOutsideHomogeneousPlane(clipCorners, clipPlanes[planeIndex])) - { - return false; - } - } - - if (!hasScissorConstraint) - return true; - - Span scissorPlanes = stackalloc Vector4[4] - { - new( 1f, 0f, 0f, -aabb.X), - new(-1f, 0f, 0f, aabb.Z), - new( 0f, 1f, 0f, -aabb.Y), - new( 0f, -1f, 0f, aabb.W), - }; - for (int planeIndex = 0; planeIndex < scissorPlanes.Length; planeIndex++) - { - if (IsAabbOutsideHomogeneousPlane(clipCorners, scissorPlanes[planeIndex])) - { - return false; - } - } - - return true; - } - - private static bool IsAabbOutsideHomogeneousPlane( - ReadOnlySpan clipCorners, - Vector4 plane) - { - // A linear half-space reaches its maximum over the transformed AABB at - // one of the eight corners. If every corner is negative, no point in - // the cell box can survive this GPU clip plane. - for (int corner = 0; corner < clipCorners.Length; corner++) - { - if (Vector4.Dot(plane, clipCorners[corner]) >= 0f) - return false; - } - - return true; - } - private void EnsureCapacity(int newCapacity) { if (newCapacity <= _alloc.Capacity) diff --git a/src/AcDream.App/Rendering/WorldPassSurface.cs b/src/AcDream.App/Rendering/WorldPassSurface.cs index 6905e995..f7d56d59 100644 --- a/src/AcDream.App/Rendering/WorldPassSurface.cs +++ b/src/AcDream.App/Rendering/WorldPassSurface.cs @@ -20,10 +20,12 @@ internal interface IRenderFrameGlState /// and heavily tested — is written once for both backends. /// /// Everything else those executors do is delegation to a renderer. What is -/// left is exactly this: the clip-frame publication, the doorway scissor, -/// gl_ClipDistance enablement, and retail's interior depth clear. Four -/// concerns, each of which genuinely differs between GL and Vulkan, and none of -/// which is expressible on the pinned RHI contract. +/// left is exactly this: the clip-frame publication, gl_ClipDistance +/// enablement, and retail's interior depth clear. Three concerns, each of +/// which genuinely differs between GL and Vulkan, and none of which is +/// expressible on the pinned RHI contract. (S3 landing hygiene, H4: the +/// doorway scissor this used to also cover was retired at S3 chunk 4 fix +/// round 2 — see 's implementation doc.) /// internal interface IWorldPassSurface { @@ -31,13 +33,6 @@ internal interface IWorldPassSurface /// Publishes this frame's per-cell clip-region table, and routes it to the /// renderers that read it. /// - /// is how many distinct terrain - /// clip blocks the frame will issue. GL reserves that many arena records - /// before the first draw, because reallocating the arena while an earlier - /// slice can still reference it is the hazard the reservation exists for. The - /// RHI arm ignores it: a ring allocation is distinct memory by construction - /// and lives until the frame retires. - /// /// S3 chunk 4 fix round 1 (K3) deleted the walk's per-frame terrain /// screen-space clip publish this used to also carry — its only writer /// was retired along with the per-outside-view-slice terrain/sky/weather @@ -45,17 +40,29 @@ internal interface IWorldPassSurface /// declaration of that clip block along with the section binder that used /// to re-assert it, so this contract now publishes exactly one section, /// the per-cell clip-region table. + /// + /// S3 landing hygiene (H2) deleted the GL-era reservation-count + /// parameter this used to also take (how many distinct terrain clip + /// blocks the frame would issue): the RHI arm always ignored it (a ring + /// allocation is distinct memory by construction and lives until the + /// frame retires), and the last GL implementation that read it + /// (GlWorldPassSurface) was deleted at Campaign V slice V11 — + /// nothing was left to size a reservation for. /// - void PrepareClipFrame(int terrainUploadCount); + void PrepareClipFrame(); /// /// Enables every gl_ClipDistance slot. /// /// Vulkan activates every element the shader declares and has no /// enable, so this is a no-op there — and that is safe rather than a - /// divergence: all three world vertex shaders already write 1.0 - /// ("keep everything") into every slot past the active count, so a frame with - /// no clip planes clips nothing on either backend (plan §5.5.14 item 4). + /// divergence: only portal_depth.vert writes gl_ClipDistance + /// in the world pass now (S3 landing hygiene, H4 — S3 chunk 4 fix round 2 + /// (L3) deleted the sky and terrain shaders' own declaration of the block + /// this contract used to also publish; see 's + /// doc). It still writes 1.0 ("keep everything") into every slot + /// past its active count, so a frame with no clip planes clips nothing on + /// either backend (plan §5.5.14 item 4). /// void EnableClipDistances(); @@ -73,11 +80,10 @@ internal interface IWorldPassSurface } /// -/// The clip tables are ring sections published on the world pass scope, the -/// scissor is dynamic state on the borrowed encoder, and the depth clear is a -/// scoped vkCmdClearAttachments. The raw-GL implementation this used to -/// sit alongside (GlWorldPassSurface) was deleted at Campaign V slice -/// V11. +/// The clip table is a ring section published on the world pass scope, and the +/// depth clear is a scoped vkCmdClearAttachments. The raw-GL +/// implementation this used to sit alongside (GlWorldPassSurface) was +/// deleted at Campaign V slice V11. /// internal sealed class RhiWorldPassSurface : IWorldPassSurface { @@ -95,12 +101,8 @@ internal sealed class RhiWorldPassSurface : IWorldPassSurface _clipFrame = clipFrame ?? throw new ArgumentNullException(nameof(clipFrame)); } - public void PrepareClipFrame(int terrainUploadCount) + public void PrepareClipFrame() { - // The reservation count has no RHI counterpart: each publication below - // takes its own ring slice, which is distinct memory that outlives every - // draw recorded against it in this frame. - _ = terrainUploadCount; _scope.Sections.ClipRegions = Publish( _clipFrame.RegionBytes, GpuRingUsage.Storage); diff --git a/src/AcDream.App/Rendering/WorldScenePassExecutor.cs b/src/AcDream.App/Rendering/WorldScenePassExecutor.cs index 4a46e908..2da09ff1 100644 --- a/src/AcDream.App/Rendering/WorldScenePassExecutor.cs +++ b/src/AcDream.App/Rendering/WorldScenePassExecutor.cs @@ -110,7 +110,7 @@ internal sealed class WorldScenePassExecutor : IWorldScenePassExecutor _environmentCells.SetClipRouting(null); } - public void PrepareFlatWorldClip() => _surface.PrepareClipFrame(1); + public void PrepareFlatWorldClip() => _surface.PrepareClipFrame(); public void DrawFlatSky( in WorldCameraFrame camera, diff --git a/tests/AcDream.App.Tests/Rendering/ClipPlaneSetTests.cs b/tests/AcDream.App.Tests/Rendering/ClipPlaneSetTests.cs index cc1a57d7..85a20921 100644 --- a/tests/AcDream.App.Tests/Rendering/ClipPlaneSetTests.cs +++ b/tests/AcDream.App.Tests/Rendering/ClipPlaneSetTests.cs @@ -47,9 +47,8 @@ public class ClipPlaneSetTests var fromPolygon = ClipPlaneSet.From(polygon); Assert.Equal(fromView.Count, fromPolygon.Count); - Assert.Equal(fromView.UseScissorFallback, fromPolygon.UseScissorFallback); + Assert.Equal(fromView.IsPlaneOverflow, fromPolygon.IsPlaneOverflow); Assert.Equal(fromView.IsNothingVisible, fromPolygon.IsNothingVisible); - Assert.Equal(fromView.ScissorNdcAabb, fromPolygon.ScissorNdcAabb); Assert.Equal(fromView.Planes, fromPolygon.Planes); } @@ -75,14 +74,14 @@ public class ClipPlaneSetTests } [Fact] - public void From_NineEdgePolygon_FallsBackToScissor() + public void From_NineEdgePolygon_FallsBackToPlaneOverflow() { var poly = RegularNgonCellView(n: 9, radius: 0.6f); var cps = ClipPlaneSet.From(poly); - Assert.True(cps.UseScissorFallback || cps.Count <= 8); - if (cps.UseScissorFallback) + Assert.True(cps.IsPlaneOverflow || cps.Count <= 8); + if (cps.IsPlaneOverflow) { - Assert.Equal(0, cps.Count); // AABB carries the gate + Assert.Equal(0, cps.Count); // draws unclipped, no plane gate } } @@ -95,7 +94,7 @@ public class ClipPlaneSetTests // --- Multi-polygon safety (the under-inclusion guard) --------------------- [Fact] - public void From_MultiplePolygons_FallsBackToUnionScissor_NeverEmitsOnePolygonsPlanes() + public void From_MultiplePolygons_FallsBackToPlaneOverflow_NeverEmitsOnePolygonsPlanes() { // Two disjoint squares: a CONVEX plane set can never represent their union. var cv = new CellView(); @@ -111,29 +110,19 @@ public class ClipPlaneSetTests var cps = ClipPlaneSet.From(cv); // MUST NOT emit a single polygon's convex planes (that would hide the other). - Assert.True(cps.UseScissorFallback); + Assert.True(cps.IsPlaneOverflow); Assert.Equal(0, cps.Count); Assert.Empty(cps.Planes); - - // Scissor AABB is the UNION of both polygons (over-include, safe). - Assert.Equal(-0.8f, cps.ScissorNdcAabb.X, 5); // minX - Assert.Equal(-0.8f, cps.ScissorNdcAabb.Y, 5); // minY - Assert.Equal(0.8f, cps.ScissorNdcAabb.Z, 5); // maxX - Assert.Equal(0.8f, cps.ScissorNdcAabb.W, 5); // maxY - - // The far corner of the second square is inside the union AABB → not hidden. - Assert.True(0.7f >= cps.ScissorNdcAabb.X && 0.7f <= cps.ScissorNdcAabb.Z); - Assert.True(0.7f >= cps.ScissorNdcAabb.Y && 0.7f <= cps.ScissorNdcAabb.W); } - // --- Distinguishing the three Count==0 states ---------------------------- + // --- Distinguishing the two Count==0 states ------------------------------- [Fact] - public void Empty_IsNothingVisible_NotScissorFallback() + public void Empty_IsNothingVisible_NotPlaneOverflow() { var e = ClipPlaneSet.From(new CellView()); Assert.Equal(0, e.Count); - Assert.False(e.UseScissorFallback); // NOT "draw the AABB box" + Assert.False(e.IsPlaneOverflow); // NOT "draws unclipped" Assert.True(e.IsNothingVisible); // "draw nothing" Assert.Empty(e.Planes); } @@ -143,23 +132,17 @@ public class ClipPlaneSetTests { var e = ClipPlaneSet.Empty; Assert.Equal(0, e.Count); - Assert.False(e.UseScissorFallback); + Assert.False(e.IsPlaneOverflow); Assert.True(e.IsNothingVisible); - // Degenerate AABB (min > max) so a consumer that naively scissors on it draws nothing. - Assert.True(e.ScissorNdcAabb.X > e.ScissorNdcAabb.Z); - Assert.True(e.ScissorNdcAabb.Y > e.ScissorNdcAabb.W); } [Fact] - public void ScissorFallback_IsNotNothingVisible() + public void PlaneOverflow_IsNotNothingVisible() { var poly = RegularNgonCellView(n: 9, radius: 0.6f); var cps = ClipPlaneSet.From(poly); - Assert.True(cps.UseScissorFallback); - Assert.False(cps.IsNothingVisible); // "draw the AABB box", not "draw nothing" - // The fallback AABB bounds the 9-gon (radius 0.6). - Assert.True(cps.ScissorNdcAabb.Z - cps.ScissorNdcAabb.X > 0.5f); - Assert.True(cps.ScissorNdcAabb.W - cps.ScissorNdcAabb.Y > 0.5f); + Assert.True(cps.IsPlaneOverflow); + Assert.False(cps.IsNothingVisible); // "draws unclipped", not "draw nothing" } // --- Plane-sign correctness for every edge ------------------------------- @@ -242,13 +225,13 @@ public class ClipPlaneSetTests Assert.Equal(4, cps.Count); // the redundant collinear edge is merged away } - // --- Exactly 8 edges fit (octagon); 9 spills to scissor (after merge fails to help). + // --- Exactly 8 edges fit (octagon); 9 spills to plane overflow (after merge fails to help). [Fact] public void From_RegularOctagon_FitsInEightPlanes() { var cps = ClipPlaneSet.From(RegularNgonCellView(n: 8, radius: 0.6f)); - Assert.False(cps.UseScissorFallback); + Assert.False(cps.IsPlaneOverflow); Assert.Equal(8, cps.Count); var center = new Vector4(0, 0, 0, 1); foreach (var p in cps.Planes) diff --git a/tests/AcDream.App.Tests/Rendering/Issue130DoorwayStripTests.cs b/tests/AcDream.App.Tests/Rendering/Issue130DoorwayStripTests.cs deleted file mode 100644 index 2d3028dd..00000000 --- a/tests/AcDream.App.Tests/Rendering/Issue130DoorwayStripTests.cs +++ /dev/null @@ -1,326 +0,0 @@ -using System; -using System.Collections.Generic; -using System.Numerics; -using AcDream.App.Rendering; -using DatReaderWriter; -using DatReaderWriter.Options; -using Xunit; -using Xunit.Abstractions; - -namespace AcDream.App.Tests.Rendering; - -/// -/// #130 — background-color strip along the TOP outer edge of a doorway when -/// looking out from inside. Mechanism model (2026-06-12 evidence sweep): for -/// an interior root the SEAL stamps the FULL raw dat portal polygon at true -/// depth (PortalDepthMaskRenderer, root-cell slice = full screen), while -/// terrain/sky COLOR used to be gated per fragment by the OutsideView region — -/// the same dat polygon run through ProjectToClip → ClipToRegion (1-px -/// MergeSubPixelVertices) → ClipPlaneSet.From (0.5° collinear merge) → planes, -/// with a Floor/Ceil pixel scissor (the sky's own doorway scissor bracket) -/// on the slice AABB on top. Every one of those passes could only SHRINK the gate, so any -/// shave showed as a strip of clear color between the gate's top edge and the -/// aperture's rasterized top edge. -/// -/// RETIRED MECHANISM (S3 chunk 4 fix round 2, L7): the scissor half of this -/// story no longer applies. Retail draws the sky and the landscape unclipped -/// (LScape::draw never installs a view before either — S3 chunk 3/4's -/// own findings), and acdream now matches that: the sky's own doorway -/// scissor bracket, IWorldPassSurface.BeginScissor/EndScissor, and -/// every per-slice terrain/sky/weather clip they used to bracket are deleted -/// outright. An -/// interior root's aperture exactness comes instead from the depth clear, the -/// exit seals, and the interior repaint (WalkFrameDriver's interior -/// turn draws the real cell geometry back over whatever the unclipped -/// landscape painted through the doorway) — see the S3 chunk 4 plan section -/// for the retail citations. -/// -/// What remains below still pins something production reads: the outside-view -/// polygon pipeline (ProjectToClip → ClipToRegion → -/// ClipPlaneSet.From) that the KEPT punch-fan clip still consumes -/// (RetailPViewPassExecutor.DrawWalkPunchFan reads -/// clipAssembly.OutsideViewSlices[activeViewIndex].Planes — see -/// ClipFrameLayoutTests's punch-fan equivalence pin for the synthetic- -/// view version of this same proof). This harness measures the PLANE gap -/// headlessly at the real Holtburg corner building exit door (A9B4 0x0170, -/// the HouseExitWalkReplay door): project the aperture, run the production -/// flood + assembler, then walk sample points just inside the aperture's top -/// edge downward until the plane gate admits them. -/// -/// VERDICT (2026-06-12, 147 eye/gaze combos, plane half only — the original -/// sweep also measured a scissor gap, since retired): the CPU polygon -/// pipeline is sub-pixel exact (worst 0.54 px) — the W=0 clip port 987313a -/// and both merge passes are EXONERATED. PIN 2 below still asserts that -/// bound. -/// -[Trait("Lane", "InstalledDat")] -public class Issue130DoorwayStripTests -{ - private readonly ITestOutputHelper _out; - public Issue130DoorwayStripTests(ITestOutputHelper output) => _out = output; - - private const uint ExitCellId = CornerFloodReplayTests.Landblock | 0x0170u; - - // Production projection convention (CornerFloodReplayTests.ViewProjFor): - // FovY 1.2 rad, 1280x720 viewport, near 1, far 5000. The flood clip is - // near-independent so near/far exactness is not load-bearing. - private static Matrix4x4 ViewProjFor(Vector3 eye, Vector3 lookAt) - { - var view = Matrix4x4.CreateLookAt(eye, lookAt, Vector3.UnitZ); - var proj = Matrix4x4.CreatePerspectiveFieldOfView(1.2f, 1280f / 720f, 1f, 5000f); - return view * proj; - } - - [Fact] - public void ExitDoorTopEdge_GateCoversTheDrawnApertureWithinPixelTolerance() - { - var datDir = CornerFloodReplayTests.ResolveDatDir(); - if (datDir is null) { _out.WriteLine("SKIP: dats unavailable"); Assert.Fail("Lane=InstalledDat requires an installed retail DAT directory; see docs/release-gate.md."); } - - using var dats = new DatCollection(datDir, DatAccessType.Read); - var cells = CornerFloodReplayTests.LoadBuilding(dats); - var root = cells[ExitCellId]; - LoadedCell? Lookup(uint id) => cells.TryGetValue(id, out var c) ? c : null; - - // Find the exit portal (OtherCellId == 0xFFFF) and its world polygon. - int exitIdx = -1; - for (int i = 0; i < root.Portals.Count; i++) - { - if (root.Portals[i].OtherCellId == 0xFFFF && i < root.PortalPolygons.Count - && root.PortalPolygons[i].Length >= 3) - { exitIdx = i; break; } - } - Assert.True(exitIdx >= 0, "0x0170 has no exit portal polygon"); - - var localPoly = root.PortalPolygons[exitIdx]; - // Campaign FW3.3: ShellDrawLiftZ is retired — shells, seal fans, and - // the gate all live in the ONE dat space, so the drawn aperture IS - // the physics polygon. This test survives as the coverage proof - // (gate covers the drawn hole within tolerance across the sweep); - // the historical lifted-vs-unlifted strip test's premise (two - // spaces) no longer exists and that test is deleted. - var worldPoly = new Vector3[localPoly.Length]; - for (int i = 0; i < localPoly.Length; i++) - worldPoly[i] = Vector3.Transform(localPoly[i], root.WorldTransform); - - Vector3 centroid = Vector3.Zero; - foreach (var w in worldPoly) centroid += w; - centroid /= worldPoly.Length; - - // Inward direction: the portal plane normal signed toward the cell - // interior (ClipPlanes carries InsideSide from the load). - var plane = root.ClipPlanes[exitIdx]; - var worldNormal = Vector3.TransformNormal(plane.Normal, root.WorldTransform); - var cellCenterWorld = Vector3.Transform( - (root.LocalBoundsMin + root.LocalBoundsMax) * 0.5f, root.WorldTransform); - if (Vector3.Dot(worldNormal, cellCenterWorld - centroid) < 0) - worldNormal = -worldNormal; - worldNormal = Vector3.Normalize(worldNormal); - - _out.WriteLine(FormattableString.Invariant( - $"exit portal idx={exitIdx} verts={localPoly.Length} centroid=({centroid.X:F2},{centroid.Y:F2},{centroid.Z:F2}) inward=({worldNormal.X:F2},{worldNormal.Y:F2},{worldNormal.Z:F2})")); - for (int i = 0; i < worldPoly.Length; i++) - _out.WriteLine(FormattableString.Invariant( - $" poly[{i}] world=({worldPoly[i].X:F3},{worldPoly[i].Y:F3},{worldPoly[i].Z:F3})")); - - float worstPlaneGapPx = 0f; - string worstDesc = "(none)"; - - // Eye sweep: back off the doorway along the inward normal at several - // distances/heights/lateral offsets; gaze at the centroid plus raised / - // lowered targets (NDC alignment of the top edge varies with gaze). - var lateral = Vector3.Normalize(Vector3.Cross(worldNormal, Vector3.UnitZ)); - float[] dists = { 0.6f, 1.0f, 1.6f, 2.4f, 3.5f }; - float[] heights = { 0.9f, 1.4f, 1.7f }; - float[] laterals = { -0.8f, 0f, 0.8f }; - float[] gazeRaise = { -0.4f, 0f, 0.4f, 0.9f }; - - int evaluated = 0; - foreach (float d in dists) - foreach (float h in heights) - foreach (float lat in laterals) - foreach (float gz in gazeRaise) - { - var eye = centroid + worldNormal * d + lateral * lat; - eye.Z = centroid.Z - 1.0f + h; // door centroid sits mid-opening; bias to floor-ish - var look = centroid + new Vector3(0, 0, gz); - var viewProj = ViewProjFor(eye, look); - - // Aperture truth: the seal's footprint = the raw polygon's projection. - var clip = new Vector4[worldPoly.Length]; - float minW = float.MaxValue; - for (int i = 0; i < worldPoly.Length; i++) - { - clip[i] = Vector4.Transform(new Vector4(worldPoly[i], 1f), viewProj); - minW = MathF.Min(minW, clip[i].W); - } - if (minW <= 0.05f) continue; // eye in/behind the door plane — out of #130's scenario - var aperture = new Vector2[clip.Length]; - for (int i = 0; i < clip.Length; i++) - aperture[i] = new Vector2(clip[i].X / clip[i].W, clip[i].Y / clip[i].W); - - var pv = PortalVisibilityBuilder.Build(root, eye, Lookup, viewProj, - buildingMembership: null); - var asm = ClipFrameAssembler.Assemble(ClipFrame.NoClip(), pv); - if (asm.OutsideViewSlices.Length == 0) - { - _out.WriteLine(FormattableString.Invariant( - $"d={d} h={h} lat={lat} gz={gz}: NO outside slice (outPolys={pv.OutsideView.Polygons.Count})")); - continue; - } - evaluated++; - - (float planeGapPx, float atX) = - MeasureTopEdgeGap(aperture, asm.OutsideViewSlices, 1080); - - if (planeGapPx > worstPlaneGapPx) - { - worstDesc = FormattableString.Invariant( - $"d={d} h={h} lat={lat} gz={gz} minW={minW:F2} atX={atX:F3} slices={asm.OutsideViewSlices.Length} outVerts={DescribePolys(pv.OutsideView)} apVerts={aperture.Length}"); - worstPlaneGapPx = planeGapPx; - } - - if (planeGapPx > 0.55f) - { - _out.WriteLine(FormattableString.Invariant( - $"GAP d={d} h={h} lat={lat} gz={gz}: planeGap={planeGapPx:F2}px atX={atX:F3} outVerts={DescribePolys(pv.OutsideView)}")); - float apTop = TopBoundaryY(aperture, atX); - foreach (var slice in asm.OutsideViewSlices) - _out.WriteLine(FormattableString.Invariant( - $" slice slot={slice.Slot} planes={slice.Planes.Length} aabb=({slice.NdcAabb.X:F4},{slice.NdcAabb.Y:F4},{slice.NdcAabb.Z:F4},{slice.NdcAabb.W:F4}) apTopAtX={apTop:F4}")); - foreach (var poly in pv.OutsideView.Polygons) - { - var sb = new System.Text.StringBuilder(" outPoly:"); - foreach (var v in poly.Vertices) - sb.Append(FormattableString.Invariant($" ({v.X:F4},{v.Y:F4})")); - _out.WriteLine(sb.ToString()); - } - } - } - - _out.WriteLine(FormattableString.Invariant( - $"evaluated={evaluated} worstPlaneGapPx={worstPlaneGapPx:F2} @ {worstDesc}")); - - Assert.True(evaluated > 100, $"sweep degenerated: only {evaluated} eye/gaze combos evaluated"); - // PIN (canary): the CPU polygon pipeline (ProjectToClip → ClipToRegion - // merges → ClipPlaneSet planes) stays sub-pixel exact against the raw - // aperture projection. Observed 0.54 px worst (2026-06-12); the - // production vertex-merge floor is ~1 px — beyond 1.2 px means a new - // under-inclusion shaver entered the pipeline. - Assert.True(worstPlaneGapPx <= 1.2f, FormattableString.Invariant( - $"plane gate under-covers the aperture top edge by {worstPlaneGapPx:F2}px @ {worstDesc}")); - } - - private static string DescribePolys(CellView view) - { - var parts = new List(); - foreach (var p in view.Polygons) parts.Add(p.Vertices.Length.ToString()); - return $"[{string.Join(",", parts)}]"; - } - - /// - /// For sample x positions across the aperture's projected top edge, find the - /// aperture boundary's top y, then walk downward until the plane gate - /// admits the point. Returns the worst gap in 1080p pixels, and the x of - /// the worst gap. S3 chunk 4 fix round 2 (L7): the scissor-gap half this - /// used to measure alongside the plane gap is deleted — the scissor - /// mechanism it modeled no longer exists in production. - /// - private static (float planeGapPx, float atX) MeasureTopEdgeGap( - Vector2[] aperture, ClipViewSlice[] slices, int fbH, - ITestOutputHelper? debug = null) - { - const float Inset = 1e-4f; // dodge exact-boundary ambiguity - const float StepY = 0.0002f; // ~0.1 px at 1080p - const float CapY = 0.02f; // stop searching beyond ~10 px - - float minX = float.MaxValue, maxX = float.MinValue; - foreach (var v in aperture) { minX = MathF.Min(minX, v.X); maxX = MathF.Max(maxX, v.X); } - float span = maxX - minX; - if (span <= 0.01f) return (0, 0); - - float worstPlane = 0, atX = 0; - const int Samples = 160; - for (int s = 0; s <= Samples; s++) - { - float x = minX + span * (0.01f + 0.98f * s / Samples); - if (MathF.Abs(x) > 0.98f) continue; // off screen — no pixel exists there - float topY = TopBoundaryY(aperture, x); - if (float.IsNaN(topY) || MathF.Abs(topY) > 0.98f) continue; // off screen / no boundary - - var p = new Vector2(x, topY - Inset); - - float planeGap = GapBelow(p, q => AnySliceAdmitsPlanes(slices, q), StepY, CapY); - - if (debug is not null && planeGap > 0.005f) - debug.WriteLine(FormattableString.Invariant( - $" sample x={x:F4} apTop={topY:F4} planeGap={planeGap * fbH / 2f:F2}px")); - - if (planeGap > worstPlane) { worstPlane = planeGap; atX = x; } - } - // NDC y → pixels at the given framebuffer height. - return (worstPlane * fbH / 2f, atX); - } - - private static float GapBelow(Vector2 start, Func admitted, float step, float cap) - { - if (admitted(start)) return 0f; - for (float dy = step; dy <= cap; dy += step) - { - if (admitted(new Vector2(start.X, start.Y - dy))) - return dy; - } - return cap; - } - - // Production semantics: each OutsideView polygon is one slice; the union of - // slices is drawn. A slice with planes gates per fragment via - // gl_ClipDistance (dot((nx,ny,0,d),(x,y,z,1)) >= 0 for an NDC point); - // a planeless slice (>8-edge zero-plane fallback, ClipFrameAssembler.cs) admits its whole NDC AABB. - private static bool AnySliceAdmitsPlanes(ClipViewSlice[] slices, Vector2 p) - { - foreach (var slice in slices) - { - if (slice.Planes.Length == 0) - { - if (p.X >= slice.NdcAabb.X && p.Y >= slice.NdcAabb.Y - && p.X <= slice.NdcAabb.Z && p.Y <= slice.NdcAabb.W) - return true; - continue; - } - bool inside = true; - foreach (var pl in slice.Planes) - { - if (pl.X * p.X + pl.Y * p.Y + pl.W < 0f) { inside = false; break; } - } - if (inside) return true; - } - return false; - } - - /// Highest boundary y of the polygon at vertical line x (NaN when - /// the line misses the polygon). - private static float TopBoundaryY(Vector2[] poly, float x) - { - float best = float.NaN; - for (int i = 0; i < poly.Length; i++) - { - var a = poly[i]; - var b = poly[(i + 1) % poly.Length]; - if (MathF.Abs(a.X - b.X) < 1e-9f) - { - if (MathF.Abs(a.X - x) < 1e-6f) - { - float hi = MathF.Max(a.Y, b.Y); - if (float.IsNaN(best) || hi > best) best = hi; - } - continue; - } - float t = (x - a.X) / (b.X - a.X); - if (t < 0f || t > 1f) continue; - float y = a.Y + t * (b.Y - a.Y); - if (float.IsNaN(best) || y > best) best = y; - } - return best; - } -} diff --git a/tests/AcDream.App.Tests/Rendering/RetailPViewPassExecutorTests.cs b/tests/AcDream.App.Tests/Rendering/RetailPViewPassExecutorTests.cs index ebff31e9..702c55ec 100644 --- a/tests/AcDream.App.Tests/Rendering/RetailPViewPassExecutorTests.cs +++ b/tests/AcDream.App.Tests/Rendering/RetailPViewPassExecutorTests.cs @@ -159,7 +159,9 @@ public sealed class RetailPViewPassExecutorTests /// ClearClipRouting + the old DrawLandscapeSliceLate leaf, /// one call per active landscape view) is deleted — DrawLandscapeDynamicsPhase now calls /// exactly once, - /// unconditionally, with no loop of any kind around it. This + /// conditional on + /// (see + /// — the L1 pin), with no loop of any kind around it. This /// Assert.Single alone proved insufficient at fix round 1 (K1): /// it counts DISTINCT call-site offsets, so it stays green even with a /// foreach wrapped around the one call site (the exact round-1 @@ -201,7 +203,14 @@ public sealed class RetailPViewPassExecutorTests /// the compiled foreach 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. + /// unconditional call to make it pass again. Note for reviewers (added + /// at S3 landing hygiene, H4, matching 's + /// own note): reads only + /// br/brtrue/brfalse-family single-target branches — + /// it does not decode a compiled switch jump table, but no C# loop + /// construct (for/foreach/while/do) ever + /// compiles to one, so this pin's blind spot is not a loop shape it + /// could miss. /// [Fact] public void DrawLandscapeDynamicsPhase_DrawWeatherOnceCallSiteHasNoEnclosingBackwardBranch() @@ -283,6 +292,65 @@ public sealed class RetailPViewPassExecutorTests && branch.TargetOffset > drawOffset); } + /// + /// S3 landing hygiene (H5): strengthens the L1 pin above against a + /// conjoined gate the post-hoc three-lens review found it does not + /// reject. L1's check (b) only looks at branches STRICTLY BETWEEN the + /// WeatherTurnFired getter call and the DrawWeatherOnce + /// call — so if (clipAssembly.OutsideViewSlices.Length != 0 && + /// walkDriver.WeatherTurnFired) passes.DrawWeatherOnce(ctx); still + /// passes it: the compiler evaluates OutsideViewSlices.Length != 0 + /// FIRST, so ITS OWN brfalse lands BEFORE the getter call's + /// offset — outside L1's window — while calls[c-1] is still the + /// getter (the added condition reads OutsideViewSlices, a + /// property getter, then .Length, a non-call ldlen, so no + /// OTHER call intervenes before the getter). This pin widens the window + /// to start at the call immediately before the whole gate — + /// , + /// the outdoor-emitters call the method's own comment names as the last + /// call before the gate — and counts EVERY branch in that wider window + /// with Assert.Single: production has exactly one, the forward + /// brfalse/brfalse.s right after the getter. A conjoined + /// gate's extra, earlier condition adds a second branch this wider + /// window catches but L1's narrower one cannot. + /// MUTATION: change the gate to if + /// (clipAssembly.OutsideViewSlices.Length != 0 && + /// walkDriver.WeatherTurnFired) — the branch count in the window + /// goes from 1 to 2 and Assert.Single fails (see the commit body + /// for the exact recorded failure text). + /// + [Fact] + public void DrawLandscapeDynamicsPhase_ExactlyOneBranchGuardsDrawWeatherOnce() + { + MethodInfo method = typeof(RetailPViewRenderer).GetMethod( + "DrawLandscapeDynamicsPhase", + BindingFlags.Instance | BindingFlags.NonPublic)!; + IReadOnlyList calls = CompiledCallGraph.Read(method); + int particlesIndex = RequiredCallIndex( + calls, + typeof(RetailPViewPassExecutor), + nameof(RetailPViewPassExecutor.DrawUnattachedSceneParticles)); + int drawIndex = RequiredCallIndex( + calls, + typeof(RetailPViewPassExecutor), + nameof(RetailPViewPassExecutor.DrawWeatherOnce)); + int particlesOffset = calls[particlesIndex].Offset; + int drawOffset = calls[drawIndex].Offset; + + IReadOnlyList branches = CompiledCallGraph.ReadBranches(method); + CompiledBranch onlyGuard = Assert.Single( + branches, + branch => branch.Offset > particlesOffset && branch.Offset < drawOffset); + + Assert.True( + onlyGuard.OpCode == OpCodes.Brfalse || onlyGuard.OpCode == OpCodes.Brfalse_S, + $"Expected the sole branch between DrawUnattachedSceneParticles and " + + $"DrawWeatherOnce to be a brfalse, was {onlyGuard.OpCode}."); + Assert.True( + onlyGuard.TargetOffset > drawOffset, + "Expected the guard branch to skip forward past DrawWeatherOnce."); + } + /// /// S3 chunk 4 fix round 2 (L8): the identical loop-shape question K1 /// asked of 's call diff --git a/tests/AcDream.App.Tests/Rendering/TerrainParticleCellVisibilityTests.cs b/tests/AcDream.App.Tests/Rendering/TerrainParticleCellVisibilityTests.cs index c681890f..e51bdcec 100644 --- a/tests/AcDream.App.Tests/Rendering/TerrainParticleCellVisibilityTests.cs +++ b/tests/AcDream.App.Tests/Rendering/TerrainParticleCellVisibilityTests.cs @@ -3,6 +3,19 @@ using AcDream.App.Rendering; namespace AcDream.App.Tests.Rendering; +/// +/// S3 landing hygiene (H2): +/// is frustum-only now — its clip-plane-list and NDC-AABB scissor +/// parameters and the CPU/GPU clip-region equivalence check they fed had +/// exactly one production caller (), +/// and that caller never actually passed either (grep). The doorway-clip/scissor +/// rejection cases this file used to pin (RejectsCellsOutsideDoorwayClipPlanes, +/// RejectsCellsOutsideDoorwayScissorAabb, UnionsCellsFromEveryLandscapeSlice) +/// pinned a mechanism nothing in production ever fed — deleted with the +/// parameters. What remains still pins the one thing this method's callers +/// actually rely on: it publishes the visible terrain cell set per landblock, +/// respecting an explicit frustum when one is given. +/// public sealed class TerrainParticleCellVisibilityTests { [Fact] @@ -16,9 +29,7 @@ public sealed class TerrainParticleCellVisibilityTests Vector3.Zero, zMin: 0f, zMax: 20f, - frustum: null, - Matrix4x4.Identity, - ReadOnlySpan.Empty); + frustum: null); Assert.Equal(64, cells.Count); Assert.Contains(0xA9B40001u, cells); @@ -40,80 +51,8 @@ public sealed class TerrainParticleCellVisibilityTests new Vector3(100f, 100f, 100f), zMin: 100f, zMax: 120f, - frustum, - Matrix4x4.Identity, - ReadOnlySpan.Empty); + frustum); Assert.Empty(cells); } - - [Fact] - public void CollectVisibleCells_RejectsCellsOutsideDoorwayClipPlanes() - { - var cells = new HashSet(); - var planes = new[] { new Vector4(-1f, 0f, 0f, -2f) }; - - TerrainModernRenderer.CollectVisibleCells( - cells, - 0xA9B4FFFFu, - Vector3.Zero, - zMin: 0f, - zMax: 20f, - frustum: null, - Matrix4x4.Identity, - planes); - - Assert.Empty(cells); - } - - [Fact] - public void CollectVisibleCells_RejectsCellsOutsideDoorwayScissorAabb() - { - var cells = new HashSet(); - - TerrainModernRenderer.CollectVisibleCells( - cells, - 0xA9B4FFFFu, - Vector3.Zero, - zMin: 0f, - zMax: 20f, - frustum: null, - Matrix4x4.Identity, - ReadOnlySpan.Empty, - ndcClipAabb: new Vector4(-4f, -4f, -2f, -2f)); - - Assert.Empty(cells); - } - - [Fact] - public void CollectVisibleCells_UnionsCellsFromEveryLandscapeSlice() - { - var cells = new HashSet(); - - TerrainModernRenderer.CollectVisibleCells( - cells, - 0xA9B4FFFFu, - Vector3.Zero, - zMin: 0f, - zMax: 20f, - frustum: null, - Matrix4x4.Identity, - ReadOnlySpan.Empty, - ndcClipAabb: new Vector4(1f, -1_000f, 23f, 1_000f)); - TerrainModernRenderer.CollectVisibleCells( - cells, - 0xA9B4FFFFu, - Vector3.Zero, - zMin: 0f, - zMax: 20f, - frustum: null, - Matrix4x4.Identity, - ReadOnlySpan.Empty, - ndcClipAabb: new Vector4(49f, -1_000f, 71f, 1_000f)); - - Assert.Equal(16, cells.Count); - Assert.Contains(0xA9B40001u, cells); - Assert.Contains(0xA9B40011u, cells); - Assert.DoesNotContain(0xA9B40009u, cells); - } }