fix(render): shadow-gated-off receiver frames light from the authored sun again (Campaign VM VM6 review 5)

Round 4 (eec95535) fixed wind but introduced a new lighting bug: the
receiver VERTEX shaders source the sun direction from the shadow block,
not only the shadow visibility term. mesh_atmospheric.vert's
accumulateLights read uShadowLightDirectionAndSource unconditionally for
every directional light; terrain_atmospheric.vert did the same for its
single sun term. Round 4's PublishDisabledReceiverBinding writes
direction (0,0,1) into that block on every shadow-gated-off frame (user
sun-shadow-strength 0 in daylight, indoor/portal cover, night), so every
such frame was lighting outdoor terrain and objects from straight
overhead instead of the authored sun. Publishing the environment's real
direction would not have restored parity either -- the celestial shadow
source direction (sun/moon disc) is not the authored light direction.

F1 (BLOCKER): fixed in the shaders themselves, exact parity with the
plain pipeline. Both receiver verts now branch on the same flag bit
acdreamDirectionalShadowVisibility already reads
((uShadowTextureAndFlags.w & 1u) == 0u) and, when clear, use the EXACT
plain-pipeline expression instead of the shadow block's direction:
-uLights[i].dirAndRange.xyz in mesh_atmospheric.vert (matching
mesh_modern.vert, hoisted out of the light loop as a uniform branch);
-uLights[0].dirAndRange.xyz in terrain_atmospheric.vert (matching
terrain_modern.vert's sunDir/-sunDir form). The (0,0,1) word in the
disabled block stays as the documented normalize()-cannot-NaN guard; its
comment now says so explicitly since it is no longer read as a light
direction when the flag is clear.

F2: RenderPrepared's cascadeCount == 0 return is a third bufferless-
disabled path reachable from a frame that already passed Render's own
two gates (the cascade fitter can still find zero usable cascades) --
publishes the same disabled binding now, via the same
PublishDisabledReceiverBinding helper (re-signatured to take a bare
AtmosphericFrameBufferBinding so all three call sites -- Render's two
early-outs plus this one -- share it).

F3: removed a stray duplicated " -- Closeout and merge" fragment under
the plan's VM7 heading.

F4: corrected the false "the flag bit makes it numerically the plain
lighting sum" claim in the plan's round-4 paragraph and in
WbDrawDispatcher.DirectionalShadowReceivers.cs -- the flag bit alone
only fixed the shadow VISIBILITY term (already correct before round 4);
it took both that AND round 5's light-DIRECTION fallback to actually
match the plain pipeline.

T1: extracted Render's gate prologue (environment evaluate -> two
early-outs -> PublishDisabledReceiverBinding) into internal
EvaluateGateAndPublishDisabledBinding(frame, in input, out environment,
out environmentGateTicks), behaviour-preserving, called by Render before
it touches world/terrain -- the ArgumentNullException.ThrowIfNull(world)/
ThrowIfNull(terrain) calls keep their exact position relative to the
gate. No test in this suite constructs a real WbDrawDispatcher +
TerrainModernRenderer pair (still true), so this extraction is what
makes the gate itself testable; two new tests drive it directly with
PlayerInsideCell: true and with ResidentMaximumReachMeters <=
CameraNearMeters, asserting TryGetCurrentFrameBinding true / IsValidFor
false for both.

T2: proves the actual composition WbDrawDispatcher.PipelinesFor and
TerrainModernRenderer both use -- TryGetCurrentFrameBinding feeding
ShouldSelectReceiverPipeline -- selects the receiver pipeline for the
atmospheric world pass once a disabled binding is published, and still
refuses a non-atmospheric pass name.

T3: shader-source guard (same style as AtmosphericPostProcessGraphTests'
existing shader-text tests) pinning that both receiver verts contain the
flag-gated fallback and reference the same uLights expression the plain
verts use, so a future edit that drops the fallback fails this test
instead of only showing up in a pixel capture.

