diff --git a/docs/plans/2026-08-22-visualmaster-campaign.md b/docs/plans/2026-08-22-visualmaster-campaign.md index e2421b09..8127b462 100644 --- a/docs/plans/2026-08-22-visualmaster-campaign.md +++ b/docs/plans/2026-08-22-visualmaster-campaign.md @@ -653,7 +653,54 @@ deterministic amount between two resolves — proving both the first-advance snap and that a SECOND advance still eases normally — without a real-time `Thread.Sleep`. +**Review fix round 4 (2026-08-23): wind decoupled from the shadow gate.** +The reviewer's offline pixel apparatus caught a real design defect the +first three rounds' CPU-side reasoning could not see: foliage wind was +welded to "directional shadows rendered this frame." Evidence: at the +High preset with `sun-shadow-strength=0` and wind-strength 2 + 1 m +lean/branch amplitude, wind-on vs wind-off at the same pinned clock +differed by only 49–65 px — inside the apparatus's own 22 px run-to-run +noise floor, i.e. no measurable motion at all. A CPU probe independently +confirmed `ResolveFoliageWind` itself was correct (first advance snaps +exactly to Clear's 0.25/0.15, the gate is 1, one graph instance) — the +correct uniform was computed but never reached the world pass. Root +cause: `DirectionalSunShadowRenderer.Render` left `_currentFrameBinding` +at its pure `Disabled` (no-buffer) default on both early-out paths +(`!environment.ShouldRender`, `ResidentWindowUnavailable`); +`WbDrawDispatcher.PipelinesFor`/`TerrainModernRenderer`'s matching +selection logic only chose the atmospheric receiver pipeline +(`mesh_atmospheric`, which alone `#include`s `foliage_wind.glsl`) when +`TryGetCurrentFrameBinding` returned true; with no buffer it always +returned false, so the world pass silently ran the plain `mesh_modern` +pipeline instead — which has no wind code at all. Because the shadow +gate is `ActiveDayGroupMultiplier = dayGroupPolicy × elevationResponse × +strength`, this killed wind every NIGHT (elevation response → 0), at +user `sun-shadow-strength` 0, and under the portal/login cover — not +just in the artificial `strength=0` repro. +Fix (decouple, don't patch): `DirectionalShadowFrameBinding` gained +`IsBindableFor` ("a real current-frame allocation exists") separate from +`IsValidFor` ("...and it is Enabled with real shadow content" — the +volumetric pass still gates on this, unchanged); +`TryGetCurrentFrameBinding` now returns `IsBindableFor`. When the +built-in pack supplies an `AtmosphericFrame` binding (declared packs +never do, so they are unaffected), `Render`'s two early-out paths call a +new `PublishDisabledReceiverBinding`: it allocates one real ring slice +and writes a DISABLED `DirectionalShadowUniforms` block — every matrix +Identity, every control/bias term zero, `TextureAndFlags` all zero (bit +0 clear is exactly what `directional_shadow_receiver.glsl`'s +`acdreamDirectionalShadowVisibility` already reads as "no shadow, full +visibility" via its existing early `return 1.0`), and a UNIT light +direction `(0,0,1)` so a fragment shader's `normalize()` can never +produce NaN. `BindDirectionalShadowReceiver` and +`TerrainModernRenderer`'s shadow-buffer bind now check `Buffer is not +null` instead of `Enabled`, so this disabled block actually gets bound +once it is selected. Consequence in one sentence: the built-in pack's +world pass now always runs the receiver shader; with shadows gated off, +the shadow block's own flag bit makes its shadow term numerically the +plain lighting sum, while wind keeps moving correctly. + ## VM7 — Closeout and merge + — Closeout and merge - Full gates: `tools/run-release-gate.ps1` (hermetic lanes), the AR reference matrix re-run for the changed presets, VM0's masked comparison diff --git a/src/AcDream.App/Rendering/DirectionalShadowReceiver.cs b/src/AcDream.App/Rendering/DirectionalShadowReceiver.cs index 5a1c4531..7d4f5c0e 100644 --- a/src/AcDream.App/Rendering/DirectionalShadowReceiver.cs +++ b/src/AcDream.App/Rendering/DirectionalShadowReceiver.cs @@ -30,11 +30,36 @@ internal readonly record struct DirectionalShadowFrameBinding( { internal static DirectionalShadowFrameBinding Disabled => default; - internal bool IsValidFor(IGpuFrame frame) => - Enabled - && Buffer is not null + /// + /// Campaign VM VM6 review fix round 4 (item 1): "there IS a real, + /// current-frame ring allocation here" — independent of whether the + /// shadow content it carries is Enabled. A directional-shadow-gated-off + /// frame (indoors, portal cover, night, sun-shadow-strength 0, ...) + /// still publishes a DISABLED-content block via + /// PublishDisabledReceiverBinding when an AtmosphericFrame binding is + /// available, specifically so the world receiver pipeline (which reads + /// wind from set 3/binding 5, not the shadow block) keeps running + /// regardless of shadow gating. This is the predicate consumers use to + /// decide "is there something here to bind" — see + /// BindDirectionalShadowReceiver and TerrainModernRenderer's own + /// receiver-pipeline selection. + /// + internal bool IsBindableFor(IGpuFrame frame) => + Buffer is not null && FrameSerial == frame.Serial - && SizeBytes == DirectionalShadowUniforms.SizeInBytes + && SizeBytes == DirectionalShadowUniforms.SizeInBytes; + + /// + /// "There is a real, current-frame, ENABLED shadow map here" — the + /// stricter predicate consumers that need actual shadow content (texture + /// slot, cascade count) must use, e.g. VolumetricShaftRenderer's own + /// gate. A disabled block published by PublishDisabledReceiverBinding + /// deliberately fails this (TextureSlot.Unassigned, CascadeCount 0) even + /// though it passes IsBindableFor. + /// + internal bool IsValidFor(IGpuFrame frame) => + IsBindableFor(frame) + && Enabled && TextureSlot.IsAssigned && CascadeCount is >= 2 and <= 4; } @@ -47,6 +72,20 @@ internal interface IDirectionalShadowReceiverSource { DirectionalShadowPipelineShaders PipelineShaders { get; } + /// + /// Campaign VM VM6 review fix round 4 (item 2): returns TRUE whenever + /// is bindable for + /// () — NOT + /// whether shadows are Enabled this frame. This deliberately decouples + /// world-receiver pipeline selection (which needs binding 5's wind + /// data, unconditionally, from any bound AtmosphericFrame) from the + /// directional-shadow gate (indoors, portal cover, night, strength 0, + /// ...), which used to also silently disable foliage wind because it + /// left the world pass on the plain (no-wind) pipeline. A caller that + /// needs actual shadow CONTENT — texture slot, cascade count — must + /// additionally check + /// on the returned binding (e.g. VolumetricShaftRenderer's own gate). + /// bool TryGetCurrentFrameBinding( IGpuFrame frame, out DirectionalShadowFrameBinding binding); diff --git a/src/AcDream.App/Rendering/DirectionalSunShadowRenderer.cs b/src/AcDream.App/Rendering/DirectionalSunShadowRenderer.cs index f3c1cb84..152b7657 100644 --- a/src/AcDream.App/Rendering/DirectionalSunShadowRenderer.cs +++ b/src/AcDream.App/Rendering/DirectionalSunShadowRenderer.cs @@ -313,7 +313,7 @@ internal sealed class DirectionalSunShadowRenderer : IDirectionalShadowReceiverS { ArgumentNullException.ThrowIfNull(frame); binding = _currentFrameBinding; - return !_disposed && binding.IsValidFor(frame); + return !_disposed && binding.IsBindableFor(frame); } internal DirectionalSunShadowDiagnostics Render( @@ -336,16 +336,20 @@ internal sealed class DirectionalSunShadowRenderer : IDirectionalShadowReceiverS ? Stopwatch.GetTimestamp() - cpuStageStarted : 0L; if (!environment.ShouldRender) + { + PublishDisabledReceiverBinding(frame, in input); return Disabled( in environment, new DirectionalSunShadowCpuStageTicks( environmentGateTicks, 0L, 0L, 0L, 0L)); + } if (input.ResidentMaximumReachMeters <= input.CameraNearMeters) { environment = environment with { Reason = DirectionalShadowGateReason.ResidentWindowUnavailable, }; + PublishDisabledReceiverBinding(frame, in input); return Disabled( in environment, new DirectionalSunShadowCpuStageTicks( @@ -447,6 +451,81 @@ internal sealed class DirectionalSunShadowRenderer : IDirectionalShadowReceiverS } } + /// + /// Campaign VM VM6 review fix round 4 (item 3): the offline pixel gate + /// found wind welded to "directional shadows rendered this frame" — + /// 's two early-disabled paths left + /// at its pure + /// default (no + /// buffer at all), so returned + /// false, the world receiver pipeline selection + /// (DirectionalShadowReceiverPolicy.ShouldSelectReceiverPipeline) + /// fell back to the plain mesh_modern/mesh_atmospheric-less + /// pipeline, and foliage_wind.glsl's include never ran — even though + /// the built-in pack's CPU wind state (ResolveFoliageWind) kept + /// ticking correctly the whole time. The shadow gate fires far more + /// often than "shadows visibly missing" suggests: every night + /// (elevation response goes to 0), at authored sun-shadow-strength 0, + /// indoors, and under the portal/login cover. + /// + /// Fix: when the built-in pack supplies an AtmosphericFrame + /// binding (declared packs never do — their receiver shaders don't + /// declare set 3 binding 5, so they correctly keep today's plain- + /// pipeline behaviour when shadows are gated off), publish a REAL ring + /// allocation carrying a DISABLED shadow block — every matrix Identity, + /// every control/bias term zero, TextureAndFlags all zero (bit 0 + /// clear is exactly what directional_shadow_receiver.glsl's + /// acdreamDirectionalShadowVisibility already reads as "no + /// shadow, full visibility" — see its early return 1.0), and a + /// UNIT light direction (never the zero vector a fragment shader's + /// normalize() could turn into NaN). This binding passes + /// (there IS + /// a current-frame allocation) but fails + /// (Enabled is + /// false, TextureSlot is Unassigned, CascadeCount is 0) — so the world + /// receiver pipeline runs (and reads correct wind data from the + /// AtmosphericFrame half of the same binding), while anything that + /// actually needs shadow content (VolumetricShaftRenderer's own gate) + /// still correctly reports no current directional shadow. + /// internal, not private: a hermetic test drives + /// this directly (constructing the full environment gate plus a real + /// WbDrawDispatcher/TerrainModernRenderer pair just to reach Render's + /// early-out paths is not a cheap test — no test in this suite + /// constructs either) instead of standing up that dependency chain. + /// + internal void PublishDisabledReceiverBinding( + IGpuFrame frame, + in DirectionalSunShadowRenderInput input) + { + if (!input.AtmosphericFrame.IsBound) + return; + + var disabledUniforms = new DirectionalShadowUniforms( + Matrix4x4.Identity, + Matrix4x4.Identity, + Matrix4x4.Identity, + Matrix4x4.Identity, + Vector4.Zero, + Vector4.Zero, + Vector4.Zero, + new UInt4(0u, 0u, 0u, 0u), + new Vector4(0f, 0f, 1f, 0f)); + GpuRingAllocation allocation = frame.AllocateRing( + DirectionalShadowUniforms.SizeInBytes, + GpuRingUsage.Uniform); + MemoryMarshal.Write(allocation.Data, in disabledUniforms); + + _currentFrameBinding = new DirectionalShadowFrameBinding( + frame.Serial, + Enabled: false, + allocation.Buffer, + allocation.OffsetBytes, + DirectionalShadowUniforms.SizeInBytes, + GpuTextureSlot.Unassigned, + CascadeCount: 0, + AtmosphericFrame: input.AtmosphericFrame); + } + internal static DirectionalShadowCasterClassDiagnostics CompleteCasterClassDiagnostics( in DirectionalShadowCasterBuildStats casterStats, diff --git a/src/AcDream.App/Rendering/TerrainModernRenderer.Rhi.cs b/src/AcDream.App/Rendering/TerrainModernRenderer.Rhi.cs index b688cb4f..0d9a0f0d 100644 --- a/src/AcDream.App/Rendering/TerrainModernRenderer.Rhi.cs +++ b/src/AcDream.App/Rendering/TerrainModernRenderer.Rhi.cs @@ -255,7 +255,19 @@ public sealed unsafe partial class TerrainModernRenderer BindTilingTable(encoder); WorldFrameSectionBinding.BindSceneLighting(encoder, scope.Sections, frame); WorldFrameSectionBinding.BindTerrainClip(encoder, scope.Sections, frame); - if (shadowBinding.Enabled && shadowBinding.Buffer is not null) + // Campaign VM VM6 review fix round 4 (item 4): bind on BINDABLE, + // not Enabled — same rule as WbDrawDispatcher.BindDirectionalShadowReceiver. + // TryGetCurrentFrameBinding now returns true for a disabled-content + // binding whenever the built-in pack published one (shadows gated + // off but AtmosphericFrame still bound), which also switches + // ShouldSelectReceiverPipeline to the receiver pipeline above — + // that pipeline expects SOMETHING bound at set 3/binding 6. Without + // this fix the stale `Enabled` check would skip the bind here, + // leaving binding 6 reading whatever a prior pass left there + // instead of the safe all-zero disabled block. Terrain has no wind + // (only the world mesh receiver reads AtmosphericFrame), so this + // fix only concerns the shadow block, not foliage. + if (shadowBinding.Buffer is not null) { encoder.BindUniformBuffer( GpuBindingModel.UniformDirectionalShadow, diff --git a/src/AcDream.App/Rendering/Wb/WbDrawDispatcher.DirectionalShadowReceivers.cs b/src/AcDream.App/Rendering/Wb/WbDrawDispatcher.DirectionalShadowReceivers.cs index 5b4ba103..244e7d95 100644 --- a/src/AcDream.App/Rendering/Wb/WbDrawDispatcher.DirectionalShadowReceivers.cs +++ b/src/AcDream.App/Rendering/Wb/WbDrawDispatcher.DirectionalShadowReceivers.cs @@ -117,7 +117,19 @@ public sealed partial class WbDrawDispatcher IGpuPassEncoder encoder, in DirectionalShadowFrameBinding binding) { - if (!binding.Enabled || binding.Buffer is null) + // Campaign VM VM6 review fix round 4 (item 4): bind on BINDABLE, + // not Enabled. A directional-shadow-gated-off frame still publishes + // a real (disabled-content) binding via + // DirectionalSunShadowRenderer.PublishDisabledReceiverBinding so + // the world receiver pipeline (selected on the exact same + // TryGetCurrentFrameBinding predicate — see + // WbDrawDispatcher.PipelinesFor above) can bind BOTH words: the + // disabled shadow block (its flags bit 0 clear makes + // directional_shadow_receiver.glsl's acdreamDirectionalShadowVisibility + // return 1.0 unconditionally — numerically the plain lighting sum) + // and the real AtmosphericFrame wind data mesh_atmospheric.vert + // needs regardless of shadow gating. + if (binding.Buffer is null) return; encoder.BindUniformBuffer( GpuBindingModel.UniformDirectionalShadow, diff --git a/tests/AcDream.App.Tests/Rendering/DirectionalShadowGpuTests.cs b/tests/AcDream.App.Tests/Rendering/DirectionalShadowGpuTests.cs index a55f1868..7e9467f6 100644 --- a/tests/AcDream.App.Tests/Rendering/DirectionalShadowGpuTests.cs +++ b/tests/AcDream.App.Tests/Rendering/DirectionalShadowGpuTests.cs @@ -538,6 +538,145 @@ public sealed class DirectionalShadowGpuTests Assert.Equal("shadow-frame", shadowBind.BufferName); } + /// + /// Campaign VM VM6 review fix round 4, test (a): the offline pixel gate + /// found foliage wind welded to "directional shadows rendered this + /// frame" — a shadow-gated-off frame used to leave + /// TryGetCurrentFrameBinding returning false, which fell the world + /// receiver pipeline back to the plain (no-wind) mesh pipeline. This + /// proves the fix directly against PublishDisabledReceiverBinding + /// (made internal for exactly this reason — see its doc comment): + /// given a bound AtmosphericFrame, it publishes a binding that IS + /// bindable (TryGetCurrentFrameBinding true) but is NOT a valid shadow + /// (IsValidFor false, Enabled false), and the written block's flags + /// word is exactly 0 (directional_shadow_receiver.glsl's bit-0-clear + /// "full visibility" contract) with a unit light direction (0,0,1) — + /// never the zero vector a shader's normalize() could turn into NaN. + /// + [Fact] + public void PublishDisabledReceiverBinding_IsBindableButNotValidWithZeroFlagsAndUnitDirection() + { + using var device = new RecordingGpuDevice(); + using var renderer = new DirectionalSunShadowRenderer(device, DirectionalShadowPreset.Low); + using IGpuBuffer atmosphericBuffer = Buffer(device, "wind-only-frame", GpuBufferUsage.Uniform); + var atmosphericFrame = new AtmosphericFrameBufferBinding( + atmosphericBuffer, + OffsetBytes: 0u, + SizeBytes: 192u); + var input = new DirectionalSunShadowRenderInput( + default, + Matrix4x4.Identity, + Matrix4x4.Identity, + new DirectionalShadowCasterFrame(), + AtmosphericFrame: atmosphericFrame); + device.Clear(); + using IGpuFrame frame = device.BeginFrame(); + + renderer.PublishDisabledReceiverBinding(frame, in input); + + Assert.True(renderer.TryGetCurrentFrameBinding(frame, out DirectionalShadowFrameBinding binding)); + Assert.True(binding.IsBindableFor(frame)); + Assert.False(binding.IsValidFor(frame)); + Assert.False(binding.Enabled); + Assert.Equal(0, binding.CascadeCount); + Assert.False(binding.TextureSlot.IsAssigned); + Assert.True(binding.AtmosphericFrame.IsBound); + Assert.Same(atmosphericBuffer, binding.AtmosphericFrame.Buffer); + + DirectionalShadowUniforms written = MemoryMarshal.Read( + device.RingBytes.Slice( + (int)binding.OffsetBytes, + DirectionalShadowUniforms.SizeInBytes)); + Assert.Equal(0u, written.TextureAndFlags.W); + Assert.Equal(new Vector4(0f, 0f, 1f, 0f), written.LightDirectionAndSource); + } + + [Fact] + public void PublishDisabledReceiverBinding_NoOpsWhenAtmosphericFrameIsUnboundPreservingTodaysBehaviour() + { + // Campaign VM VM6 review fix round 4, test (b): a declared (non + // built-in) pack never supplies an AtmosphericFrame binding, so + // this must stay a pure no-op for it — TryGetCurrentFrameBinding + // keeps returning false exactly like before this fix, and a + // declared pack's receiver shader (which never declares set 3 + // binding 5) is unaffected. + using var device = new RecordingGpuDevice(); + using var renderer = new DirectionalSunShadowRenderer(device, DirectionalShadowPreset.Low); + var input = new DirectionalSunShadowRenderInput( + default, + Matrix4x4.Identity, + Matrix4x4.Identity, + new DirectionalShadowCasterFrame()); + device.Clear(); + using IGpuFrame frame = device.BeginFrame(); + + renderer.PublishDisabledReceiverBinding(frame, in input); + + Assert.False(renderer.TryGetCurrentFrameBinding(frame, out _)); + Assert.Empty(device.OfKind()); + } + + [Fact] + public void BindDirectionalShadowReceiver_WithADisabledBindingEmitsBothShadowAndAtmosphericBinds() + { + // Campaign VM VM6 review fix round 4, test (c): the whole point of + // publishing a disabled-content binding instead of leaving + // TryGetCurrentFrameBinding at false is that BindDirectionalShadowReceiver + // (fixed in this same round to check Buffer, not Enabled) actually + // emits BOTH binds for it — the safe all-zero shadow block AND the + // real wind data — so the world receiver pipeline (selected because + // the binding is bindable) has everything it declares. + using var device = new RecordingGpuDevice(); + using IGpuBuffer shadowBuffer = Buffer(device, "disabled-shadow-frame", GpuBufferUsage.Uniform); + using IGpuBuffer atmosphericBuffer = Buffer(device, "disabled-atmospheric-frame", GpuBufferUsage.Uniform); + var binding = new DirectionalShadowFrameBinding( + FrameSerial: 1, + Enabled: false, + Buffer: shadowBuffer, + OffsetBytes: 0u, + SizeBytes: DirectionalShadowUniforms.SizeInBytes, + TextureSlot: GpuTextureSlot.Unassigned, + CascadeCount: 0, + AtmosphericFrame: new AtmosphericFrameBufferBinding( + atmosphericBuffer, + OffsetBytes: 0u, + SizeBytes: 192u)); + + device.Clear(); + var target = device.CreateRenderTarget(new GpuRenderTargetDescription( + "test-world-hdr", + 640, + 480, + GpuTextureFormat.Rgba16FloatRenderTarget, + GpuTextureFormat.Depth24Stencil8, + SampleCount: 1)); + using IGpuFrame frame = device.BeginFrame(); + using (IGpuPassEncoder encoder = frame.BeginPass(new GpuPassDescription + { + Name = "test-world-hdr", + Color = new GpuColorAttachment( + target, + GpuLoadOp.Clear, + GpuStoreOp.Store, + Vector4.Zero), + Depth = new GpuDepthAttachment(GpuLoadOp.Clear, GpuStoreOp.Store, 1f, 0), + SampleCount = 1, + })) + { + WbDrawDispatcher.BindDirectionalShadowReceiver(encoder, in binding); + } + frame.End(); + + GpuRecordedUniformBind shadowBind = Assert.Single( + device.OfKind(), + call => call.Binding == GpuBindingModel.UniformDirectionalShadow); + Assert.Equal("disabled-shadow-frame", shadowBind.BufferName); + GpuRecordedUniformBind atmosphericBind = Assert.Single( + device.OfKind(), + call => call.Binding == GpuBindingModel.UniformAtmosphericFrame); + Assert.Equal("disabled-atmospheric-frame", atmosphericBind.BufferName); + } + [Fact] public void StableTopology_ReusesRetainedCommandBuffersWithoutFrameRingCopies() { diff --git a/tests/AcDream.App.Tests/Rendering/Packs/VolumetricShaftRendererTests.cs b/tests/AcDream.App.Tests/Rendering/Packs/VolumetricShaftRendererTests.cs index 3964c8f1..9bee6a2e 100644 --- a/tests/AcDream.App.Tests/Rendering/Packs/VolumetricShaftRendererTests.cs +++ b/tests/AcDream.App.Tests/Rendering/Packs/VolumetricShaftRendererTests.cs @@ -134,6 +134,52 @@ public sealed class VolumetricShaftRendererTests Assert.Empty(device.CreatedRenderTargets); } + [Fact] + public void DisabledShadowContentBindingStillGatesToNoCurrentDirectionalShadow() + { + // Campaign VM VM6 review fix round 4, test (d): PublishDisabledReceiverBinding + // (DirectionalSunShadowRenderer) now publishes a REAL, bindable + // ring allocation even when shadows are gated off, specifically so + // the world receiver pipeline keeps reading wind data. This proves + // the volumetric pass is unaffected by that change: its own gate + // reads IsValidFor, which a disabled-content binding (Enabled + // false, TextureSlot Unassigned, CascadeCount 0) still fails, so it + // reports NoCurrentDirectionalShadow exactly as it would for no + // binding at all — never mistaking "bindable" for "has real shadow + // content." + var device = new RecordingGpuDevice(); + using var renderer = Renderer(device, "high"); + GpuTextureSlot depth = TextureSlot(device, "scene-depth"); + using IGpuFrame frame = device.BeginFrame(); + GpuRingAllocation allocation = frame.AllocateRing( + DirectionalShadowUniforms.SizeInBytes, + GpuRingUsage.Uniform); + allocation.Data.Clear(); + var disabledContent = new DirectionalShadowFrameBinding( + frame.Serial, + Enabled: false, + allocation.Buffer, + allocation.OffsetBytes, + DirectionalShadowUniforms.SizeInBytes, + GpuTextureSlot.Unassigned, + CascadeCount: 0); + AtmosphericFrameInputs inputs = Inputs(1280, 720); + + VolumetricShaftOutput output = renderer.Render( + frame, + in inputs, + in disabledContent, + depth); + frame.End(); + + Assert.True(disabledContent.IsBindableFor(frame)); + Assert.False(disabledContent.IsValidFor(frame)); + Assert.Equal( + VolumetricShaftGateReason.NoCurrentDirectionalShadow, + output.Diagnostics.GateReason); + Assert.False(output.HasTexture); + } + [Fact] public void ResizeAtomicallyReplacesTargetAndResetsMixedResolutionPerformanceWindow() {