fix(render): correct foliage-wind classification, receiver/caster desync, and frame binding (Campaign VM VM6 review)
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>
This commit is contained in:
parent
27a446f05c
commit
43e3abed4d
28 changed files with 724 additions and 123 deletions
|
|
@ -357,9 +357,9 @@ the owner does not want.
|
|||
### The rule for what sways (no guessing, data-driven)
|
||||
|
||||
- **Candidate set:** procedural scenery only — entity ids in the
|
||||
`0x8XXYYIII` namespace (`ProceduralSceneryIdAllocator`, bit 31). Landblock
|
||||
statics (fences, signposts, buildings), weenies, creatures and players never
|
||||
sway.
|
||||
`0x8XXYYIII` namespace (`ProceduralSceneryIdAllocator`, top nibble 0x8).
|
||||
Landblock statics (fences, signposts, buildings), weenies, creatures and
|
||||
players never sway.
|
||||
- **Foliage subsets:** within a candidate, the **alpha-cutout material
|
||||
subsets** (leaves, fronds, bushes, grass tufts). Rocks have none and stay
|
||||
still; a tree's trunk is opaque and gets only the slow lean (below).
|
||||
|
|
@ -369,7 +369,7 @@ the owner does not want.
|
|||
descriptor) for the rare scenery object that is cutout but not foliage.
|
||||
There is no include list: the rule is the rule.
|
||||
|
||||
This is a render-only classification flag per batch — bit 1 of
|
||||
This is a render-only classification flag per batch — bits 1 and 2 of
|
||||
`BatchData.flags` (`mesh_atmospheric.vert` already reads `flags`; bit 0 is
|
||||
the #226 built-mesh marker). The dispatcher sets it once at classification
|
||||
time from the entity id namespace and the subset's blend class; the pack-off
|
||||
|
|
@ -377,7 +377,7 @@ time from the entity id namespace and the subset's blend class; the pack-off
|
|||
|
||||
### The motion (vertex shader, pack variant only)
|
||||
|
||||
Applied in `mesh_atmospheric.vert` and in all six
|
||||
Applied in `mesh_atmospheric.vert` and in all four
|
||||
`directional_shadow_world_*` vertex shaders (constraint: **the shadow must
|
||||
move with the leaf** — the caster and receiver displacement are one shared
|
||||
include, `foliage_wind.glsl`, so they cannot drift apart).
|
||||
|
|
@ -417,20 +417,41 @@ vertex hash decorrelates leaves on the same tree; the instance phase
|
|||
decorrelates trees. The `p.z` term is the cheap length-preserving correction
|
||||
so a bent canopy sinks slightly instead of growing.
|
||||
|
||||
**Deliberate divergence from the pseudocode above (review fix round A7):**
|
||||
both `h`'s divisor and the `p.z` correction's divisor guard `maxHeight`
|
||||
(`amp.w`/`uAtmosphereWindAmplitude.w`) with `max(maxHeight, 0.5)`, i.e.
|
||||
`h = clamp((p.z - o.z) / max(maxHeight, 0.5), 0, 1)` and
|
||||
`p.z -= 0.5 * dot(d, d) / max(h * maxHeight, 0.5)`. `wind-canopy-height-metres`
|
||||
is an author-facing pack setting with no enforced floor; without the guard a
|
||||
misconfigured near-zero canopy height would divide by a near-zero value and
|
||||
either blow `h` up to a huge (then clamped) number with a discontinuous
|
||||
derivative right at the base, or make the bend-shortening term explode.
|
||||
Flooring at 0.5 m (half the shortest plausible sapling) keeps both terms
|
||||
well-behaved for any authored value, including 0, and is invisible for every
|
||||
realistic canopy height (the built-in default is 8 m). Both the shader
|
||||
(`foliage_wind.glsl`) and the CPU mirror (`FoliageWindModel.Displace`) apply
|
||||
the guard identically.
|
||||
|
||||
Normals are **not** rotated (Gouraud on a cutout leaf with flipped lighting
|
||||
would flicker); AC's flat-lit foliage does not need it.
|
||||
|
||||
### Weather drives it (AC owns the weather)
|
||||
|
||||
The pack's `AtmospherePolicyDeclaration` gains a `FoliageWind` table keyed by
|
||||
the same categorical `activeDayGroup` the rays already use:
|
||||
The pack's `AtmospherePolicyDeclaration` gains a `FoliageWindByWeather` table
|
||||
keyed by the DAT-classified `AcDream.Core.World.WeatherKind` — not the raw
|
||||
`activeDayGroup` index, which carries no weather meaning by itself.
|
||||
`WeatherState.cs` already classifies each day group's authored DAT name into
|
||||
one of these five real kinds, and `AtmosphericFrameInputs.Weather` /
|
||||
`uAtmosphereWeather.x` already threads that classification through the
|
||||
frame — this table reuses it instead of re-guessing from the index:
|
||||
|
||||
| Day group | mean | gust |
|
||||
| WeatherKind | mean | gust |
|
||||
|---|---|---|
|
||||
| Clear | 0.25 | 0.15 |
|
||||
| Cloudy | 0.45 | 0.30 |
|
||||
| Overcast | 0.60 | 0.35 |
|
||||
| Rainy | 0.85 | 0.60 |
|
||||
| Rain | 0.85 | 0.60 |
|
||||
| Snow | 0.35 | 0.20 |
|
||||
| Storm | 1.00 | 0.75 |
|
||||
|
||||
plus a global **Wind** slider (0–2×) and an **Off** setting. Transitions
|
||||
between day groups interpolate over the existing weather delta seconds
|
||||
|
|
@ -459,15 +480,17 @@ direction to read, and the register row says so.
|
|||
|
||||
### Acceptance
|
||||
|
||||
- Automated: the classification flag is set only for bit-31 entities' cutout
|
||||
subsets (and lean-only for their opaque subsets); pack-off pipelines never
|
||||
- Automated: the classification flag is set only for top-nibble-0x8 entities'
|
||||
cutout subsets (and lean-only for their opaque subsets); pack-off pipelines never
|
||||
read the flag and VM0's masked comparison is unchanged; the shared include
|
||||
is byte-identical between receiver and caster variants (test reads both
|
||||
SPIR-V inputs' source); weather table interpolates without discontinuity;
|
||||
budget row passes.
|
||||
- **Owner visual gate**, in this order: (1) Holtburg outskirts, Clear, noon —
|
||||
"barely moving, alive"; (2) same place, Rainy — "clearly windy, still not
|
||||
a flag"; (3) watch one tree for 30 s — gusts arrive and leave, neighbours
|
||||
"barely moving, alive"; (2) same place, Rain or Storm (the WeatherKind
|
||||
comes from WeatherState's classification of the active day group's DAT
|
||||
name — `/time`-cycle day groups or wait for weather) — "clearly windy,
|
||||
still not a flag"; (3) watch one tree for 30 s — gusts arrive and leave, neighbours
|
||||
out of step; (4) the tree's shadow on the ground moves with it; (5) walk
|
||||
indoors — nothing moves; (6) pack off — nothing moves. The owner's words
|
||||
decide; the numbers above are starting points to tune live.
|
||||
|
|
@ -507,14 +530,39 @@ vertex shaders, reading the ABI v2 `uAtmosphereClockWind`/
|
|||
`uAtmosphereWindAmplitude` members `AtmosphericPostProcessGraph.ResolveFoliageWind`
|
||||
resolves once per `frame.Serial` (so the caster, which runs first each
|
||||
frame, and the receiver read byte-identical values). `FoliageWindModel` is
|
||||
the CPU mirror pinned by hermetic tests. Known limitation, documented and
|
||||
accepted rather than engineered around: classification is per mesh-subset
|
||||
(a `BatchData.flags` word shared by every instance drawn in one indirect
|
||||
command), not per entity instance — the rare case of the exact same mesh
|
||||
subset being reachable from both a procedural-scenery and a non-scenery
|
||||
placement would classify all of that subset's instances alike. Register row
|
||||
IA-25. Owed: the six-step owner visual gate above (implementation is
|
||||
otherwise code-complete and the automated acceptance criteria pass).
|
||||
the CPU mirror pinned by hermetic tests. Register row IA-25. Owed: the
|
||||
six-step owner visual gate above (implementation is otherwise code-complete
|
||||
and the automated acceptance criteria pass).
|
||||
|
||||
**Review fix round (2026-08-22/23):** an Opus dual-lens review of the three
|
||||
implementation commits found two blockers, both fixed. (A1) The procedural-
|
||||
scenery classifier tested bit 31 alone instead of the full top nibble
|
||||
`0xF000_0000 == 0x8000_0000`, which incorrectly matched `LandblockStatic-
|
||||
EntityIdAllocator`'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 and `FoliageWindClassification.IsProceduralScenery` delegates to it.
|
||||
(A2) `GroupKey` (the world-receiver instance-batching key) did not include
|
||||
`FoliageFlags` while the caster's dedup key already did, so a scenery
|
||||
instance and a non-scenery instance sharing the same mesh subset coalesced
|
||||
into one receiver `InstanceGroup` whose flags were whichever entity
|
||||
classified it last — the "known limitation" paragraph this outcome section
|
||||
used to carry. `GroupKey` now carries `FoliageFlags`, set exactly once at
|
||||
group creation from the key, never re-stamped; the two placements now land
|
||||
in two distinct groups and the receiver agrees with the caster by
|
||||
construction. Two should-fix items also landed: (A3) the world receiver
|
||||
pass now explicitly binds `UniformAtmosphericFrame` from the caster's own
|
||||
per-frame binding (carried on `DirectionalShadowFrameBinding`) instead of
|
||||
relying on Vulkan not resetting the caster pass's leftover binding; (A4) a
|
||||
Setup-composed tree's opaque trunk part now gets the trunk flag by OR-ing
|
||||
`HasCutoutSubset` across all of the entity's currently-resolved sibling
|
||||
parts (`FoliageWindClassification.ComputeEntityHasCutoutSubset`) instead of
|
||||
consulting only the trunk part's own (cutout-free) mesh data. Nits A5
|
||||
(flutter hash relative to instance origin, not absolute world XY — fp32
|
||||
precision at far landblock corners) and A8 (`FoliageWindExclusions` as a
|
||||
`FrozenSet`) also landed; A7 is the divergence note on the `max(maxHeight,
|
||||
0.5)` guard above.
|
||||
|
||||
## VM7 — Closeout and merge
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue