c407104ab9
Resolve the Fix D contradiction with decomp (workflow wf_f660eb88 + adversarial verify) + 4 live cdb captures. The D3D-FF model was the WRONG oracle: retail has TWO light systems — STATIC torches BAKE into wall vertices (calc_point_light, triple-clamped: range gate + per-channel min(scale*color,color) + per-vertex [0,1] from black), DYNAMIC lights go D3D hardware. The captured intensity=100 is the purple PORTAL (magenta, dynamic), not a wall torch. Ground truth: 38 static warm torches (orange (1,0.588,0.314)/cream, intensity=100, falloff 3-5) + 2 dynamic. acdream over-brightness = two confirmed bugs: D-1 mesh_modern.vert folds ambient+sun+torches into one UNCLAMPED accumulator (single frag clamp) -> warm blowout; D-2 EnvCellRenderer never binds SSBO 4/5 so the cell shell reads a leaked light set. Spec: D-1 in-shader clamp-split (clamp the torch sum on its own before ambient/sun); D-2 bind the shell's own per-cell light set (mirror WbDrawDispatcher); LightBake.cs is the C# conformance oracle. Adds the 4 reusable cdb capture scripts. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
211 lines
12 KiB
Markdown
211 lines
12 KiB
Markdown
# A7 Fix D — warm torch over-brightness on indoor walls (#140)
|
||
|
||
**Date:** 2026-06-18 **Milestone:** M1.5 (Indoor world feels right) → A7 lighting
|
||
**Status:** design approved (user pre-approved 2026-06-18); ready for implementation plan.
|
||
**Investigation source of truth:**
|
||
[`docs/research/2026-06-18-lighting-a7-fixABC-shipped-fixD-handoff.md`](../../research/2026-06-18-lighting-a7-fixABC-shipped-fixD-handoff.md)
|
||
(RESOLVED section) + `claude-memory/reference_retail_ambient_values.md`.
|
||
|
||
## Problem
|
||
|
||
The Holtburg meeting-hall walls (and outdoor objects near torches) blow out
|
||
**warm/bright** in acdream vs **dim** in retail. Fix A/B/C (shipped) did not touch this.
|
||
|
||
The handoff "contradiction" (D3D-FF math `color×100×N·L/d` says walls should go WHITE,
|
||
yet retail is DIM) is **resolved**: the D3D-FF hardware model is the **wrong oracle**
|
||
for these walls. Two SEPARATE retail light systems (Ghidra xrefs, unambiguous):
|
||
|
||
- **STATIC lights → CPU vertex BAKE**: `DrawEnvCell` (0x0059F170) →
|
||
`SetStaticLightingVertexColors` (0x0059CFE0) → `calc_point_light` (0x0059C8B0, its
|
||
SOLE caller). Wall torches are STATIC objects → baked into vertex colours.
|
||
- **DYNAMIC lights → D3D hardware FF**: `add_dynamic_light` → `config_hardware_light`
|
||
(0x0059AD30); `minimize_envcell_lighting` (0x0054C170) enables ONLY the dynamic
|
||
subset for a cell. The previously-captured `intensity=100` light is on THIS path.
|
||
|
||
`calc_point_light` is mathematically **bounded**: range gate `d < falloff×1.3`; the
|
||
decisive **per-channel cap `min(scale·color, color)`** (a torch adds at most its own
|
||
sub-1.0 colour, any intensity); caller sums from **BLACK** then clamps the sum to
|
||
`[0,1]` (no ambient/sun in the bake accumulator). White needs many in-range lights;
|
||
a hall has a handful, each warm-capped.
|
||
|
||
### Ground truth (live cdb, `tools/cdb/a7-fixd-*.cdb`; `Render::world_lights` @ 0x008672a0)
|
||
|
||
Holtburg: **38 static + 2 dynamic** lights.
|
||
|
||
| Light | path | type | intensity | falloff | colour (r,g,b) |
|
||
|---|---|---|---|---|---|
|
||
| viewer light | dynamic / HW | point | 2.25 | 10 | (1, 1, 1) white |
|
||
| **portal** | dynamic / HW | point | **100** | 6 | **(0.784, 0, 0.784) magenta** ← the captured "intensity=100"; NOT a wall torch |
|
||
| 38× wall torch | static / **bake** | point | 100 | 3–5 | **(1.0, 0.588, 0.314) orange** / (0.980, 0.843, 0.612) cream |
|
||
|
||
Torches carry `intensity=100` too, but the per-channel cap pins each to its warm
|
||
colour ⇒ retail walls go warm, never white.
|
||
|
||
## Root cause in acdream (both verified in source)
|
||
|
||
Two independent bugs, both touching the meeting-hall walls; this spec fixes both.
|
||
|
||
**D-1 (math, primary): unclamped accumulator folding ambient + sun + torches.**
|
||
[`mesh_modern.vert`](../../../src/AcDream.App/Rendering/Shaders/mesh_modern.vert)
|
||
`accumulateLights` starts `lit = uCellAmbient.xyz` (:184), adds sun (:196), adds each
|
||
capped torch (:206), returns UNCLAMPED (:208); the only clamp is `min(lit,1.0)` in
|
||
`mesh_modern.frag:92` after a lightning bump. The per-light cap (vert:180) is faithful.
|
||
But pouring ambient + sun + up-to-8 intensity-100 WARM torches into ONE bucket and
|
||
trimming only at the end overflows to warm-white. Retail clamps the torch sum on its
|
||
OWN (from black); ambient/sun are a separate material-lit term.
|
||
|
||
**D-2 (state, compounding): EnvCell shell SSBO binding leak.**
|
||
[`EnvCellRenderer.RenderModernMDIInternal`](../../../src/AcDream.App/Rendering/Wb/EnvCellRenderer.cs)
|
||
binds SSBO 0/1/2/3 only, NEVER **4** (`gLights`) or **5** (`instanceLightIdx`) — which
|
||
the shared `mesh_modern.vert` reads unconditionally (:204-206). Only `WbDrawDispatcher`
|
||
binds 4/5. Indoor `DrawInside` interleaves the two, so a cell shell reads whatever
|
||
LEAKED light set the last `WbDrawDispatcher` draw left bound (a different entity's
|
||
torches, wrong per-instance indices) ⇒ wrong/over-bright walls.
|
||
|
||
`LightBake.cs` (verbatim CPU port of the bake) exists but is UNWIRED (zero callers).
|
||
|
||
## Design
|
||
|
||
Decisions (user, 2026-06-18): **D-1 = small in-shader clamp split** (not a CPU bake);
|
||
**D-1 + D-2 land together**, single visual verification.
|
||
|
||
### D-1 — clamp the torch sum on its own (mirrors `SetStaticLightingVertexColors`)
|
||
|
||
In `mesh_modern.vert` `accumulateLights`, give point/spot lights their own accumulator,
|
||
saturate it to `[0,1]` BEFORE it joins ambient + sun. The per-light cap and
|
||
`pointContribution` are unchanged; the only new operation is one `min(pointAcc, 1.0)`.
|
||
|
||
```glsl
|
||
vec3 accumulateLights(vec3 N, vec3 worldPos, int instanceIndex) {
|
||
// ambient + sun = retail's material-lit term
|
||
vec3 lit = uCellAmbient.xyz;
|
||
int activeLights = int(uCellAmbient.w);
|
||
for (int i = 0; i < 8; ++i) {
|
||
if (i >= activeLights) break;
|
||
if (int(uLights[i].posAndKind.w) != 0) continue; // directional only
|
||
vec3 Ldir = -uLights[i].dirAndRange.xyz;
|
||
float ndl = max(0.0, dot(N, Ldir));
|
||
lit += uLights[i].colorAndIntensity.xyz * uLights[i].colorAndIntensity.w * ndl;
|
||
}
|
||
// point/spot torches: their OWN accumulator, clamped to [0,1] (retail baked emissive)
|
||
vec3 pointAcc = vec3(0.0);
|
||
int base = instanceIndex * 8;
|
||
for (int k = 0; k < 8; ++k) {
|
||
int gi = instanceLightIdx[base + k];
|
||
if (gi < 0) continue;
|
||
pointAcc += pointContribution(N, worldPos, gLights[gi]); // per-light cap unchanged
|
||
}
|
||
lit += min(pointAcc, vec3(1.0)); // <-- THE FIX
|
||
return lit; // frag still does final min(lit, 1.0)
|
||
}
|
||
```
|
||
|
||
Behaviour change is confined to surfaces whose torch sum currently exceeds 1.0 —
|
||
normally-lit surfaces are byte-identical (no regression). Shared by every mesh using
|
||
this shader (outdoor objects AND cell walls), matching the issue's scope.
|
||
`mesh_modern.frag:92`'s final `min(lit, 1.0)` stays as-is (it clamps the total to the
|
||
retail FF pixel clamp). The lightning bump (frag:89) is unaffected.
|
||
|
||
### D-2 — the EnvCell shell binds its OWN light set
|
||
|
||
`EnvCellRenderer` must own its lighting like `WbDrawDispatcher` does, instead of reading
|
||
leaked SSBO state. Mirror `WbDrawDispatcher`'s proven pattern
|
||
(`ComputeEntityLightSet`/`AppendCurrentLightSet`/`UploadGlobalLights`):
|
||
|
||
1. **Wire `LightManager` in** via `Initialize(...)` (alongside `_shader`). Self-contained
|
||
pass — per `feedback_render_self_contained_gl_state`, EnvCellRenderer already
|
||
re-uploads its own `uViewProjection`; it now also uploads/binds its own lights.
|
||
2. **Binding 4 (global lights):** upload `LightManager.PointSnapshot` itself, packed
|
||
identically to `WbDrawDispatcher.UploadGlobalLights` (the `GlobalLight` SSBO layout:
|
||
`posAndKind`, `dirAndRange`, `colorAndIntensity`, `coneAngleEtc`). Same snapshot →
|
||
same indices both renderers reference. `BuildPointLightSnapshot` is already called
|
||
once per frame before rendering. **Extract the packing into a shared helper** so the
|
||
two renderers cannot drift (a `GlobalLightPacker` in `AcDream.App/Rendering/Wb/` or a
|
||
static on the snapshot type) — do not copy-paste the struct layout.
|
||
3. **Binding 5 (per-instance light set):** per **cell** (keyed on `allInstances[i].CellId`),
|
||
compute the set ONCE with `LightManager.SelectForObject(snapshot, cellCenter,
|
||
cellRadius, set)` (camera-independent; cache per CellId, reuse for all that cell's
|
||
part-instances — like `WbDrawDispatcher` reuses one set per entity). Write the 8-int
|
||
set per instance into a new buffer parallel to `_gpuInstanceTransforms` (same shape
|
||
as `_clipSlotData`); bind at binding 5. On a no-lights frame, fill -1 (shader adds no
|
||
point light) and still bind a ≥1-element buffer so the SSBO is never unbound.
|
||
4. **Cell centre/radius:** world-space bounding sphere of the cell geometry — reuse the
|
||
cell's existing visibility bound (the BSP/AABB sphere already computed for culling).
|
||
The exact field is pinned during planning by reading the cell-storage structs in
|
||
`EnvCellRenderer` / `EnvCellLandblock`; fallback = centre from the cell-part transform
|
||
translation, radius from the cell vertex AABB. **This is the one detail to confirm
|
||
against code in the plan.**
|
||
|
||
Order independence: D-1 and D-2 are orthogonal (shader math vs buffer binding) and can
|
||
be implemented in either order, but ship together.
|
||
|
||
## Testing (TDD)
|
||
|
||
`LightBake.cs` already encodes the correct math: `PointContribution` = per-light capped
|
||
(matches `mesh_modern.vert` pointContribution line-for-line), `ComputeVertexColor` = sum
|
||
reaching point lights → clamp `[0,1]`, skip directional. The new shader `pointAcc` clamp
|
||
mirrors `ComputeVertexColor`'s final clamp exactly.
|
||
|
||
New conformance test in `tests/AcDream.Core.Tests/` (e.g. `LightBakeConformanceTests`):
|
||
|
||
- **Golden warm torch, bounded:** an orange `(1, 0.588, 0.314)` `intensity=100`
|
||
`falloff=4` (Range = 4×1.3 = 5.2 m) torch lighting a wall vertex (facing it) at
|
||
d = 1, 2, 3, 4, 5 m → result is warm (R ≥ G ≥ B, hue preserved) and **every channel
|
||
≤ 1.0** (never white); at d ≥ Range the contribution is 0 (range gate).
|
||
- **No-blowout under stacking:** 8 overlapping `intensity=100` near-white torches summed
|
||
via `ComputeVertexColor` → each channel clamps to ≤ 1.0 (the `[0,1]` saturate holds).
|
||
- **Hue preserved:** a single orange torch's bounded result keeps B < G < R (warm), not
|
||
desaturated toward white.
|
||
|
||
These pin the contract the shader must match. GLSL is not unit-testable in-process
|
||
(standard for this project per the render digest); the shader `pointContribution` +
|
||
`pointAcc` clamp are matched to `LightBake` by **line-for-line review** with the C#
|
||
oracle as the pinned reference (call it out in the implementation commit).
|
||
|
||
## Bookkeeping — divergence register
|
||
|
||
- **Correct stale row AP-35** (`docs/architecture/retail-divergence-register.md`): it
|
||
describes the point-light path as per-pixel `mesh_modern.frag:52` with the half-Lambert
|
||
wrap "NOT ported". Reality since Fix A (`aa94ced`): per-vertex Gouraud in
|
||
`mesh_modern.vert:163` WITH the wrap ported. Update the row to match; the D-1 clamp
|
||
makes the accumulator MORE faithful (no new deviation introduced).
|
||
- **EnvCell shell per-cell 8-light selection** (D-2) inherits Fix B's existing
|
||
per-object approximation (retail bakes per-VERTEX over the full static list; acdream
|
||
selects up to 8 per cell-sphere then gates per-vertex in-shader). Confirm Fix B's
|
||
register row covers EnvCell shells; extend that row if needed — do NOT add a
|
||
contradicting row.
|
||
|
||
## Files
|
||
|
||
- `src/AcDream.App/Rendering/Shaders/mesh_modern.vert` — D-1 clamp split.
|
||
- `src/AcDream.App/Rendering/Shaders/mesh_modern.frag` — verify final clamp stays correct
|
||
(expected no change).
|
||
- `src/AcDream.App/Rendering/Wb/EnvCellRenderer.cs` — D-2: `LightManager` ref, per-cell
|
||
light sets, bind SSBO 4 + 5, per-instance light-set buffer.
|
||
- `src/AcDream.App/Rendering/Wb/WbDrawDispatcher.cs` (+ a shared `GlobalLightPacker`) —
|
||
extract the binding-4 global-lights packing so both renderers share it.
|
||
- `src/AcDream.App/Rendering/GameWindow.cs` — wire `LightManager` into
|
||
`EnvCellRenderer.Initialize` (minimal).
|
||
- `tests/AcDream.Core.Tests/Lighting/LightBakeConformanceTests.cs` — new.
|
||
- `docs/architecture/retail-divergence-register.md` — AP-35 update.
|
||
|
||
## Acceptance criteria
|
||
|
||
- `dotnet build` green; `dotnet test` green including the new conformance test.
|
||
- Conformance test passes on the captured golden torch values (warm, bounded, hue-preserved).
|
||
- Shader `pointContribution` + new `pointAcc` clamp reviewed line-for-line against
|
||
`LightBake` (cited in the commit).
|
||
- AP-35 corrected; any D-2 register note reconciled with Fix B's row.
|
||
- **Visual (user):** outdoor objects near torches no longer blow out warm-white, and the
|
||
Holtburg meeting-hall walls render warm-but-dim like retail.
|
||
|
||
## Out of scope (explicit)
|
||
|
||
- **Do NOT port the D3D-FF hardware model** (`config_hardware_light`'s
|
||
`color×intensity`, `(0,1,0)=1/d`, `Range=falloff×1.5`) — it lights GfxObjs/dynamics,
|
||
not the baked walls. Wrong oracle (handoff warning stands).
|
||
- **Do NOT** wire the CPU vertex bake (`LightBake.cs` as the runtime path) — chosen
|
||
approach is the in-shader clamp split. `LightBake.cs` stays the test oracle.
|
||
- Sun handling on indoor walls is unchanged (kept in the material-lit term as today);
|
||
any "should indoor walls receive sun at all" refinement is a separate question.
|
||
- The purple portal is correct — do not touch it.
|