diff --git a/src/AcDream.App/Rendering/ClipFrame.cs b/src/AcDream.App/Rendering/ClipFrame.cs index 5e890cda..a447d097 100644 --- a/src/AcDream.App/Rendering/ClipFrame.cs +++ b/src/AcDream.App/Rendering/ClipFrame.cs @@ -25,11 +25,15 @@ // Pure CPU byte-packing. The GL upload machinery this file used to carry // alongside the packing (a per-flight-slot region SSBO + terrain UBO arena, // reservation/upload-once bookkeeping, and their disposal) was deleted at -// Campaign V slice V11: the RHI arm (see RhiWorldPassSurface.PrepareClipFrame / -// SetTerrainClip in WorldPassSurface.cs) reads RegionBytes/TerrainBytes below and -// copies them into a frame ring allocation instead, so nothing here owns a GPU -// resource anymore. The byte layout is asserted by ClipFrameLayoutTests so a -// silent std430/std140 drift can't reach the GPU. +// Campaign V slice V11: the RHI arm (see RhiWorldPassSurface.PrepareClipFrame +// in WorldPassSurface.cs) reads RegionBytes below and copies it into a frame +// ring allocation instead, so nothing here owns a GPU resource anymore. S3 +// chunk 4 fix round 1 (K3) deleted SetTerrainClip (its only writer) — the +// terrain UBO half of PrepareClipFrame's publication is gone with it; +// WorldFrameSectionBinding.BindTerrainClip's existing zeroed-ring fallback +// binds TerrainBytes's permanent all-zero state instead. The byte layout is +// asserted by ClipFrameLayoutTests so a silent std430/std140 drift can't +// reach the GPU. using System; using System.Numerics; using System.Runtime.InteropServices; @@ -226,20 +230,18 @@ public sealed class ClipFrame : IDisposable count * sizeof(float) * 4)); } - /// - /// Set the terrain OutsideView clip region (the single region the terrain - /// shader gates against). length 0 ungates terrain - /// (count 0). U.3 callers never touch this — leaves it - /// at count 0. U.4 calls it with the OutsideView planes. - /// - public void SetTerrainClip(ReadOnlySpan planes) - { - int count = Math.Min(planes.Length, MaxPlanes); - Array.Clear(_terrainBytes); - WriteInt(_terrainBytes, 0, count); - for (int i = 0; i < count; i++) - WriteVec4(_terrainBytes, CellClipPlanesOffset + i * 16, planes[i]); - } + // S3 chunk 4 fix round 1 (K3): SetTerrainClip (the terrain OutsideView + // writer) is deleted — no production caller has written a non-empty + // terrain clip since S3 chunk 4's original round deleted the walk's + // per-outside-view-slice sky/terrain-clip loop, and fix round 1's K4 + // deletes the sky loop's own now-dead SetTerrainClip wrapper too. Every + // KEEP clip (exit seals, punch fans) already read their planes through + // GetSlotPlanes above / ClipViewSlice.Planes directly, never through + // this terrain-specific slot. _terrainBytes therefore stays at its + // NoClip/Reset default (count 0, all zero) for the life of a frame — + // see WorldPassSurface.cs's PrepareClipFrame for how the TerrainClip + // UBO section still gets bound as that all-zero disabled block even + // with nothing left to publish it. /// /// No-op: this container owns no GPU resource of its own on the RHI arm — diff --git a/src/AcDream.App/Rendering/RetailPViewPassExecutor.WalkLeaf.cs b/src/AcDream.App/Rendering/RetailPViewPassExecutor.WalkLeaf.cs index f8f41497..79d15f09 100644 --- a/src/AcDream.App/Rendering/RetailPViewPassExecutor.WalkLeaf.cs +++ b/src/AcDream.App/Rendering/RetailPViewPassExecutor.WalkLeaf.cs @@ -11,65 +11,43 @@ namespace AcDream.App.Rendering; /// methods — the cutover changes ORDER only. internal sealed partial class RetailPViewPassExecutor { - /// The walk's single sky turn: retail draws GameSky once - /// inside LScape::draw, clipped by the active views; looping - /// today's per-slice scissor + terrain-clip block reproduces today's - /// pixels while the driver sees ONE turn. - internal void DrawWalkSky( - RetailPViewFrameInput frame, ClipFrameAssembly clipAssembly) + /// The walk's single sky turn — retail draws GameSky exactly + /// once per frame, UNCLIPPED, for both root kinds: + /// LScape::draw @0x00506330 calls GameSky::Draw(sky,0) + /// @0x0050633c once, before the block loop, with no view clip of any + /// kind (view-culled landscape content — terrain, the sky dome, the + /// weather pass alike — is never GPU-clipped to a portal view; S3 + /// chunk 3's terrain turn and chunk 1's weather turn already draw this + /// way). + /// + /// S3 chunk 4 fix round 1 (K4): this used to loop the OUTDOOR case + /// once per active landscape view under a doorway scissor + the terrain + /// clip block (FW4 slice 6 had already retired that loop for the + /// INTERIOR case, on the same reasoning this now applies uniformly) — + /// an interior root's exit-portal aperture exactness comes from the + /// depth clear, the exit seals, and the interior repaint (FW4 slice 6's + /// own argument), not a GPU clip on the sky dome; the same is true + /// outdoors, where there is no aperture at all to bound. The former + /// per-slice scissor left stale sky color wherever the authored + /// exit-portal bands were narrower than the real opening. + internal void DrawWalkSky(RetailPViewFrameInput frame) { if (!frame.RenderSky) return; - // FW4 slice 6: an INTERIOR root draws the sky ONCE, unclipped — - // the same LScape::draw rule as the terrain turn below (view-culled - // landscape content is never view-clipped; the clear + seals + - // interior repaint own aperture exactness). The former per-slice - // scissor left stale sky color wherever the authored exit-portal - // bands were narrower than the real opening. - if (!frame.RootCell.IsOutdoorNode) + _sky?.RenderSky( + frame.Camera, + frame.CameraWorldPosition, + frame.DayFraction, + frame.ActiveDayGroup, + frame.SkyKeyframe, + frame.EnvironOverrideActive); + if (_particles is not null && _particleRenderer is not null) { - _sky?.RenderSky( + _particleRenderer.Draw( frame.Camera, frame.CameraWorldPosition, - frame.DayFraction, - frame.ActiveDayGroup, - frame.SkyKeyframe, - frame.EnvironOverrideActive); - if (_particles is not null && _particleRenderer is not null) - { - _particleRenderer.Draw( - frame.Camera, - frame.CameraWorldPosition, - ParticleRenderPass.SkyPreScene); - } - return; - } - - ReadOnlySpan slices = clipAssembly.OutsideViewSlices; - for (int sliceIndex = 0; sliceIndex < slices.Length; sliceIndex++) - { - ClipViewSlice slice = slices[sliceIndex]; - bool scissor = BeginDoorwayScissor(slice.NdcAabb); - _surface.BindTerrainClip(); - EnableClipDistances(); - _sky?.RenderSky( - frame.Camera, - frame.CameraWorldPosition, - frame.DayFraction, - frame.ActiveDayGroup, - frame.SkyKeyframe, - frame.EnvironOverrideActive); - DisableClipDistances(); - if (_particles is not null && _particleRenderer is not null) - { - _particleRenderer.Draw( - frame.Camera, - frame.CameraWorldPosition, - ParticleRenderPass.SkyPreScene); - } - if (scissor) - _surface.EndScissor(); + ParticleRenderPass.SkyPreScene); } } @@ -160,11 +138,10 @@ internal sealed partial class RetailPViewPassExecutor /// ORDER, never the leaf mechanics: /// /// -/// → the sky block of DrawLandscapeSlice, -/// looped over the active slices internally (retail draws GameSky once -/// inside LScape::draw clipped by the active views; the per-slice -/// scissor+clip here reproduces today's pixels while the driver still sees -/// ONE sky turn). +/// → S3 chunk 4 fix round 1's single unclipped +/// sky draw (retail's GameSky::Draw(sky,0) inside LScape::draw, +/// before the block loop) — one call, no scissor, no clip, for both root +/// kinds. /// → S3 chunk 3's per-land-cell /// terrain turn, UNCLIPPED for both root kinds (retail never view-clips /// terrain) — no scissor, no terrain clip, one call per order-merged @@ -232,7 +209,7 @@ internal sealed class WalkProductionLeafRenderer : IWalkFrameLeafRenderer ?? throw new ArgumentNullException(nameof(drawExitSeals)); } - public void DrawSky() => _passes.DrawWalkSky(_frame, _clipAssembly); + public void DrawSky() => _passes.DrawWalkSky(_frame); public void DrawLandCellBatch( IReadOnlyList<(uint LandblockId, int SideCellCount, int CellIndex)> cells) => diff --git a/src/AcDream.App/Rendering/RetailPViewPassExecutor.cs b/src/AcDream.App/Rendering/RetailPViewPassExecutor.cs index dd62c895..100a8116 100644 --- a/src/AcDream.App/Rendering/RetailPViewPassExecutor.cs +++ b/src/AcDream.App/Rendering/RetailPViewPassExecutor.cs @@ -214,6 +214,23 @@ public RetailPViewPassExecutor( /// the end of the walk for an outdoor root) instead of wherever the GPU /// draw happens to run at Replay. /// + /// + /// S3 chunk 4 fix round 1 (K2): the caller () now gates + /// this call on WalkFrameDriver.WeatherTurnFired — the walk's own + /// record of "a Landscape turn ran with the weather gate open" — rather + /// than re-deriving the gate independently, so this method's own + /// check is a + /// provably redundant safety net (the caller reads the same + /// 's toggles the flag was computed from). + /// + /// + /// S3 chunk 4 fix round 1 (K8): the weather MESH draws before the rain + /// PARTICLE emitters below purely as this method's own call order — + /// retail's single GameSky::Draw(sky,1) call imposes no ordering + /// between acdream's two substitutes (the mesh and the particle + /// system), so this order is a bookkeeping choice, not a retail fact. + /// /// public void DrawWeatherOnce(RetailPViewFrameInput frame) { @@ -430,10 +447,16 @@ public RetailPViewPassExecutor( return submitted; } - private bool BeginDoorwayScissor(Vector4 ndcAabb) => - _surface.BeginScissor(ndcAabb); - - private void EnableClipDistances() => _surface.EnableClipDistances(); + // S3 chunk 4 fix round 1 (K4): BeginDoorwayScissor and the + // EnableClipDistances wrapper are deleted — their only caller was + // DrawWalkSky's per-outside-view-slice loop (RetailPViewPassExecutor. + // WalkLeaf.cs), itself deleted by the same fix (retail draws the sky + // ONCE, unclipped, exactly like the terrain and the weather — see + // DrawWalkSky's own doc comment). _surface.BeginScissor/EndScissor and + // IWorldPassSurface.EnableClipDistances stay: RhiWorldPassSurface. + // ClearInteriorDepth still ends an active scissor, and + // WorldScenePassExecutor (the separate flat-world path, out of this + // chunk's scope) still calls EnableClipDistances directly. private void DisableClipDistances() => _surface.DisableClipDistances(); } diff --git a/src/AcDream.App/Rendering/RetailPViewRenderer.cs b/src/AcDream.App/Rendering/RetailPViewRenderer.cs index 6421abab..cbde681f 100644 --- a/src/AcDream.App/Rendering/RetailPViewRenderer.cs +++ b/src/AcDream.App/Rendering/RetailPViewRenderer.cs @@ -451,7 +451,8 @@ internal sealed class RetailPViewRenderer DrawLandscapeDynamicsPhase( ctx, passes, - clipAssembly); + clipAssembly, + walkDriver); }; try { @@ -461,7 +462,8 @@ internal sealed class RetailPViewRenderer DrawLandscapeDynamicsPhase( ctx, passes, - clipAssembly); + clipAssembly, + walkDriver); } } finally @@ -624,16 +626,17 @@ internal sealed class RetailPViewRenderer private void DrawLandscapeDynamicsPhase( RetailPViewFrameInput ctx, RetailPViewPassExecutor passes, - ClipFrameAssembly clipAssembly) + ClipFrameAssembly clipAssembly, + Walk.WalkFrameDriver walkDriver) { - if (clipAssembly.OutsideViewSlices.Length == 0) - return; - - // Ownerless OUTDOOR-cell emitters — now unconditional: the old - // hasBuildingLookIns gate only existed to sequence this submission - // around the OLD static barrier drains, which no longer run here - // (the walk owns its own alpha barriers — WalkFrameDriver.OnBuildingTurn). - passes.DrawUnattachedSceneParticles(ctx, outdoorCells: true); + if (clipAssembly.OutsideViewSlices.Length != 0) + { + // Ownerless OUTDOOR-cell emitters — now unconditional: the old + // hasBuildingLookIns gate only existed to sequence this submission + // around the OLD static barrier drains, which no longer run here + // (the walk owns its own alpha barriers — WalkFrameDriver.OnBuildingTurn). + passes.DrawUnattachedSceneParticles(ctx, outdoorCells: true); + } // S3 chunk 4 (§10.2): the former per-outside-view-slice loop // (SetTerrainClip + ClearClipRouting + DrawLandscapeSliceLate, one @@ -648,7 +651,19 @@ internal sealed class RetailPViewRenderer // emitter. The OC transcript print no longer lives here — it moved // to WalkFrameDriver.OnWeatherTurn, fired by RetailFrameWalk. // DrawLandscape at Collect time (S3 chunk 4, O3). - passes.DrawWeatherOnce(ctx); + // + // S3 chunk 4 fix round 1 (K2): gated on walkDriver.WeatherTurnFired + // — the SAME predicate that gated the Collect-time "OC" transcript + // print (WalkFrameDriver.OnWeatherTurn, fired only when a Landscape + // turn actually ran with ctx.WeatherGateOpen open) — instead of + // independently re-deriving "clipAssembly.OutsideViewSlices.Length + // > 0", which could diverge from the print's gate on an interior + // root whose landscape turn ran but whose reassembled outside-view + // slices ended up empty (the transcript would then report a weather + // turn this call never actually drew). Gate closed -> neither the + // print nor this draw fires; gate open -> both do. + if (walkDriver.WeatherTurnFired) + passes.DrawWeatherOnce(ctx); } /// Campaign FW4 slice 2 — the walk-flood seal draw. Retail's diff --git a/src/AcDream.App/Rendering/Walk/WalkFrameDriver.cs b/src/AcDream.App/Rendering/Walk/WalkFrameDriver.cs index 81ef1453..c4d4844d 100644 --- a/src/AcDream.App/Rendering/Walk/WalkFrameDriver.cs +++ b/src/AcDream.App/Rendering/Walk/WalkFrameDriver.cs @@ -597,6 +597,23 @@ internal sealed class WalkFrameDriver : IWalkEventSink, IWalkLookInViewSource private readonly List _floodViewRouteScratch = new(); private WalkPlane _lookInCyPlane; + /// S3 chunk 4 fix round 1 (K2): set exactly when fires this frame — i.e. a + /// Landscape turn actually ran (retail's RetailFrameWalk.DrawLandscape + /// only reaches that call for an outdoor root, or an interior root whose + /// flood still has a surviving exit view) AND ctx.WeatherGateOpen + /// held. This is "the walk's own predicate" the S3 chunk-4 fix round 1 + /// unifies the transcript print and the Replay-time weather draw + /// on — before this, the print's own gate (ctx.WeatherGateOpen + /// alone) and the draw's gate (clipAssembly.OutsideViewSlices.Length + /// > 0, re-derived independently at Replay) could diverge on an + /// interior root whose landscape turn ran but whose reassembled outside- + /// view slices ended up empty, letting the transcript report a weather + /// turn the frame never actually drew. Reset every Collect + /// (/); read at Replay + /// by . + internal bool WeatherTurnFired { get; private set; } + /// S3 chunk 3 (§9.2 B2), fix round 1 (F2), round 2 (F10): /// 's PENDING terrain batch — every /// event since the last flush, @@ -693,6 +710,25 @@ internal sealed class WalkFrameDriver : IWalkEventSink, IWalkLookInViewSource return routeIndex; } + /// Test-only accessor (S3 chunk 4 fix round 1, K7): the packed + /// GPU clip slot one look-in route's slice was assigned by 's call to — the + /// SAME value resolves + /// through _clipFrame.GetSlotPlanes(slice.ClipSlot) for the root- + /// flood case. Restores the "a real look-in slice never reuses the + /// reserved no-clip slot 0" pin the deleted VisibleClipSlotsInLookInTurn + /// used to prove. indexes _lookInTurns + /// directly, matching 's own + /// parameter (not the flood-cell indirection goes through). + internal uint LookInSliceClipSlotAt(int routeIndex, int sliceOffset = 0) + { + WalkLookInTurn turn = _lookInTurns[routeIndex]; + if ((uint)sliceOffset >= (uint)turn.SliceCount) + throw new ArgumentOutOfRangeException(nameof(sliceOffset)); + return _lookInSlices[turn.SliceStart + sliceOffset].ClipSlot; + } + internal List VisitedBuildings { get; } = new(); internal HashSet VisitedLandscapeCellIds { get; } = new(); @@ -764,6 +800,7 @@ internal sealed class WalkFrameDriver : IWalkEventSink, IWalkLookInViewSource _landscapeTurnsThisFrame = 0; _currentDcStage = null; _readyToReplay = false; + WeatherTurnFired = false; _stream.Reset(); _events.Clear(); _markPositions.Clear(); @@ -917,6 +954,7 @@ internal sealed class WalkFrameDriver : IWalkEventSink, IWalkLookInViewSource _landscapeTurnsThisFrame = 0; _currentDcStage = null; _readyToReplay = false; + WeatherTurnFired = false; _stream.Reset(); _events.Clear(); _markPositions.Clear(); @@ -1550,20 +1588,25 @@ internal sealed class WalkFrameDriver : IWalkEventSink, IWalkLookInViewSource /// S3 chunk 4 (O3): retail's GameSky::Draw(sky,1) /// @0x00506ff0 weather pass — - /// only calls this when its own gate already held, so this handler is - /// print-only, matching 's own - /// shape (no stream side effect; the flag gate wraps the entire body). - /// The GPU draw stays where chunk 1 round 2 put it — , called directly - /// from at - /// Replay time — this hook exists ONLY to move the "OC" transcript print - /// to retail's real Collect-time turn. + /// only calls this when its own gate already held, so this handler + /// records unconditionally (S3 chunk 4 + /// fix round 1, K2 — the one predicate the Replay-time draw now reads, + /// so a landscape turn that ran with the weather gate open can never be + /// under-reported to the draw side) and is otherwise print-only, + /// matching 's own shape (no + /// stream side effect). The GPU draw stays where chunk 1 round 2 put + /// it — , called + /// directly from + /// at Replay time, now gated on THIS flag instead of re-deriving its + /// own independent condition. void IWalkEventSink.OnWeatherTurn(uint viewerCellId) { + RequireOpenFrame(); + WeatherTurnFired = true; + if (!AcDream.Core.Rendering.RenderingDiagnostics.DumpWalkTranscriptEnabled) return; - RequireOpenFrame(); WalkTranscriptDump.PrintObjectCellTurn(viewerCellId); } diff --git a/src/AcDream.App/Rendering/WorldPassSurface.cs b/src/AcDream.App/Rendering/WorldPassSurface.cs index 6b17adca..5ac23b23 100644 --- a/src/AcDream.App/Rendering/WorldPassSurface.cs +++ b/src/AcDream.App/Rendering/WorldPassSurface.cs @@ -29,8 +29,8 @@ internal interface IRenderFrameGlState internal interface IWorldPassSurface { /// - /// Publishes this frame's per-cell clip-region table and terrain clip block, - /// and routes both to the renderers that read them. + /// 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 @@ -38,12 +38,21 @@ internal interface IWorldPassSurface /// 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): this no longer also publishes the + /// terrain clip block — ClipFrame.SetTerrainClip, its only + /// writer, is deleted, so stays at + /// its permanent NoClip/Reset default (count 0, all zero) for the life of + /// a frame. The terrain/sky shaders still declare the TerrainClip + /// UBO (binding=2), so 's + /// existing zeroed-ring fallback (used whenever nothing published the + /// section) now binds that all-zero disabled block on every frame instead + /// of only when something raced the publish — the observable result is + /// identical either way, since a zeroed publish and a zeroed fallback are + /// the same bytes. /// void PrepareClipFrame(int terrainUploadCount); - /// Replaces the terrain clip planes and republishes the block. - void SetTerrainClip(ReadOnlySpan planes); - /// /// Re-asserts the terrain clip block at its binding. /// @@ -122,13 +131,12 @@ internal sealed class RhiWorldPassSurface : IWorldPassSurface _scope.Sections.ClipRegions = Publish( _clipFrame.RegionBytes, GpuRingUsage.Storage); - PublishTerrainClip(); - } - - public void SetTerrainClip(ReadOnlySpan planes) - { - _clipFrame.SetTerrainClip(planes); - PublishTerrainClip(); + // S3 chunk 4 fix round 1 (K3): the terrain-clip publish half is + // deleted — SetTerrainClip (its only writer) is gone, so publishing + // ClipFrame.TerrainBytes here would only ever republish permanent + // zero bytes. WorldFrameSectionBinding.BindTerrainClip's existing + // zeroed-ring fallback (for when nothing published the section) + // binds that same all-zero disabled block instead. } /// No-op: there is no ambient binding to re-assert. See the interface. @@ -176,11 +184,6 @@ internal sealed class RhiWorldPassSurface : IWorldPassSurface _scope.ClearInteriorDepth(); } - private void PublishTerrainClip() => - _scope.Sections.TerrainClip = Publish( - _clipFrame.TerrainBytes, - GpuRingUsage.Uniform); - private GpuBufferSection Publish(ReadOnlySpan data, GpuRingUsage usage) { IGpuFrame frame = _frames.CurrentFrame diff --git a/tests/AcDream.App.Tests/Rendering/ClipFrameLayoutTests.cs b/tests/AcDream.App.Tests/Rendering/ClipFrameLayoutTests.cs index 72a7f709..dca1080a 100644 --- a/tests/AcDream.App.Tests/Rendering/ClipFrameLayoutTests.cs +++ b/tests/AcDream.App.Tests/Rendering/ClipFrameLayoutTests.cs @@ -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 bytes, int offset, Vector4 expected) { @@ -255,15 +246,21 @@ public class ClipFrameLayoutTests /// KEEP item 2 — punch fans: RetailPViewPassExecutor.DrawWalkPunchFan /// reads its clip planes through clipAssembly.OutsideViewSlices /// [activeViewIndex].Planes — 's - /// Planes field, which ClipFrameAssembler.Assemble sets - /// DIRECTLY to cps.PlaneArray (no packed-byte round trip at - /// all). This pin is therefore the more fundamental of the two: it - /// proves '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. + /// Planes field. S3 chunk 4 fix round 1 (K6): this pin now builds + /// that slice through the REAL production assembly — + /// ClipFrameAssembler.Assemble's own + /// outsideSlicesList.Add(new ClipViewSlice(slot, AabbOf(poly), + /// planes)) line, the exact construction + /// ReassembleOutsideViewFromWalk (the walk's real interior-root + /// producer) shares — instead of hand-constructing a + /// directly from '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 ClipViewSlice 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. /// [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); } diff --git a/tests/AcDream.App.Tests/Rendering/RetailPViewPassExecutorTests.cs b/tests/AcDream.App.Tests/Rendering/RetailPViewPassExecutorTests.cs index 17b752c7..b793bfac 100644 --- a/tests/AcDream.App.Tests/Rendering/RetailPViewPassExecutorTests.cs +++ b/tests/AcDream.App.Tests/Rendering/RetailPViewPassExecutorTests.cs @@ -157,11 +157,15 @@ public sealed class RetailPViewPassExecutorTests /// DrawLandscapeSliceLate leaf, one call per active landscape /// view) is deleted — DrawLandscapeDynamicsPhase now calls /// 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 Assert.Single fail. + /// unconditionally, 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 + /// regression this file's review caught) — see + /// + /// 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 Assert.Single fail. /// [Fact] public void DrawLandscapeDynamicsPhase_CallsDrawWeatherOnceExactlyOnce() @@ -177,6 +181,47 @@ public sealed class RetailPViewPassExecutorTests && call.Target.Name == nameof(RetailPViewPassExecutor.DrawWeatherOnce)); } + /// + /// S3 chunk 4 fix round 1 (K1, blocking): the real loop-shape pin. + /// 's + /// Assert.Single over call-site offsets still passes when the ONE + /// call site sits inside a foreach — 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 + /// for/foreach/while) enclose the + /// DrawWeatherOnce 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 foreach (var slice in clipAssembly.OutsideViewSlices) — + /// 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. + /// + [Fact] + public void DrawLandscapeDynamicsPhase_DrawWeatherOnceCallSiteHasNoEnclosingBackwardBranch() + { + MethodInfo method = typeof(RetailPViewRenderer).GetMethod( + "DrawLandscapeDynamicsPhase", + BindingFlags.Instance | BindingFlags.NonPublic)!; + IReadOnlyList calls = CompiledCallGraph.Read(method); + int callIndex = RequiredCallIndex( + calls, + typeof(RetailPViewPassExecutor), + nameof(RetailPViewPassExecutor.DrawWeatherOnce)); + int callOffset = calls[callIndex].Offset; + + IReadOnlyList branches = CompiledCallGraph.ReadBranches(method); + Assert.DoesNotContain( + branches, + branch => branch.TargetOffset < branch.Offset + && branch.TargetOffset <= callOffset + && callOffset < branch.Offset); + } + /// /// S3 chunk 1 fix round 2 (§11.6 H1): is diff --git a/tests/AcDream.App.Tests/Rendering/Walk/WalkFrameDriverTests.cs b/tests/AcDream.App.Tests/Rendering/Walk/WalkFrameDriverTests.cs index 56cefa41..78afdc76 100644 --- a/tests/AcDream.App.Tests/Rendering/Walk/WalkFrameDriverTests.cs +++ b/tests/AcDream.App.Tests/Rendering/Walk/WalkFrameDriverTests.cs @@ -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 diff --git a/tests/AcDream.App.Tests/Rendering/Walk/WalkFrameDriverTranscriptTests.cs b/tests/AcDream.App.Tests/Rendering/Walk/WalkFrameDriverTranscriptTests.cs index a2370ef9..9ce4e67d 100644 --- a/tests/AcDream.App.Tests/Rendering/Walk/WalkFrameDriverTranscriptTests.cs +++ b/tests/AcDream.App.Tests/Rendering/Walk/WalkFrameDriverTranscriptTests.cs @@ -711,4 +711,79 @@ public sealed partial class WalkFrameDriverTests RenderingDiagnostics.DumpWalkTranscriptEnabled = previous; } } + + /// + /// 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 — + /// — instead of two + /// independently re-derived conditions that could diverge. Before this + /// fix, the print fired on ctx.WeatherGateOpen alone while the + /// Replay-time draw additionally required + /// clipAssembly.OutsideViewSlices.Length > 0, 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. is exactly the flag + /// RetailPViewRenderer.DrawLandscapeDynamicsPhase now gates + /// DrawWeatherOnce on, so this test's flag assertion doubles as + /// the draw-side pin: gate closed -> the flag stays false (so + /// DrawWeatherOnce would not fire either) AND no "OC" line + /// prints; gate open -> the flag becomes true (so + /// DrawWeatherOnce would fire) AND exactly one "OC" line prints. + /// + [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()); + 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 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; + } + } }