diff --git a/docs/plans/2026-08-22-visualmaster-campaign.md b/docs/plans/2026-08-22-visualmaster-campaign.md index a44290e0..a8973f0d 100644 --- a/docs/plans/2026-08-22-visualmaster-campaign.md +++ b/docs/plans/2026-08-22-visualmaster-campaign.md @@ -721,7 +721,14 @@ plain-pipeline expression (`-uLights[i].dirAndRange.xyz` in `terrain_modern.vert`) instead of the shadow block's direction. Corrected statement: with the flag bit clear the receiver shaders take visibility 1.0 AND fall back to the authored `uLights` direction (round 5), so a -gated-off frame is numerically the plain pipeline. Wind is unaffected — +gated-off frame matches the plain pipeline to within float summation- +order rounding (review fix round 6, N2: "numerically the plain pipeline" +overstated it — the direction expression is bit-for-bit, but the SUM is +not, because the atmospheric fragment's split ambient+point vs +directional accumulation reassociates the terms relative to the plain +pipeline's single varying, and terrain's two separate varyings vs the +plain pipeline's one do the same; measured mean |Δ| 0.007 on the offline +scene, well under the apparatus's 65 px floor). Wind is unaffected — `foliage_wind.glsl` reads only the AtmosphericFrame half of set 3, never the shadow block. diff --git a/src/AcDream.App/Rendering/DirectionalSunShadowRenderer.cs b/src/AcDream.App/Rendering/DirectionalSunShadowRenderer.cs index f2691404..035ffa9f 100644 --- a/src/AcDream.App/Rendering/DirectionalSunShadowRenderer.cs +++ b/src/AcDream.App/Rendering/DirectionalSunShadowRenderer.cs @@ -329,7 +329,12 @@ internal sealed class DirectionalSunShadowRenderer : IDirectionalShadowReceiverS /// are the validated values /// Render's remaining stages use). Behaviour-preserving: identical /// order, identical PublishDisabledReceiverBinding calls, identical - /// Disabled(...) construction as before the extraction. + /// Disabled(...) construction as before the extraction. Round 6 (N4): + /// self-contained — resets itself + /// on entry instead of relying on the caller to have done it first, + /// so a future direct caller of this method (not just Render) cannot + /// observe a stale binding from a previous frame. Idempotent with + /// Render's own reset immediately before calling this. /// internal DirectionalSunShadowDiagnostics? EvaluateGateAndPublishDisabledBinding( IGpuFrame frame, @@ -337,6 +342,7 @@ internal sealed class DirectionalSunShadowRenderer : IDirectionalShadowReceiverS out DirectionalShadowEnvironmentState environment, out long environmentGateTicks) { + _currentFrameBinding = DirectionalShadowFrameBinding.Disabled; long cpuStageStarted = input.MeasureCpuStages ? Stopwatch.GetTimestamp() : 0L; environment = DirectionalShadowEnvironmentGate.Evaluate( input.Environment, @@ -636,7 +642,18 @@ internal sealed class DirectionalSunShadowRenderer : IDirectionalShadowReceiverS ArgumentNullException.ThrowIfNull(worldDraws); ArgumentNullException.ThrowIfNull(terrainDraws); if (!environment.ShouldRender) + { + // Campaign VM VM6 review fix round 6 (N4): a fourth bufferless- + // disabled exit, same shape as Render's own two and the + // cascadeCount == 0 exit below (F2). Unreachable via Render + // itself (its own gate already validated ShouldRender before + // calling here), but RenderPrepared is also called directly + // (tests, and any future caller) — one rule covers every exit + // on a frame that draws the world: publish when the pack's + // AtmosphericFrame is bound. + PublishDisabledReceiverBinding(frame, atmosphericFrame); return Disabled(in environment, cpuStages); + } if (!worldDraws.Commands.IsEmpty && worldGeometry is null) throw new ArgumentNullException(nameof(worldGeometry)); if (!terrainDraws.Commands.IsEmpty && terrainGeometry is null) diff --git a/src/AcDream.App/Rendering/Shaders/mesh_atmospheric.vert b/src/AcDream.App/Rendering/Shaders/mesh_atmospheric.vert index 967b40ae..cec1ba8c 100644 --- a/src/AcDream.App/Rendering/Shaders/mesh_atmospheric.vert +++ b/src/AcDream.App/Rendering/Shaders/mesh_atmospheric.vert @@ -278,8 +278,13 @@ vec3 accumulateAmbientLocalLights( // 0, indoors, portal cover) from straight overhead. Fall back // to the EXACT plain mesh_modern.vert expression // (-uLights[i].dirAndRange.xyz, unnormalized — matched - // bit-for-bit) so a gated-off frame is numerically the plain - // pipeline. Hoisted out of the loop: a uniform branch, + // bit-for-bit) so a gated-off frame matches the plain + // pipeline to within float summation-order rounding + // (review fix round 6, N2: the atmospheric fragment's split + // ambient+point vs directional accumulation reassociates the + // sum relative to the plain pipeline's single varying — + // measured mean |Δ| 0.007 on the offline scene, not bit- + // identical). Hoisted out of the loop: a uniform branch, // evaluated once per vertex regardless of light count. bool shadowGatedOff = (uShadowTextureAndFlags.w & 1u) == 0u; int activeLights = int(uCellAmbient.w); diff --git a/src/AcDream.App/Rendering/Shaders/spv/shaders.manifest.json b/src/AcDream.App/Rendering/Shaders/spv/shaders.manifest.json index c8cd1094..e0c93c12 100644 --- a/src/AcDream.App/Rendering/Shaders/spv/shaders.manifest.json +++ b/src/AcDream.App/Rendering/Shaders/spv/shaders.manifest.json @@ -215,7 +215,7 @@ "stages": [ { "stage": "vert", - "sourceSha256": "a6b7aa2b0c1c38f92e4b35e76a58acdf5adc916f6ed0cebf94c8bea3f8843666", + "sourceSha256": "c03ba560fbc2c7ce12e67326dcc79114f740999423fade53cd2d18804bf8f81a", "compiled": true }, { @@ -327,7 +327,7 @@ "stages": [ { "stage": "vert", - "sourceSha256": "8cc199b3c6582831e5c9614e19afdff4f0f5fcdb8d96271c8cb3f427e29c68c9", + "sourceSha256": "882979f63c858760e977ecafb708416ab01f7adabe4bb38c0630020c087f1c28", "compiled": true }, { diff --git a/src/AcDream.App/Rendering/Shaders/terrain_atmospheric.vert b/src/AcDream.App/Rendering/Shaders/terrain_atmospheric.vert index a4399b9c..706a085b 100644 --- a/src/AcDream.App/Rendering/Shaders/terrain_atmospheric.vert +++ b/src/AcDream.App/Rendering/Shaders/terrain_atmospheric.vert @@ -165,8 +165,11 @@ void main() { // carries only a (0,0,1) NaN guard, NOT a real light direction. Fall // back to the EXACT plain terrain_modern.vert expression // (-uLights[0].dirAndRange.xyz, unnormalized — matched bit-for-bit) - // so a gated-off frame is numerically the plain pipeline — see the - // matching comment in mesh_atmospheric.vert's accumulateLights. + // so a gated-off frame matches the plain pipeline to within float + // summation-order rounding (review fix round 6, N2: terrain's two + // separate varyings vs the plain pipeline's one reassociate the sum + // — measured mean |Δ| 0.007 on the offline scene, not bit-identical) + // — see the matching comment in mesh_atmospheric.vert's accumulateLights. bool shadowGatedOff = (uShadowTextureAndFlags.w & 1u) == 0u; vec3 surfaceToLight = shadowGatedOff ? -uLights[0].dirAndRange.xyz diff --git a/tests/AcDream.App.Tests/Rendering/DirectionalShadowGpuTests.cs b/tests/AcDream.App.Tests/Rendering/DirectionalShadowGpuTests.cs index e99b0f34..be4bd238 100644 --- a/tests/AcDream.App.Tests/Rendering/DirectionalShadowGpuTests.cs +++ b/tests/AcDream.App.Tests/Rendering/DirectionalShadowGpuTests.cs @@ -756,6 +756,53 @@ public sealed class DirectionalShadowGpuTests bindingValid)); } + [Fact] + public void RenderPrepared_CascadeFitterReturningZeroCascadesPublishesBindableNotValidBinding() + { + // Campaign VM VM6 review fix round 5 (F2), coverage added round 6 + // (N3): RenderPrepared's OWN cascadeCount == 0 exit is reachable + // from a frame that already passed the environment gate + // (ShouldRender true) but whose cascade fitter still finds zero + // usable cascades — DirectionalShadowCascadeFitter.Fit returns 0 + // whenever min(Quality.MaximumReachMeters, + // ResidentMaximumReachMeters) <= CameraNearMeters, the same + // effective constraint Render's own early-out checks, but + // reached HERE via the fitter's own arithmetic on a frame that + // got PAST that early-out (this test drives RenderPrepared + // directly, bypassing Render's gate entirely, to isolate the + // fitter's own zero-cascade path). Proves it publishes the same + // disabled receiver binding Render's two early-outs do. + using var device = new RecordingGpuDevice(); + using var renderer = new DirectionalSunShadowRenderer(device, DirectionalShadowPreset.Low); + using IGpuBuffer atmosphericBuffer = Buffer(device, "cascade-zero-frame", GpuBufferUsage.Uniform); + var atmosphericFrame = new AtmosphericFrameBufferBinding(atmosphericBuffer, 0u, 192u); + device.Clear(); + using IGpuFrame frame = device.BeginFrame(); + WorldTransformFrameSlice transforms = PublishSharedTransforms( + frame, + ReadOnlySpan.Empty); + + DirectionalSunShadowDiagnostics diagnostics = renderer.RenderPrepared( + frame, + EnabledEnvironment(), + Matrix4x4.Identity, + Matrix4x4.CreatePerspectiveFieldOfView(1f, 1f, 0.1f, 500f), + 0.1f, + 48f, + new DirectionalShadowPreparedDraws(), + new DirectionalShadowTerrainPreparedDraws(), + null, + null, + transforms, + residentMaximumReachMeters: 0.05f, // <= cameraNearMeters (0.1f): the fitter returns 0 + atmosphericFrame: atmosphericFrame); + + Assert.Equal(DirectionalShadowGateReason.ResidentWindowUnavailable, diagnostics.GateReason); + Assert.True(renderer.TryGetCurrentFrameBinding(frame, out DirectionalShadowFrameBinding binding)); + Assert.True(binding.IsBindableFor(frame)); + Assert.False(binding.IsValidFor(frame)); + } + [Fact] public void BindDirectionalShadowReceiver_WithADisabledBindingEmitsBothShadowAndAtmosphericBinds() { diff --git a/tests/AcDream.App.Tests/Rendering/Packs/AtmosphericPostProcessGraphTests.cs b/tests/AcDream.App.Tests/Rendering/Packs/AtmosphericPostProcessGraphTests.cs index 0c8293ea..776abd6c 100644 --- a/tests/AcDream.App.Tests/Rendering/Packs/AtmosphericPostProcessGraphTests.cs +++ b/tests/AcDream.App.Tests/Rendering/Packs/AtmosphericPostProcessGraphTests.cs @@ -1,6 +1,7 @@ using System.Globalization; using System.Numerics; using System.Runtime.InteropServices; +using System.Text.RegularExpressions; using AcDream.App.Plugins; using AcDream.App.Rendering; using AcDream.App.Rendering.Gpu; @@ -1225,28 +1226,38 @@ public sealed class AtmosphericPostProcessGraphTests [Fact] public void ReceiverVertexShadersFallBackToThePlainLightDirectionWhenTheShadowFlagIsClear() { - // Campaign VM VM6 review fix round 5 (F1 BLOCKER, test T3): pins - // the shader SOURCE TEXT so a future edit that removes the - // flag-gated fallback (reintroducing "a shadow-gated-off frame - // lights outdoor terrain/objects from the disabled block's - // (0,0,1) placeholder instead of the authored sun") fails this - // test instead of only showing up in a pixel capture. Both - // receiver vertex shaders must (1) test the SAME flags bit - // acdreamDirectionalShadowVisibility already tests, and (2) - // reference the SAME uLights-derived expression the corresponding - // plain (mesh_modern.vert / terrain_modern.vert) pipeline uses, - // proving the fallback branch is not some other, unverified - // direction. + // Campaign VM VM6 review fix round 5 (F1 BLOCKER, test T3), hardened + // round 6 (N1): the original version of this test asserted only + // that the plain-pipeline direction SUBSTRING appeared somewhere + // in the receiver file — but the round-5 fix's own explanatory + // comments ALSO quote that exact substring in prose ("(-uLights[i] + // .dirAndRange.xyz, ... matched bit-for-bit)"), so the check was + // satisfied by the COMMENT even with the CODE mutated. Mutation- + // tested locally against this rewritten test: (1) swapping the + // ternary's true/false operands — PASSED the old substring-only + // test, FAILS this one; (2) deleting the fallback entirely + // (always taking the shadow-block direction, i.e. collapsing the + // ternary to just its false arm) — PASSED the old test, FAILS + // this one. Comment lines are stripped before matching, and the + // assertion is the exact branch SHAPE — the gated-off condition, + // then the plain uLights expression on the TRUE arm, then + // uShadowLightDirectionAndSource on the FALSE arm — via an + // ordered regex, not a substring search that a comment can also + // satisfy. string shaderRoot = Path.Combine( RepositoryRoot(), "src", "AcDream.App", "Rendering", "Shaders"); - string meshAtmospheric = File.ReadAllText(Path.Combine(shaderRoot, "mesh_atmospheric.vert")); - string meshModern = File.ReadAllText(Path.Combine(shaderRoot, "mesh_modern.vert")); - string terrainAtmospheric = File.ReadAllText(Path.Combine(shaderRoot, "terrain_atmospheric.vert")); - string terrainModern = File.ReadAllText(Path.Combine(shaderRoot, "terrain_modern.vert")); + string meshAtmospheric = StripLineComments( + File.ReadAllText(Path.Combine(shaderRoot, "mesh_atmospheric.vert"))); + string meshModern = StripLineComments( + File.ReadAllText(Path.Combine(shaderRoot, "mesh_modern.vert"))); + string terrainAtmospheric = StripLineComments( + File.ReadAllText(Path.Combine(shaderRoot, "terrain_atmospheric.vert"))); + string terrainModern = StripLineComments( + File.ReadAllText(Path.Combine(shaderRoot, "terrain_modern.vert"))); const string flagTest = "(uShadowTextureAndFlags.w & 1u) == 0u"; Assert.Contains(flagTest, meshAtmospheric, StringComparison.Ordinal); @@ -1254,7 +1265,8 @@ public sealed class AtmosphericPostProcessGraphTests // mesh: both files negate the per-light uniform inline (no // intermediate variable), so the literal expression matches - // string-for-string. + // string-for-string once comments can no longer supply a false + // positive. const string meshPlainDirection = "-uLights[i].dirAndRange.xyz"; Assert.Contains(meshPlainDirection, meshModern, StringComparison.Ordinal); Assert.Contains(meshPlainDirection, meshAtmospheric, StringComparison.Ordinal); @@ -1269,8 +1281,46 @@ public sealed class AtmosphericPostProcessGraphTests "-" + terrainLightUniform, terrainAtmospheric, StringComparison.Ordinal); + + // Branch-shape guard (N1): fails on a swapped ternary or a + // deleted fallback even though the substrings above would still + // be present somewhere in the (comment-stripped) file. + var meshBranchShape = new Regex( + @"shadowGatedOff\s*\?\s*-uLights\[i\]\.dirAndRange\.xyz\s*:\s*normalize\(uShadowLightDirectionAndSource\.xyz\)", + RegexOptions.Singleline); + Assert.True( + meshBranchShape.IsMatch(meshAtmospheric), + "mesh_atmospheric.vert must branch exactly: " + + "shadowGatedOff ? -uLights[i].dirAndRange.xyz : normalize(uShadowLightDirectionAndSource.xyz)"); + + var terrainBranchShape = new Regex( + @"shadowGatedOff\s*\?\s*-uLights\[0\]\.dirAndRange\.xyz\s*:\s*normalize\(uShadowLightDirectionAndSource\.xyz\)", + RegexOptions.Singleline); + Assert.True( + terrainBranchShape.IsMatch(terrainAtmospheric), + "terrain_atmospheric.vert must branch exactly: " + + "shadowGatedOff ? -uLights[0].dirAndRange.xyz : normalize(uShadowLightDirectionAndSource.xyz)"); } + /// + /// Campaign VM VM6 review fix round 6 (N1): removes everything from + /// the first // on each line onward. GLSL has no string + /// literals in these files, so a per-line truncation is safe and + /// cannot itself corrupt real code — it exists specifically so a + /// shader-text assertion cannot be satisfied by a comment that quotes + /// the expected code as prose instead of by the code itself. + /// + private static string StripLineComments(string glsl) + { + string[] lines = glsl.Split('\n'); + for (int i = 0; i < lines.Length; i++) + { + int index = lines[i].IndexOf("//", StringComparison.Ordinal); + if (index >= 0) + lines[i] = lines[i][..index]; + } + return string.Join('\n', lines); + } [Fact] public void FirstAdvanceSnapsExactlyToTheWeatherTargetInsteadOfEasingFromZero() {