diff --git a/docs/research/2026-06-23-fps-distance-degrade-handoff.md b/docs/research/2026-06-23-fps-distance-degrade-handoff.md new file mode 100644 index 00000000..7806ceae --- /dev/null +++ b/docs/research/2026-06-23-fps-distance-degrade-handoff.md @@ -0,0 +1,282 @@ +# Handoff — FPS in dense towns = MISSING DISTANCE-DEGRADE (port it) — 2026-06-23 + +**Branch:** `claude/thirsty-goldberg-51bb9b` (continue in THIS worktree: +`C:\Users\erikn\source\repos\acdream\.claude\worktrees\thirsty-goldberg-51bb9b`). +**Working tree:** clean. Everything below is committed on the branch. +**References / named-retail / WorldBuilder** live in the MAIN repo +`C:\Users\erikn\source\repos\acdream\` (read via absolute path; they are NOT in the worktree). + +> **Read with fresh eyes.** This session SHIPPED a real, verified fix (the `_datLock` +> contention that caused the 30↔200 *swing*) — keep it, do not revisit it. But that fix +> did NOT solve the user's actual pain: **sustained ~30 FPS in dense towns (Fort Tethana), +> and it changes with view direction.** That second symptom was then root-caused, end to +> end, to a different cause: **acdream has no distance-based LOD/degrade — it draws every +> object in the frustum at full detail at any distance.** Retail degrades then hides distant +> objects. **The next session's job: port retail's per-frame distance-degrade.** The data is +> already in our dat tables; only the per-frame distance decision is missing. + +--- + +## 0. TL;DR + +1. **DONE + committed (do not touch):** the `_datLock`-contention fix. The streaming worker + now pre-reads the terrain-apply's dats into a `PhysicsDatBundle` so `ApplyLoadedTerrainLocked` + makes zero `DatCollection` calls and its `lock(_datLock)` is removed. **Measured: apply + lock-wait 24 ms median / 88 ms p95 → 0.2 µs.** This killed the streaming-induced *spikes*. +2. **STILL OPEN (the real FPS pain):** sustained ~30 FPS in dense towns, **view-direction- + dependent** ⇒ GPU/render-bound. Root cause **verified**: no distance LOD/degrade; every + frustum-visible object (statics + the huge volume of procedural scenery) is drawn at full + detail regardless of distance. ~1,400 draw calls / ~24,000 instances per frame in dense town. +3. **THE FIX (next session):** port retail `CPhysicsPart::UpdateViewerDistance` → + `GfxObjDegradeInfo::get_degrade` — per-frame, per-object: pick a lower-detail degrade slot + by camera distance, and hide past max distance. **Render-pipeline change** (objects bake + their close-mesh at spawn today; the draw must start carrying degrade variants + selecting + per frame). It's a faithful port, not a redesign — but render changes have been reverted + here when rushed, so do it via the grep→pseudocode→port→verify workflow with a fresh head. + +--- + +## 1. What this session SHIPPED — the `_datLock` fix (committed; DO NOT REVERT) + +**Symptom it fixed:** the wild 30↔200 *swing* while moving/streaming/portal-hopping. + +**Root cause (measured + confirmed by the code's own comment):** `DatCollection` is not +thread-safe, so a single global `_datLock` serializes ALL `DatCollection.Get`. The streaming +worker holds that lock for the ENTIRE per-landblock build (`BuildLandblockForStreaming`, +`GameWindow.cs:~5910`; comment at `:~5885` literally predicted this and named the fix). The +update thread's `ApplyLoadedTerrain` also took `_datLock` (for its own dat reads), so it BLOCKED +on the worker — measured **24 ms median / 88 ms p95** of pure lock-wait. A 43 ms apply = a 23-fps +frame; idle = 200 fps. That asymmetry was invisible to the title-bar ms (apply runs in `OnUpdate`, +FPS counter is `OnRender`). + +**Fix (design A1, spec + plan committed):** the worker (already under `_datLock`) pre-reads the six +dats the apply needs into a `PhysicsDatBundle` carried on `LoadedLandblock`. The apply reads from +the bundle, makes zero `Get` calls, and its `lock(_datLock)` is deleted. Safe because `_datLock`'s +only cross-thread job is serializing `DatCollection`; the apply's other writes are update-thread-only +(`_physicsEngine`, `ShadowObjects`, renderer, `_worldState`, `_buildingRegistries`) or already +concurrent-safe (`PhysicsDataCache` is all `ConcurrentDictionary`). + +**Commits (newest→oldest, prior HEAD was `92e95be`):** +| SHA | What | +|---|---| +| `b3925f4` | chore(diag): [FRAME-DIAG] StreamingController counters (apparatus) | +| `536f1c0` | **fix(streaming): drop `_datLock` from the terrain apply** (the fix) | +| `81a5605` | refactor(apply): read dats from the bundle, not DatCollection | +| `4a99b55` | feat(streaming): worker pre-reads the apply's dats into the bundle | +| `3a0e349` | feat(streaming): `PhysicsDatBundle` on `LoadedLandblock` | +| `5ab5d39` | docs: implementation plan | +| `c715e55` | docs: design spec | + +**Verified (empirical, full session town-walk + 4 teleports):** `lockwait` = 0.1 µs median / +0.2–0.3 µs p95 on EVERY line (was 24 ms / 88 ms). apply town p95 37 ms → 11 ms. Build + 1568 +Core tests green. **The lock fix is correct and real. Keep it.** (It only addressed the *swing*, +not the dense-town floor — see §2.) + +**Files the fix touched:** +- `src/AcDream.Core/World/PhysicsDatBundle.cs` (NEW) — the bundle record; `.Empty` for far tier. + Note: `Environment` aliased to `DatEnvironment` (collides with `System.Environment`). +- `src/AcDream.Core/World/LoadedLandblock.cs` — added `PhysicsDatBundle? PhysicsDats = null` + (trailing default → back-compatible). +- `src/AcDream.App/Rendering/GameWindow.cs` — `BuildPhysicsDatBundle(landblockId, entities)` + (mirrors the apply's 6 read sites; called from `BuildLandblockForStreamingLocked` near return); + the 6 apply sites rewritten `_dats.Get(id)` → `datBundle.X[id]` via `TryGetValue`; the + `lock(_datLock)` removed from `ApplyLoadedTerrain`. +- `src/AcDream.App/Streaming/StreamingController.cs` — only diag counters (apparatus). +- Spec: `docs/superpowers/specs/2026-06-23-datlock-contention-fix-design.md`. +- Plan: `docs/superpowers/plans/2026-06-23-datlock-contention-fix.md` (Task 5 — apparatus strip + + spec "verified" mark — is NOT done; see §4). + +--- + +## 2. STILL OPEN — the REAL FPS pain (why Fort Tethana is still ~30 FPS) + +**User's report (the axiom):** "When I portaled in it was high, as soon as I enter the town it +drops, also depending on what direction I look." → portal-in over empty terrain = high; turn toward +the dense town = drop. + +**Diagnosis (locked):** FPS that changes with **view direction** can only be the cost of drawing +what's in the frustum — i.e. **GPU/render-bound**. This is NOT the apply, the streaming, or the lock +(those are view-independent). The lock fix cannot touch it. + +**Root cause (VERIFIED against source):** there is **no distance-based LOD or degrade** on entities +or scenery. The draw path culls by frustum-AABB only, then draws everything in view at full detail: +- `WbDrawDispatcher.WalkEntitiesInto` (`src/AcDream.App/Rendering/Wb/WbDrawDispatcher.cs:~744-747`): + per-entity cull is `FrustumCuller.IsAabbVisible(...)` — **no distance check**. +- `:~779-780`: for any in-frustum entity, **every `MeshRef` is emitted as an instance, every + frame**, regardless of distance. +- `src/AcDream.Core/Meshing/GfxObjDegradeResolver.cs` resolves each object to its CLOSE-detail mesh + (`Degrades[0]`) and its own comment (`~line 57-59`) says: *"We don't yet have distance-based LOD + plumbing, so this resolver always returns slot 0."* **We ported the degrade DATA, not the per-frame + distance DECISION.** (It already uses `Degrades[i].MaxDist` in `IsRuntimeHiddenMarker` for the #136 + "red cone" — so the per-slot distances ARE available to us.) +- Scenery (trees/bushes/rocks) is procedurally generated per landblock (`SceneryGenerator`), is the + BULK of dense-town entity count, and goes through the same no-distance path. +- (Co-factors, lower priority: MSAA 4× on the High preset amplifies fill-rate; 2-pass translucency + for foliage. Don't chase these first — the missing distance-degrade is the structural cause.) + +**Measurement caveat carried forward:** the existing `[WB-DIAG] gpu_us` timer brackets ONLY the +entity MDI dispatch (opaque+transparent) — it does NOT measure total GPU frame time (terrain shading, +sky, particles, MSAA resolve, fill-rate stalls). So "gpu_us≈400µs" is NOT the frame cost. **The next +session needs a real total-frame-time / GPU-frame-time meter** (swap-to-swap delta, or a glFinish- +bracketed full-frame timer) to (a) confirm GPU-bound and (b) prove the fix. Do not trust the existing +gpu_us as the frame budget. + +--- + +## 3. THE FIX (next session) — port retail's per-frame distance-degrade + +**Retail mechanism (named-retail decomp; addresses verified this session):** +- `CPhysicsObj::UpdateViewerDistance` (`0x0050f340` / `0x00510b30`) → `CPartArray::UpdateViewerDistance` + → per-part `CPhysicsPart::UpdateViewerDistance` (`0x0050e030`, named-retail line ~275517). Per + frame, per part: compute distance to viewer, call `get_degrade(distance / gfxobj_scale.z, ...)`. +- `GfxObjDegradeInfo::get_degrade` (`0x0051e4b0`, named-retail line ~293086). Algorithm: + - if `degrades_disabled` → slot 0. + - `Render::force_level` → debug override (skip). + - **`effective_dist = |distance| − Render::s_rDegradeDistance`** (the user's "Degrade Distance" + setting, a 0–100 slider; `Render_DegradeDistance`, named-retail line ~3270-3272). + - bias = `Render::auto_update_deg_mul ? Render::deg_mul : Render::s_rUserSuppliedDegradeBias`. + - walk degrade slots; each slot has **`ideal_dist`** and **`max_dist`**; threshold per slot = + `ideal_dist − (ideal_dist − max_dist) * bias`. Pick the slot whose band contains `effective_dist`; + past the last band, use the last slot (whose `gfxobj_id` may be **0 = hidden**). +- `GfxObjDegradeInfo::get_max_degrade_distance` (`0x0051e2d0`, line ~292918) — the hide-past-this distance. +- `CPhysicsPart::Draw` (`0x0050d7a0`) draws `gfxobj[deg_level]`. +- **READ THESE FULLY before porting** (this handoff only summarizes): `get_degrade` body at + named-retail `acclient_2013_pseudo_c.txt:293086+`, `UpdateViewerDistance` at `:275517+`. + Cross-check the degrade-slot struct fields (`ideal_dist`, `max_dist`, `degrade_mode`, `gfxobj_id`) + against `docs/research/named-retail/acclient.h` and acdream's `GfxObjDegradeInfo` / `GfxObjInfo` + dat structs (we already expose `Degrades[i].Id` + `Degrades[i].MaxDist`; confirm whether `ideal_dist` + is also exposed or must be added). + +**The render-pipeline change (the hard part — design it first):** today an object resolves to +`Degrades[0]` at SPAWN and bakes `MeshRef`s. Distance-degrade needs the **draw** to select the slot +**per frame** by camera distance. Options to brainstorm: +- (a) carry ALL degrade-slot GfxObj variants per part on the entity/`MeshRef`, and have + `WbDrawDispatcher` pick the slot per-frame (true retail; most work; per-frame `get_degrade` per part). +- (b) **hide-only first cut**: keep slot-0 mesh, but in `WalkEntitiesInto` skip emitting instances for + entities beyond their `get_max_degrade_distance` (scaled by the DegradeDistance setting). This is the + big, low-risk win (distant scenery/buildings stop drawing) WITHOUT per-frame mesh-swapping. Plumb a + per-entity max-draw-distance (read from the degrade table at spawn) + a per-entity camera-distance + check in the walk. **Recommended phase 1.** +- (c) full LOD-mesh selection (slot swap at intermediate distances) as phase 2 on top of (b). + +**Open design questions for the brainstorm:** +- Where to evaluate distance: per-entity (cheap, approximate) vs per-part (retail-exact)? Start + per-entity. +- The `Render_DegradeDistance` setting: pick the retail default; wire through `QualitySettings` / + `RuntimeOptions` (per CLAUDE.md rule 4). Don't hardcode a magic distance — derive from the degrade + table + the setting. +- Hysteresis/popping: retail's bands prevent thrash; a naive per-frame threshold can pop. Use the + ideal/max band, not a single cutoff. +- Animated entities currently BYPASS the per-entity cull (they're landblock-tracked). Decide whether + they also distance-degrade (retail does, but be careful with the player + nearby NPCs — they should + stay slot 0 / never hide). +- **Divergence register:** the "no distance-degrade" deviation is NOT yet a row in + `docs/architecture/retail-divergence-register.md`. Add the row when the deviation is formally + acknowledged, and DELETE it in the commit that lands the port (per the register rules). + +**Acceptance:** user confirms Fort Tethana / Holtburg FPS no longer drops to 30 when facing the dense +town. Prove it with the new total-frame-time meter (before/after) AND user eyes. NO "fixed" claim on a +sub-metric again (see §6). + +--- + +## 4. Apparatus — `[FRAME-DIAG]` (committed; needs strip; build a NEW meter for the next phase) + +- **`[FRAME-DIAG]`** is committed across `GameWindow.cs` (fields, `OnUpdate` flush, the apply timing + + cell/bsp/shadow/upload/lockwait sub-spans, the `MaybeFlushTerrainDiag` print) and + `StreamingController.cs` (the counters). Gated on `ACDREAM_WB_DIAG=1`, flushes every ~5 s on outdoor + frames. Line format: + `[FRAME-DIAG] apply_us=Xm/Yp95 lockwait=… [cell=… bsp=… shadow=…] (upl=…) entUpl_us=… applies_max/upd=N deferred=N forceReload=N(drop=N) esg=N spawn=N resident=N walked=N` +- **It measures the APPLY (update) side — now PROVEN fixed (lockwait→0).** It does NOT measure the + total render frame time, which is what the distance-degrade work needs. **Next phase: add a + total-frame-time + GPU-frame-time meter** (the missing measurement; §2 caveat). +- **STRIP all of `[FRAME-DIAG]` + any new meter** when the FPS work fully lands (mirrors `92e95be`). + Plan Task 5 (apparatus strip + spec "verified" mark) was deliberately NOT done so the before/after + stays measurable. + +--- + +## 5. Verified file:line map (carry forward; lines drift — match by symbol) + +| Thing | Location | +|---|---| +| Worker holds `_datLock` for full build (the old contention) | `GameWindow.cs:~5910` `BuildLandblockForStreaming`; comment `:~5885` | +| Apply wrapper (lock now REMOVED) | `GameWindow.cs:~6608` `ApplyLoadedTerrain` | +| Apply body (now dat-free; reads `datBundle`) | `GameWindow.cs:~6786` `ApplyLoadedTerrainLocked` | +| Bundle gather (worker) | `GameWindow.cs` `BuildPhysicsDatBundle` | +| **No-distance-cull (the open bug)** | `WbDrawDispatcher.cs:~744-747` (frustum only), `:~779-780` (emit all meshrefs) | +| **Degrade resolver stub ("always slot 0")** | `GfxObjDegradeResolver.cs:~57-59`; `IsRuntimeHiddenMarker` uses `MaxDist` | +| Entity store (drawn set) | `GpuWorldState.cs` `_flatEntities` / `LandblockEntries` / `Entities` | +| `[WB-DIAG]` entity-draw CPU+GPU timer (partial — entity MDI only) | `WbDrawDispatcher.cs:~301-329` | +| Retail `get_degrade` | named-retail `acclient_2013_pseudo_c.txt:293086` (`0x0051e4b0`) | +| Retail per-part `UpdateViewerDistance` | named-retail `:275517` (`0x0050e030`) | +| Retail `get_max_degrade_distance` | named-retail `:292918` (`0x0051e2d0`) | +| Retail `Render_DegradeDistance` setting (0–100) | named-retail `:3270-3272`, `:169441` | +| WorldBuilder reference (no distance LOD either — we must add it) | `references/WorldBuilder/.../SceneryRenderManager.cs`, `VisibilityManager.cs` (MAIN repo) | + +--- + +## 6. Repro + capture + +Launch (Release; user drives + watches FPS; user manages client lifecycle): +```powershell +$env:ACDREAM_DAT_DIR="$env:USERPROFILE\Documents\Asheron's Call"; $env:ACDREAM_LIVE="1" +$env:ACDREAM_TEST_HOST="127.0.0.1"; $env:ACDREAM_TEST_PORT="9000" +$env:ACDREAM_TEST_USER="notan"; $env:ACDREAM_TEST_PASS="MittSnus81!" +$env:ACDREAM_WB_DIAG="1" # turn OFF for the clean FPS number +dotnet run --project src\AcDream.App\AcDream.App.csproj --no-build -c Release 2>&1 | Tee-Object -FilePath "fps.log" +``` +Character `+Je` (`notan` / `MittSnus81!`). Repro: portal in (high FPS) → enter the dense town → **look +toward the town vs away** (FPS drops facing the dense geometry). Fort Tethana / Holtburg are the test +beds. Build Release for any FPS read (Debug is not representative). The old probe logs from this +session are in the worktree root (`frame-diag*.log`, `fps.log` — untracked; ignore/clean). + +--- + +## 7. DO-NOT-RETRY / lessons (read before you act) + +- **"lock-wait = 0" is NOT "FPS fixed."** This session declared the FPS fixed on the lockwait sub-metric + and was wrong — the user was still at 30 in Fort Tethana. **Never claim FPS fixed without measuring the + TOTAL frame time the user actually sees, plus user-eyes confirmation.** +- **Verify subagent claims against source.** Two Explore agents this session drifted line numbers and one + fabricated a "rehydrate replays the player" race that source refuted (`SelectGuidsToRehydrate` excludes + the player). Read the cited code yourself before designing on it. +- **Render changes get reverted when rushed/speculative** (AP-48 cull deleted `_lastSpawnByGuid` → missing + walls; the sky-fog shader edits). The distance-degrade port is retail-faithful, but do it via + grep→pseudocode→port→verify, brainstorm the draw-path approach first, and measure before/after. +- **The `_datLock` fix is correct — do not re-investigate or revert it.** lockwait 88ms→0 is proven. +- **The existing `[WB-DIAG] gpu_us` under-reports** (entity MDI only) — don't use it as the frame budget. +- **DatCollection is not thread-safe**; only the worker may `Get` off the update thread (under `_datLock`). + The apply is now dat-free — keep it that way (the bundle is how it gets dats). + +--- + +## 8. Other OPEN follow-ups (deferred this session — lower priority than §3) + +- **H3 — unbounded entity leak / GPU heat-up over hops.** `_entitiesByServerGuid` + `_lastSpawnByGuid` + never evict across teleports (`esg`/`spawn` climbed 1→528 over hops; ACE doesn't DeleteObject across + teleport). Inflates the in-frustum drawn set, so it COMPOUNDS the §3 cost. Fix = the SAFE render-only + eviction (drop far entities' render/GPU state, KEEP `_lastSpawnByGuid`, re-materialize on re-entry) — + NOT the naive AP-48 cull. Pruner: `RemoveLiveEntityByServerGuid` (`GameWindow.cs:~3831`, only caller = + DeleteObject handler + respawn de-dup). Distance-degrade (§3) may make this less urgent. +- **Invisible char on outdoor portal (intermittent).** VERIFIED mechanism: `ForceReloadWindow` (sky-arcs + fix `9945d46`) tears down the whole window on every outdoor teleport; the player is persistent-rescued + (`GpuWorldState.RemoveLandblock:~298`) → re-injected via `DrainRescued`/`AppendLiveEntity`, but lands in + `_pendingByLandblock` (not drawn) until its landblock re-loads. The fade can lift before the player is + flushed into the drawn set → invisible. (The Explore agent's "rehydrate replays player" theory was + WRONG — player is excluded from rehydrate at `LandblockEntityRehydrator.cs:79`.) Likely fix: gate the + teleport fade-lift on "player is in the drawn set," not just terrain residency. Needs a player-draw- + membership probe to pin "fade-too-early" vs "stuck-in-pending." + +--- + +## 9. References (read FIRST per the relevant thread) +- `docs/superpowers/specs/2026-06-23-datlock-contention-fix-design.md` — the shipped fix's design. +- `docs/research/2026-06-23-fps-gpu-churn-handoff.md` (MAIN repo) — the handoff that STARTED this session. +- `docs/research/2026-06-22-world-load-fps-rootcause-report.md` (MAIN repo) — prior FPS deep-dive. +- named-retail `docs/research/named-retail/acclient_2013_pseudo_c.txt` — `get_degrade` (:293086), + `UpdateViewerDistance` (:275517). `acclient.h` for the degrade structs. +- `references/WorldBuilder/` (MAIN repo) — scenery/object render path acdream extracted (no distance LOD; + read `docs/architecture/worldbuilder-inventory.md` first). +- `claude-memory/project_render_pipeline_digest.md` + `project_physics_collision_digest.md` (DO-NOT-RETRY tables). +- CLAUDE.md "Development workflow: grep named → decompile → verify → port" — MANDATORY for the distance-degrade port.