acdream/src/AcDream.App/Rendering/Wb/FoliageWindClassification.cs
Erik fccba8390d refactor(render): one group-creation seam, required foliage key field, first-frame wind snap (Campaign VM VM6 review 3)
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>
2026-08-23 03:03:56 +02:00

111 lines
5.2 KiB
C#

using AcDream.Core.Meshing;
using AcDream.Core.World;
namespace AcDream.App.Rendering.Wb;
/// <summary>
/// Campaign VM VM6: the two <c>BatchData.flags</c> bits that mark a world
/// draw batch as swaying foliage — bit 0 remains #226's built-mesh marker
/// (see <c>mesh_detail.frag</c>'s <c>vBatchFlags &amp; 1u</c> check) and is
/// never touched here. <c>mesh_atmospheric.vert</c> and the four
/// <c>directional_shadow_world_*.vert</c> caster shaders read these bits
/// through the shared <c>foliage_wind.glsl</c> include
/// (<c>acdreamFoliageDisplace</c>'s <c>batchFlags &amp; 0x6u</c> gate); the
/// retail <c>mesh_modern</c> pipelines never read them, so setting them does
/// not change pack-off output.
/// </summary>
internal static class FoliageWindClassification
{
/// <summary>
/// Bit 1: the subset is an alpha-cutout/ClipMap material AND its owning
/// entity is procedural scenery (see <see cref="Classify"/>). Leaves,
/// fronds, bushes, grass tufts.
/// </summary>
internal const uint CutoutFoliageFlag = 0x2u;
/// <summary>
/// Bit 2: the subset is opaque AND its owning entity is procedural
/// scenery AND that same entity owns at least one cutout subset — a
/// tree trunk or branch, not a rock (rocks have no cutout subset).
/// </summary>
internal const uint TrunkFlag = 0x4u;
/// <summary>
/// Pure classification: no allocation, no scan. <paramref name="meshHasCutoutSubset"/>
/// is expected to be computed once per GfxObj/Setup mesh and cached with
/// the mesh record (<see cref="AcDream.App.Rendering.Wb.ObjectRenderData.HasCutoutSubset"/>),
/// not recomputed per call.
/// </summary>
internal static uint Classify(
uint entityId,
bool isExcluded,
TranslucencyKind translucency,
bool meshHasCutoutSubset)
{
if (!IsProceduralScenery(entityId) || isExcluded)
return 0u;
if (translucency == TranslucencyKind.ClipMap)
return CutoutFoliageFlag;
if (translucency == TranslucencyKind.Opaque && meshHasCutoutSubset)
return TrunkFlag;
return 0u;
}
/// <summary>
/// Campaign VM VM6 review fix round (A4): a Setup composite's parts are
/// separate GfxObjs, each with its own independently-cached
/// <c>ObjectRenderData.HasCutoutSubset</c> — an opaque trunk part carries
/// no cutout batches of its own (the leaves are a DIFFERENT part), so
/// classifying it from only its own <c>HasCutoutSubset</c> never gives
/// it <see cref="TrunkFlag"/>. Both the world-receiver and
/// directional-shadow-caster Setup-part walks, and the packed production
/// classifier, call this ONCE per entity per frame (not per batch) to OR
/// every currently-resolved part's <c>HasCutoutSubset</c> 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 <c>ObjectRenderData</c> or the mesh adapter — the caller supplies
/// <paramref name="hasCutoutSubset"/> to look each part's value up
/// however it already does. Short-circuits on the first
/// <see langword="true"/>; a part the caller cannot currently resolve
/// simply contributes nothing (the caller marks the entity incomplete
/// separately and reclassifies once every part loads).
///
/// <para>Campaign VM VM6 review fix round 2 (F3), round 3 (N5): takes
/// the would-be-captured value (e.g. the mesh adapter) as an explicit
/// <typeparamref name="TContext"/> argument to a
/// <see langword="static"/> lambda instead of letting a caller's lambda
/// close over an instance field — a closure-capturing lambda would
/// allocate a fresh closure object AND delegate on every call, and this
/// runs once per Setup entity per frame from all three production
/// classifiers. A <see langword="static"/> lambda with no captures lets
/// the C# compiler cache a single delegate instance for the method's
/// lifetime instead. Round 3 deleted the single-generic predecessor of
/// this overload — every call site (production and test) now goes
/// through this one, so there is exactly one
/// <c>ComputeEntityHasCutoutSubset</c> to keep correct.</para>
/// </summary>
internal static bool ComputeEntityHasCutoutSubset<T, TContext>(
IReadOnlyList<T> setupParts,
TContext context,
Func<TContext, T, bool> hasCutoutSubset)
{
for (int i = 0; i < setupParts.Count; i++)
{
if (hasCutoutSubset(context, setupParts[i]))
return true;
}
return false;
}
/// <summary>
/// The full top-nibble <c>0x8...</c> test via
/// <see cref="ProceduralSceneryIdAllocator.IsInNamespace"/> — NOT bit 31
/// alone. Bit 31 alone also matches <c>LandblockStaticEntityIdAllocator</c>'s
/// <c>0xC...</c> ids (fences, gates, building shells) and the synthetic
/// render ids <c>0xDA11_D0xx</c> / <c>0xFFFF_FF01</c> — using bit 31
/// alone here would sway non-scenery objects that happen to have a
/// cutout subset.
/// </summary>
internal static bool IsProceduralScenery(uint entityId) =>
ProceduralSceneryIdAllocator.IsInNamespace(entityId);
}