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>
Closes the automated half of slice VM6 at 754d59d9 and records how it was
proven. The first pixel method (clock pin 0 s vs 3 s, wind-off pair as the
control) is recorded as confounded: the treeline silhouette carries a
bimodal 0-or-~280 px rasterisation churn between runs of the same binary,
and two conclusions drawn from it were retracted. The replacement
apparatus (same pin, wind on vs off, two captures per arm, robust mask =
AND of the four cross pairs minus both repeat pairs, shadows off so only
geometry can move) found the round-4 defect (wind welded to the shadow
gate: 1 m wind = 49-65 px vs a 22 px floor while the CPU state was
correct) and, after round 5, gives 14,993 robust wind pixels vs a 65 px
floor, every one on a treeline tree, hillside tree or shoreline bush, with
the wind-off frame matching the pre-round-4 plain pipeline at mean |d|
0.007 (round 4's eec95535: 1.77, the overhead-sun regression the narrow
review caught).
tools/vm6/wind-pixel-proof.py is the analysis command the closeout names;
evidence overlays under docs/research/evidence/vm6. Plan: VM6 ledger row
CODE-COMPLETE 2026-08-23, owner visual gate owed (section VM6 Acceptance).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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>
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>
Narrow re-review of a82959f1: APPROVE, with follow-ups. All items landed.
N2 (structural): (a) extracted the ONE shared InstanceGroup-from-key
construction seam, WbDrawDispatcher.CreateGroupFromKey(key, registration,
frame) — before this there were two near-identical `new InstanceGroup
{ ... }` initializers (GetOrCreateInstanceGroup and GetOrCreatePackedGroup)
that had already drifted once (the round-2 F1 bug). Both routes call it now;
CreateGroupFromKey's own `new()` is the only production InstanceGroup
construction site repo-wide, same precedent as AppendPackedInstance. (b)
GroupKey.FoliageFlags lost its `= 0u` default and moved before CullMode in
the declaration (CullMode keeps its default, C# requires optional params to
trail required ones), so a `new GroupKey(...)` that omits it is a compile
error. Fixed every real construction site the reorder/requirement touched:
the 2 production sites, ToKey (a reconstruction from InstanceGroup the
review didn't count but the reorder broke), and 5 test sites (one more than
the review's "4" — InstanceGroupClearTests had a second, implicit
target-typed `MakeKey` factory the original count missed). Verified by a
full solution build.
N1: added CreateGroupFromKey_CopiesFoliageFlagsFromTheKey
(InstanceGroupClearTests) — a key carrying FoliageFlags 0x2 in, the created
group's FoliageFlags 0x2 out. That test plus N2b's required field are what
actually guard the round-2 F1 blocker; reworded PackedDispatcherOracleTests'
existing test comment to say what IT proves (the classification-to-
BuildIndirectArrays-to-BatchData.flags path), not that it guards the
classifier.
N3: corrected the plan's round-2 paragraph — folding FoliageFlags into the
G2/G3 digest is correct and symmetric, but CompareClassifiedOutput only
runs from RenderScenePViewFrameProductController.BuildAndCompare, which has
no production caller anywhere in src/AcDream.App/, and both of
RenderScenePViewFrameProductTests's own callers construct the controller
without the optional dispatcher argument — so the fold catches nothing
until that oracle is wired to an actual caller.
N4: the plan's F6 note now names both classification caches — the classic
route's EntityClassificationCache.EntityCacheEntry (self-heals per entity
on its own next eviction) and the packed route's
PackedProjectionClassificationEntry/PackedClassifiedBatch.Key
(PackedProjectionClassificationCache.BeginFrame clears its entire cache in
one shot on a RenderSceneGeneration change) — and notes neither mechanism
is keyed to a pack switch specifically.
N5: deleted the now-unused single-generic ComputeEntityHasCutoutSubset<T>
overload; its 4 test call sites now use the two-generic, zero-alloc
overload with an unused int context and a static (_, value) => value
lambda, so there is exactly one ComputeEntityHasCutoutSubset to keep
correct.
A6 (reviewer-filed): ResolveFoliageWind's _windMean/_windGust started at 0
and always eased toward the weather target by clock delta, with no
distinction for a graph's first-ever advance. A pinned clock
(ACDREAM_SKY_PHASE_SECONDS, the offline pixel gate's determinism pin) never
advances between calls, so the wind reached only whatever fraction the
first (1-second-clamped) step produced and sat there forever; live, the
first 10 s after a graph is constructed (pack selection / login) spun up
from dead calm even though the weather already IS what it is. Fixed at the
root: the first advance (_windFrameSerial == -1, the constructor sentinel)
now snaps _windMean/_windGust straight to the target; every later advance
eases over WeatherSystem.TransitionSeconds exactly as before. Added a
SetWindClockSecondsOverrideForTesting seam (_windClockSecondsOverride is no
longer readonly) so a hermetic test can advance the pinned clock by an
exact amount between two resolves without a real-time Thread.Sleep; two new
tests prove the first-advance snap is exact and a second advance still
eases at the normal rate. The three existing indoor/wind-disabled/amplitude
gate tests pass unchanged.
Verify: Release build 0 warnings/0 errors. App hermetic-lane filter
6,046/0 failed. Core.Tests 4,695/0 failed. Full hermetic-filtered solution:
15,274/0 failed across 15 projects.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Narrow re-review of 43e3abed found every A1-A5/A7/A8 item resolved but
one new blocker in the production packed classifier.
F1 (BLOCKER): RetailPViewPassExecutor.DrawPackedProductionRoute — the
route production world geometry actually draws from — never computed
FoliageFlags at all. WbDrawDispatcher.PackedOracle.cs's
ClassifyPackedBatches built its GroupKey with the field defaulting to
0u, and GetOrCreatePackedGroup never copied it onto the created
InstanceGroup, so production BatchData.flags bits 1/2 were always zero
for every scenery entity: the world geometry never swayed even though
the independently-classified shadow caster did, so shadows visibly
swayed under rigid trees. Both classifier call sites now compute
FoliageFlags via the identical FoliageWindClassification.Classify call
and entity-scoped HasCutoutSubset OR the classic (non-packed) path
uses, and GetOrCreatePackedGroup copies it exactly like
GetOrCreateInstanceGroup always has. The G2/G3 classified-output
digest (AddOpaqueSubmissionGroup/BuildTransparentSubmissionDigest) now
also folds GroupKey.FoliageFlags into its hash — present in the key
since round 1 but never actually read by either digest function, so a
content-level (not just group-count-level) classic-vs-packed
divergence is now caught.
F2 (medium): the delayed-alpha replay path (PrepareDeferredAlphaDraws)
hardcoded Flags = 1, dropping bits 1/2 for any group replayed through
it — a trunk instance promoted into the alpha-blend group mid-fade
(the #188 translucency-promotion case) would stop swaying for the
duration of its fade. Now 1u | key.FoliageFlags.
F3 (nit): ComputeEntityHasCutoutSubset's three call sites (classic,
caster, and the newly-fixed packed classifier) each allocated a
closure over _meshAdapter per Setup entity per frame. A new
context-taking overload passes the mesh adapter as an explicit
argument to a static lambda instead, letting the compiler cache one
delegate for the method's lifetime rather than allocating fresh ones.
A3 test gap: WbDrawDispatcher.BindDirectionalShadowReceiver is now
internal so DirectionalShadowGpuTests can drive it directly with a
bare RecordingGpuDevice pass encoder, proving it emits
UniformAtmosphericFrame with the exact buffer/offset/size a
DirectionalShadowFrameBinding carries — paired with the existing test
proving that binding carries the caster's real bind forward untouched.
F1's missing test: PackedDispatcherOracleTests chains
FoliageWindClassification.Classify (called with the packed
classifier's exact argument shape) for a real 0x8... scenery entity id
through BuildIndirectArrays — the same shared, already-tested
production step both classic and packed group lists feed into BatchData —
proving the resulting flags word carries bit 0x2. Driving
ClassifyPackedBatches/GetOrCreatePackedGroup directly was not a "cheap
test": both are private instance methods reachable only through the
full RetailPViewPassExecutor route, which needs a real IGpuDevice,
world-pass scope, mesh manager, and compiled pipelines to construct —
no test anywhere in the App test project stands one up.
Nits: F4 corrects foliage_wind.glsl's header comment from "bit 31" to
the top-nibble test; F5 documents at the receiver bind site that the
caster's own AtmosphericFrameBufferBinding has its seven ABI v1
members zero/Identity by construction (only the two v2 wind members
are valid) — safe today because mesh_atmospheric.vert reads that
binding solely for wind displacement, flagged as a footgun for a
future v1-reading addition to that shader; F6 notes in the plan
(rather than fixes) that EntityCacheEntry does not proactively
invalidate when FoliageWindExclusions changes on a pack switch —
harmless with the pack off, self-heals on the entry's next natural
eviction.
foliage_wind.glsl's F4 comment-only change updated the SPIR-V
manifest's source hashes for the five includers (mesh_atmospheric.vert
+ four directional_shadow_world_* casters); the compiled .spv bytes
are byte-identical since comments do not affect bytecode.
Verify: Release build 0 warnings/0 errors. App hermetic-lane filter
6,043/0 failed. Core.Tests 4,695/0 failed. RenderPackValidator 30/30.
Full hermetic-filtered solution: 15,271/0 failed across 15 projects.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Opus dual-lens review of the three VM6 commits (0930c35d, 39e8408c,
6cc5e183) found two blockers and two should-fix issues; all landed here
along with the review's nits and documentation corrections.
Blockers:
- A1: the procedural-scenery classifier tested bit 31 alone instead of
the full top nibble (0xF000_0000 == 0x8000_0000), so it also matched
LandblockStaticEntityIdAllocator's 0xC... namespace (fences/gates/
building shells with a cutout subset), the 0xDA11_D0xx paperdoll id,
and the 0xFFFF_FF01 portal-tunnel id as procedural scenery — all
three would have swayed. ProceduralSceneryIdAllocator.IsInNamespace
now does the exact top-nibble test; FoliageWindClassification
delegates to it.
- A2: GroupKey (the receiver's instance-batching key) did not carry
FoliageFlags while the caster's dedup key already did, so a scenery
instance and a non-scenery instance sharing a mesh subset coalesced
into one receiver InstanceGroup whose flags were last-writer-wins —
disagreeing with the correctly-keyed caster. GroupKey now carries
FoliageFlags, computed before key construction and set exactly once
at group creation; the imperative re-stamp is gone, and CachedBatch's
now-redundant FoliageFlags field is removed.
Should-fix:
- A3: the world receiver pass bound UniformAtmosphericFrame only by
accident (leftover from the caster pass, which runs first each
frame, since Vulkan binding state isn't reset between passes).
DirectionalShadowFrameBinding now carries the caster's exact
AtmosphericFrameBufferBinding and BindDirectionalShadowReceiver binds
it explicitly.
- A4: a Setup-composed tree's opaque trunk part never got the trunk
flag because HasCutoutSubset is cached per GfxObj part, not per
entity. FoliageWindClassification.ComputeEntityHasCutoutSubset now
ORs HasCutoutSubset across an entity's resolved sibling parts once
per entity, threaded into ClassifyBatches/AddDirectionalShadowBatches
via a new optional override parameter.
Nits: A5 hashes the per-vertex flutter seed relative to the instance
origin instead of absolute world XY (fp32 sin() precision loss at far
landblock corners), mirrored in both foliage_wind.glsl and
FoliageWindModel; A7 documents the max(maxHeight, 0.5) divide-guard as
a deliberate pseudocode divergence; A8 switches FoliageWindExclusions'
construction to ToFrozenSet() and softens the "never stale" doc
comment to "no slower than one frame behind."
Tests added: top-nibble classification (0xFFFFFFFFu now correctly
false), GroupKey inequality across entity-driven scenery/landblock-
static classification, a caster-batch test proving the same pairing
never coalesces, ComputeEntityHasCutoutSubset unit + end-to-end
two-part-Setup tests, the caster→receiver AtmosphericFrame binding
carry-through, flutter-hash translation invariance relative to
instance origin, and a Storm-wind mid-height displacement floor
guarding against a "no motion" regression.
Docs: plan VM6 body corrected to the five-row WeatherKind table, "bits
1 and 2", "all four" caster shaders, and top-nibble wording throughout;
the owner gate checklist's Rain/Storm step; the stale v1-only shader-
interface compatibility entry; semantic-bindings-v1.md's v2 members
folded into the main 192-byte block; the IA-25 register row's top-
nibble wording; AtmosphericFrameInputs.cs's ABI size reference.
foliage_wind.glsl's A5 change recompiled exactly the five shaders that
include it (mesh_atmospheric.vert, the four directional_shadow_world_*
casters) plus the manifest; no other .spv changed.
Verify: Release build 0 warnings/0 errors. App hermetic-lane filter
6,041/0 failed (no environment-specific failures this run).
RenderPackValidator 30/30. Full hermetic-filtered solution: 15,269/0
failed across 15 projects.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
FoliageWindByDayGroup / FoliageWindDayGroupPoint(int ActiveDayGroup, ...)
becomes FoliageWindByWeather / FoliageWindWeatherPoint(string WeatherKind,
...) in AtmospherePolicyDeclaration (Plugin.Abstractions is BCL-only, so
the key is the exact member name of AcDream.Core.World.WeatherKind rather
than the enum itself). The raw activeDayGroup index carries no weather
meaning by itself; WeatherState.cs already classifies each day group's
authored DAT name into one of five real weather kinds, and that fact was
already threaded through AtmosphericFrameInputs.Weather / uAtmosphereWeather.x
— this reuses it instead of guessing an index-to-category mapping.
Built-in table (BuiltInAtmosphericRenderPack.AtmospherePolicy()): Clear
0.25/0.15, Overcast 0.60/0.35, Rain 0.85/0.60, Snow 0.35/0.20, Storm
1.00/0.75 — all five WeatherKind members declared, the invented "Cloudy"
row dropped. RenderPackAtmospherePolicyEvaluation.FoliageWind now takes a
WeatherKind and matches by weather.ToString() (ordinal) against each
declared point's name; a kind absent from the table falls back to the
declared Clear row, then to (0,0) if Clear itself is undeclared. The
delta-seconds EMA interpolation (EaseTowardTarget) is unchanged.
AtmosphericPostProcessGraph.ResolveFoliageWind and its two callers
(RenderPostProcess via inputs.Weather; RenderDirectionalShadows via
foundation.Atmosphere.Kind) now pass WeatherKind instead of the day-group
int.
RenderPackValidation.ValidateAtmosphere (runs for every pack declaring an
AtmospherePolicy, not gated to Tier2/shadow packs) now rejects an unknown
or non-exact-case weather-kind name and a repeated kind, mirroring the
existing ActiveDayGroupMultiplier duplicate-key check.
Tests: RenderPackAtmospherePolicyEvaluationTests rewritten for the
kind-keyed API (all five kinds resolve to their declared row, an unlisted
kind falls back to Clear, ordinal exact-case matching, null-table
handling); RenderPackSpirvValidatorTests gains four descriptor-validation
cases (unknown name, wrong case, duplicate kind, the five-kind table
accepted); AtmosphericPostProcessGraphTests' three foliage-wind cases now
select WeatherKind.Storm via `with` instead of an assumed day-group index.
Spot-check (per the coordinator's ask, not changed here): yes —
ActiveDayGroupMultiplier / EvaluateDayGroupPolicy (pre-existing, Campaign
AR/VM3-era — BuiltInAtmosphericRenderPack.AtmospherePolicy()'s three rows
`new ActiveDayGroupMultiplier(0, 1.0), (1, 0.35), (2, 0.20)`) key the
sun-ray/shadow/volumetric day-group strength multiplier by the same raw
activeDayGroup index with an undocumented assumed meaning (0=brightest ...
2=dimmest), the identical class of issue this commit fixes for foliage
wind. Left unchanged per instruction; flagging for the coordinator to file.
Full solution Debug and Release builds green. App hermetic filter
6024/6026 — the same 2 pre-existing failures as VM6a/VM6b. Both were
re-run in isolation per the verification ask: both still fail alone (not
a load-flake in this environment) — confirmed via git stash earlier this
session that both already fail on the unmodified pre-VM6 baseline, so
they are pre-existing and unrelated to this change. Core.Tests hermetic
4697/4697. RenderPackValidator.Tests 30/30. No shader/spv changes in this
commit (pure C#/docs fix).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Procedural-scenery foliage (trees/bushes — entity ids in the
ProceduralSceneryIdAllocator's 0x8XXYYIII namespace) sways with weather in
mesh_atmospheric.vert and all four directional_shadow_world_* caster vertex
shaders, both calling the identical new foliage_wind.glsl include so the
shadow moves with the leaf by construction.
Classification (FoliageWindClassification, AcDream.App.Rendering.Wb): two
new BatchData.flags bits, computed once per (entity, subset) from four
inputs — entity id (bit 31 for procedural scenery), the pack's declared
FoliageExclusions membership, the subset's TranslucencyKind, and
ObjectRenderData.HasCutoutSubset (computed once per mesh at build time, not
per frame). Bit 1 marks an alpha-cutout leaf subset; bit 2 marks an opaque
trunk subset (only when its own mesh also owns a cutout subset, so rocks
stay still). WbDrawDispatcher.ClassifyBatches (world receiver) and
AddDirectionalShadowBatches (caster) call this with the same four inputs, so
casters and receivers classify identically without needing to share state.
Retail's mesh_modern/terrain_modern/mesh_detail pipelines never read these
bits, so pack-off output is unaffected.
Motion model (foliage_wind.glsl, mirrored bit-for-bit in the new
FoliageWindModel for hermetic CPU tests): height-squared-scaled slow lean
for every foliage subset, plus branch swing and per-vertex-hash-decorrelated
flutter for cutout subsets only. AtmosphericPostProcessGraph.ResolveFoliageWind
resolves the wind block once per frame.Serial — advanced by whichever of
RenderDirectionalShadows (which runs first) or RenderPostProcess is called
first that frame, with the second reading the already-advanced state, which
is what keeps the caster and receiver reading byte-identical clock/strength
values. The per-day-group mean/gust target (AtmospherePolicyDeclaration.
FoliageWindByDayGroup, keyed by the same day-group index convention
ActiveDayGroupMultipliers already established: Clear/Cloudy/Overcast/Rainy)
eases toward its target over WeatherSystem.TransitionSeconds (10s) so a
weather change never snaps; wind-enabled off or indoor instead gates the
OUTPUT to an exact zero (not an asymptotic approach) so a settings toggle or
cell transition is immediate. The wind clock is a Stopwatch started at graph
construction (monotonic, session-relative magnitude for GPU sin() accuracy),
overridable by the same ACDREAM_SKY_PHASE_SECONDS pin SkyRenderer already
uses, for deterministic offline gates.
New settings: wind-enabled, wind-strength, wind-direction-degrees (225°
default — no authored retail wind direction exists to read),
wind-lean-metres, wind-branch-metres, wind-flutter-metres (0 on Low),
wind-canopy-height-metres.
Register row IA-25 files this as an intentional, strictly opt-in divergence:
retail applies no per-vertex wind displacement to any geometry. Known,
accepted limitation: classification is per mesh-subset (one BatchData.flags
word per indirect-draw batch), not per entity instance, so the rare case of
one mesh subset being reachable from both a procedural-scenery and a
non-scenery placement would classify all of that subset's instances alike.
Tests: FoliageWindClassificationTests (the full classification matrix),
FoliageWindModelTests (identity on non-foliage/calm-wind/base-vertex,
canopy-top displacement bound, z-never-increases, trunk has no flutter
term), RenderPackAtmospherePolicyEvaluationTests (exact day-group lookup,
no interpolation across day-group ids, easing convergence without overshoot
or discontinuity), AtmosphericShaderAbiTests (each of the five shaders calls
acdreamFoliageDisplace exactly once; mesh_modern/terrain/mesh_detail call it
never), and four AtmosphericPostProcessGraphTests additions (indoor/disabled
exact-zero gating, settings-to-UBO wiring, same-frame-Serial idempotency —
the last proxies the caster/receiver agreement invariant without needing
this hermetic harness's WbDrawDispatcher/TerrainModernRenderer dependency
chain to exercise RenderDirectionalShadows directly).
App hermetic filter: 6015/6017 (the same 2 pre-existing failures as VM6a,
confirmed unrelated). Core.Tests hermetic: 4697/4697. RenderPackValidator.Tests:
30/30. Full solution Debug and Release builds green. Shader recompile
touched exactly the 5 edited files' .spv (plus manifest); the retail oracle
set and every other pack shader are byte-identical.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Opus narrow re-review of 51178f7c: APPROVE. Residuals closed: the AR plan
no longer says '<=1 LSB' unqualified (99.99% of pixels; 95 foliage-
silhouette pixels up to 73 LSB, 58 isolated); the campaign doc says the
same; AtmosphericColorPipelineTests now read the SHIPPED exposure/vignette
defaults from BuiltInAtmosphericRenderPack.Descriptor and the graph's named
DefaultVignetteStrengthFallback instead of literals.
Measured for the gate (offline Holtburg hillside, High defaults vs pack
off): mean luminance -17% noon, -44% dusk, p95 unchanged, clip 0.06% both;
neutral High vs pack off: 110,561 px at |d|=1, 95 at >=5 (foliage edges).
Evidence images under docs/research/evidence/vm3/.
#422 filed: one High-default offline capture exited with
STATUS_HEAP_CORRUPTION after a clean managed shutdown; 1 in 8 runs, never
under validation layers. VM7 gate item.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Fix round from Opus review of 87677f9c (APPROVE WITH FIXES):
1. (A1/A3) Hardened the atmospheric_filmic.frag / atmospheric_bloom_
downsample.frag shader-source pinning test: asserts the exact decode
call count (6 - three in lowFusedScene, three in main's non-fused
branch), that the 2.2 display-gamma exponent and 1/2.2 inverse in
atmospheric_common.glsl are formatted FROM AtmosphericColorPipeline
.DisplayGamma (so shader literal and CPU-tested value cannot drift),
that the 0.18 contrast pivot in atmospheric_filmic.frag is formatted
from AtmosphericColorPipeline.LinearMidGrey, and pins
AtmosphericPostProcessGraph.BloomKneeLinear/BloomThresholdLinear as
exact literals (0.73f / 1f). Also removed a stray trailing "()" from
an existing comment in atmospheric_filmic.frag that was inflating the
decode-call count to 7.
2. (B1) Re-derived the "vignette-strength" default for the linear-light
post stack. The vignette multiply now happens on linear colour before
the final encode, so a corner factor of (1 - strength) displays as
(1 - strength)^(1/2.2), not (1 - strength) directly. The accepted
look was strength 0.12 under the OLD gamma-space pipeline: a 12%
on-screen corner darkening. Under strength 0.12 in the new linear
pipeline that same 0.88 corner multiplier would only display as
0.88^(1/2.2) ~= 0.9435 (5.6% darkening - visibly weaker). Solving
(1 - strength)^(1/2.2) = 0.88 gives strength = 1 - 0.88^2.2 ~= 0.245,
which reproduces the accepted 12% corner darkening. Since
RenderPackSettingValueCodec requires every declared default to be
step-aligned from the minimum and 0.245 is not a multiple of the old
0.01 step, the step also moves to 0.005 (a finer slider, not
coarser) so the exact derived default is a valid grid point -
verified by running the ExternalTierTwoPackCanRenameEveryOwnedId
AndShaderAsset validation test, which failed with "invalid default
value" before this correction. Also updated the matching fallback in
AtmosphericPostProcessGraph.FromDescriptor (0.12f -> 0.245f) for
consistency, and added
AtmosphericColorPipelineTests.VignetteDefaultReproducesTheAccepted
TwelvePercentCornerDarkening pinning encode(1-0.245) ~= 0.88.
3. (A4) Renamed VolumetricShaftFrameParameters.LinearSunColor ->
AuthoredSunColor in VolumetricShaftQuality.cs (internal, 2 references,
both in that file - safe). Left LightSource.ColorLinear unrenamed:
grep shows 13 files depend on it (GlobalLightPacker, SceneLightingUbo,
LightBake, LightManager, EnvCellRenderer, RenderingDiagnostics, and
several Core tests) across the shared retail default-path lighting
UBO pipeline - renaming it is out of VM3's pack-only scope and would
touch the mandatory-unchanged default path. Added a pointer comment
on the field in LightSource.cs (and a one-line note at its
WorldRenderFrameBuilder.cs call site) documenting the same
display-space-not-linear fact and explaining why the rename is
deferred to its own default-path colour-space pass.
4. (B4) Added a citation beside acesFitted in both atmospheric_filmic
.frag and its C# mirror (AtmosphericColorPipeline.AcesFitted):
Krzysztof Narkowicz, "ACES Filmic Tone Mapping Curve" (2016). The fit
takes linear scene light in and returns linear display light in
[0,1] - it does not itself gamma-encode. Evidence: acesFitted(0.80 *
decode(0.46)) = 0.2064 un-encoded versus the accepted 0.51 on screen.
5. (B2) Rewrote the VM3 section of docs/plans/2026-08-22-visualmaster-
campaign.md with the shipped truth in place of the pre-implementation
guess: exposure stays 0.80 (at exposure 1.0 the linear pipeline maps
gamma-0.5 to 0.6017, essentially the same 0.6163 the owner called too
bright), bloom threshold stays 1.0 (a fixed point of both exponents),
knee moves 0.45 -> 0.73, vignette-strength moves 0.12 -> 0.245. Added
the old-vs-new curve table at exposure 0.80 across ten gamma inputs.
Replaced the acceptance criteria's "new automated test on the
recording RHI" with the CPU mirror + shader-source pins actually
used, and recorded that the real-frame masked capture WAS run
(retail vs High-with-every-effect-neutral, artifacts/vm3):
independently re-verified by re-running the pixel diff against the
checked-in screenshots - 110,561 px at |delta|=1 and exactly 95
pixels at |delta|>=5, confined to foliage-canopy silhouette edges
against sky with nothing on any ground/building/water surface. Noted
the Stage-1 luminance table re-capture is still owed at the owner
gate.
6. (B3) Corrected docs/plans/2026-08-21-atmospheric-rendering.md's VM3
summary sentence: the bloom intermediate is already linear after
extraction (no separate "bloom read" decode), and the neutral-preset
claim is now phrased as a measured numerical identity (<=1 LSB on a
real frame) rather than an unqualified "is" statement.
7. (A5) Corrected toolchain attribution: tools/compile-shaders.ps1 used
the managed Silk.NET.Shaderc path (shaderc_shared.dll) to compile in
both this round and the original VM3 commit - a Vulkan SDK glslc was
detected and its path recorded, but the managed compiler is what
actually ran. Regenerating this round only changed the atmospheric_
filmic frag stage's manifest hash (comment-only edits); the compiled
.spv bytes are unchanged, and every retail-oracle shader
(mesh_modern, terrain_modern, mesh_detail, etc.) remains untouched.
8. Replaced an invented motive in the atmospheric_filmic.frag contrast-
pivot comment ("rounded up for a stronger gamma-space contrast
feel") with the actual reason: the previous 0.5 was simply the [0,1]
midpoint of the standard contrast formula, not a deliberately chosen
value; in linear the perceptual mid-grey is 0.18.
Verify: Release build 0 warnings / 0 errors. App hermetic-filter tests:
5970 passed / 0 failed / 0 skipped. VulkanShaderManifestTests: 7/7 pass
(retail-oracle SPIR-V byte-identical; only the atmospheric_filmic frag
manifest hash changed, no .spv bytes changed).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Fixed step-count volumetric ray marches (Low/Medium presets, e.g. 40 steps)
sample the same t values at every pixel, so a shadow-cascade edge crossing
lands at the same march step across a whole neighbourhood of pixels and
reads as visible concentric banding around light shafts. Offset each
pixel's march phase with interleaved gradient noise (Jimenez 2014,
fract(52.9829189 * fract(dot(gl_FragCoord.xy, vec2(0.06711056,
0.00583715))))) instead of the fixed 0.5 sub-step center: t = (step + ign)
/ steps. This decorrelates the step boundaries across pixels, turning the
banding into fine per-pixel noise that the existing quarter/half-res
upsample already averages away. Pack-on only — atmospheric_volumetric is
never used on the retail default path; extinction, colour, and early-outs
are unchanged.
Regenerated SPIR-V via tools/compile-shaders.ps1: only
atmospheric_volumetric.frag.spv and shaders.manifest.json's source hash
for that stage changed.
Also files #421 (docs/ISSUES.md) for Campaign AR review finding F7: the
directional-shadow pass's DirectionalShadowTransformBufferSet duplicates
the main pass's N.5 instance SSBO instead of binding it, to be resolved
once the planned GPU-culling step for shadow cascades lands. Updates the
VM5 ledger row and outcome note in
docs/plans/2026-08-22-visualmaster-campaign.md.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Dated 'VM4 correction' notes, not silent rewrites:
- AR plan: NoOpRenderPackProductionIntegrationTests is a 2x2 recording-
device fixture, not a production pin; the real invariance is VM0's.
Both '14,928/14,928 zero skips' totals qualified: hermetic lane filter.
- Stage-2 connected report: the dense-town CPU figures ran under the
automation observer (9.8 MB/frame alloc) and say nothing about the
owner's hitch on the product; clean numbers are VM0's.
- Track A report + findings doc: TerrainUtils.GetNormal only orients
scenery; the render normals were already smooth; A2 replaced central
differences with retail's incident-face average (the reviewer's own
premise error).
- Findings doc: 'retail brightens' is the fallback path only; real
hardware runs the single-pass lerp (VM2). LandscapeDetailTextures = 0
answers the landscape open question. VM0 production table added as the
baseline.
- Completion audit retail-path row and the review's F1/F2/F3/F5 headers
annotated with their closures.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Opus narrow re-review of ae651312: APPROVE. Closes its three residuals:
- AP-232 filed: retail's single-pass stage-1 OUTPUT alpha
(MODULATE(TEXTURE, CURRENT) @0x0059c549) is the blend weight for a
translucent subset; acdream's two-draw model is exact for opaque
subsets (fog identity pinned) and a bounded weight difference on
translucent ones. Distinct from AP-34 (queue order). Owed since
05970306.
- TerrainAtlas.DetailSamplerDescription names the production sampler
(WRAP/LINEAR x3 per ACRender::SetDetailSurfaceInternal @0x006b6280);
the test now asserts that constant's properties instead of a
test-local copy.
- Plan VM1 section: fragment now described as fogged; VM1 marked CLOSED
with the Holtburg measurement (+2.17/+0.57/+0.16 vs predicted
+2.2/+0.66/+0.16) and the detail-on cost (+0.3-0.5 ms CPU at Arwic).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Exact-pixel and production-perf comparison of 6c79d35c (+ only the A2
normal files, so no terrain mask is needed) against c51b07ef with no pack:
connected as +Acdream, visible window, one isolated config clone per
variant, pinned clocks. Open field: the only differences are idle pose,
mana digits and a passing flyer. Holtburg: same-binary-twice defines the
dynamic mask (9.8%); in the static 90% both self-diffs have ZERO pixels
with |d|>=8 while base+normals vs HEAD-off has 841/729 - all streaks inside
the animated lifestone. Buildings, ground, trees, sky and UI are clean.
Perf (uncapped Release, no automation observer, ACDREAM_FRAME_PROF=1):
Holtburg CPU p50 4.7 -> 4.1 ms, Arwic 6.0 -> 5.2 ms, GPU unchanged,
alloc/frame 574 KB -> 21 KB. No regression; F5b's '27.8 ms retail CPU' was
the observer.
Three false alarms recorded so nobody repeats them: the isolated gate
settings lack fieldOfView (90 vs the real 86.33 -> a 0.952 zoom); the real
%APPDATA% settings still selected acdream.atmospheric/low (pack ON); a
minimized GLFW window is throttled and never settles.
Tools: tools/vm0/capture-visible.ps1 (pre-campaign gate + -Exe/-Live/
-ConfigDir/-CharacterName/-PreCaptureCommand), tools/vm0/perf-run.sh,
and -BuildingDetailTextures on run-offline-pixel-gate.ps1. Baseline
patches under docs/research/evidence/vm0/.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Binding rule: when retail gates a mechanism behind a capability, preference,
or trysinglepass-style switch, the port is not done until the gate's live
value is cdb-read on the PDB-paired client and cited with the binary GUID.
Records that the owner's live 'too bright' was the pack-on gamma-space
tonemap (VM3), not the detail overlay (+3% fallback / -10% real path).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Live cdb read on the PDB-paired retail client (owner's AMD GPU):
m_caps.bCanDoSinglePassDetailing = 1, trysinglepass = 1,
MaxSimultaneousTextures = 8, bTexOpDotProduct3 = 1,
LandscapeDetailTextures = 0, EnvironmentDetailTextures = 1,
landscape/object detail surfaces null, building/environment non-null,
tiling 4 everywhere.
So retail's detail combine on modern hardware is the texture-stage path in
D3DPolyRender::SetSurface (stage0 alpha PREMODULATE, stage1 colour
BLENDCURRENTALPHA): lerp(base*diffuse, detail.rgb, detail.a*diffuse.a) -
a mild DARKENING (~-10% mid-tones with 0x06006D58), not the
DSTCOLOR+INVSRCALPHA brightening that Campaign AR ported and that the
2026-08-21 findings doc + AR review both analysed. Those described the
fallback for adapters without D3DTEXOPCAPS_PREMODULATE; the reviewer's
guess that consumer drivers rarely advertised it was wrong.
VM1 now carries the re-port (SRCALPHA+INVSRCALPHA, detail.rgb /
detail.a*diffuseAlpha, neutral at a==0) together with the fade removal.
No distance fade exists on either path. Scripts are read-only attaches
with no breakpoints.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Adds the independent Campaign AR review (docs/research/2026-08-22-campaign-
ar-review.md): architecture, technical correctness, performance, coverage.
Findings F1-F8; F1 (default-path invariance proven only on a 2x2 recording-
device oracle) blocks merge; F2 (10 m/50 m detail fade has no retail anchor);
F3 (the port is retail's two-pass fallback; single-pass stage path
undetermined); F4 (post stack runs in gamma space); F5 (four overclaiming
sentences, one premise error the reviewer owns).
Campaign VM plans the closure: VM0 masked pixel + production perf A/B vs
6c79d35c; VM1 removes the fade (retail attenuates by mips); VM2 one cdb read
of m_caps.bCanDoSinglePassDetailing; VM3 linear-light post stack with a
numerically neutral preset; VM4 truthful docs; VM5 volumetric jitter; VM6
weather-driven foliage wind - procedural-scenery (bit-31 id namespace) cutout
subsets only, three motions (lean/branch/flutter) with gusts and per-tree
phase, shadows share the displacement include; VM7 closeout + merge.
Roadmap gains the Campaign VM pointer beside Campaign AR.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Erik