diff --git a/docs/plans/2026-04-11-roadmap.md b/docs/plans/2026-04-11-roadmap.md index 6bad6c33..4f462e06 100644 --- a/docs/plans/2026-04-11-roadmap.md +++ b/docs/plans/2026-04-11-roadmap.md @@ -2072,6 +2072,8 @@ OpenGL ceiling; revisit macOS only if a supported graphics backend is chosen. **Future / unscheduled — Campaign AR:** the opt-in [Atmospheric Rendering / Shader Packs campaign](2026-08-21-atmospheric-rendering.md) makes moving authored sun-and-moon directional shadows from trees, monsters, players, and buildings its Tier-2 headline while preserving acdream's current retail-faithful renderer as the default and leaving physics, collision, gameplay, and network behavior unchanged; the project owner assigned Campaign AR on 2026-08-22 without displacing active M4 gameplay work. The [celestial source contract](../research/2026-08-22-dereth-celestial-shadow-sources.md) selects sun, dominant moon, then secondary moon by rendered direction while retaining AC's single authored directional-energy channel; sun rays and volumetrics remain sun-only. The previously referenced #268 + TS-8 package is complete and retired. Stage 1's automated correctness, performance, lifetime, locked-restore, Release, evidence, documentation, and project-owner live gates completed on 2026-08-22 after the opt-in exposure correction. The [Stage 2 connected and closeout gate](../research/2026-08-22-atmospheric-stage2-connected-gate.md) now closes every available machine-local ACE, dense A/B, long-lifetime, graphical-package, shader, Release, and complete-test row. The implemented two-client gate proved that ACE rejects two concurrent characters on the one available account and is ready for separate observer credentials without recording secrets. A distinct-account remote-player row, unavailable physical GPU classes, and final project-owner pack-off/pack-on acceptance remain external gates; Campaign AR is not yet declared shipped. +**Planned, owner-directed — Campaign VM (VisualMaster):** the [VisualMaster campaign](2026-08-22-visualmaster-campaign.md) closes the [Campaign AR review](../research/2026-08-22-campaign-ar-review.md) findings with evidence (pack-off pixel/perf invariance vs `6c79d35c`, the un-anchored detail fade, a linear-light post stack, truthful closeouts, volumetric jitter) and adds weather-driven foliage wind behind the render pack. Named by the owner 2026-08-22; awaiting the owner's goal. + --- ## Cross-cutting work tracked in parallel diff --git a/docs/plans/2026-08-22-visualmaster-campaign.md b/docs/plans/2026-08-22-visualmaster-campaign.md new file mode 100644 index 00000000..468d44c0 --- /dev/null +++ b/docs/plans/2026-08-22-visualmaster-campaign.md @@ -0,0 +1,382 @@ +# Campaign VM — VisualMaster + +**Date:** 2026-08-22 · **Status:** PLANNED — awaiting the owner's goal +**Phase id:** **Campaign VM** — named by the project owner on 2026-08-22 +**Branch:** `claude/git-sync-status-5fb1d2` (= main + Campaign AR, `c51b07ef`) +**Predecessor:** [Campaign AR](2026-08-21-atmospheric-rendering.md) and its +[independent review](../research/2026-08-22-campaign-ar-review.md) +**Scheduling:** owner-directed, like Campaign AR. Does not displace M4. + +## Goal + +Make Campaign AR **provably safe to merge** and then **finish the look**: +close every review finding with evidence rather than assertion, put the +Tier-1 post stack in linear light so its controls mean what they say, and add +the one cheap, high-impact enhancement still missing from the discussed +scope — **gentle, believable wind in Dereth's foliage**, driven by AC's own +authored weather, opt-in through the same render-pack contract, with the +default retail-faithful path untouched. + +Campaign VM is shipped when both sides of the final matrix pass: + +- **pack off:** pixel-identical to the pre-campaign build outside the two + intended parity changes (terrain normals, building detail), with production + CPU/GPU/allocation within the pre-campaign noise envelope; and +- **pack on:** linear-light post stack, registered-or-removed detail fade, + moving foliage that the owner judges "real, not a screensaver", shadows + that move with the leaves, and all of it inside Campaign AR's declared + budgets. + +## Operating model + +The launcher/AR cadence, unchanged: + +- **Fable** plans, verifies claims against source/decomp, writes the + closeouts, and drives the work order. Nothing in a closeout may claim more + than its artifact shows — every "passes" names the command and the lane. +- **Sonnet** implements each slice against the pinned contract below. +- **Opus** reviews every slice with two lenses (architectural / retail- + faithful) before its closeout; findings go through a fix round and a narrow + re-review. +- **The owner** is stopped for exactly three things: the cdb read on the + retail client (VM2), the visual gates (VM3, VM6, VM7), and the merge. + +Rules binding on every slice: no workarounds (CLAUDE.md), no guessed +AC-specific constants — a number without a decomp/DAT anchor gets a register +row in the same commit — and the pack-off path is the oracle: any slice that +moves a pack-off pixel outside its declared mask is a bug, not a tuning. + +## Slice ledger + +| Slice | Closes | Gate | +|---|---|---| +| VM0 | F1 — default-path invariance | automated + Fable report | +| VM1 | F2 — invented detail fade | automated | +| VM2 | F3 — which retail detail path ran | **owner action** (launch retail) | +| VM3 | F4 — linear-light post stack | automated + **owner visual** | +| VM4 | F5 — overclaiming docs, incl. the reviewer's own | docs | +| VM5 | F8 — volumetric banding; F7 filed | automated | +| VM6 | Foliage wind | automated + **owner visual** | +| VM7 | Closeout: full gates, register, roadmap, merge | **owner** | + +Order is VM0 → VM1 → VM4 → VM5 → VM3 → VM6 → VM2 (whenever the owner has +retail up) → VM7. VM0 goes first because everything after it is measured +against the baseline it establishes. + +--- + +## VM0 — Default-path invariance (F1) + +**Why:** the only "pre-campaign oracle" is a 2×2 synthetic, one-draw +recording-device fixture. `WbDrawDispatcher.Rhi.cs` was refactored underneath +the default path. Nobody has compared real pack-off pixels or production +performance against `6c79d35c`. + +**Implementation** + +1. Build `6c79d35c` in a throwaway worktree (Release). Capture with + `tools/run-offline-pixel-gate.ps1 -Out artifacts/vm0/base` — the tool's + own documented baseline mode. +2. Capture HEAD pack-off twice with the same tool and camera set: + `BuildingDetailTextures=false` (isolates the A2 normal change) and + `=true` (the shipped default). +3. Extend the tool's compare step with a **terrain mask** (same mechanism as + the existing `sky-mask.png`): pixels whose depth/material belong to + `terrain_modern` are excluded from the identity assertion and reported + separately. +4. Assert: HEAD(detail=false) vs base — **zero** differing pixels outside the + terrain mask; inside the mask, differences are reported as a histogram + (expected: low-amplitude shading deltas, no structural change). + HEAD(detail=true) vs HEAD(detail=false) — differences confined to + building/EnvCell surfaces (second mask from the detail replay's own + command set). +5. Production performance A/B, **no automation observer, no validation + layers, uncapped Release**, at the CLAUDE.md production-profile camera and + at pinned dense Arwic: base vs HEAD pack-off. Record CPU/GPU p50/p95/p99, + FPS, `alloc_kb p50`, GC counts. The pre-campaign references are + 519.7 FPS / 1.869 ms CPU p50 (profile) and ~3.0/4.9 ms CPU p50/p95 (dense + Caul, digest §render). Deltas must sit inside run-to-run noise; any + allocation growth on the pack-off path is a regression to fix in this + slice, not to file. +6. Add the masked comparison as a repeatable tool mode + (`-Baseline … -TerrainMask`) so VM3/VM6 can re-run it. + +**Acceptance:** the two pixel assertions in step 4 hold; the perf A/B shows +no default-path regression; the Fable report records the exact commands, +commits and artifact paths. If a regression is found, it is fixed here and +the report names the cause — no "accepted difference" without a diff that +explains it. + +## VM1 — Remove the invented detail fade (F2) + +**Why:** `RetailDetailTextureContract.FullDetailDistanceMetres = 10` / +`ZeroDetailDistanceMetres = 50` have no retail anchor. Retail's +`DrawBuilding`/`DrawEnvCell`/`RenderMeshSubset`/`SetDetailSurfaceInternal` +carry no distance term; attenuation is the LINEAR mip chain. + +**Decision (Fable, retail-faithful rule):** remove the ramp. Mip averaging +already converges the live category texture to its 1.033 mean factor. + +**Implementation** + +- Delete the two constants and `vDetailFade` from `mesh_detail.vert/.frag` + (`mesh_detail.vert:83`); the fragment outputs `detail.rgba` unscaled. +- Confirm the detail texture is uploaded with a full mip chain and sampled + LINEAR/LINEAR/LINEAR, WRAP — that *is* retail's attenuation; test it. +- Rename `RetailDetailTextureContract` members so nothing un-anchored is + labelled retail; keep the blend-factor helper and its tests. +- Update the #226 pseudocode note: strike the "10 m / 50 m" lines, add the + decomp citations above, and state that attenuation is mip-driven. +- If the owner later wants a ramp back, it returns as a **pack setting** + (enhancement), never on the default path. + +**Acceptance:** no reference to a distance fade remains in src or the note; +VM0's masked comparison re-run shows the only change is on detail surfaces; +at >50 m a building reads the same as at 60 m (mips), not a hard step. + +## VM2 — Which detail path did retail hardware run? (F3, owner-gated) + +**Why:** the port is of the two-pass framebuffer fallback. With +`m_caps.bCanDoSinglePassDetailing`, retail used stage-1 texture ops +(`PREMODULATE` + `BLENDCURRENTALPHA` — a lerp). The cap needs +`D3DTEXOPCAPS_PREMODULATE` (0x0059f6c6), which consumer drivers rarely +exposed — probable, not proven. + +**Implementation:** one cdb script (`tools/cdb/vm2-detail-caps.cdb`): attach +to the PDB-paired `acclient.exe`, `dt acclient!RenderDevice::render_device` +→ `m_caps.bCanDoSinglePassDetailing`, `m_caps.bTexOpDotProduct3`, and the raw +`m_D3DCaps.TextureOpCaps`; `qd`. Record the answer in the #226 note and the +review doc. + +- `bCanDoSinglePassDetailing == 0` → the port is the path players saw; close. +- `== 1` → file the single-pass stage math as the correct target, register + the current blend as a bounded divergence, and schedule the re-port as its + own slice (it is a fragment-shader change only). + +**Acceptance:** the value is recorded with the binary GUID and the date. + +## VM3 — Linear-light post stack (F4) + +**Why:** bloom threshold, ACES, Rec.709 luma, saturation and the 0.5 +contrast pivot all assume linear light; they receive gamma-encoded retail +colours and the output is never re-encoded. `exposure = 0.80` is the +compensation. + +**Design** + +- The main-world intermediate stays gamma-encoded `Rgba16Float` — the world + pass and its alpha blending are retail's and must not change. +- Every pack read of world colour decodes once: `acdreamDecode(c) = pow(c, 2.2)` + (a named function in `atmospheric_common.glsl`; 2.2 is the retail-era + display assumption, documented as such — not sRGB piecewise, which would + imply a precision the source never had). +- Bloom chain, sun-ray composite and volumetric composite operate in linear. +- `atmospheric_filmic.frag`: exposure → ACES → grade → vignette in linear, + then `acdreamEncode(c) = pow(c, 1/2.2)` to the UNORM swapchain. +- Defaults re-tuned so the **neutral** preset reproduces the pack-off image + within the VM0 tolerance when every effect is at its neutral value + (exposure 1.0, bloom 0, tonemap mix 0, saturation 1, contrast 1, + vignette 0) — the existing "every effect can be set to neutral" acceptance + now actually holds numerically, and is asserted by a test. +- Opinionated defaults (what the owner accepted at 0.80 exposure) are + re-derived in linear and presented for the visual gate; expect exposure to + return to ~1.0 and the bloom threshold to move up. + +**Acceptance:** neutral preset ≡ pack-off within tolerance (new automated +test on the recording RHI plus VM0's masked tool on a real capture); Stage-1 +luminance table re-captured; owner visual gate: "same look as accepted, no +clipping, highlights roll off". Budget unchanged (two `pow` per pixel). + +## VM4 — Truthful documents (F5) + +Correct, in the AR plan and its reports, the four sentences the review +named: the 2×2 oracle described as a production pin; the turning-hitch +conclusion drawn under the automation observer; the "zero skips" headline +(say "hermetic lanes"); and the Track A premise that `TerrainUtils.GetNormal` +produced a faceted look. Also correct the reviewer's own findings doc §4 +(`2026-08-21-terrain-and-atmospheric-rendering-findings.md`): that function +orients scenery; the render normals were already smooth; A2 replaced +central-difference with retail's split-aware incident-face average. +Add the VM0 numbers as the new baseline table. Docs only; no code. + +## VM5 — Volumetric jitter, and file the shared transform buffer (F8, F7) + +- `atmospheric_volumetric.frag`: interleaved-gradient-noise offset per pixel + on the march start (`fract(52.9829189 * fract(dot(gl_FragCoord.xy, + vec2(0.06711056, 0.00583715))))`), plus the existing quarter/half-res + upsample. Automated: a fixed-camera capture's step-banding metric (row + autocorrelation at the step period) drops below the current value; + GPU delta < 0.02 ms. +- File issue: the shadow pass uploads a second transform buffer; a future + GPU-culling step should bind the main pass's instance SSBO. Not built here. + +## VM6 — Foliage wind (the feature) + +### What "real movement" means here + +Trees do not wave like flags. Real foliage has **three motions at three +speeds**: the whole tree leans slowly with the mean wind (seconds), branches +swing at their own natural frequency (about one second), and leaves flutter +fast and independently (fractions of a second). Gusts come and go over tens +of seconds. Neighbouring trees are out of step. Trunk bases do not move. +Anything that ignores one of these reads as "screensaver" — which is what +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. +- **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). +- **Opaque subsets of a candidate that also owns cutout subsets** (trunks, + branches) receive the slow lean only, scaled by height, never the flutter. +- A pack may **exclude** object ids (`FoliageExclusions`, a list in the + 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 +`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 +`mesh_modern` pipelines never read it. + +### The motion (vertex shader, pack variant only) + +Applied in `mesh_atmospheric.vert` and in all six +`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). + +Inputs: instance world origin `o` (translation column of +`Instances[i].transform`), vertex world position `p`, `time` (from +`uCameraAndTime.w`, already in the vertex stage), and the pack's wind block +(new `uFoliageWind` in `atmospheric_common.glsl`): + +``` +vec4 uFoliageWindDirection; // xy unit direction, z = mean strength [0..1], w = gust strength +vec4 uFoliageWindParams; // x = lean amplitude m, y = branch amplitude m, z = flutter amplitude m, w = max height m +``` + +All amplitudes are **metres at the canopy top**; they are pack settings with +bounded ranges, not shader constants. + +``` +h = clamp((p.z - o.z) / maxHeight, 0, 1) // 0 at the base, 1 at the top +k = h * h // bend grows with height²: bases stay put +ph = dot(o.xy, vec2(0.137, 0.291)) // per-tree phase from world position +g = 0.5 + 0.5 * sin(0.05 * t + ph) // gust envelope, ~20 s + + 0.25 * sin(0.13 * t + 1.7 * ph) +s = mean + gust * g // instantaneous strength + +lean = k * leanAmp * s * (0.8 + 0.2 * sin(0.35 * t + ph)) // slow lean +branch = k * branchAmp * s * sin(1.1 * t + ph + 2.0 * h) // ~1 Hz, phase runs up the tree +flutter = h * flutterAmp * s * sin(6.0 * t + 7.0 * fract(sin(dot(p.xy, vec2(12.9898, 78.233))) * 43758.5453)) + +d = dir * (lean + branch) + perp(dir) * 0.35 * branch + vec2(flutter) * normalize(vec2(cos, sin) of vertex hash) +p.xy += d +p.z -= 0.5 * dot(d, d) / max(h * maxHeight, 0.5) // bend shortens, it does not stretch +``` + +Opaque (trunk) subsets use `lean` only. Cutout subsets use all three. The +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. + +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: + +| Day group | mean | gust | +|---|---|---| +| Clear | 0.25 | 0.15 | +| Cloudy | 0.45 | 0.30 | +| Overcast | 0.60 | 0.35 | +| Rainy | 0.85 | 0.60 | + +plus a global **Wind** slider (0–2×) and an **Off** setting. Transitions +between day groups interpolate over the existing weather delta seconds +(`uAtmosphereWeather.z`), so a weather change never snaps. Wind direction is +a pack setting (default NE→SW, 225°); there is no authored retail wind +direction to read, and the register row says so. + +### Indoor, shadows, physics, picking + +- Indoors (`!IsOutdoor`) wind is zero — EnvCells have no scenery anyway. +- Shadow casters apply the identical displacement (shared include). +- Physics is untouched by construction: the collision BSP is the trunk and + nothing here touches Runtime or Core physics. The acceptance test asserts + the Runtime/physics diff is empty. +- World picking (`WorldPicker`) picks against the undisplaced mesh; a leaf + may be up to `leanAmp + branchAmp` metres from its pick volume. That is + accepted and registered (one row, "render-only foliage displacement"); + nobody picks leaves. + +### Budget and quality scaling + +- Cost: ~25 ALU per foliage vertex, zero CPU, zero submissions. Target: + < 0.05 ms GPU at 1080p dense foliage route; measured with the AR matrix + tooling (`tools/run-atmospheric-performance-matrix.ps1`). +- Low preset: lean + branch only (no flutter). Medium/High: all three. + +### 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 + 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 + 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. + +## VM7 — Closeout and merge + +- Full gates: `tools/run-release-gate.ps1` (hermetic lanes), the AR + reference matrix re-run for the changed presets, VM0's masked comparison + on the final binary, the connected lifecycle/reconnect route. +- Register: rows for the wind direction constant, the render-only + displacement vs picking, and whatever VM2 decides; TS-52 stays retired. +- Roadmap: move Campaign VM to shipped; AR's "final owner gate" rows that + VM0/VM3/VM6 satisfy are marked so in the AR plan. +- Memory: a `project_visualmaster_campaign.md` digest entry (the wind rule, + the linear-light decision, the VM0 oracle recipe). +- Owner: final pack-off / pack-on visual matrix, then merge to main. + +## What this campaign does NOT do + +- It does not touch the retail-faithful default path except through VM0's + evidence and VM1's removal of an un-anchored constant. +- It does not add normal maps, PBR, SSAO, water, or a second weather system. +- It does not move trunks' collision, walkability, or anything in Runtime. +- It does not animate landblock statics, weenies, grass decals that are not + scenery, or creatures. +- It does not rebuild the shadow transform buffer (filed, VM5). +- It does not re-port #226 to the single-pass path unless VM2 proves retail + ran it. + +## Proposed owner goal + +> **Campaign VM — VisualMaster.** Drive +> `docs/plans/2026-08-22-visualmaster-campaign.md` to shipped on branch +> `claude/git-sync-status-5fb1d2`: VM0 proves the pack-off path is +> pixel-identical to `6c79d35c` outside the terrain/detail masks with no +> production perf or allocation regression; VM1 removes the un-anchored +> detail fade; VM3 puts the post stack in linear light with a numerically +> neutral preset; VM4 makes every document truthful; VM5 de-bands the +> volumetrics; VM6 adds weather-driven foliage wind (scenery-only, cutout +> subsets, three motions, shadows follow) behind the render pack. Fable plans +> and verifies, Sonnet implements, Opus dual-lens reviews every slice. Stop +> me only for: the retail cdb read (VM2), the three visual gates (VM3, VM6, +> VM7), and the merge. Every closeout claim must name its command, lane and +> artifact. diff --git a/docs/research/2026-08-22-campaign-ar-review.md b/docs/research/2026-08-22-campaign-ar-review.md new file mode 100644 index 00000000..667aa03c --- /dev/null +++ b/docs/research/2026-08-22-campaign-ar-review.md @@ -0,0 +1,254 @@ +# Campaign AR — independent deep review + +**Date:** 2026-08-22 · **Reviewer:** Claude (Fable 5), report-only · **Scope:** +`main (6c79d35c) .. c51b07ef` — 378 files, +52,639 / −954 — codex's atmospheric +rendering + terrain fidelity campaign, plus the imported signal-sequencing +commit. Axes: architecture, technical correctness, performance, coverage of +the discussed scope. + +Every claim below was checked against the source, the decomp, or a test run +made during this review. Where codex's docs and the evidence disagree, the +evidence is cited. + +--- + +## Verdict + +A substantial, technically competent body of work that delivers almost +everything we discussed. The shadow implementation in particular is the +textbook recipe done properly. The architecture is sound and honours the +project's boundaries (Runtime/physics diff is empty, Core changes are additive +metadata, the built-in pack uses the same registry as external packs). + +The weaknesses are mostly in **evidence and provenance**, not code: + +1. default-path invariance is asserted on a toy oracle, never measured + against the pre-campaign build; +2. the #226 detail-fade constants are invented and presented as retail; +3. the Tier-1 post stack runs in gamma space; +4. several closeout sentences claim more than their artifacts prove. + +**Recommendation:** do not merge to main until the two F1 checks below have +run; decide F2; schedule F3/F4 as follow-ups; then the owner visual gate. + +--- + +## Coverage against the discussed scope + +| Discussed | Status | Notes | +|---|---|---| +| Dynamic shadows from trees, monsters, houses | **Delivered** | Camera-relative stable CSM, 2–4 cascades, opaque + alpha-cutout casters, animated casters, terrain + world receivers. Extended to moon shadows (doc records owner approval 2026-08-22). | +| Bloom, ACES tonemap, colour grade, vignette | **Delivered** | Gamma-space caveat (F4). | +| Screen-space sun rays | **Delivered** | Sky-depth occlusion mask, 48-tap radial; sun-only. | +| Volumetric shafts | **Delivered** | Reuses shadow depth; sun-only; no jitter (F8). | +| Rays composited before tonemap | **Delivered** | | +| Ray/shaft intensity from authored weather | **Delivered** | `AtmospherePolicyDeclaration` maps `activeDayGroup` + sun elevation in the pack descriptor. | +| Opt-in, retail default authoritative, via plugin API | **Delivered** | BCL-only `AcDream.Plugin.Abstractions.Rendering`, SDK validator, three samples. | +| #226 detail overlay | **Delivered, with caveats** | Buildings + EnvCells (correct per decomp), through the previously dead checkbox. See F2/F3. | +| Terrain vertex normals (A2) | **Delivered, verified** | Retail `calc_lighting` 0x00531700 port. See F5d for the premise correction. | +| Subdivision (A3) | Rejected | Agree. | +| Tree wind / sway | **Not present** | Raised after codex's brief; not a gap in their work. Still the cheapest item on the list. | + +--- + +## Findings, by severity + +### F1 — HIGH (evidence): default-path invariance is unproven + +The plan says `NoOpRenderPackProductionIntegrationTests` "pins the +pre-campaign pass list, pipeline set, draw/dispatch tuple, framebuffer +SHA-256". It does — for a **2×2 synthetic framebuffer, one draw call, one +pipeline, on a `RecordingGpuDevice` that does not rasterize** +(`tests/AcDream.App.Tests/Rendering/Packs/NoOpRenderPackProductionIntegrationTests.cs`). +It proves the controller arm adds nothing to a toy composition. It does not +prove the real default frame is unchanged. + +Meanwhile the hot default draw path was **refactored, not just extended**: +`WbDrawDispatcher.Rhi.cs` +512/−44 replaces the `MeshPipelineSet`, +`BindRingSection`/`WriteRingSection` instance and indirect-command handling +with variant-aware equivalents. `WbDrawDispatcher.cs` +120, `WorldSceneRenderer` ++127, `TerrainAtlas` +187. And the default path legitimately changed in two +places (A2 normals; the detail overlay, which is **on by default**). + +`tools/run-offline-pixel-gate.ps1` has a `-Baseline` mode designed for exactly +this ("capture a baseline at the parent commit, then gate the slice"). No +record shows it was run against a `6c79d35c` capture. The six physical +"retail rows" record zero pack *work* and performance, not pixel identity. + +**Ask:** +1. Capture `6c79d35c` pack-off with the pixel gate; capture HEAD pack-off with + `BuildingDetailTextures=false`; diff. Expected: differences confined to + terrain shading (A2). Anything outside a terrain mask is a regression. +2. A clean uncapped Release production A/B (no automation observer, no + validation layers) at the CLAUDE.md profile camera: CPU/GPU p50/p99 and + `alloc_kb` pre vs post. The connected dense-town numbers cannot serve (see + F5b). + +### F2 — HIGH (process): the #226 fade constants are invented and unregistered + +`RetailDetailTextureContract.FullDetailDistanceMetres = 10f` / +`ZeroDetailDistanceMetres = 50f` drive `mesh_detail.vert:83` +(`clamp((50 − z) / 40, 0, 1)`). The pseudocode note presents "full through +10 m; linear 10–50 m" as part of the retail contract. **Nothing in retail has +a distance gate on detail:** `RenderDeviceD3D::DrawBuilding` (0x0059f2a0), +`DrawEnvCell` (0x0059f170), `D3DPolyRender::RenderMeshSubset` (0x0059ca10) and +`ACRender::SetDetailSurfaceInternal` (0x006b6280) install surface, tiling, +WRAP, LINEAR×3 and the blend — no viewer-distance term. Retail's distance +attenuation is mip averaging (LINEAR mip filter), which for the live category +texture (mean factor 1.033) converges to near-neutral on its own. + +This is a guessed AC-specific constant in a class named `Retail…Contract`, +with TS-52 retired and no replacement register row — a double breach of the +workflow rules. **Ask:** either drop the ramp and rely on mips (retail), or +keep it and file the register row naming it an acdream adaptation. + +### F3 — MEDIUM (premise): #226 ports retail's two-pass fallback; which path real hardware ran is undetermined + +`SetDetailSurfaceInternal` sets the `DSTCOLOR + INVSRCALPHA` framebuffer blend +**only when `stage == 0`** (the two-pass fallback). With +`m_caps.bCanDoSinglePassDetailing`, `RenderMeshSubset` and `landPolyDraw` +call it with stage 1 and the combine is the texture-stage setup in +`D3DPolyRender::SetSurface` (0x0059c4d0): stage 0 alpha `PREMODULATE`, stage 1 +colour `BLENDCURRENTALPHA(texture, current)` — a **lerp toward the detail +colour by alpha**, not a multiply. That capability requires +`D3DTEXOPCAPS_BLENDCURRENTALPHA` and `D3DTEXOPCAPS_PREMODULATE` (0x0059f6c6). +PREMODULATE was rarely advertised by consumer drivers, so the fallback is +*probably* what players saw — but that is a recollection of driver caps, not +evidence. One cdb read settles it: +`dt acclient!RenderDevice::render_device->m_caps` on the PDB-paired binary. +Note this caveat also applies to the reviewer's own earlier "retail's detail +pass brightens" finding, which was derived from the same fallback path. + +### F4 — MEDIUM (technical): the Tier-1 post stack runs in gamma space + +The main world renders retail's fixed-function, gamma-encoded colours into +`Rgba16Float`. Nothing decodes them: bloom thresholds Rec.709 luma of +gamma values, `acesFitted` (Narkowicz) is applied to gamma values, saturation +and the 0.5 contrast pivot assume linear, and the result is written to the +UNORM swapchain without re-encoding (`atmospheric_filmic.frag`, +`atmospheric_bloom_downsample.frag`; no `pow`/sRGB anywhere in the +atmospheric shaders). The default `exposure = 0.80` is the compensation — ACES +maps gamma-0.5 to 0.62 at exposure 1.0, which is the "too bright" the owner +corrected live. + +It is an opt-in look and the owner accepted it, so this is not a blocker. +But the fix is cheap (decode `pow(c, 2.2)` where the world colour is read, +encode at the end) and would make threshold/exposure/contrast behave like +every other tool's controls. Schedule as a slice. + +### F5 — MEDIUM (docs): closeout claims exceed their evidence + +a. "Pins the pre-campaign … framebuffer SHA-256" — see F1. +b. "The user's turning hitch reproduces on the retail path" is drawn from + `connected-dense-town-20260822-133727` (retail/off: CPU p50 **27.8 ms**, + `alloc_kb p50 = 9,830`, 31 Gen0 GCs in 216 frames). The Slice H closeout + already documents that the connected automation observer "intentionally + allocates in proportion to scene size; 3.9–10.5 MiB/frame outdoor readings + are automation observer cost". Pre-campaign production dense Caul measured + 3.0/4.9 ms CPU p50/p95. The A/B is valid for the *Medium-minus-retail + increment* (~0.7–0.9 ms GPU); it says nothing about the owner's hitch or + about production default-path cost. +c. "14,928/14,928, zero skips" holds only under the hermetic lane filter. A + raw `dotnet test AcDream.slnx` shows 74 skips and 36 failures across five + assemblies; re-run in isolation with the repo filter, all five pass + (7,560 / 0). Fine — but the headline should say "hermetic lanes". +d. The Track A report repeats the findings doc's wrong premise that + `TerrainUtils.GetNormal` produced "the faceted look". That function only + orients scenery (`SceneryGenerator.cs:166`). The rendered mesh already had + smooth central-difference normals (Phase 3b). The real change — + central-difference → retail's split-aware incident-face average — is a + genuine, decomp-verified parity port (0x00531700: sum unit plane normals + per vertex id, normalise, `(0,0,1)` fallback), but a subtler visual change + than either document implies. The reviewer owns the original error. + +### F6 — LOW (architecture): the pack API is heavier than what it can express + +42 public types in `AcDream.Plugin.Abstractions.Rendering`. +`RenderPassSemantic` is the built-in pack's pass list (BloomDownsample, +BloomBlur×2, SunOcclusion, SunRays, VolumetricShafts, FilmicComposite) plus +`CustomFullscreen`. A third-party pack can swap shaders within the built-in +pipeline and add fullscreen passes; it cannot introduce a new pass kind. That +is a sensible v1 (Iris/OptiFine are fixed programs with named slots) — the +docs should describe it that way rather than as a general framework. The +SPIR-V validator living in the BCL-only abstractions assembly is unusual but +lets the SDK validator run without App/Vulkan references. The built-in pack +registers through `BufferedRenderPackRegistry.Register(descriptor, assets)` +exactly like externals — good. + +### F7 — LOW (perf/architecture): the shadow pass keeps its own transform buffer + +`DirectionalShadowTransformBufferSet` uploads a second matrix buffer. The +matrices are recomposed from the same `MeshRef.PartTransform × LocalToWorld` +through the same `WbDrawDispatcher.ComposePartWorldMatrix`, so there is no +second pose (constraint 5 honoured in spirit), but there is a second +composition and upload per animated part per frame, and this buffer is where +the 65,536-matrix ceiling bug lived. Measured cost is small (0.11–0.21 ms CPU +incremental). A future GPU-culling step will want one shared buffer. + +### F8 — LOW (quality) + +- `atmospheric_volumetric.frag` marches up to 64 steps with no per-pixel + jitter → visible banding at Low/Medium step counts. +- The sun-occlusion mask is "depth ≈ far plane" only; no disc/sky-radiance + weighting, so rays are uniform-coloured. +- `SkyDescLoader.ResolveSortCenter` swallows exceptions to `Vector3.Zero`; + acceptable for enhancement metadata, but note it against the silent-catch + rule. The other 26 added `catch` sites are cleanup-then-rethrow or + convert-to-failure-outcome, consistent with the fail-safe contract. + +--- + +## What is good (specifically) + +- **Receiver shader** (`directional_shadow_receiver.glsl`): bias in world + metres (constant + slope-scaled + normal offset), scaled per cascade by + texel density — the #129 NDC-bias trap is handled by construction; true + compare-then-interpolate PCF via `textureGather`; cascade blend band and + terminal reach fade in metres so FOV cannot move seams. +- **Cascade fitter**: bounding-sphere per split, radius quantised to 1/16 m, + centre snapped to the light-space texel grid — stable CSM done right. +- **Caster pass**: per cascade, one terrain MDI plus the opaque/cutout MDI + runs over the **full resident set**; no per-cascade CPU reculling, with + classification counters so the gate is enforced rather than hoped. + Alpha-cutout casters sample and discard. Low uses multiview. +- **Indoor gating**: `IsOutdoor = RenderSky && !CameraInsideCell`. +- **Lifecycle**: off-side candidate preparation, atomic activation, withdrawal + at a frame boundary, no-retry per registration, Auto with 180-sample + hysteresis and a visible reason string. Device loss is correctly treated as + terminal to the device lifetime. +- **Boundaries**: `git diff main..HEAD -- src/AcDream.Runtime src/*/Physics` + is empty; Core edits are additive (`TranslucencyFadeManager.Revision`, + `SkyObjectData.AuthoredSortCenter`); Headless references no pack types. +- **#226 blend**: `src = (d.rgb·f, d.a·f)` under `DSTCOLOR + INVSRCALPHA` gives + `dest × (1 + f·(d.rgb − d.a))` — exactly neutral at f=0, retail's measured + factor at f=1. The dead `BuildingDetailTextures` checkbox is now live + without a second option. +- **A2**: the decomp claim is correct and the port matches it (unit face + normals summed per shared vertex, per-landblock like retail). +- **Tests**: green under the repo's hermetic filter — 7,560/0 in the five + assemblies re-run in isolation, plus Runtime 1,818, Core 4,877, + UI.Abstractions 884, Content 160, MossTank 48, RenderPackValidator 30. + +--- + +## Performance summary + +Reference adapter (RX 9070 XT, 1080p capped, fixed dense 9,498-caster scene): +Low 0.108 ms CPU incr / 0.90 ms GPU incl / 40 MiB; Medium 0.116 / 1.00 / 74; +High 0.121 / 1.24 / 114; volumetric +0.19 ms GPU. Credible and inside the +declared budgets. Caveats: one high-end adapter; the integrated-AMD row proves +only safe fallback; the connected dense-town figures carry the observer tax +(F5b); the default path's own pre/post cost is unmeasured (F1). + +--- + +## Recommended order + +1. F1 — run both checks (pixel gate vs `6c79d35c`; clean production A/B). + Blocking for merge. +2. F2 — decide: drop the ramp (retail) or register it. Small. +3. F3 — one cdb read on the retail client. Small; also settles the + reviewer's earlier finding. +4. F4 — linear-light post stack as a follow-up slice. +5. F5 — correct the four sentences in the plan/reports. +6. Owner visual gate per the plan's own final-gate list; then ship.