T4: the (0,0,1) test's doc comment and an inline assertion comment now
say the value is a NaN guard, not a light direction.

Regenerated SPIR-V: mesh_atmospheric.vert and terrain_atmospheric.vert
recompiled to different bytes this time (a real code change, not a
comment); manifest updated to match.

Verify: Release build 0 warnings/0 errors. App hermetic-lane filter
6,054/0 failed. Core.Tests 4,695/0 failed. RenderPackValidator 30/30.
Full hermetic-filtered solution: 15,282/0 failed across 15 projects.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
Erik 2026-08-23 03:58:41 +02:00
parent eec9553582
commit 754d59d949
10 changed files with 360 additions and 51 deletions

View file

@ -316,28 +316,37 @@ internal sealed class DirectionalSunShadowRenderer : IDirectionalShadowReceiverS
return !_disposed && binding.IsBindableFor(frame);
}
internal DirectionalSunShadowDiagnostics Render(
/// <summary>
/// Campaign VM VM6 review fix round 5 (T1): the gate prologue Render
/// runs before it touches world/terrain, extracted into its own
/// internal method so a hermetic test can drive it directly —
/// constructing a real WbDrawDispatcher/TerrainModernRenderer pair
/// just to reach these two early-out paths is not a cheap test (see
/// PublishDisabledReceiverBinding's own doc comment). Returns the
/// disabled diagnostics Render should return immediately, or null when
/// the gate passed and Render should continue (in which case
/// <paramref name="environment"/> and
/// <paramref name="environmentGateTicks"/> are the validated values
/// Render's remaining stages use). Behaviour-preserving: identical
/// order, identical PublishDisabledReceiverBinding calls, identical
/// Disabled(...) construction as before the extraction.
/// </summary>
internal DirectionalSunShadowDiagnostics? EvaluateGateAndPublishDisabledBinding(
IGpuFrame frame,
in DirectionalSunShadowRenderInput input,
WbDrawDispatcher world,
TerrainModernRenderer terrain)
out DirectionalShadowEnvironmentState environment,
out long environmentGateTicks)
{
ObjectDisposedException.ThrowIf(_disposed, this);
ArgumentNullException.ThrowIfNull(frame);
_currentFrameBinding = DirectionalShadowFrameBinding.Disabled;
ArgumentNullException.ThrowIfNull(world);
ArgumentNullException.ThrowIfNull(terrain);
long cpuStageStarted = input.MeasureCpuStages ? Stopwatch.GetTimestamp() : 0L;
DirectionalShadowEnvironmentState environment =
DirectionalShadowEnvironmentGate.Evaluate(
input.Environment,
_atmospherePolicy);
long environmentGateTicks = input.MeasureCpuStages
environment = DirectionalShadowEnvironmentGate.Evaluate(
input.Environment,
_atmospherePolicy);
environmentGateTicks = input.MeasureCpuStages
? Stopwatch.GetTimestamp() - cpuStageStarted
: 0L;
if (!environment.ShouldRender)
{
PublishDisabledReceiverBinding(frame, in input);
PublishDisabledReceiverBinding(frame, input.AtmosphericFrame);
return Disabled(
in environment,
new DirectionalSunShadowCpuStageTicks(
@ -349,14 +358,35 @@ internal sealed class DirectionalSunShadowRenderer : IDirectionalShadowReceiverS
{
Reason = DirectionalShadowGateReason.ResidentWindowUnavailable,
};
PublishDisabledReceiverBinding(frame, in input);
PublishDisabledReceiverBinding(frame, input.AtmosphericFrame);
return Disabled(
in environment,
new DirectionalSunShadowCpuStageTicks(
environmentGateTicks, 0L, 0L, 0L, 0L));
}
return null;
}
cpuStageStarted = input.MeasureCpuStages ? Stopwatch.GetTimestamp() : 0L;
internal DirectionalSunShadowDiagnostics Render(
IGpuFrame frame,
in DirectionalSunShadowRenderInput input,
WbDrawDispatcher world,
TerrainModernRenderer terrain)
{
ObjectDisposedException.ThrowIf(_disposed, this);
ArgumentNullException.ThrowIfNull(frame);
_currentFrameBinding = DirectionalShadowFrameBinding.Disabled;
ArgumentNullException.ThrowIfNull(world);
ArgumentNullException.ThrowIfNull(terrain);
DirectionalSunShadowDiagnostics? gated = EvaluateGateAndPublishDisabledBinding(
frame,
in input,
out DirectionalShadowEnvironmentState environment,
out long environmentGateTicks);
if (gated is not null)
return gated.Value;
long cpuStageStarted = input.MeasureCpuStages ? Stopwatch.GetTimestamp() : 0L;
DirectionalShadowPreparedDraws worldDraws =
world.PrepareDirectionalShadowDraws(input.Casters);
DirectionalShadowTerrainPreparedDraws terrainDraws =
@ -495,9 +525,9 @@ internal sealed class DirectionalSunShadowRenderer : IDirectionalShadowReceiverS
/// </summary>
internal void PublishDisabledReceiverBinding(
IGpuFrame frame,
in DirectionalSunShadowRenderInput input)
AtmosphericFrameBufferBinding atmosphericFrame)
{
if (!input.AtmosphericFrame.IsBound)
if (!atmosphericFrame.IsBound)
return;
var disabledUniforms = new DirectionalShadowUniforms(
@ -509,6 +539,14 @@ internal sealed class DirectionalSunShadowRenderer : IDirectionalShadowReceiverS
Vector4.Zero,
Vector4.Zero,
new UInt4(0u, 0u, 0u, 0u),
// Campaign VM VM6 review fix round 5 (F1): (0,0,1) is a
// normalize()-cannot-NaN guard ONLY — round 5 fixed
// mesh_atmospheric.vert/terrain_atmospheric.vert to fall back
// to the plain pipeline's uLights-derived direction whenever
// this block's flags bit 0 is clear, so this vector is never
// actually read as a light direction any more. It stays a
// unit vector regardless, in case a future reader forgets that
// rule and reads it directly.
new Vector4(0f, 0f, 1f, 0f));
GpuRingAllocation allocation = frame.AllocateRing(
DirectionalShadowUniforms.SizeInBytes,
@ -523,7 +561,7 @@ internal sealed class DirectionalSunShadowRenderer : IDirectionalShadowReceiverS
DirectionalShadowUniforms.SizeInBytes,
GpuTextureSlot.Unassigned,
CascadeCount: 0,
AtmosphericFrame: input.AtmosphericFrame);
AtmosphericFrame: atmosphericFrame);
}
internal static DirectionalShadowCasterClassDiagnostics
@ -632,6 +670,16 @@ internal sealed class DirectionalSunShadowRenderer : IDirectionalShadowReceiverS
{
Reason = DirectionalShadowGateReason.ResidentWindowUnavailable,
};
// Campaign VM VM6 review fix round 5 (F2): a third bufferless-
// disabled exit reachable from a frame that already passed
// Render's own two gates (ShouldRender, the constructor-level
// ResidentMaximumReachMeters check) — the cascade fitter can
// still find zero usable cascades. Publish the same disabled
// receiver binding as Render's two early-outs so this path
// doesn't reintroduce the F1/round-4 bug for the rarer case
// where a frame draws the world but the shadow fitter itself
// bails out.
PublishDisabledReceiverBinding(frame, atmosphericFrame);
return Disabled(in unavailable, cpuStages);
}

