test(render): foliage-wind receiver fallback guard asserts branch shape; cascade-0 publish covered (Campaign VM VM6 review 5 nits)
Round-5 narrow re-review of 754d59d9: APPROVE (verified to the SPIR-V
disassembly; shadows-on path proven opcode-identical). Four
non-blocking nits.
N1 (test robustness): T3 (ReceiverVertexShadersFallBackToThePlainLightDirectionWhenTheShadowFlagIsClear)
was satisfied by comment prose -- round 5's own explanatory comments
quote the exact plain-pipeline direction substring
("-uLights[i].dirAndRange.xyz, ... matched bit-for-bit"), so the
substring-only assertion passed even with the code mutated. Fixed:
comment lines are now stripped (new StripLineComments helper) before
any assertion, and a new ordered regex per shader asserts the exact
branch SHAPE -- shadowGatedOff ? -uLights[...] : normalize(uShadow...)
-- not just substring presence. Mutation-tested locally against the
rewritten test: (1) swapping the ternary's true/false operands --
FAILED (previously passed); (2) deleting the fallback entirely,
collapsing to the pre-round-5 buggy expression -- FAILED (previously
passed). Original file restored and reverified passing after each
mutation.
N2 (doc accuracy): "numerically the plain pipeline" overstated the
round-5 fix in three places (plan doc, mesh_atmospheric.vert,
terrain_atmospheric.vert). The direction expression is bit-for-bit;
the SUM is not, because the atmospheric shaders' split ambient+point
vs directional accumulation (and terrain's two varyings vs the plain
pipeline's one) reassociates float summation order by ~1 ulp -- which
is exactly the measured mean |Delta| 0.007 the coordinator's own
pixel-proof evidence already recorded (well under the 65 px noise
floor). All three rewritten to say the receiver "matches the plain
pipeline to within float summation-order rounding (measured mean
|Delta| 0.007 on the offline scene)."
N3 (coverage): RenderPrepared's own cascadeCount == 0 exit (the F2
fix) had no direct test even though RenderPrepared already has 8
direct call sites in this file. Added one: ResidentMaximumReachMeters
at/below CameraNearMeters, passed straight to RenderPrepared with an
otherwise-fully-valid environment (so the fitter, not the environment
gate, is what returns zero cascades), asserting IsBindableFor true /
IsValidFor false.
N4 (latent): RenderPrepared's OWN "if (!environment.ShouldRender)"
exit is a fourth bufferless-disabled path -- unreachable via Render
(whose own gate already validates ShouldRender first) but the same
shape, and RenderPrepared is called directly by tests and any future
caller. Took the preferred fix: publishes the disabled binding there
too, via the same helper, so every exit on a frame that draws the
world publishes when the pack's AtmosphericFrame is bound. Also made
EvaluateGateAndPublishDisabledBinding self-contained: it now resets
_currentFrameBinding to Disabled on its own entry instead of relying
on Render having done so first (idempotent with Render's own reset).
Regenerated SPIR-V: mesh_atmospheric.vert and terrain_atmospheric.vert
are comment-only changes (N2), so only the manifest's source hashes
changed -- compiled .spv bytes are unchanged, consistent with round 3's
precedent for comment-only shader edits.
Verify: Release build 0 warnings/0 errors. App hermetic-lane filter
6,055/0 failed. Full hermetic-filtered solution: 15,283/0 failed
across 15 projects.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
Erik
parent
9a49c66d47
commit
1f15124271
7 changed files with 154 additions and 25 deletions
|
|
@ -329,7 +329,12 @@ internal sealed class DirectionalSunShadowRenderer : IDirectionalShadowReceiverS
|
|||
/// <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.
|
||||
/// Disabled(...) construction as before the extraction. Round 6 (N4):
|
||||
/// self-contained — resets <see cref="_currentFrameBinding"/> 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.
|
||||
/// </summary>
|
||||
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)
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
|
|
|
|||
|
|
@ -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
|
||||
},
|
||||
{
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue