fix(render): foliage wind no longer depends on the directional-shadow gate (Campaign VM VM6 review 4)
The reviewer's offline pixel apparatus found a real design defect, not a
test artefact: foliage wind was welded to "directional shadows rendered
this frame." Evidence: offline High preset, sun-shadow-strength=0,
wind-strength 2 + lean/branch 1 m — 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 (no measurable motion). A CPU probe independently
confirmed ResolveFoliageWind was correct (first advance snaps to Clear
0.25/0.15, gate 1, one graph) — the correct uniform never reached the
world pass.
Root cause: DirectionalSunShadowRenderer.Render's two early-out paths
(!environment.ShouldRender, ResidentWindowUnavailable) left
_currentFrameBinding at its pure Disabled (no-buffer) default.
WbDrawDispatcher.PipelinesFor and TerrainModernRenderer's matching
selection logic only choose the atmospheric receiver pipeline
(mesh_atmospheric, the only pipeline that #includes foliage_wind.glsl)
when TryGetCurrentFrameBinding returns true; with no buffer it always
returned false, so the world pass silently fell back to the plain
mesh_modern pipeline, which has no wind code at all. Because the shadow
gate is ActiveDayGroupMultiplier = dayGroupPolicy x elevationResponse x
strength, this killed wind every night (elevation response -> 0), at
user sun-shadow-strength 0, and under the portal/login cover.
Fix (decouple, not patch): DirectionalShadowFrameBinding gained
IsBindableFor ("a real current-frame allocation exists") separate from
IsValidFor ("...and it is Enabled with real shadow content" -- kept
exactly as VolumetricShaftRenderer's own gate needs it).
TryGetCurrentFrameBinding now returns IsBindableFor. When the built-in
pack supplies an AtmosphericFrame binding (declared packs never do, so
their receiver shaders -- which never declare set 3 binding 5 -- 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
the disabled block actually gets bound once it is selected.
PublishDisabledReceiverBinding is internal (not private) specifically so
it is testable without standing up a real WbDrawDispatcher/
TerrainModernRenderer pair -- no test in this suite constructs either.
New tests: (a)/(b) PublishDisabledReceiverBinding is bindable-not-valid
with a bound AtmosphericFrame and a genuine no-op with an unbound one;
(c) BindDirectionalShadowReceiver emits both UniformDirectionalShadow and
UniformAtmosphericFrame binds for a disabled binding; (d)
VolumetricShaftRenderer's gate still reports NoCurrentDirectionalShadow
for a disabled binding. ShouldSelectReceiverPipeline itself is untouched
and its existing tests (parametrized directly on bindingValid) remain
valid; no existing test asserted the old "disabled shadows -> plain
pipeline / no binding" behaviour in a way this fix invalidates -- every
existing caller either bypasses Render (calls RenderPrepared directly)
or uses a stale-serial binding IsBindableFor still correctly rejects.
Verify: Release build 0 warnings/0 errors. App hermetic-lane filter
6,050/0 failed. Core.Tests 4,695/0 failed. Full hermetic-filtered
solution: 15,278/0 failed across 15 projects.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
parent
fccba8390d
commit
eec9553582
7 changed files with 381 additions and 7 deletions
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue