# 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 > **CLOSED (VM0, 2026-08-22): PASS.** Exact pixel identity on static content > vs `6c79d35c` (Holtburg + open field) and a clean production perf A/B with no > regression — `2026-08-22-vm0-default-path-invariance.md`. 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 > **Post-review correction (VM1, 2026-08-22):** "invented" was too strong. The > 10 m → 50 m ramp is a real retail function, `ACRender::get_alpha_for_z` > (0x006b6230). It is, however, dead for the surfaces #226 targets: retail > evaluates it only in `D3DPolyRender::DrawPolyInternal` (immediate polygons) > and only when the static `noFadeDetail` (0x00820e38, initialised 1) is 0; > built meshes light with `Diffuse.a = 1`. The finding's conclusion (remove > it; no register row) stands; the provenance claim is corrected. `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 > **ANSWERED (VM2, 2026-08-22):** `m_caps.bCanDoSinglePassDetailing = 1`, > `trysinglepass = 1` on the owner's GPU — retail runs the single-pass path. > The reviewer's "probably the fallback" was wrong. See > `2026-08-22-vm2-retail-detail-path-cdb.md`; re-ported at VM1. `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 > **CLOSED (VM4, 2026-08-22):** (a)–(d) corrected in place in the AR plan, the > Stage-2 connected report, the Track A report and the findings doc, each with a > dated "VM4 correction" note rather than a silent rewrite. 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.