diff --git a/docs/architecture/retail-divergence-register.md b/docs/architecture/retail-divergence-register.md index 49ab8269..f0c187cc 100644 --- a/docs/architecture/retail-divergence-register.md +++ b/docs/architecture/retail-divergence-register.md @@ -62,7 +62,7 @@ accepted-divergence entries (#96, #49, #50). | IA-22 | **Filed 2026-08-13 (#391, user-directed: "we should only support modern resolutions. Not any old format").** The Config Resolution dropdown offers a CURATED list — the monitor's real mode enumeration filtered to modern widescreen families (16:9/16:10/21:9/32:9, ≥1280 wide, fitting the desktop; `DisplayModeCatalog.Curate`) — and its Defaults value is the desktop's own mode. Retail offered the adapter's complete enumeration including 4:3 legacy modes and authored `800x600` as the row default (`gmConfigUI::InitOptions SetDefaultValue(0x03200258)`; `gmClient::Init @0x004047af` `Device::ForceDisplayResolution(1, 0x320, 0x258)`). | `src/AcDream.App/Rendering/DisplayModeCatalog.cs`; `src/AcDream.App/UI/Layout/ConfigOptionsPageController.cs` (Resolution row); fixture fallback `src/AcDream.UI.Abstractions/Panels/Settings/DisplaySettings.cs` (`AvailableResolutions`, 800x600 removed) | Explicit product direction. **Amended 2026-08-16 (#407, Campaign CC gate round 1):** the dropdown now offers `DisplayModeCatalog.WindowedResolutions` — the curated hardware modes UNIONed with the static modern-ladder sizes that fit the desktop — because a WINDOWED pick is a plain Size write needing no video mode, and remote/RDP virtual displays advertise almost no modes (the live RDP display exposed only 1920x1080 + the 2056x1290 desktop, starving the dropdown). The original "an offered mode is supported by construction" invariant now holds for the FULLSCREEN half only: the fullscreen apply still validates against the hardware `Resolutions` list plus `GlfwDisplayModeSwitcher`'s monitor-mode-list hard guard, so a fullscreen pick of a windowed-only entry refuses safely (log-and-stay, #388; the #392 apply-result seam is that family's open follow-up) — "Graphics mode not supported" crashes remain unreachable from the dropdown. | A user wanting a genuine legacy 4:3 mode cannot pick it; retail-parity comparisons of the Config tab's list/default will show the deviation. | decomp sites in the Divergence column; ISSUES #391 | | IA-23 | **Filed 2026-08-17 at the night-round review fix round (F8).** `gmHouseUI::DisplayPurchaseTimeText @0x004a3110`'s not-yet-expired branch renders `"You may buy another landscape house at " + strftime("%c", localtime(timestamp + 0x278d00)) + ". This restriction does not apply to apartments."` — byte-decoded from raw pushed literals at `@0x004a3265`/`@0x004a321d`/`@0x004a3235` (all three text pieces confirmed; a prior filing had wrongly called this "unrecoverable"). This port renders the SAME three pieces, in the same order, with the same expiry-timestamp math, but formats the middle date/time piece with .NET's culture-default `DateTime.ToString()` (no explicit format string) rather than the C runtime's `strftime("%c", ...)` — the two engines do not share a format table, so the RENDERED SHAPE of the date/time differs (e.g. .NET's short numeric date+time vs the CRT's `Ddd Mon DD HH:MM:SS YYYY`-style locale string) even though both express "the process's own locale's full date+time" and use the SAME underlying instant (local time, matching retail's `localtime()`). | `src/AcDream.Runtime/Gameplay/RuntimeHouseState.cs` (`Recompute`'s not-expired branch) | Both are "whatever the process locale says" full date+time strings; no game-logic reads or parses this text back, it is pure chat-scroll presentation, so a differently-shaped (but equally legible) date string carries no functional risk | A retail-side-by-side visual comparison will show a differently formatted date/time (not a byte-identical `strftime("%c")` reproduction) — cosmetic only | `gmHouseUI::DisplayPurchaseTimeText @0x004a3110`; `strftime`/`localtime` CRT calls at `@0x004a322c`/`@0x004a3216` | | IA-24 | **Filed 2026-08-22, Campaign AR.** An explicitly selected atmospheric render pack adds cascaded real-time directional shadows from terrain, trees, buildings, players, monsters, and other retained outdoor casters. The one shadow direction follows the visible authored sun, then the dominant haloed moon (`0x01001F6A`), then the secondary moon (`0x01001F67`); a moon supplies direction only while colour/energy remains retail's single interpolated `SkyTimeOfDay.DirColor × DirBright` channel. Retail renders none of these real-time object-shadow maps and does not expose a second moon light. | `src/AcDream.App/Rendering/Packs/AuthoredCelestialShadowSource.cs`; `src/AcDream.App/Rendering/DirectionalSunShadowRenderer.cs`; pack-only receiver shaders; evidence `docs/research/2026-08-22-dereth-celestial-shadow-sources.md` | This is the user-requested headline graphics enhancement and is strictly opt-in. The retail path remains the default and authoritative fallback; pack-off does not build/select shadow work or change `SceneLighting`. One selected source reuses one cascade array, so moon support does not multiply shadow resources. | Pack-on output intentionally differs from retail. A wrong celestial identity/transform or stale source transition would visibly misalign shadows from the sky; pack-off output changing would violate the campaign's primary safety contract. | `SkyDesc::GetLighting @0x00500A80`; `GameSky::UseTime @0x005075B0`; installed Region `0x13000000`; cited research note | -| IA-25 | **Filed 2026-08-22, Campaign VM VM6.** An explicitly selected atmospheric render pack sways procedural-scenery foliage (trees/bushes — entity ids in the `0x8XXYYIII` `ProceduralSceneryIdAllocator` namespace) in `mesh_atmospheric.vert` and the four `directional_shadow_world_*` caster vertex shaders, driven by a weather-table lean/branch/flutter vertex displacement (`foliage_wind.glsl`, `FoliageWindModel` CPU mirror) whose mean/gust strength is looked up per DAT-classified `AcDream.Core.World.WeatherKind` (Clear/Overcast/Rain/Snow/Storm — the same classification `WeatherState.cs` already derives from the active day group's authored name, not the day group's raw index, which carries no weather meaning by itself) and eases toward its target over `WeatherSystem.TransitionSeconds` (10 s) so a weather change never snaps. Retail's fixed-function renderer applies no per-vertex wind displacement to any scenery mesh — Dereth's trees are static geometry. Wind direction (`wind-direction-degrees`, default 225°) is a plain pack default: there is no authored retail wind direction to read (no wind data exists in retail at all). Render-only: `WorldPicker` picks the undisplaced mesh, so a swaying leaf can be up to `lean + branch` metres from its pick volume at the moment of a click; foliage subsets are cosmetic scenery, not interactable in retail either. | `src/AcDream.App/Rendering/Shaders/foliage_wind.glsl`; `src/AcDream.App/Rendering/Wb/FoliageWindClassification.cs`; `src/AcDream.App/Rendering/Packs/FoliageWindModel.cs`; `src/AcDream.App/Rendering/Packs/AtmosphericPostProcessGraph.cs` (`ResolveFoliageWind`); `src/AcDream.App/Rendering/Packs/BuiltInAtmosphericRenderPack.cs` (wind settings + `FoliageWindByWeather`) | Explicitly opt-in graphics enhancement — the retail path (`mesh_modern`, `terrain_modern`, `mesh_detail`) never reads `BatchData.flags` bits 1/2 and is pixel-identical with the pack off. The classification never touches Runtime/Core physics — the collision BSP is the trunk, and picking against the undisplaced mesh has no gameplay consequence since foliage is not interactable. | Pack-on output intentionally differs from retail (moving foliage where retail has none). A wrong classification bit would sway a non-foliage object or leave a real tree still; a caster/receiver clock or amplitude mismatch would visibly misalign a leaf's shadow from the leaf itself. Pack-off output changing would violate the campaign's primary safety contract. | None — retail applies no vertex wind displacement to any geometry; `ProceduralSceneryIdAllocator` (bit 31 entity-id namespace, existing acdream mechanism, not retail) | +| IA-25 | **Filed 2026-08-22, Campaign VM VM6.** An explicitly selected atmospheric render pack sways procedural-scenery foliage (trees/bushes — entity ids in the `0x8XXYYIII` `ProceduralSceneryIdAllocator` namespace) in `mesh_atmospheric.vert` and the four `directional_shadow_world_*` caster vertex shaders, driven by a weather-table lean/branch/flutter vertex displacement (`foliage_wind.glsl`, `FoliageWindModel` CPU mirror) whose mean/gust strength is looked up per DAT-classified `AcDream.Core.World.WeatherKind` (Clear/Overcast/Rain/Snow/Storm — the same classification `WeatherState.cs` already derives from the active day group's authored name, not the day group's raw index, which carries no weather meaning by itself) and eases toward its target over `WeatherSystem.TransitionSeconds` (10 s) so a weather change never snaps. Retail's fixed-function renderer applies no per-vertex wind displacement to any scenery mesh — Dereth's trees are static geometry. Wind direction (`wind-direction-degrees`, default 225°) is a plain pack default: there is no authored retail wind direction to read (no wind data exists in retail at all). Render-only: `WorldPicker` picks the undisplaced mesh, so a swaying leaf can be up to `lean + branch` metres from its pick volume at the moment of a click; foliage subsets are cosmetic scenery, not interactable in retail either. | `src/AcDream.App/Rendering/Shaders/foliage_wind.glsl`; `src/AcDream.App/Rendering/Wb/FoliageWindClassification.cs`; `src/AcDream.App/Rendering/Packs/FoliageWindModel.cs`; `src/AcDream.App/Rendering/Packs/AtmosphericPostProcessGraph.cs` (`ResolveFoliageWind`); `src/AcDream.App/Rendering/Packs/BuiltInAtmosphericRenderPack.cs` (wind settings + `FoliageWindByWeather`) | Explicitly opt-in graphics enhancement — the retail path (`mesh_modern`, `terrain_modern`, `mesh_detail`) never reads `BatchData.flags` bits 1/2 and is pixel-identical with the pack off. The classification never touches Runtime/Core physics — the collision BSP is the trunk, and picking against the undisplaced mesh has no gameplay consequence since foliage is not interactable. | Pack-on output intentionally differs from retail (moving foliage where retail has none). A wrong classification bit would sway a non-foliage object or leave a real tree still; a caster/receiver clock or amplitude mismatch would visibly misalign a leaf's shadow from the leaf itself. Pack-off output changing would violate the campaign's primary safety contract. | None — retail applies no vertex wind displacement to any geometry; `ProceduralSceneryIdAllocator` (top-nibble-0x8 entity-id namespace, existing acdream mechanism, not retail) | --- diff --git a/docs/plans/2026-08-22-visualmaster-campaign.md b/docs/plans/2026-08-22-visualmaster-campaign.md index 9162e3c3..3f1ecb15 100644 --- a/docs/plans/2026-08-22-visualmaster-campaign.md +++ b/docs/plans/2026-08-22-visualmaster-campaign.md @@ -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 diff --git a/docs/render-packs/compatibility-and-failure-v1.md b/docs/render-packs/compatibility-and-failure-v1.md index b4a806d4..aad3a2aa 100644 --- a/docs/render-packs/compatibility-and-failure-v1.md +++ b/docs/render-packs/compatibility-and-failure-v1.md @@ -93,7 +93,7 @@ transactional fallback rules above. | Declaration | `Pass 'blur' reads resource 'bloom-a' before it is written` | Correct the descriptor | | User setting | `user override 'exposure' has invalid Float value '1,5'` | Correct/remove that stable setting-ID override; retail remains active | | Asset | `asset 'bloom.frag.spv' is not valid SPIR-V` | Rebuild/reinstall the pack | -| Shader interface | `AtmosphericFrame block does not match v1` | Recompile against the v1 binding table | +| Shader interface | `AtmosphericFrame must match ABI v1 (seven members, 160 bytes) or ABI v2 (nine members, 192 bytes)` | Recompile against the v1 or v2 binding table — v2 is additive over v1, so existing v1 modules remain valid and need no rebuild; this fires only when a module's `AtmosphericFrame` block matches neither accepted layout | | Resource ceiling | `preset 'high' exceeds the pack memory ceiling` | Reduce the preset declaration | | Auto performance | `Low remained over its declared performance budget for 180 stable samples` | Complete pack falls back to Retail; select explicit Low only after reviewing the measured limits | | Candidate build | `pipeline creation failed for 'directional-shadow-world-cutout'` | Driver/asset diagnosis; retail for this session | diff --git a/docs/render-packs/semantic-bindings-v1.md b/docs/render-packs/semantic-bindings-v1.md index efd8707a..3647c2a3 100644 --- a/docs/render-packs/semantic-bindings-v1.md +++ b/docs/render-packs/semantic-bindings-v1.md @@ -98,6 +98,9 @@ layout(std140, set = 3, binding = 5) uniform AtmosphericFrame { vec4 uAtmosphereSunDirection; // @64: surface-to-sun xyz, authored direction brightness vec4 uAtmospherePolicy; // @80: day group, group factor, shadow factor, shaft factor mat4 uAtmosphereInverseViewProjection; // @96 + // ABI v2 (Campaign VM VM6) — additive, see below: + vec4 uAtmosphereClockWind; // @160: elapsed seconds, wind mean [0..1], wind gust [0..1], wind direction radians + vec4 uAtmosphereWindAmplitude; // @176: lean amplitude m, branch amplitude m, flutter amplitude m, max canopy height m }; ``` @@ -136,17 +139,9 @@ and depth range match the shared push-block `viewProjection`. ### ABI v2 (additive) — Campaign VM VM6 -`AtmosphericFrame` grew from 160 to 192 bytes by appending two members after -`uAtmosphereInverseViewProjection`. Nothing above this line moved or changed -meaning: - -```glsl -layout(std140, set = 3, binding = 5) uniform AtmosphericFrame { - // ... the seven v1 members, unchanged ... - vec4 uAtmosphereClockWind; // @160: elapsed seconds, wind mean [0..1], wind gust [0..1], wind direction radians - vec4 uAtmosphereWindAmplitude; // @176: lean amplitude m, branch amplitude m, flutter amplitude m, max canopy height m -}; -``` +`AtmosphericFrame` grew from 160 to 192 bytes by appending the two members +shown above after `uAtmosphereInverseViewProjection`. Nothing before offset +160 moved or changed meaning. `uAtmosphereClockWind`/`uAtmosphereWindAmplitude` feed the shared `foliage_wind.glsl` include, which `mesh_atmospheric.vert` and the four diff --git a/src/AcDream.App/Rendering/DirectionalShadowReceiver.cs b/src/AcDream.App/Rendering/DirectionalShadowReceiver.cs index 0e023b9a..5a1c4531 100644 --- a/src/AcDream.App/Rendering/DirectionalShadowReceiver.cs +++ b/src/AcDream.App/Rendering/DirectionalShadowReceiver.cs @@ -14,7 +14,19 @@ internal readonly record struct DirectionalShadowFrameBinding( uint OffsetBytes, uint SizeBytes, GpuTextureSlot TextureSlot, - int CascadeCount) + int CascadeCount, + // Campaign VM VM6 review fix round (A3): the SAME AtmosphericFrame + // binding the caster pass bound this frame (see + // DirectionalSunShadowRenderer.RenderPrepared's atmosphericFrame + // parameter), carried on this existing per-frame seam so the world + // receiver pass (mesh_atmospheric.vert, which reads + // uAtmosphereClockWind/uAtmosphereWindAmplitude) binds set 3/binding 5 + // itself instead of relying on whatever the caster pass happened to + // leave bound earlier in the frame. Unbound (default) is valid — it + // just means no caster ran this frame (or the source never supplied + // one, e.g. a declared pack); BindDirectionalShadowReceiver skips + // binding 5 in that case exactly like the caster side does. + AtmosphericFrameBufferBinding AtmosphericFrame = default) { internal static DirectionalShadowFrameBinding Disabled => default; diff --git a/src/AcDream.App/Rendering/DirectionalSunShadowRenderer.cs b/src/AcDream.App/Rendering/DirectionalSunShadowRenderer.cs index 8fc7a684..f3c1cb84 100644 --- a/src/AcDream.App/Rendering/DirectionalSunShadowRenderer.cs +++ b/src/AcDream.App/Rendering/DirectionalSunShadowRenderer.cs @@ -590,11 +590,15 @@ internal sealed class DirectionalSunShadowRenderer : IDirectionalShadowReceiverS DirectionalShadowUniforms.SizeInBytes); // Campaign VM VM6: the caster pass reads foliage-wind clock/ // amplitude inputs through the same shared AtmosphericFrame - // block the world receiver binds (set 3/binding 5), so a - // displaced leaf's shadow moves with it by construction. Bound - // only when the caller supplied one — declared (non-built-in) - // packs leave this unbound and their caster shaders, which never - // declare binding 5, are unaffected. + // block (set 3/binding 5) the world receiver pass ALSO + // explicitly binds — this exact buffer/offset is carried + // forward on DirectionalShadowFrameBinding.AtmosphericFrame + // (review fix round A3) so WbDrawDispatcher.BindDirectionalShadowReceiver + // binds it itself rather than depending on this caster bind + // surviving un-reset until the receiver pass runs later in the + // frame. Bound only when the caller supplied one — declared + // (non-built-in) packs leave this unbound and their caster + // shaders, which never declare binding 5, are unaffected. if (atmosphericFrame.IsBound) { encoder.BindUniformBuffer( @@ -647,7 +651,11 @@ internal sealed class DirectionalSunShadowRenderer : IDirectionalShadowReceiverS uniformAllocation.OffsetBytes, DirectionalShadowUniforms.SizeInBytes, _textureSlot, - cascadeCount); + cascadeCount, + // Campaign VM VM6 review fix round (A3): the exact same + // buffer/offset the caster pass just bound above, carried + // forward so the receiver pass can bind it itself. + AtmosphericFrame: atmosphericFrame); (bool hasGpu, double gpuMilliseconds) = ResolveGpu(cascadeCount); int drawsPerCascade = terrainDraws.Commands.IsEmpty ? 0 : 1; diff --git a/src/AcDream.App/Rendering/Packs/AtmosphericFrameInputs.cs b/src/AcDream.App/Rendering/Packs/AtmosphericFrameInputs.cs index abf8cfa0..cb59ea3e 100644 --- a/src/AcDream.App/Rendering/Packs/AtmosphericFrameInputs.cs +++ b/src/AcDream.App/Rendering/Packs/AtmosphericFrameInputs.cs @@ -133,7 +133,7 @@ internal sealed class AtmosphericFrameInputState : IAtmosphericWorldFrameSink /// six std140 vec4 values, one mat4, then two more std140 vec4 values — /// 192 bytes. The two appended members carry the foliage-wind clock/weather /// and amplitude inputs foliage_wind.glsl reads; every earlier member -/// keeps its ABI v1 offset. See +/// keeps its ABI v1 offset. See /// and atmospheric_common.glsl for the byte-level contract both /// backends and the SPIR-V validator agree on. /// diff --git a/src/AcDream.App/Rendering/Packs/AtmosphericPostProcessGraph.cs b/src/AcDream.App/Rendering/Packs/AtmosphericPostProcessGraph.cs index 36c116e1..9b17d55e 100644 --- a/src/AcDream.App/Rendering/Packs/AtmosphericPostProcessGraph.cs +++ b/src/AcDream.App/Rendering/Packs/AtmosphericPostProcessGraph.cs @@ -1,3 +1,4 @@ +using System.Collections.Frozen; using System.Diagnostics; using System.Numerics; using System.Runtime.InteropServices; @@ -312,7 +313,7 @@ internal sealed class AtmosphericPostProcessGraph : userSettingOverrides); _foliageWindExclusions = (descriptor.AtmospherePolicy?.FoliageExclusions - ?? (IReadOnlyList)[]).ToHashSet(); + ?? (IReadOnlyList)[]).ToFrozenSet(); _windClockSecondsOverride = windClockSecondsOverride; _packSettings = PackSettingsUniforms.Create( descriptor, diff --git a/src/AcDream.App/Rendering/Packs/FoliageWindModel.cs b/src/AcDream.App/Rendering/Packs/FoliageWindModel.cs index a1ab5d3a..fc730cc7 100644 --- a/src/AcDream.App/Rendering/Packs/FoliageWindModel.cs +++ b/src/AcDream.App/Rendering/Packs/FoliageWindModel.cs @@ -70,9 +70,14 @@ internal static class FoliageWindModel if ((batchFlags & FoliageWindClassification.CutoutFoliageFlag) != 0u) { float branch = k * amplitude.Y * s * MathF.Sin((1.1f * t) + ph + (2.0f * h)); - var worldXY = new Vector2(worldPos.X, worldPos.Y); + // Review fix round A5: relative to the instance origin, not + // absolute world XY — matches foliage_wind.glsl exactly (see + // its comment for the fp32-precision-at-far-landblocks reason). + var relativeXY = new Vector2( + worldPos.X - instanceOrigin.X, + worldPos.Y - instanceOrigin.Y); float vh = Frac( - MathF.Sin(Vector2.Dot(worldXY, new Vector2(12.9898f, 78.233f))) * 43758.5453f); + MathF.Sin(Vector2.Dot(relativeXY, new Vector2(12.9898f, 78.233f))) * 43758.5453f); float flutter = h * amplitude.Z * s * MathF.Sin((6.0f * t) + (7.0f * vh)); d += (dir * branch) + (perp * 0.35f * branch) diff --git a/src/AcDream.App/Rendering/Shaders/foliage_wind.glsl b/src/AcDream.App/Rendering/Shaders/foliage_wind.glsl index bd03ee37..5837f61e 100644 --- a/src/AcDream.App/Rendering/Shaders/foliage_wind.glsl +++ b/src/AcDream.App/Rendering/Shaders/foliage_wind.glsl @@ -72,10 +72,15 @@ vec3 acdreamFoliageDisplace( { // Cutout (leaves): branch swing at ~1 Hz with a phase that runs up // the tree, plus a fast per-vertex flutter decorrelated by a hash of - // the vertex's own world XY so leaves on the same tree do not move - // in lockstep. + // the vertex's position RELATIVE TO ITS OWN INSTANCE ORIGIN (not + // absolute world XY — review fix round A5: a landblock far from the + // world origin can put worldPos.xy in the tens of thousands, and + // sin() of a large fp32 argument loses precision fast, which showed + // up as visibly patterned/quantized flutter near far map corners) + // so leaves on the same tree do not move in lockstep. float branch = k * amp.y * s * sin(1.1 * t + ph + 2.0 * h); - float vh = fract(sin(dot(worldPos.xy, vec2(12.9898, 78.233))) * 43758.5453); + vec2 relativeXY = worldPos.xy - instanceOrigin.xy; + float vh = fract(sin(dot(relativeXY, vec2(12.9898, 78.233))) * 43758.5453); float flutter = h * amp.z * s * sin(6.0 * t + 7.0 * vh); d += dir * branch + perp * 0.35 * branch + vec2(cos(6.2832 * vh), sin(6.2832 * vh)) * flutter; diff --git a/src/AcDream.App/Rendering/Shaders/spv/directional_shadow_world_cutout.vert.spv b/src/AcDream.App/Rendering/Shaders/spv/directional_shadow_world_cutout.vert.spv index 573e5db9..11d90bba 100644 Binary files a/src/AcDream.App/Rendering/Shaders/spv/directional_shadow_world_cutout.vert.spv and b/src/AcDream.App/Rendering/Shaders/spv/directional_shadow_world_cutout.vert.spv differ diff --git a/src/AcDream.App/Rendering/Shaders/spv/directional_shadow_world_cutout_multiview.vert.spv b/src/AcDream.App/Rendering/Shaders/spv/directional_shadow_world_cutout_multiview.vert.spv index 6c6892f9..8e94e2ae 100644 Binary files a/src/AcDream.App/Rendering/Shaders/spv/directional_shadow_world_cutout_multiview.vert.spv and b/src/AcDream.App/Rendering/Shaders/spv/directional_shadow_world_cutout_multiview.vert.spv differ diff --git a/src/AcDream.App/Rendering/Shaders/spv/directional_shadow_world_opaque.vert.spv b/src/AcDream.App/Rendering/Shaders/spv/directional_shadow_world_opaque.vert.spv index a1250943..becff4c2 100644 Binary files a/src/AcDream.App/Rendering/Shaders/spv/directional_shadow_world_opaque.vert.spv and b/src/AcDream.App/Rendering/Shaders/spv/directional_shadow_world_opaque.vert.spv differ diff --git a/src/AcDream.App/Rendering/Shaders/spv/directional_shadow_world_opaque_multiview.vert.spv b/src/AcDream.App/Rendering/Shaders/spv/directional_shadow_world_opaque_multiview.vert.spv index ea2d05cd..c4ffe953 100644 Binary files a/src/AcDream.App/Rendering/Shaders/spv/directional_shadow_world_opaque_multiview.vert.spv and b/src/AcDream.App/Rendering/Shaders/spv/directional_shadow_world_opaque_multiview.vert.spv differ diff --git a/src/AcDream.App/Rendering/Shaders/spv/mesh_atmospheric.vert.spv b/src/AcDream.App/Rendering/Shaders/spv/mesh_atmospheric.vert.spv index 31404c4f..3cc72ed5 100644 Binary files a/src/AcDream.App/Rendering/Shaders/spv/mesh_atmospheric.vert.spv and b/src/AcDream.App/Rendering/Shaders/spv/mesh_atmospheric.vert.spv differ diff --git a/src/AcDream.App/Rendering/Shaders/spv/shaders.manifest.json b/src/AcDream.App/Rendering/Shaders/spv/shaders.manifest.json index e367da29..cd8b246a 100644 --- a/src/AcDream.App/Rendering/Shaders/spv/shaders.manifest.json +++ b/src/AcDream.App/Rendering/Shaders/spv/shaders.manifest.json @@ -151,7 +151,7 @@ "stages": [ { "stage": "vert", - "sourceSha256": "fe67e9bff0e528b08fa699f0be17d1c49660ad63ab34f7ac7dd31f79a2253006", + "sourceSha256": "ae6026d4bc04bc07e5c67c1dca6c3dfd16ad377104153808a2e11b60bbe492f7", "compiled": true }, { @@ -167,7 +167,7 @@ "stages": [ { "stage": "vert", - "sourceSha256": "2a155a7964ff15bd936394b8ffd3a331d5598e140ca36268759520bab86b1bdc", + "sourceSha256": "c26158346a1db03e305192a23b595d288ed27a3622b8c2b49cc2fe6b27550946", "compiled": true }, { @@ -183,7 +183,7 @@ "stages": [ { "stage": "vert", - "sourceSha256": "74b1aa64396dd74185d644de7e9f40626c9422f335da60deb7c606f2a45d92c2", + "sourceSha256": "c752d8f85e0aa4a5bd2b7cb8679276ba46850844362eef0efce885401021433b", "compiled": true }, { @@ -199,7 +199,7 @@ "stages": [ { "stage": "vert", - "sourceSha256": "f15425837f7a8f84337f4493989bcd7cb1af87f916f7678fc016947f68d26be5", + "sourceSha256": "e483aae19e35ad9bdd7226cb1604db33268f8ae4474d774dfa08d5ca681717ee", "compiled": true }, { @@ -215,7 +215,7 @@ "stages": [ { "stage": "vert", - "sourceSha256": "7e3e49483024145bb71929f715a68087380e5126e15b5bef2c1c5c7d5e2e3516", + "sourceSha256": "949a50149a6dd7f8f2b44fa035ab80a4227a71a1cbd4ec812bf451d1c2971db3", "compiled": true }, { diff --git a/src/AcDream.App/Rendering/Wb/CachedBatch.cs b/src/AcDream.App/Rendering/Wb/CachedBatch.cs index b4f219db..3cadb4b1 100644 --- a/src/AcDream.App/Rendering/Wb/CachedBatch.cs +++ b/src/AcDream.App/Rendering/Wb/CachedBatch.cs @@ -27,15 +27,7 @@ internal readonly record struct CachedBatch( Matrix4x4 RestPose, Vector3 LocalSortCenter = default, WbDrawDispatcher.InstanceGroup? Group = null, - long GroupRegistration = 0, - // Campaign VM VM6: the foliage-wind classification bits ClassifyBatches - // computed for this subset (0 for anything that isn't procedural-scenery - // foliage). Preserved here — not just on the live InstanceGroup — so a - // cache-hit replay that recreates an evicted InstanceGroup (see - // WbDrawDispatcher.ApplyCacheHitDirect's stale-registration path) - // re-stamps the correct flags instead of leaving a freshly-recreated - // group at its zero default. - uint FoliageFlags = 0u); + long GroupRegistration = 0); /// /// Immutable retail-picking descriptor for one static entity part. The cache diff --git a/src/AcDream.App/Rendering/Wb/FoliageWindClassification.cs b/src/AcDream.App/Rendering/Wb/FoliageWindClassification.cs index 1270410b..46f9a045 100644 --- a/src/AcDream.App/Rendering/Wb/FoliageWindClassification.cs +++ b/src/AcDream.App/Rendering/Wb/FoliageWindClassification.cs @@ -1,4 +1,5 @@ using AcDream.Core.Meshing; +using AcDream.Core.World; namespace AcDream.App.Rendering.Wb; @@ -29,8 +30,6 @@ internal static class FoliageWindClassification /// internal const uint TrunkFlag = 0x4u; - private const uint SceneryEntityIdBit = 0x8000_0000u; - /// /// Pure classification: no allocation, no scan. /// is expected to be computed once per GfxObj/Setup mesh and cached with @@ -53,10 +52,44 @@ internal static class FoliageWindClassification } /// - /// Bit 31 of the entity id — see ProceduralSceneryIdAllocator's - /// 0x8XXYYIII namespace. No other consumer decodes more than this - /// one bit. + /// Campaign VM VM6 review fix round (A4): a Setup composite's parts are + /// separate GfxObjs, each with its own independently-cached + /// ObjectRenderData.HasCutoutSubset — an opaque trunk part carries + /// no cutout batches of its own (the leaves are a DIFFERENT part), so + /// classifying it from only its own HasCutoutSubset never gives + /// it . Both the world-receiver and + /// directional-shadow-caster Setup-part walks call this ONCE per entity + /// per frame (not per batch) to OR every currently-resolved part's + /// HasCutoutSubset into one entity-scoped value before + /// classifying each part against it. Generic over the caller's own part + /// representation so it needs no dependency on ObjectRenderData + /// or the mesh adapter — the caller supplies + /// to look each part's value up however it already does. Short-circuits + /// on the first ; a part the caller cannot + /// currently resolve simply contributes nothing (the caller marks the + /// entity incomplete separately and reclassifies once every part loads). + /// + internal static bool ComputeEntityHasCutoutSubset( + IReadOnlyList setupParts, + Func hasCutoutSubset) + { + for (int i = 0; i < setupParts.Count; i++) + { + if (hasCutoutSubset(setupParts[i])) + return true; + } + return false; + } + + /// + /// The full top-nibble 0x8... test via + /// — NOT bit 31 + /// alone. Bit 31 alone also matches LandblockStaticEntityIdAllocator's + /// 0xC... ids (fences, gates, building shells) and the synthetic + /// render ids 0xDA11_D0xx / 0xFFFF_FF01 — using bit 31 + /// alone here would sway non-scenery objects that happen to have a + /// cutout subset. /// internal static bool IsProceduralScenery(uint entityId) => - (entityId & SceneryEntityIdBit) != 0u; + ProceduralSceneryIdAllocator.IsInNamespace(entityId); } diff --git a/src/AcDream.App/Rendering/Wb/GroupKey.cs b/src/AcDream.App/Rendering/Wb/GroupKey.cs index 3a79567f..8d01842c 100644 --- a/src/AcDream.App/Rendering/Wb/GroupKey.cs +++ b/src/AcDream.App/Rendering/Wb/GroupKey.cs @@ -22,6 +22,16 @@ namespace AcDream.App.Rendering.Wb; /// (CompareOpaqueSubmissionOrder / CompareTransparentSubmissionOrder), /// and the delayed-alpha path sorts by viewer distance then submission ordinal. /// The key reaches only equality, hashing, and the scene-digest fingerprints. +/// +/// Campaign VM VM6 review fix round: joined +/// the key so a mesh subset reachable from BOTH a procedural-scenery entity +/// and a non-scenery entity (same index range/texture/translucency/cull +/// mode) buckets into two DIFFERENT groups instead of coalescing into one +/// group whose InstanceGroup.FoliageFlags was last-writer-wins between +/// the two classifications — which flickered the shared group's flags +/// between frames and let the caster (keyed correctly from the start) and +/// the receiver (previously keyed without this field) disagree about the +/// same subset. /// internal readonly record struct GroupKey( uint FirstIndex, @@ -30,4 +40,5 @@ internal readonly record struct GroupKey( GpuTextureSlot TextureSlot, uint TextureLayer, TranslucencyKind Translucency, - CullMode CullMode = CullMode.CounterClockwise); + CullMode CullMode = CullMode.CounterClockwise, + uint FoliageFlags = 0u); diff --git a/src/AcDream.App/Rendering/Wb/WbDrawDispatcher.DirectionalShadowReceivers.cs b/src/AcDream.App/Rendering/Wb/WbDrawDispatcher.DirectionalShadowReceivers.cs index d86570fa..a7d66221 100644 --- a/src/AcDream.App/Rendering/Wb/WbDrawDispatcher.DirectionalShadowReceivers.cs +++ b/src/AcDream.App/Rendering/Wb/WbDrawDispatcher.DirectionalShadowReceivers.cs @@ -119,6 +119,22 @@ public sealed partial class WbDrawDispatcher binding.Buffer, binding.OffsetBytes, binding.SizeBytes); + // Campaign VM VM6 review fix round (A3): mesh_atmospheric.vert reads + // uAtmosphereClockWind/uAtmosphereWindAmplitude from set 3/binding 5 + // — bind it here explicitly with the EXACT buffer/offset the caster + // pass bound earlier this same frame (carried on + // DirectionalShadowFrameBinding.AtmosphericFrame), rather than + // depending on that earlier bind surviving un-reset until this pass + // runs. Unbound (the declared-pack / no-caster-this-frame case) is a + // no-op here exactly like the caster side. + if (binding.AtmosphericFrame.IsBound) + { + encoder.BindUniformBuffer( + GpuBindingModel.UniformAtmosphericFrame, + binding.AtmosphericFrame.Buffer!, + binding.AtmosphericFrame.OffsetBytes, + binding.AtmosphericFrame.SizeBytes); + } } private void DisposeDirectionalShadowReceiverPipelines() diff --git a/src/AcDream.App/Rendering/Wb/WbDrawDispatcher.DirectionalShadows.cs b/src/AcDream.App/Rendering/Wb/WbDrawDispatcher.DirectionalShadows.cs index fd00b9ba..885e8c3f 100644 --- a/src/AcDream.App/Rendering/Wb/WbDrawDispatcher.DirectionalShadows.cs +++ b/src/AcDream.App/Rendering/Wb/WbDrawDispatcher.DirectionalShadows.cs @@ -1024,6 +1024,17 @@ public sealed partial class WbDrawDispatcher if (renderData.IsSetup && renderData.SetupParts.Count > 0) { + // Campaign VM VM6 review fix round (A4) — see + // FoliageWindClassification.ComputeEntityHasCutoutSubset's + // doc comment. Same shared helper the world-receiver + // Setup-part loop uses, so the caster's trunk-part + // classification agrees with the receiver's. + bool entityHasCutoutSubset = FoliageWindClassification + .ComputeEntityHasCutoutSubset( + renderData.SetupParts, + part => _meshAdapter.TryGetRenderData(part.GfxObjId) + is { HasCutoutSubset: true }); + for (int setupPartIndex = 0; setupPartIndex < renderData.SetupParts.Count; setupPartIndex++) @@ -1069,7 +1080,8 @@ public sealed partial class WbDrawDispatcher in transformSource, ref batches, ref rejectedTransparent, - ref unresolvedCutoutTextures); + ref unresolvedCutoutTextures, + entityHasCutoutSubset); } } else @@ -1151,8 +1163,16 @@ public sealed partial class WbDrawDispatcher in DirectionalShadowTransformSource transformSource, ref int sourceBatches, ref int rejectedTransparent, - ref int unresolvedCutoutTextures) + ref int unresolvedCutoutTextures, + // Campaign VM VM6 review fix round (A4): the ENTITY/Setup-scoped OR + // of every resolved part's HasCutoutSubset — see the matching + // parameter on WbDrawDispatcher.ClassifyBatches (the world receiver) + // for why a Setup composite's own per-part HasCutoutSubset is not + // enough. Null (the default) means "use renderData.HasCutoutSubset + // directly", correct for the non-Setup flat-MeshRef caller below. + bool? entityHasCutoutSubsetOverride = null) { + bool entityHasCutoutSubset = entityHasCutoutSubsetOverride ?? renderData.HasCutoutSubset; for (int batchIndex = 0; batchIndex < renderData.Batches.Count; batchIndex++) @@ -1203,7 +1223,7 @@ public sealed partial class WbDrawDispatcher candidate.LocalEntityId, FoliageWindExclusions.Contains(meshRef.GfxObjId), batch.Translucency, - renderData.HasCutoutSubset); + entityHasCutoutSubset); _directionalShadowDraws.Add( batch.FirstIndex, diff --git a/src/AcDream.App/Rendering/Wb/WbDrawDispatcher.cs b/src/AcDream.App/Rendering/Wb/WbDrawDispatcher.cs index 55febcde..38f393e3 100644 --- a/src/AcDream.App/Rendering/Wb/WbDrawDispatcher.cs +++ b/src/AcDream.App/Rendering/Wb/WbDrawDispatcher.cs @@ -490,9 +490,9 @@ public sealed partial class WbDrawDispatcher : IDisposable /// material would otherwise classify them as foliage. Empty (no /// exclusions) when no atmospheric pack is active. Assigned every frame /// by AtmosphericPostProcessGraph.RenderDirectionalShadows from - /// the currently-selected pack's declaration — self-contained by - /// construction, so a pack switch or deactivation can never leave a - /// stale exclusion set applied. + /// the currently-selected pack's declaration, so a pack switch or + /// deactivation is reflected on the very next frame that runs the + /// assignment — not "never," just "no slower than one frame behind." /// public IReadOnlySet FoliageWindExclusions { get; set; } = System.Collections.Frozen.FrozenSet.Empty; @@ -1841,6 +1841,24 @@ public sealed partial class WbDrawDispatcher : IDisposable bool drewAny = false; if (renderData.IsSetup && renderData.SetupParts.Count > 0) { + // Campaign VM VM6 review fix round (A4): a Setup composite's + // parts are separate GfxObjs with their own independently + // cached ObjectRenderData/HasCutoutSubset — a tree's opaque + // trunk part carries no cutout batches of its own (the + // leaves are a DIFFERENT part), so classifying each part + // from only its own HasCutoutSubset would never give the + // trunk part the trunk flag. One bounded scan of THIS + // Setup's own (small, fixed-per-object) part list, once per + // entity per frame — not per batch, not per instance — ORs + // every currently-resolved part's HasCutoutSubset into one + // entity-scoped value classification uses below instead of + // each part's own. A part missing this frame contributes + // nothing; the entity is already marked incomplete by the + // loop below and reclassifies once every part has loaded. + bool entityHasCutoutSubset = FoliageWindClassification.ComputeEntityHasCutoutSubset( + renderData.SetupParts, + part => _meshAdapter.TryGetRenderData(part.GfxObjId) is { HasCutoutSubset: true }); + // #188: setupPartIndex is the SAME index space // TransparentPartHook.PartIndex addresses — retail's CPartArray // numbers parts by their ordinal position in the Setup's own @@ -1905,7 +1923,7 @@ public sealed partial class WbDrawDispatcher : IDisposable opacityMultiplier = 1f - translucencyValue; // CMaterial::SetTranslucencySimple 0x005396f0 } - if (!ClassifyBatches(partData, model, entity, meshRef, paletteIdentity, restPose, opacityMultiplier, collector)) + if (!ClassifyBatches(partData, model, entity, meshRef, paletteIdentity, restPose, opacityMultiplier, collector, entityHasCutoutSubset)) currentEntityIncomplete = true; _selectionSink?.AddVisiblePart( entity.ServerGuid, @@ -2378,7 +2396,8 @@ public sealed partial class WbDrawDispatcher : IDisposable g.TextureSlot, g.TextureLayer, g.Translucency, - g.CullMode); + g.CullMode, + g.FoliageFlags); private void ObserveCurrentDispatcherSubmission( int visibleInstanceCount, @@ -2996,15 +3015,19 @@ public sealed partial class WbDrawDispatcher : IDisposable internal static void ApplyCacheHit( EntityCacheEntry entry, Matrix4x4 entityWorld, - Action appendInstance) + Action appendInstance) { foreach (var cached in entry.Batches) { + // Campaign VM VM6 review fix round: cached.Key.FoliageFlags is + // now the ONLY source of truth for a replayed group's foliage + // classification (GetOrCreateInstanceGroup derives it from the + // key at creation) — no separate FoliageFlags field to drift + // out of sync with it. appendInstance( cached.Key, cached.RestPose * entityWorld, - cached.LocalSortCenter, - cached.FoliageFlags); + cached.LocalSortCenter); } } @@ -3033,7 +3056,7 @@ public sealed partial class WbDrawDispatcher : IDisposable GroupRegistration = group.Registration, }; } - AppendInstanceToGroup(group!, model, cached.LocalSortCenter, cached.FoliageFlags); + AppendInstanceToGroup(group!, model, cached.LocalSortCenter); } } @@ -3147,11 +3170,10 @@ public sealed partial class WbDrawDispatcher : IDisposable private void AppendInstanceToGroup( GroupKey key, Matrix4x4 model, - Vector3 localSortCenter, - uint foliageFlags) + Vector3 localSortCenter) { InstanceGroup grp = GetOrCreateInstanceGroup(key); - AppendInstanceToGroup(grp, model, localSortCenter, foliageFlags); + AppendInstanceToGroup(grp, model, localSortCenter); } private InstanceGroup GetOrCreateInstanceGroup(GroupKey key) @@ -3177,6 +3199,13 @@ public sealed partial class WbDrawDispatcher : IDisposable TextureLayer = key.TextureLayer, Translucency = key.Translucency, CullMode = key.CullMode, + // Campaign VM VM6 review fix round: FoliageFlags is now part of + // GroupKey (see GroupKey's doc comment), so it is set exactly + // once here, at group creation, from the SAME key that decides + // group identity — never re-stamped imperatively afterward, + // which is what let a shared group's classification flicker + // between whichever caller ran last. + FoliageFlags = key.FoliageFlags, Registration = _nextGroupRegistration++, LastUsedFrame = _groupFrame, }; @@ -3187,16 +3216,9 @@ public sealed partial class WbDrawDispatcher : IDisposable private void AppendInstanceToGroup( InstanceGroup grp, Matrix4x4 model, - Vector3 localSortCenter, - uint foliageFlags) + Vector3 localSortCenter) { grp.LastUsedFrame = _groupFrame; - // Campaign VM VM6: re-stamp on every append (idempotent — the value - // is a deterministic function of the mesh subset the group's key - // already identifies) so a stale-registration replay that recreates - // an evicted InstanceGroup (see ApplyCacheHitDirect above) never - // leaves it at its zero default. - grp.FoliageFlags = foliageFlags; grp.Matrices.Add(model); grp.LocalSortCenters.Add(localSortCenter); grp.SubmissionOrders.Add(_nextInstanceSubmissionOrder++); @@ -3337,8 +3359,18 @@ public sealed partial class WbDrawDispatcher : IDisposable PaletteCompositeIdentity paletteIdentity, Matrix4x4 restPose, float opacityMultiplier = 1.0f, - List? collector = null) + List? collector = null, + // Campaign VM VM6 review fix round (A4): HasCutoutSubset is a + // per-PART (per-GfxObj) fact — a Setup composite's parts are + // separate GfxObjs, so a tree's opaque trunk part's OWN + // renderData.HasCutoutSubset is false even though the SAME Setup's + // leaves part has one. The caller passes the ENTITY/Setup-scoped OR + // across every resolved part here; null (the default) means "use + // renderData.HasCutoutSubset directly", which is already correct for + // a non-Setup single-mesh entity (see the flat MeshRef call site). + bool? entityHasCutoutSubsetOverride = null) { + bool entityHasCutoutSubset = entityHasCutoutSubsetOverride ?? renderData.HasCutoutSubset; bool allTexturesReady = true; for (int batchIdx = 0; batchIdx < renderData.Batches.Count; batchIdx++) { @@ -3369,9 +3401,25 @@ public sealed partial class WbDrawDispatcher : IDisposable GpuTextureSlot texSlot = texture.Slot; uint texLayer = texture.Layer; + // Campaign VM VM6 review fix round: classify BEFORE constructing + // the key and fold the result INTO the key (rather than + // stamping it onto whatever group the key already resolves to). + // Classification is from the RAW (pre-#188-promotion) + // batch.Translucency — a mid-fade trunk is still a trunk, it + // just landed in the alpha-blend group instead of opaque. This + // is what keeps a scenery instance and a non-scenery instance + // of the identical mesh subset in two SEPARATE groups instead of + // coalescing into one group whose classification depends on + // whichever entity classified it last. + uint foliageFlags = FoliageWindClassification.Classify( + entity.LocalEntityId, + FoliageWindExclusions.Contains(meshRef.GfxObjId), + batch.Translucency, + entityHasCutoutSubset); var key = new GroupKey( batch.FirstIndex, (int)batch.BaseVertex, - batch.IndexCount, texSlot, texLayer, translucency, batch.CullMode); + batch.IndexCount, texSlot, texLayer, translucency, batch.CullMode, + foliageFlags); InstanceGroup grp = GetOrCreateInstanceGroup(key); grp.Matrices.Add(model); @@ -3381,23 +3429,13 @@ public sealed partial class WbDrawDispatcher : IDisposable AppendCurrentLightSet(grp); // Fix B — 8 ints per instance, parallel to Matrices grp.Opacities.Add(opacityMultiplier); // #188 — parallel to Matrices grp.SelectionLighting.Add(_currentEntitySelectionLighting); - // Campaign VM VM6: classify from the RAW (pre-#188-promotion) - // batch.Translucency — a mid-fade trunk is still a trunk, it - // just landed in the alpha-blend group instead of opaque. - uint foliageFlags = FoliageWindClassification.Classify( - entity.LocalEntityId, - FoliageWindExclusions.Contains(meshRef.GfxObjId), - batch.Translucency, - renderData.HasCutoutSubset); - grp.FoliageFlags = foliageFlags; collector?.Add(new CachedBatch( key, texSlot, restPose, renderData.SortCenter, grp, - grp.Registration, - foliageFlags)); + grp.Registration)); } return allTexturesReady; } @@ -3851,9 +3889,14 @@ public sealed partial class WbDrawDispatcher : IDisposable // into BatchData.flags alongside #226's bit 0 at BuildIndirectArrays. // Group-level, not per-instance, because BatchData is read once per // draw call (Batches[gl_DrawIDARB]) — every instance sharing one - // mesh-subset draw shares its classification. Set by ClassifyBatches - // on a fresh classification and re-stamped by AppendInstanceToGroup - // on every cache-hit replay (see CachedBatch.FoliageFlags). + // mesh-subset draw shares its classification. Review fix round: this + // is now set EXACTLY ONCE, in GetOrCreateInstanceGroup, from the + // owning GroupKey.FoliageFlags — never re-stamped imperatively after + // creation, so a shared group's classification can no longer + // flicker between whichever caller classified it last. A mesh + // subset reachable from both a scenery and a non-scenery entity now + // buckets into two distinct groups (distinct GroupKey.FoliageFlags) + // instead of coalescing into one. public uint FoliageFlags; public float SortDistance; // squared distance from camera to first instance, for opaque sort diff --git a/src/AcDream.Core/World/ProceduralSceneryIdAllocator.cs b/src/AcDream.Core/World/ProceduralSceneryIdAllocator.cs index 885e665d..a9c6e58a 100644 --- a/src/AcDream.Core/World/ProceduralSceneryIdAllocator.cs +++ b/src/AcDream.Core/World/ProceduralSceneryIdAllocator.cs @@ -4,11 +4,19 @@ namespace AcDream.Core.World; /// Allocates stable, collision-free ids for procedurally generated scenery. /// /// -/// The top nibble is fixed at 0x8, so bit 31 continues to identify -/// procedural scenery to every renderer and physics consumer. The remaining -/// 28 bits are X(8), Y(8), and a 12-bit per-landblock counter: -/// 0x8XXYYIII. No consumer decodes the former byte-aligned -/// 0x80XXYYII layout; they classify only on bit 31. +/// The top nibble is fixed at 0x8: tests +/// (id & 0xF0000000u) == 0x80000000u, the full nibble, not bit 31 +/// alone. Bit 31 alone is NOT a reliable procedural-scenery test — it is +/// also set by LandblockStaticEntityIdAllocator's 0xC... +/// namespace (top nibble 1100) and by the synthetic render ids +/// 0xDA11_D0xx (paperdoll) and 0xFFFF_FF01 (portal tunnel). +/// LandblockPhysicsPublisher.cs's isOutdoorMesh predicate +/// intentionally tests bit 31 alone — it wants "any of these broader +/// synthetic/outdoor namespaces", not specifically procedural scenery — but +/// that is the one deliberate exception; every OTHER consumer, including +/// foliage-wind classification, must use . The +/// remaining 28 bits are X(8), Y(8), and a 12-bit per-landblock counter: +/// 0x8XXYYIII. /// /// /// @@ -36,4 +44,13 @@ public static class ProceduralSceneryIdAllocator return Base(landblockX, landblockY) + counter++; } + + /// + /// The full top-nibble test — 0x8..., not bit 31 alone. Bit 31 + /// alone also matches LandblockStaticEntityIdAllocator's + /// 0xC... ids (top nibble 1100 also has bit 31 set) and + /// the synthetic render ids 0xDA11_D0xx / 0xFFFF_FF01. + /// + public static bool IsInNamespace(uint entityId) => + (entityId & 0xF0000000u) == 0x80000000u; } diff --git a/tests/AcDream.App.Tests/Rendering/DirectionalShadowGpuTests.cs b/tests/AcDream.App.Tests/Rendering/DirectionalShadowGpuTests.cs index 253f4b32..93375d0a 100644 --- a/tests/AcDream.App.Tests/Rendering/DirectionalShadowGpuTests.cs +++ b/tests/AcDream.App.Tests/Rendering/DirectionalShadowGpuTests.cs @@ -390,6 +390,83 @@ public sealed class DirectionalShadowGpuTests } } + /// + /// Campaign VM VM6 review fix round (A3): the receiver pass must not + /// depend on the caster pass's own AtmosphericFrame bind surviving + /// un-reset until the receiver pass runs later in the frame — it must + /// bind set 3/binding 5 itself, from the EXACT buffer/offset/size the + /// caster bound. This asserts the seam that makes that possible: + /// TryGetCurrentFrameBinding's DirectionalShadowFrameBinding.AtmosphericFrame + /// carries the identical buffer identity, offset, and size the caster's + /// own recorded GpuRecordedUniformBind calls used — the value + /// WbDrawDispatcher.BindDirectionalShadowReceiver reads to issue its own + /// bind. + /// + [Fact] + public void CasterFrameBindingCarriesTheExactAtmosphericFrameBufferForTheReceiverSeam() + { + using var device = new RecordingGpuDevice(); + using var renderer = new DirectionalSunShadowRenderer(device, DirectionalShadowPreset.Medium); + DirectionalShadowPreparedDraws world = CreateWorldDraws(device.DefaultTextureSlot); + DirectionalShadowTerrainPreparedDraws terrain = CreateTerrainDraws(); + using IGpuBuffer worldVertices = Buffer(device, "world-v", GpuBufferUsage.Vertex); + using IGpuBuffer worldIndices = Buffer(device, "world-i", GpuBufferUsage.Index); + using IGpuBuffer terrainVertices = Buffer(device, "terrain-v", GpuBufferUsage.Vertex); + using IGpuBuffer terrainIndices = Buffer(device, "terrain-i", GpuBufferUsage.Index); + var worldGeometry = new DirectionalShadowMeshGeometry(worldVertices, worldIndices); + var terrainGeometry = new DirectionalShadowTerrainGeometry(terrainVertices, terrainIndices); + var environment = new DirectionalShadowEnvironmentState( + DirectionalShadowGateReason.Enabled, + Vector3.Normalize(new Vector3(0.1f, 0.2f, 1f)), + 0.9f, + 0.75f, + 1.1f, + AuthoredCelestialShadowSourceKind.Sun, + SourceObjectIndex: -1, + SourceGfxObjId: 0); + using IGpuBuffer atmosphericBuffer = Buffer(device, "test-atmospheric-frame", GpuBufferUsage.Uniform); + var atmosphericFrame = new AtmosphericFrameBufferBinding( + atmosphericBuffer, + OffsetBytes: 64u, + SizeBytes: 192u); + + device.Clear(); + using IGpuFrame frame = device.BeginFrame(); + WorldTransformFrameSlice sharedTransforms = PublishSharedTransforms(frame, world.Transforms); + renderer.RenderPrepared( + frame, + environment, + Matrix4x4.Identity, + Matrix4x4.CreatePerspectiveFieldOfView(MathF.PI / 3f, 16f / 9f, 0.1f, 500f), + cameraNearMeters: 0.1f, + casterDepthPaddingMeters: 48f, + world, + terrain, + worldGeometry, + terrainGeometry, + sharedTransforms, + atmosphericFrame: atmosphericFrame); + + // The caster pass's OWN recorded bind used exactly this buffer/offset/size. + GpuRecordedUniformBind casterAtmosphericBind = Assert.Single( + device.OfKind() + .DistinctBy(call => (call.BufferName, call.OffsetBytes, call.SizeBytes)), + call => call.Binding == GpuBindingModel.UniformAtmosphericFrame); + Assert.Equal("test-atmospheric-frame", casterAtmosphericBind.BufferName); + Assert.Equal(64u, casterAtmosphericBind.OffsetBytes); + Assert.Equal(192u, casterAtmosphericBind.SizeBytes); + + // The receiver seam carries forward the IDENTICAL binding — this is + // what BindDirectionalShadowReceiver reads to bind set 3/binding 5 + // itself, rather than depending on the caster's bind surviving + // un-reset until the receiver pass runs. + Assert.True(renderer.TryGetCurrentFrameBinding(frame, out DirectionalShadowFrameBinding binding)); + Assert.True(binding.AtmosphericFrame.IsBound); + Assert.Same(atmosphericBuffer, binding.AtmosphericFrame.Buffer); + Assert.Equal(atmosphericFrame.OffsetBytes, binding.AtmosphericFrame.OffsetBytes); + Assert.Equal(atmosphericFrame.SizeBytes, binding.AtmosphericFrame.SizeBytes); + } + [Fact] public void StableTopology_ReusesRetainedCommandBuffersWithoutFrameRingCopies() { diff --git a/tests/AcDream.App.Tests/Rendering/Packs/FoliageWindModelTests.cs b/tests/AcDream.App.Tests/Rendering/Packs/FoliageWindModelTests.cs index f49562ac..f3601155 100644 --- a/tests/AcDream.App.Tests/Rendering/Packs/FoliageWindModelTests.cs +++ b/tests/AcDream.App.Tests/Rendering/Packs/FoliageWindModelTests.cs @@ -203,6 +203,89 @@ public sealed class FoliageWindModelTests Assert.NotEqual(reference, farAway); } + [Fact] + public void FlutterHashIsRelativeToInstanceOriginSoTranslatingTheWholeTreeDoesNotChangeIt() + { + // Review fix round A5: the per-vertex flutter hash is computed from + // worldPos.xy - instanceOrigin.xy (relative to the tree's own base), + // not absolute world XY. A tree near the world origin and the exact + // same tree translated tens of thousands of metres away (a far + // landblock corner, where fp32 sin() of a large ABSOLUTE coordinate + // loses precision and produced visibly patterned flutter pre-fix) + // must produce identical displacement for the identical local leaf + // offset. + // + // ph = dot(instanceOrigin.xy, (0.137, 0.291)) is intentionally + // instanceOrigin-dependent (it decorrelates neighbouring trees), so + // this test holds it constant by translating along the direction + // orthogonal to (0.137, 0.291) — (0.291, -0.137) — which leaves the + // dot product, and therefore every ph-driven term (lean, branch, + // gust envelope), unchanged while moving the absolute coordinates + // by 50,000 units. Only the flutter hash's input changes shape + // between the old (absolute) and new (relative) implementation, so + // this test would have failed under the pre-fix code. + var localOffset = new Vector3(3.7f, -2.1f, 4f); // fixed offset from trunk base to this leaf + var nearOrigin = InstanceOrigin; + var translation = new Vector3(0.291f, -0.137f, 0f) * 50_000f; + var farOrigin = nearOrigin + translation; + var clockWind = new Vector4(19.5f, 0.8f, 0.6f, 2.3f); + + Vector3 nearVertex = nearOrigin + localOffset; + Vector3 farVertex = farOrigin + localOffset; + + Vector3 nearResult = FoliageWindModel.Displace( + nearVertex, + nearOrigin, + FoliageWindClassification.CutoutFoliageFlag, + clockWind, + Amplitude); + Vector3 farResult = FoliageWindModel.Displace( + farVertex, + farOrigin, + FoliageWindClassification.CutoutFoliageFlag, + clockWind, + Amplitude); + + Vector3 nearDisplacement = nearResult - nearVertex; + Vector3 farDisplacement = farResult - farVertex; + + AssertApproximatelyEqual(nearDisplacement, farDisplacement, tolerance: 1e-3f); + } + + [Fact] + public void MidHeightCutoutDisplacementHasAPositiveFloorUnderStormWind() + { + // Review fix round: pin that a cutout leaf at mid-canopy height + // (h = 0.5) actually moves under Storm-strength wind (mean 1.00, + // gust 0.75 — FoliageWindByWeather's Storm row). Guards against a + // regression that silently zeroes the whole displacement formula + // (e.g. an accidental "no motion" early-out, a broken sign, or the + // A5 relative-XY change degenerating to a constant hash). + const float floorMetres = 0.01f; + var stormClockWind = new Vector4(0f, 1.00f, 0.75f, 0f); // mean/gust match Storm + Vector3 midHeightVertex = InstanceOrigin with + { + X = InstanceOrigin.X + 2.4f, + Y = InstanceOrigin.Y - 1.1f, + Z = InstanceOrigin.Z + (0.5f * Amplitude.W), // h = 0.5 + }; + + for (float t = 0f; t < 30f; t += 2.9f) + { + Vector3 result = FoliageWindModel.Displace( + midHeightVertex, + InstanceOrigin, + FoliageWindClassification.CutoutFoliageFlag, + stormClockWind with { X = t }, + Amplitude); + + float magnitude = (result - midHeightVertex).Length(); + Assert.True( + magnitude > floorMetres, + $"t={t}: displacement magnitude {magnitude} did not clear the {floorMetres} m floor"); + } + } + private static void AssertApproximatelyEqual( Vector3 expected, Vector3 actual, diff --git a/tests/AcDream.App.Tests/Rendering/Wb/DirectionalShadowPreparedDrawTests.cs b/tests/AcDream.App.Tests/Rendering/Wb/DirectionalShadowPreparedDrawTests.cs index 6dcdd461..681c7ac4 100644 --- a/tests/AcDream.App.Tests/Rendering/Wb/DirectionalShadowPreparedDrawTests.cs +++ b/tests/AcDream.App.Tests/Rendering/Wb/DirectionalShadowPreparedDrawTests.cs @@ -129,6 +129,106 @@ public sealed class DirectionalShadowPreparedDrawTests Assert.Equal(1, product.Stats.RejectedFadedParts); } + /// + /// Campaign VM VM6 review fix round (A2): two instances of the IDENTICAL + /// mesh subset (same index range, texture, material, cull mode) — one a + /// procedural-scenery entity (top nibble 0x8), one a + /// LandblockStaticEntityIdAllocator entity (top nibble 0xC) sharing the + /// SAME GfxObj — classify to different FoliageWindClassification flags + /// (0x2 cutout vs 0x0) via the identical Classify call the world + /// receiver uses, and must land as two SEPARATE prepared caster + /// batches/commands rather than coalescing into one. This is the + /// caster-side half of "casters and receivers agree by construction": + /// DirectionalShadowDrawKey (the caster's sort/group key, unlike the + /// pre-fix receiver GroupKey) already includes FoliageFlags, so this + /// pins that it stays correct. + /// + [Fact] + public void SameSubsetDifferentFoliageClassificationNeverCoalescesIntoOneCasterBatch() + { + const uint sceneryEntityId = 0x80010203u; // ProceduralSceneryIdAllocator + const uint landblockStaticEntityId = 0xC0010203u; // LandblockStaticEntityIdAllocator + uint sceneryFlags = FoliageWindClassification.Classify( + sceneryEntityId, + isExcluded: false, + TranslucencyKind.ClipMap, + meshHasCutoutSubset: true); + uint landblockStaticFlags = FoliageWindClassification.Classify( + landblockStaticEntityId, + isExcluded: false, + TranslucencyKind.ClipMap, + meshHasCutoutSubset: true); + Assert.Equal(FoliageWindClassification.CutoutFoliageFlag, sceneryFlags); + Assert.Equal(0u, landblockStaticFlags); + + var product = new DirectionalShadowPreparedDraws(); + RenderSceneGeneration generation = RenderSceneGeneration.FromRaw(1); + Assert.True(product.TryBegin(generation, 1, estimatedInstances: 2)); + Matrix4x4 sceneryTransform = Matrix4x4.CreateTranslation(1f, 2f, 3f); + Matrix4x4 landblockStaticTransform = Matrix4x4.CreateTranslation(9f, 8f, 7f); + GpuTextureSlot sharedTexture = new(42); + + product.Add( + firstIndex: 100, + baseVertex: 5, + indexCount: 12, + sharedTexture, + textureLayer: 0, + CullMode.CounterClockwise, + DirectionalShadowCasterMaterial.AlphaCutout, + in sceneryTransform, + sceneryFlags); + product.Add( + firstIndex: 100, + baseVertex: 5, + indexCount: 12, + sharedTexture, + textureLayer: 0, + CullMode.CounterClockwise, + DirectionalShadowCasterMaterial.AlphaCutout, + in landblockStaticTransform, + landblockStaticFlags); + product.Complete( + generation, + 1, + new DirectionalShadowPreparationStats( + SourceCasters: 2, + SourceMeshRefs: 2, + SourceParts: 2, + SourceBatches: 2, + PreparedInstances: 0, + PreparedOpaqueCommands: 0, + PreparedAlphaCutoutCommands: 0, + RejectedTransparentBatches: 0, + RejectedFadedParts: 0, + MissingMeshes: 0, + UnresolvedAlphaCutoutTextures: 0)); + + // Two distinct commands/batches, NOT one command with InstanceCount=2 + // — the identical geometry/texture/material would have coalesced + // pre-fix, since only entity-level classification (which the caster + // key did not carry before A2) tells them apart. + Assert.Equal(2, product.Commands.Length); + Assert.Equal(2, product.Batches.Length); + Assert.All(product.Commands.ToArray(), command => Assert.Equal(1u, command.InstanceCount)); + + int sceneryIndex = product.Batches.ToArray() + .ToList() + .FindIndex(batch => batch.FoliageFlags == FoliageWindClassification.CutoutFoliageFlag); + int landblockStaticIndex = product.Batches.ToArray() + .ToList() + .FindIndex(batch => batch.FoliageFlags == 0u); + Assert.True(sceneryIndex >= 0, "expected one prepared batch carrying the cutout foliage flag"); + Assert.True(landblockStaticIndex >= 0, "expected one prepared batch carrying zero foliage flags"); + Assert.NotEqual(sceneryIndex, landblockStaticIndex); + Assert.Equal( + sceneryTransform, + product.Transforms[(int)product.Commands[sceneryIndex].BaseInstance]); + Assert.Equal( + landblockStaticTransform, + product.Transforms[(int)product.Commands[landblockStaticIndex].BaseInstance]); + } + [Fact] public void SameCasterBuild_ReplaysWithoutReclassificationOrStorageGrowth() { diff --git a/tests/AcDream.App.Tests/Rendering/Wb/FoliageWindClassificationTests.cs b/tests/AcDream.App.Tests/Rendering/Wb/FoliageWindClassificationTests.cs index 9dc43f36..cedfc258 100644 --- a/tests/AcDream.App.Tests/Rendering/Wb/FoliageWindClassificationTests.cs +++ b/tests/AcDream.App.Tests/Rendering/Wb/FoliageWindClassificationTests.cs @@ -97,13 +97,150 @@ public sealed class FoliageWindClassificationTests Assert.Equal(0u, flags); } + // Review fix round: the classifier tests the full top-nibble 0x8000_0000, + // NOT bit 31 alone — bit 31 is also set by LandblockStaticEntityIdAllocator's + // 0xC... namespace (top nibble 1100) and by the synthetic render ids + // 0xDA11_D0xx (paperdoll) and 0xFFFF_FF01 (portal tunnel). The prior + // [InlineData(0xFFFFFFFFu, true)] case pinned exactly this bug (0xFFFFFFFFu + // has bit 31 set but is NOT a procedural-scenery id under the top-nibble + // rule) — it is corrected to false below. [Theory] - [InlineData(0x80000000u, true)] - [InlineData(0x7FFFFFFFu, false)] + [InlineData(0x80000000u, true)] // top nibble 0x8 exactly + [InlineData(0x8FFFFFFFu, true)] // top nibble 0x8, every other bit set + [InlineData(0x7FFFFFFFu, false)] // top nibble 0x7 — bit 31 clear [InlineData(0x00000000u, false)] - [InlineData(0xFFFFFFFFu, true)] - public void IsProceduralSceneryDecodesOnlyBit31(uint entityId, bool expected) + [InlineData(0xFFFFFFFFu, false)] // top nibble 0xF — bit 31 set, NOT procedural scenery + [InlineData(0xC0010203u, false)] // LandblockStaticEntityIdAllocator (top nibble 0xC) — fence/gate/building shell + [InlineData(0xDA11D012u, false)] // ChargenPreviewEntityBuilder/CreatureAppraisalPresentation synthetic doll id + [InlineData(0xFFFFFF01u, false)] // PortalTunnelPresentation synthetic id + public void IsProceduralSceneryDecodesTheFullTopNibbleNotJustBit31(uint entityId, bool expected) { Assert.Equal(expected, FoliageWindClassification.IsProceduralScenery(entityId)); } + + /// + /// Campaign VM VM6 review fix round (A2): GroupKey now carries + /// FoliageFlags, so two otherwise-identical mesh-subset keys — same + /// geometry/texture/translucency/cull mode — with different + /// classification results are UNEQUAL keys, which is what makes a + /// scenery instance and a non-scenery instance of the same GfxObj land + /// in two separate WbDrawDispatcher groups instead of coalescing into + /// one group whose flags depended on whichever entity classified it + /// last. Flags are derived from real entity ids through + /// — a procedural- + /// scenery id (0x8…, a tree's cutout leaf subset) versus a landblock- + /// static id (0xC…, e.g. a fence with a cutout mesh subset) — matching + /// the review's exact pairing and its 0x2/0x0 expected words, and the + /// same pairing DirectionalShadowPreparedDrawTests + /// .SameSubsetDifferentFoliageClassificationNeverCoalescesIntoOneCasterBatch + /// proves the caster keeps separate. + /// + [Fact] + public void GroupKeysWithDifferentFoliageFlagsAreNeverEqualEvenWithIdenticalGeometry() + { + const uint proceduralSceneryTreeId = 0x80010203u; // top nibble 0x8 + const uint landblockStaticFenceId = 0xC0010203u; // top nibble 0xC + + uint sceneryFlags = FoliageWindClassification.Classify( + proceduralSceneryTreeId, + isExcluded: false, + TranslucencyKind.ClipMap, + meshHasCutoutSubset: true); + uint landblockStaticFlags = FoliageWindClassification.Classify( + landblockStaticFenceId, + isExcluded: false, + TranslucencyKind.ClipMap, + meshHasCutoutSubset: true); + + Assert.Equal(FoliageWindClassification.CutoutFoliageFlag, sceneryFlags); // 0x2 + Assert.Equal(0u, landblockStaticFlags); // 0x0 + + var sceneryKey = new GroupKey( + FirstIndex: 100, + BaseVertex: 5, + IndexCount: 12, + TextureSlot: new AcDream.App.Rendering.Gpu.GpuTextureSlot(42), + TextureLayer: 0, + Translucency: TranslucencyKind.ClipMap, + CullMode: DatReaderWriter.Enums.CullMode.CounterClockwise, + FoliageFlags: sceneryFlags); + var landblockStaticKey = sceneryKey with { FoliageFlags = landblockStaticFlags }; + + // Two otherwise-identical mesh-subset keys, differing only by the + // entity-driven classification, are unequal — this is what makes + // WbDrawDispatcher.GetOrCreateInstanceGroup place the two instances + // in two separate InstanceGroups instead of coalescing them. + Assert.NotEqual(sceneryKey, landblockStaticKey); + } + + /// + /// Campaign VM VM6 review fix round (A4): ComputeEntityHasCutoutSubset + /// ORs across every part regardless of WHICH part carries the cutout + /// subset — this pins the "two-part Setup, part B has cutout" scenario + /// at the algorithm level, decoupled from ObjectRenderData/mesh-adapter + /// plumbing. + /// + [Theory] + [InlineData(new[] { false, false }, false)] + [InlineData(new[] { true, false }, true)] + [InlineData(new[] { false, true }, true)] // part B (index 1) has the cutout + [InlineData(new[] { true, true }, true)] + public void ComputeEntityHasCutoutSubsetOrsAcrossEveryPart(bool[] partHasCutout, bool expected) + { + bool result = FoliageWindClassification.ComputeEntityHasCutoutSubset( + partHasCutout, + static value => value); + + Assert.Equal(expected, result); + } + + [Fact] + public void ComputeEntityHasCutoutSubsetIsFalseForAnEmptyPartList() + { + Assert.False( + FoliageWindClassification.ComputeEntityHasCutoutSubset( + Array.Empty(), + static value => value)); + } + + /// + /// Campaign VM VM6 review fix round (A4), end to end at the + /// classification level: a two-part Setup — part A is an opaque trunk + /// with no cutout batches of its own, part B is the leaves (has a + /// cutout subset). Classifying part A's opaque subset from its OWN + /// (false) HasCutoutSubset never yields the trunk flag (the pre-fix + /// bug); classifying it from the entity-scoped OR that + /// ComputeEntityHasCutoutSubset computes across BOTH parts correctly + /// yields TrunkFlag. Both WbDrawDispatcher.ClassifyBatches (receiver) + /// and AddDirectionalShadowBatches (caster) now pass the entity-scoped + /// value here instead of the part's own. + /// + [Fact] + public void TwoPartSetupGivesTheOpaqueTrunkPartTheTrunkFlagWhenAnotherPartHasCutout() + { + const uint sceneryEntityId = 0x80010203u; + bool[] partAHasCutoutSubset = [false]; // the trunk part's OWN mesh has no cutout batches + bool[] setupPartsHasCutoutSubset = [false, true]; // part A (trunk), part B (leaves) + + bool partAOwnHasCutoutSubset = FoliageWindClassification.ComputeEntityHasCutoutSubset( + partAHasCutoutSubset, + static value => value); + bool entityScopedHasCutoutSubset = FoliageWindClassification.ComputeEntityHasCutoutSubset( + setupPartsHasCutoutSubset, + static value => value); + + uint flagsFromPartOwnValue = FoliageWindClassification.Classify( + sceneryEntityId, + isExcluded: false, + TranslucencyKind.Opaque, + partAOwnHasCutoutSubset); + uint flagsFromEntityScopedValue = FoliageWindClassification.Classify( + sceneryEntityId, + isExcluded: false, + TranslucencyKind.Opaque, + entityScopedHasCutoutSubset); + + Assert.Equal(0u, flagsFromPartOwnValue); // the pre-fix bug: no flag at all + Assert.Equal(FoliageWindClassification.TrunkFlag, flagsFromEntityScopedValue); + } } diff --git a/tests/AcDream.Core.Tests/Rendering/Wb/WbDrawDispatcherBucketingTests.cs b/tests/AcDream.Core.Tests/Rendering/Wb/WbDrawDispatcherBucketingTests.cs index 1849f8a7..f0740973 100644 --- a/tests/AcDream.Core.Tests/Rendering/Wb/WbDrawDispatcherBucketingTests.cs +++ b/tests/AcDream.Core.Tests/Rendering/Wb/WbDrawDispatcherBucketingTests.cs @@ -488,7 +488,7 @@ public sealed class WbDrawDispatcherBucketingTests // at the top of the per-entity loop body in Draw. var groups = new Dictionary>(); var sortCenters = new List(); - void AppendInstance(GroupKey k, Matrix4x4 m, Vector3 localSortCenter, uint foliageFlags) + void AppendInstance(GroupKey k, Matrix4x4 m, Vector3 localSortCenter) { if (!groups.TryGetValue(k, out var list)) { @@ -497,7 +497,6 @@ public sealed class WbDrawDispatcherBucketingTests } list.Add(m); sortCenters.Add(localSortCenter); - _ = foliageFlags; } Assert.True(cache.TryGet(EntityId, LandblockId, out var entryHit)); @@ -545,7 +544,7 @@ public sealed class WbDrawDispatcherBucketingTests WbDrawDispatcher.ApplyCacheHit( entry, Matrix4x4.Identity, - (_, _, center, _) => observedCenter = center); + (_, _, center) => observedCenter = center); Assert.Equal(authoredCenter, observedCenter); } @@ -788,7 +787,7 @@ public sealed class WbDrawDispatcherBucketingTests const uint EntityId = 100; const int MeshRefCount = 3; - void AppendInstance(GroupKey k, Matrix4x4 m, Vector3 localSortCenter, uint foliageFlags) + void AppendInstance(GroupKey k, Matrix4x4 m, Vector3 localSortCenter) { if (!groups.TryGetValue(k, out var list)) { @@ -796,7 +795,6 @@ public sealed class WbDrawDispatcherBucketingTests groups[k] = list; } list.Add(m); - _ = foliageFlags; } for (int partIdx = 0; partIdx < MeshRefCount; partIdx++)