View file

@ -269,11 +269,26 @@ vec3 accumulateAmbientLocalLights(
// get no sun even in windowed buildings where the player's frame is not sun-killed.
if (uLightingMode == 0) {
if (instanceIndoor[instanceIndex] == 0u) { // #142: outdoor objects only get the sun
// Campaign VM VM6 review fix round 5 (F1 BLOCKER): when the
// shadow block's flag bit is clear (a gated-off frame — see
// DirectionalSunShadowRenderer.PublishDisabledReceiverBinding),
// uShadowLightDirectionAndSource carries only a (0,0,1) NaN
// guard, NOT a real light direction — using it here regardless
// lit every gated-off frame (every night, sun-shadow-strength
// 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,
// evaluated once per vertex regardless of light count.
bool shadowGatedOff = (uShadowTextureAndFlags.w & 1u) == 0u;
int activeLights = int(uCellAmbient.w);
for (int i = 0; i < 8; ++i) {
if (i >= activeLights) break;
if (int(uLights[i].posAndKind.w) != 0) continue; // directional only
vec3 Ldir = normalize(uShadowLightDirectionAndSource.xyz);
vec3 Ldir = shadowGatedOff
? -uLights[i].dirAndRange.xyz
: normalize(uShadowLightDirectionAndSource.xyz);
float ndl = max(0.0, dot(N, Ldir));
directionalLit += uLights[i].colorAndIntensity.xyz
* uLights[i].colorAndIntensity.w * ndl;

View file

@ -215,7 +215,7 @@
"stages": [
{
"stage": "vert",
"sourceSha256": "a00aa83a8bd8ab4791b60bfb967a80b43bafcb0bf758f5e2e056b4272f3c8b1b",
"sourceSha256": "a6b7aa2b0c1c38f92e4b35e76a58acdf5adc916f6ed0cebf94c8bea3f8843666",
"compiled": true
},
{
@ -327,7 +327,7 @@
"stages": [
{
"stage": "vert",
"sourceSha256": "06258c7ead0e123740e325c802156987ed17dabe355d591d0e90facb420b8e04",
"sourceSha256": "8cc199b3c6582831e5c9614e19afdff4f0f5fcdb8d96271c8cb3f427e29c68c9",
"compiled": true
},
{

View file

@ -160,7 +160,17 @@ void main() {
vWorldNormal = normalize(aNormal);
// Retail AdjustPlanes bake (terrain.vert:124-134 — identical math).
vec3 surfaceToLight = normalize(uShadowLightDirectionAndSource.xyz);
// Campaign VM VM6 review fix round 5 (F1 BLOCKER): when the shadow
// block's flag bit is clear (a gated-off frame), uShadowLightDirectionAndSource
// 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.
bool shadowGatedOff = (uShadowTextureAndFlags.w & 1u) == 0u;
vec3 surfaceToLight = shadowGatedOff
? -uLights[0].dirAndRange.xyz
: normalize(uShadowLightDirectionAndSource.xyz);
vec3 sunCol = uLights[0].colorAndIntensity.xyz * uLights[0].colorAndIntensity.w;
float L = max(dot(vWorldNormal, surfaceToLight), MIN_FACTOR);
// Preserve retail's authored lighting values, but keep the outdoor

View file

@ -124,11 +124,18 @@ public sealed partial class WbDrawDispatcher
// 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.
// disabled shadow block and the real AtmosphericFrame wind data
// mesh_atmospheric.vert needs regardless of shadow gating. Review
// fix round 5 (F4 correction): with the flag bit clear the
// receiver shaders take visibility 1.0 AND fall back to the
// authored uLights direction (mesh_atmospheric.vert/
// terrain_atmospheric.vert, round 5) — NOT "the flag bit alone
// makes it numerically the plain lighting sum" as an earlier round
// claimed; the vertex shaders sourced the sun direction from this
// very block unconditionally until round 5's shader fix, so the
// fallback needed BOTH the flag-gated visibility (already correct)
// and the flag-gated direction (round 5) to actually match the
// plain pipeline.
if (binding.Buffer is null)
return;
encoder.BindUniformBuffer(