fix(render): Part 1 — camera boom convergence snap (kills the at-rest viewer-cell flicker trigger)
Port retail CameraManager::UpdateCamera's convergence snap (0x00456fcd): once the per-frame lerp step is below 0.0004 m AND the rotation within 0.000199999995, freeze the damped eye at an exact fixed point instead of Vector3.Lerp's endless sub-mm asymptote. The drift was walking the 3rd-person eye across the vestibule/room portal plane at rest, flipping the per-frame viewer-cell resolve 0170<->0171 -> the indoor grey/texture flicker. The collided-eye firewall (separate publishedEye local) is already present. Adds ApplyConvergenceSnap static (TDD: 3 unit tests + 1 integration freeze test) + SnapEpsilon/RotCloseEpsilon. App suite 183 -> 187, all green. Plan: docs/superpowers/plans/2026-06-05-indoor-viewer-cell-flicker-fix.md Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Erik
parent
9601ef39c3
commit
d2212cfaea
3 changed files with 688 additions and 2 deletions
|
|
@ -0,0 +1,552 @@
|
|||
# Indoor Viewer-Cell Flicker + Bluish-Void Fix — Implementation Plan
|
||||
|
||||
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
|
||||
|
||||
**Goal:** Kill the indoor render **flicker (grey↔texture at rest)** and the **stable bluish void** at cottage cell boundaries by porting the three retail mechanisms that keep retail's `viewer_cell` rock-stable — camera-boom convergence snap, viewer-cell dead-zone, and w-space portal clip — confirmed root cause by decomp + live cdb (`docs/research/2026-06-05-viewer-cell-flicker-rootcause-and-fix-plan-handoff.md`).
|
||||
|
||||
**Architecture:** Three independent, retail-faithful ports, each TDD'd and **independently visual-verified** in this order (highest leverage first):
|
||||
1. **Camera boom stability** (`RetailChaseCamera`) — add the retail `UpdateCamera` convergence snap so the damped eye reaches an **exact fixed point** at rest instead of dithering sub-millimetre forever. Removes the *trigger* (the eye walking across a portal plane). The collided-eye firewall is already present (verified).
|
||||
2. **Viewer-cell dead-zone** (`BSPQuery.PointInsideCellBsp`) — port retail's symmetric **±0.000199999995 m** dead-zone so a point grazing a splitting plane belongs to *neither* child → membership stays sticky. Belt-and-suspenders for the flicker; shared Core primitive (also used by physics), so the full Core suite gates it.
|
||||
3. **w-space portal clip** (`PortalProjection` / `PortalVisibilityBuilder`) — verify the void is gone after 1+2, port the InitCell side-test dead-band for faithfulness, then **reassess/revert** the `9f95252` eye-in-portal flood band-aid.
|
||||
|
||||
**Tech Stack:** C# .NET 10, `System.Numerics`, xUnit. No new dependencies. Retail oracle: `docs/research/named-retail/acclient_2013_pseudo_c.txt`.
|
||||
|
||||
**DON'T (from handoff §5):**
|
||||
- No render-side debounce/grace-period for the flicker — fix the *input* (boom + cell), never the render.
|
||||
- Don't switch the render root to the *player* cell — retail roots `DrawInside` at the *viewer* cell; make the viewer cell *stable*, don't change which cell roots.
|
||||
- Don't reopen "the flood doesn't reach the cellar" — refuted.
|
||||
- Don't revert Residual A (the `update_viewer` camera-collision port) — it made the viewer cell *accurate*; we're stabilising it.
|
||||
|
||||
**Test baseline (must hold):** App **183 pass / 0 fail**. Core **1326 pass / 4 fail (documented: 2× DoorBugTrajectoryReplay LiveCompare, BSPStepUpTests.D4, DoorCollisionApparatus) / 1 skip**. Build green.
|
||||
|
||||
---
|
||||
|
||||
## File Structure
|
||||
|
||||
| File | Change | Responsibility |
|
||||
|---|---|---|
|
||||
| `src/AcDream.App/Rendering/RetailChaseCamera.cs` | Modify | Part 1: add `SnapEpsilon`/`RotCloseEpsilon` consts + `ApplyConvergenceSnap` static + wire it into `Update`'s damping branch. |
|
||||
| `tests/AcDream.App.Tests/Rendering/RetailChaseCameraTests.cs` | Modify | Part 1: unit test for `ApplyConvergenceSnap` + integration "boom freezes at rest" test. |
|
||||
| `src/AcDream.Core/Physics/BSPQuery.cs` | Modify | Part 2: add the ±0.000199999995 dead-zone to `PointInsideCellBsp` (3-way classify). |
|
||||
| `tests/AcDream.Core.Tests/Physics/PointInsideCellBspDeadZoneTests.cs` | Create | Part 2: RED→GREEN dead-zone tests + a `FindVisibleChildCell` stickiness test. |
|
||||
| `src/AcDream.App/Rendering/PortalVisibilityBuilder.cs` | Modify (Part 3, gated) | Part 3: port InitCell side-test dead-band into `CameraOnInteriorSide`; reassess/remove `EyeInsidePortalOpening` flood (`9f95252`). |
|
||||
| `tests/AcDream.App.Tests/Rendering/PortalProjectionTests.cs` | Modify (Part 3, gated) | Part 3: close-portal robustness regression if a residual void persists. |
|
||||
|
||||
Each part is one or more commits, each ending in a **VISUAL GATE** (the user looks at the running client). Parts 2 and 3 only start after the previous part's visual gate passes.
|
||||
|
||||
---
|
||||
|
||||
## Part 1 — Camera boom convergence snap (highest leverage)
|
||||
|
||||
**Root cause (decomp-confirmed):** `RetailChaseCamera.Update` lerps `_dampedEye` toward `targetEye` with `Vector3.Lerp` every frame (`RetailChaseCamera.cs:149`). `Vector3.Lerp` is asymptotic — it never reaches an exact fixed point, so the eye makes a tiny sub-millimetre step *every frame forever*. At rest that walks the eye across the vestibule/room portal plane → the per-frame viewer-cell resolve flips `0170↔0171` → the render redraws two solves → flicker. Retail's `CameraManager::UpdateCamera` (`0x00456660`) has a **convergence snap** at `0x00456fcd`: after interpolating, if the translation step `< 0.0004 m` (= `2 × 0.000199999995`, `0x00456fe1`) AND the rotation is within `0.000199999995` (`0x00456fdd`, `Frame::close_rotation`), it returns the input unchanged (`Position::Position(__return, ebx_1)`) — an exact fixed point.
|
||||
|
||||
**The collided-eye firewall is already present** — `Update` collides into a separate `publishedEye` local and never writes `_dampedEye` (`RetailChaseCamera.cs:162-172`, comment at `:153-161`). So Part 1 is ONLY the snap.
|
||||
|
||||
**Acceptance:**
|
||||
- New `ApplyConvergenceSnap` static freezes when both deltas are sub-epsilon, else returns the candidate.
|
||||
- After convergence with a constant pose, two consecutive `Update` frames produce a **bit-identical** `Position` (collision off).
|
||||
- All existing `RetailChaseCameraTests` stay green (esp. `SecondUpdate_LerpsTowardTarget` — step 0.75 ≫ epsilon, no snap).
|
||||
- **VISUAL GATE 1:** at the Holtburg cottage vestibule/room boundary, standing still, the `[flap-sweep] desiredBack` value holds flat (no `3.11→3.07` drift) and the grey↔texture flicker is gone or sharply reduced.
|
||||
|
||||
### Task 1.1: Unit-test the convergence-snap helper (RED)
|
||||
|
||||
**Files:**
|
||||
- Test: `tests/AcDream.App.Tests/Rendering/RetailChaseCameraTests.cs`
|
||||
|
||||
- [ ] **Step 1: Write the failing test.** Append these tests to the class (after the existing `Update_CollisionDoesNotCorruptDampedState`, before the closing brace):
|
||||
|
||||
```csharp
|
||||
// ── Convergence snap (Part 1: kills the at-rest boom drift) ────────
|
||||
|
||||
[Fact]
|
||||
public void ConvergenceSnap_StepBelowEpsilon_FreezesAtCurrent()
|
||||
{
|
||||
// Both the translation step and the rotation step are below the retail snap
|
||||
// thresholds (0.0004 m / 0.0002) → freeze: return the CURRENT damped state,
|
||||
// not the candidate. This is the exact fixed point retail's UpdateCamera reaches.
|
||||
var damped = new Vector3(5f, 6f, 7f);
|
||||
var forward = Vector3.Normalize(new Vector3(1f, 0f, 0f));
|
||||
var candidate = damped + new Vector3(0.0001f, 0f, 0f); // 0.1 mm step < 0.4 mm
|
||||
var candFwd = forward; // no rotation step
|
||||
|
||||
var (eye, fwd, frozen) = RetailChaseCamera.ApplyConvergenceSnap(damped, forward, candidate, candFwd);
|
||||
|
||||
Assert.True(frozen);
|
||||
Assert.Equal(damped, eye); // exact — returns the input, freezing the drift
|
||||
Assert.Equal(forward, fwd);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void ConvergenceSnap_TranslationStepAboveEpsilon_ReturnsCandidate()
|
||||
{
|
||||
var damped = new Vector3(5f, 6f, 7f);
|
||||
var forward = Vector3.Normalize(new Vector3(1f, 0f, 0f));
|
||||
var candidate = damped + new Vector3(0.01f, 0f, 0f); // 1 cm step ≫ 0.4 mm
|
||||
var candFwd = forward;
|
||||
|
||||
var (eye, fwd, frozen) = RetailChaseCamera.ApplyConvergenceSnap(damped, forward, candidate, candFwd);
|
||||
|
||||
Assert.False(frozen);
|
||||
Assert.Equal(candidate, eye); // still converging → apply the lerp step
|
||||
Assert.Equal(candFwd, fwd);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void ConvergenceSnap_RotationStepAboveEpsilon_ReturnsCandidate()
|
||||
{
|
||||
// Translation has converged but the heading is still turning — retail does NOT
|
||||
// freeze unless BOTH are close (it returns the interpolated frame). So a small
|
||||
// translation step must NOT freeze while the forward is still rotating.
|
||||
var damped = new Vector3(5f, 6f, 7f);
|
||||
var forward = Vector3.Normalize(new Vector3(1f, 0f, 0f));
|
||||
var candidate = damped + new Vector3(0.0001f, 0f, 0f); // sub-epsilon translation
|
||||
var candFwd = Vector3.Normalize(new Vector3(1f, 0.05f, 0f)); // ~0.05 rad turn ≫ 0.0002
|
||||
|
||||
var (_, _, frozen) = RetailChaseCamera.ApplyConvergenceSnap(damped, forward, candidate, candFwd);
|
||||
|
||||
Assert.False(frozen);
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run the test to verify it fails (compile error — method doesn't exist).**
|
||||
|
||||
Run: `dotnet test tests\AcDream.App.Tests\AcDream.App.Tests.csproj --filter "FullyQualifiedName~ConvergenceSnap"`
|
||||
Expected: **FAIL** — build error `'RetailChaseCamera' does not contain a definition for 'ApplyConvergenceSnap'`.
|
||||
|
||||
### Task 1.2: Implement the convergence-snap helper + constants (GREEN)
|
||||
|
||||
**Files:**
|
||||
- Modify: `src/AcDream.App/Rendering/RetailChaseCamera.cs`
|
||||
|
||||
- [ ] **Step 1: Add the snap constants.** Immediately after the `DistanceMin/Max/PitchMin/Max` const block (after `RetailChaseCamera.cs:78`, the `public const float PitchMax = 1.4f;` line), add:
|
||||
|
||||
```csharp
|
||||
|
||||
// Retail CameraManager::UpdateCamera convergence-snap thresholds (decomp
|
||||
// acclient_2013_pseudo_c.txt, 0x00456fcd–0x00457035). SnapEpsilon = 2 ×
|
||||
// 0.000199999995 m ≈ 0.0004 m — the per-frame translation step below which retail
|
||||
// freezes the boom at an exact fixed point (0x00456fe1). RotCloseEpsilon =
|
||||
// 0.000199999995 — the Frame::close_rotation tolerance (0x00456fdd). Without the
|
||||
// snap, Vector3.Lerp asymptotes forever and the boom drifts at rest, walking the eye
|
||||
// across a portal plane and flipping the viewer cell → the indoor flicker.
|
||||
private const float SnapEpsilon = 0.000199999995f * 2f;
|
||||
private const float RotCloseEpsilon = 0.000199999995f;
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Add the `ApplyConvergenceSnap` static.** Add this method just after `ComputeDampingAlpha` (after `RetailChaseCamera.cs:370`, the closing brace of `ComputeDampingAlpha`):
|
||||
|
||||
```csharp
|
||||
|
||||
/// <summary>
|
||||
/// Retail <c>CameraManager::UpdateCamera</c> convergence snap (decomp 0x00456fcd).
|
||||
/// After the per-frame lerp, if the translation step from <paramref name="dampedEye"/>
|
||||
/// to <paramref name="candidateEye"/> is below <see cref="SnapEpsilon"/> AND the
|
||||
/// rotation step is below <see cref="RotCloseEpsilon"/>, retail returns the input
|
||||
/// position unchanged — an exact fixed point. Returns <c>frozen=true</c> with the
|
||||
/// current state in that case; otherwise <c>frozen=false</c> with the candidate.
|
||||
/// Both conditions are required (retail couples origin + rotation in the snap test),
|
||||
/// so the boom keeps converging while the heading is still turning.
|
||||
/// </summary>
|
||||
internal static (Vector3 eye, Vector3 forward, bool frozen) ApplyConvergenceSnap(
|
||||
Vector3 dampedEye, Vector3 dampedForward, Vector3 candidateEye, Vector3 candidateForward)
|
||||
{
|
||||
bool translationConverged = Vector3.Distance(candidateEye, dampedEye) < SnapEpsilon;
|
||||
bool rotationConverged = Vector3.Distance(candidateForward, dampedForward) < RotCloseEpsilon;
|
||||
if (translationConverged && rotationConverged)
|
||||
return (dampedEye, dampedForward, true); // freeze: exact fixed point
|
||||
return (candidateEye, candidateForward, false);
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 3: Run the unit tests to verify they pass.**
|
||||
|
||||
Run: `dotnet test tests\AcDream.App.Tests\AcDream.App.Tests.csproj --filter "FullyQualifiedName~ConvergenceSnap"`
|
||||
Expected: **PASS** (3 tests).
|
||||
|
||||
### Task 1.3: Wire the snap into `Update` + integration test
|
||||
|
||||
**Files:**
|
||||
- Modify: `src/AcDream.App/Rendering/RetailChaseCamera.cs`
|
||||
- Test: `tests/AcDream.App.Tests/Rendering/RetailChaseCameraTests.cs`
|
||||
|
||||
- [ ] **Step 1: Write the failing integration test.** Append to `RetailChaseCameraTests`:
|
||||
|
||||
```csharp
|
||||
[Fact]
|
||||
public void Update_AtRestAfterConvergence_BoomFreezesAtExactFixedPoint()
|
||||
{
|
||||
// The retail UpdateCamera snap freezes the boom at an exact fixed point once the
|
||||
// per-frame step falls below ~0.4 mm. Without it, Vector3.Lerp asymptotes forever
|
||||
// — the eye dithers sub-millimetre every frame and walks across the portal plane,
|
||||
// flipping the viewer cell (the indoor flicker). Hold a pose DIFFERENT from the
|
||||
// init pose so the boom has to converge over many frames; with collision OFF
|
||||
// (Position == _dampedEye), two consecutive post-convergence frames must be
|
||||
// BIT-IDENTICAL. (At frame 120, α≈0.075, displacement ~7 m, the un-snapped step is
|
||||
// ~5e-5 m ≈ tens of float ULP — distinguishably nonzero — so this is a real RED.)
|
||||
bool savedAlign = CameraDiagnostics.AlignToSlope;
|
||||
bool savedColl = CameraDiagnostics.CollideCamera;
|
||||
float savedT = CameraDiagnostics.TranslationStiffness;
|
||||
float savedR = CameraDiagnostics.RotationStiffness;
|
||||
try
|
||||
{
|
||||
CameraDiagnostics.AlignToSlope = false; // deterministic heading
|
||||
CameraDiagnostics.CollideCamera = false; // Position == _dampedEye
|
||||
CameraDiagnostics.TranslationStiffness = 0.45f;
|
||||
CameraDiagnostics.RotationStiffness = 0.45f;
|
||||
|
||||
var cam = new RetailChaseCamera { Distance = 2.61f, Pitch = 0.291f };
|
||||
|
||||
// Frame 1 at pose A: init snaps the damped eye to A's target.
|
||||
cam.Update(Vector3.Zero, 0.5f, Vector3.Zero, true, Vector3.UnitZ, 1f / 60f);
|
||||
|
||||
// Hold pose B for many frames → the boom lerps A's target → B's target.
|
||||
var posB = new Vector3(5f, 5f, 0f);
|
||||
for (int i = 0; i < 120; i++)
|
||||
cam.Update(posB, 0.5f, Vector3.Zero, true, Vector3.UnitZ, 1f / 60f);
|
||||
|
||||
Vector3 a = cam.Position;
|
||||
cam.Update(posB, 0.5f, Vector3.Zero, true, Vector3.UnitZ, 1f / 60f);
|
||||
Vector3 b = cam.Position;
|
||||
|
||||
Assert.Equal(a, b); // exact — frozen, not dithering
|
||||
}
|
||||
finally
|
||||
{
|
||||
CameraDiagnostics.AlignToSlope = savedAlign;
|
||||
CameraDiagnostics.CollideCamera = savedColl;
|
||||
CameraDiagnostics.TranslationStiffness = savedT;
|
||||
CameraDiagnostics.RotationStiffness = savedR;
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run it to verify it fails.**
|
||||
|
||||
Run: `dotnet test tests\AcDream.App.Tests\AcDream.App.Tests.csproj --filter "FullyQualifiedName~BoomFreezesAtExactFixedPoint"`
|
||||
Expected: **FAIL** — `Assert.Equal()` failure, `a` and `b` differ by ~5e-5 m (the un-snapped asymptotic step).
|
||||
|
||||
- [ ] **Step 3: Wire the snap into the damping branch.** In `RetailChaseCamera.Update`, replace the `else` branch of the `if (!_initialised)` block (`RetailChaseCamera.cs:145-151`):
|
||||
|
||||
```csharp
|
||||
else
|
||||
{
|
||||
float tAlpha = ComputeDampingAlpha(CameraDiagnostics.TranslationStiffness, dt);
|
||||
float rAlpha = ComputeDampingAlpha(CameraDiagnostics.RotationStiffness, dt);
|
||||
_dampedEye = Vector3.Lerp(_dampedEye, targetEye, tAlpha);
|
||||
_dampedForward = Vector3.Normalize(Vector3.Lerp(_dampedForward, targetForward, rAlpha));
|
||||
}
|
||||
```
|
||||
|
||||
with:
|
||||
|
||||
```csharp
|
||||
else
|
||||
{
|
||||
float tAlpha = ComputeDampingAlpha(CameraDiagnostics.TranslationStiffness, dt);
|
||||
float rAlpha = ComputeDampingAlpha(CameraDiagnostics.RotationStiffness, dt);
|
||||
Vector3 candidateEye = Vector3.Lerp(_dampedEye, targetEye, tAlpha);
|
||||
Vector3 candidateForward = Vector3.Normalize(Vector3.Lerp(_dampedForward, targetForward, rAlpha));
|
||||
|
||||
// Retail UpdateCamera convergence snap (0x00456fcd): freeze at an exact fixed
|
||||
// point once the lerp step is sub-epsilon, instead of dithering forever. This is
|
||||
// the at-rest flicker fix — see ApplyConvergenceSnap + SnapEpsilon.
|
||||
(_dampedEye, _dampedForward, _) =
|
||||
ApplyConvergenceSnap(_dampedEye, _dampedForward, candidateEye, candidateForward);
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Run the integration test + the full RetailChaseCamera suite to verify pass + no regression.**
|
||||
|
||||
Run: `dotnet test tests\AcDream.App.Tests\AcDream.App.Tests.csproj --filter "FullyQualifiedName~RetailChaseCameraTests"`
|
||||
Expected: **PASS** (all existing + 4 new). Confirm `SecondUpdate_LerpsTowardTarget` and `Update_CollisionDoesNotCorruptDampedState` still pass.
|
||||
|
||||
### Task 1.4: Full build + test + commit Part 1
|
||||
|
||||
- [ ] **Step 1: Build.** Run: `dotnet build`. Expected: green.
|
||||
- [ ] **Step 2: Full App suite.** Run: `dotnet test tests\AcDream.App.Tests\AcDream.App.Tests.csproj`. Expected: **187 pass / 0 fail** (183 baseline + 4 new).
|
||||
- [ ] **Step 3: Commit.**
|
||||
|
||||
```bash
|
||||
git add src/AcDream.App/Rendering/RetailChaseCamera.cs tests/AcDream.App.Tests/Rendering/RetailChaseCameraTests.cs
|
||||
git commit -m "fix(render): Part 1 — camera boom convergence snap (kills the at-rest viewer-cell flicker trigger)
|
||||
|
||||
Port retail CameraManager::UpdateCamera's convergence snap (0x00456fcd):
|
||||
once the per-frame lerp step is below 0.0004 m AND the rotation within
|
||||
0.000199999995, freeze the damped eye at an exact fixed point instead of
|
||||
Vector3.Lerp's endless sub-mm asymptote. The drift was walking the 3rd-person
|
||||
eye across the vestibule/room portal plane at rest, flipping the per-frame
|
||||
viewer-cell resolve 0170<->0171 -> the indoor grey/texture flicker.
|
||||
|
||||
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>"
|
||||
```
|
||||
|
||||
- [ ] **Step 4: VISUAL GATE 1 — STOP.** Launch the client (CLAUDE.md "Running the client"), with `ACDREAM_PROBE_FLAP=1`. Stand still at the Holtburg cottage vestibule/room boundary. Ask the user to confirm: (a) the grey↔texture flicker at rest is gone/reduced; (b) in the log, `[flap-sweep] desiredBack` holds a constant value (no `3.11→3.07` drift). Do not start Part 2 until the user confirms.
|
||||
|
||||
---
|
||||
|
||||
## Part 2 — Viewer-cell dead-zone (belt-and-suspenders for the flicker)
|
||||
|
||||
**Root cause (decomp-confirmed):** `BSPQuery.PointInsideCellBsp` (`BSPQuery.cs:1034`) uses a hard split — `dist >= 0f` → inside, `dist < 0f` → outside — with **no dead-zone**. A point grazing a splitting plane flips inside/outside on a sub-millimetre wobble. Retail's `BSPNODE::point_inside_cell_bsp` (`0x0053c1f0`, pc:325513/325522) uses a **symmetric ±0.000199999995 m** band: a point within ±0.2 mm of a plane is in *neither* child → the traversal short-circuits and classifies it "inside this cell," so a grazing point stays in the cell it was last in (`check_cell` is null → `curr_cell` unchanged, validate_transition pc:272608). This makes the viewer/player cell sticky at boundaries.
|
||||
|
||||
**Faithful 3-way classify** (matches retail's `eax = 0 / 1 / 2`):
|
||||
- `dist >= +ε` → clearly in front → descend `PosNode` (may still reject on a deeper plane).
|
||||
- `-ε < dist < +ε` → **dead zone** → short-circuit `true` (inside this cell).
|
||||
- `dist <= -ε` → clearly behind → `false` (outside).
|
||||
|
||||
**Shared-primitive risk:** `PointInsideCellBsp` is also used by physics cell membership (`CellTransit.FindVisibleChildCell/FindCellList/FindCellSet`). This is retail-faithful (retail's `point_inside_cell_bsp` has the dead-zone for ALL callers), but the **full Core suite must stay at baseline** (1326/4/1). The change only affects the `(-ε, 0)` band (0–0.2 mm behind a plane flips outside→inside) and the `[0, +ε)` band (short-circuits true instead of testing deeper) — both ≤0.2 mm, retail-exact.
|
||||
|
||||
**Acceptance:**
|
||||
- A point 0.1 mm behind a single splitting plane returns `true` (was `false`); 1 mm behind still `false`; existing `SphereIntersectsCellBspTests.PointInsideCellBsp_PointJustOutside…` (x = −0.3 m) stays `false`.
|
||||
- A `FindVisibleChildCell` graze keeps the start cell.
|
||||
- Full Core suite at baseline; App suite at 187.
|
||||
- **VISUAL GATE 2:** the flicker is fully gone even with deliberate slow boom motion across the boundary (no residual flip).
|
||||
|
||||
### Task 2.1: RED — dead-zone unit tests
|
||||
|
||||
**Files:**
|
||||
- Create: `tests/AcDream.Core.Tests/Physics/PointInsideCellBspDeadZoneTests.cs`
|
||||
|
||||
- [ ] **Step 1: Write the failing test file.**
|
||||
|
||||
```csharp
|
||||
using System.Numerics;
|
||||
using DatReaderWriter.Enums;
|
||||
using DatReaderWriter.Types;
|
||||
using AcDream.Core.Physics;
|
||||
using Xunit;
|
||||
|
||||
namespace AcDream.Core.Tests.Physics;
|
||||
|
||||
/// <summary>
|
||||
/// Tests for the retail dead-zone in <see cref="BSPQuery.PointInsideCellBsp"/>
|
||||
/// (port of BSPNODE::point_inside_cell_bsp, acclient_2013_pseudo_c.txt:325508 /
|
||||
/// 0x0053c1f0). A point within ±0.000199999995 m of a splitting plane is in
|
||||
/// NEITHER child → classified "inside this cell" (short-circuit true). This is
|
||||
/// what keeps the viewer/player cell sticky at a boundary graze (the flicker fix).
|
||||
/// </summary>
|
||||
public class PointInsideCellBspDeadZoneTests
|
||||
{
|
||||
// One splitting plane at x = 0, normal +X → the "inside" half-space is x ≥ 0.
|
||||
private static CellBSPNode SinglePlaneTree()
|
||||
{
|
||||
var leaf = new CellBSPNode { Type = BSPNodeType.Leaf };
|
||||
return new CellBSPNode
|
||||
{
|
||||
SplittingPlane = new Plane(new Vector3(1f, 0f, 0f), 0f),
|
||||
PosNode = leaf,
|
||||
};
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void PointJustBehindPlane_WithinDeadZone_ReturnsTrue()
|
||||
{
|
||||
// 0.1 mm behind the plane → inside the ±0.2 mm dead zone → inside this cell.
|
||||
// Pre-fix this returned FALSE (hard dist < 0 → outside) → the flicker.
|
||||
var root = SinglePlaneTree();
|
||||
Assert.True(BSPQuery.PointInsideCellBsp(root, new Vector3(-0.0001f, 0f, 0f)));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void PointOnPlane_ReturnsTrue()
|
||||
{
|
||||
var root = SinglePlaneTree();
|
||||
Assert.True(BSPQuery.PointInsideCellBsp(root, new Vector3(0f, 0f, 0f)));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void PointJustInFront_ReturnsTrue()
|
||||
{
|
||||
var root = SinglePlaneTree();
|
||||
Assert.True(BSPQuery.PointInsideCellBsp(root, new Vector3(0.0001f, 0f, 0f)));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void PointClearlyBehind_BeyondDeadZone_ReturnsFalse()
|
||||
{
|
||||
// 1 mm behind → outside the ±0.2 mm band → outside the cell (unchanged).
|
||||
var root = SinglePlaneTree();
|
||||
Assert.False(BSPQuery.PointInsideCellBsp(root, new Vector3(-0.001f, 0f, 0f)));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void PointFarBehind_ReturnsFalse_RegressionGuard()
|
||||
{
|
||||
// The existing SphereIntersectsCellBspTests regression pin (x = -0.3 m) must
|
||||
// stay FALSE — the dead zone is only ±0.2 mm, 300 mm is far outside.
|
||||
var root = SinglePlaneTree();
|
||||
Assert.False(BSPQuery.PointInsideCellBsp(root, new Vector3(-0.3f, 0f, 0f)));
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run to verify it fails.**
|
||||
|
||||
Run: `dotnet test tests\AcDream.Core.Tests\AcDream.Core.Tests.csproj --filter "FullyQualifiedName~PointInsideCellBspDeadZone"`
|
||||
Expected: **FAIL** — `PointJustBehindPlane_WithinDeadZone_ReturnsTrue` fails (returns false). The other 4 pass already (they pin unchanged behaviour).
|
||||
|
||||
### Task 2.2: GREEN — add the dead-zone to `PointInsideCellBsp`
|
||||
|
||||
**Files:**
|
||||
- Modify: `src/AcDream.Core/Physics/BSPQuery.cs`
|
||||
|
||||
- [ ] **Step 1: Replace the split test.** In `PointInsideCellBsp` (`BSPQuery.cs:1034-1047`), replace the body after the leaf checks:
|
||||
|
||||
```csharp
|
||||
float dist = Vector3.Dot(node.SplittingPlane.Normal, point) + node.SplittingPlane.D;
|
||||
|
||||
// Front or on-plane → follow positive child (inside).
|
||||
if (dist >= 0f)
|
||||
return node.PosNode is not null ? PointInsideCellBsp(node.PosNode, point) : true;
|
||||
|
||||
// Behind → outside.
|
||||
return false;
|
||||
```
|
||||
|
||||
with:
|
||||
|
||||
```csharp
|
||||
float dist = Vector3.Dot(node.SplittingPlane.Normal, point) + node.SplittingPlane.D;
|
||||
|
||||
// Retail BSPNODE::point_inside_cell_bsp dead-zone (0x0053c1f0, pc:325513/325522):
|
||||
// a symmetric ±0.000199999995 m band around the splitting plane belongs to NEITHER
|
||||
// child. A point in the band short-circuits "inside this cell" (true) — this is what
|
||||
// keeps the viewer/player cell sticky at a boundary graze (no sub-mm membership flip
|
||||
// → no indoor flicker). Only a point clearly BEHIND the plane is outside.
|
||||
const float CellBspPlaneEpsilon = 0.000199999995f;
|
||||
|
||||
if (dist >= CellBspPlaneEpsilon)
|
||||
return node.PosNode is not null ? PointInsideCellBsp(node.PosNode, point) : true;
|
||||
if (dist <= -CellBspPlaneEpsilon)
|
||||
return false; // clearly behind → outside the cell
|
||||
return true; // dead zone (within ±0.2 mm) → inside this cell
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run the dead-zone tests to verify pass.**
|
||||
|
||||
Run: `dotnet test tests\AcDream.Core.Tests\AcDream.Core.Tests.csproj --filter "FullyQualifiedName~PointInsideCellBspDeadZone"`
|
||||
Expected: **PASS** (5 tests).
|
||||
|
||||
### Task 2.3: Stickiness regression at the `FindVisibleChildCell` level
|
||||
|
||||
**Files:**
|
||||
- Test: `tests/AcDream.Core.Tests/Physics/CellTransitFindVisibleChildCellTests.cs`
|
||||
|
||||
- [ ] **Step 1: Inspect the existing fixtures.** Read `tests/AcDream.Core.Tests/Physics/CellTransitFindVisibleChildCellTests.cs` to reuse its cell-cache/`CellPhysics` builder. Add one test: a point that grazes the start cell's boundary plane (within ±0.2 mm, on the outside) resolves back to the **start cell** (not a neighbour). If the existing fixtures don't expose a single-plane start cell conveniently, assert the primitive instead via `BSPQuery.PointInsideCellBsp` on the start cell's `CellBSP.Root` (the dead-zone test in Task 2.1 already covers the primitive; this task is satisfied if no cheap `FindVisibleChildCell`-level fixture exists — note that in the commit message rather than forcing a brittle fixture).
|
||||
|
||||
- [ ] **Step 2: Run the CellTransit suite.**
|
||||
|
||||
Run: `dotnet test tests\AcDream.Core.Tests\AcDream.Core.Tests.csproj --filter "FullyQualifiedName~CellTransit"`
|
||||
Expected: **PASS** (no regression).
|
||||
|
||||
### Task 2.4: Full build + test + commit Part 2
|
||||
|
||||
- [ ] **Step 1: Build.** Run: `dotnet build`. Expected: green.
|
||||
- [ ] **Step 2: FULL Core suite (shared-primitive gate).** Run: `dotnet test tests\AcDream.Core.Tests\AcDream.Core.Tests.csproj`. Expected: **1331 pass / 4 fail / 1 skip** (1326 + 5 new dead-zone tests; the 4 documented failures unchanged — verify the failing set is the SAME 4, not a new one).
|
||||
- [ ] **Step 3: Commit.**
|
||||
|
||||
```bash
|
||||
git add src/AcDream.Core/Physics/BSPQuery.cs tests/AcDream.Core.Tests/Physics/PointInsideCellBspDeadZoneTests.cs
|
||||
git commit -m "fix(physics): Part 2 — point_inside_cell_bsp dead-zone (sticky cell membership at boundary graze)
|
||||
|
||||
Port retail BSPNODE::point_inside_cell_bsp's symmetric ±0.000199999995 m
|
||||
dead-zone (0x0053c1f0, pc:325513/325522): a point within 0.2 mm of a splitting
|
||||
plane is in neither child -> short-circuit 'inside this cell'. Belt-and-suspenders
|
||||
for the indoor flicker: the viewer cell stays sticky when the boom grazes the
|
||||
vestibule/room portal plane instead of flipping 0170<->0171 on a sub-mm wobble.
|
||||
Shared with physics cell membership (retail-faithful: retail uses the same band
|
||||
for all callers). Full Core suite at baseline.
|
||||
|
||||
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>"
|
||||
```
|
||||
|
||||
- [ ] **Step 4: VISUAL GATE 2 — STOP.** Launch with `ACDREAM_PROBE_FLAP=1` + `ACDREAM_PROBE_CELL=1`. Walk the boom slowly across the vestibule/room boundary. Ask the user to confirm the flicker is fully gone and `[cell-transit]` shows clean single crossings (no oscillation). Do not start Part 3 until confirmed.
|
||||
|
||||
---
|
||||
|
||||
## Part 3 — w-space portal clip robustness + reassess `9f95252` (gated on the void state)
|
||||
|
||||
**Status after Parts 1+2:** the eye now rests stable in the *substantial* cell (room/outside), not lingering in the thin vestibule. The `proj=0` "stable bluish void" was a degenerate projection that occurs only when the eye stands IN a portal plane looking along it — a position Parts 1+2 keep the eye out of. So Part 3 is **diagnose-then-decide**, not a fixed code change.
|
||||
|
||||
**Key correction vs the handoff:** do **NOT** lower `PortalProjection.MinW` (0.05) to exactly `w = 0`. acdream's `ProjectToNdc` computes a 2D screen *region* (not a homogeneous rasterisation), so a vertex AT `w = 0` divides to `inf`/`NaN`. Retail's `polyClipFinish` produces w=0 synthetic verts because its downstream is a homogeneous rasteriser; acdream's region-clip needs `w > 0` strictly. The existing `MinW = 0.05` clip-space Sutherland–Hodgman (commit `5f596f2`) is the correct adaptation and is **kept**. The remaining faithful pieces are the InitCell side-test dead-band and removing the band-aid.
|
||||
|
||||
**Acceptance:**
|
||||
- After Parts 1+2's visual gates, capture `ACDREAM_PROBE_FLAP` at the cottage boundary + cellar; confirm whether any `proj=0` / `terrain=Skip` void remains.
|
||||
- If clean: **revert `9f95252`** (the `EyeInsidePortalOpening` flood) and re-verify the void stays gone — this is the goal (the boom + dead-zone made it redundant).
|
||||
- For faithfulness, tighten `CameraOnInteriorSide`'s `PortalSideEpsilon` toward retail's InitCell band only if it does not re-introduce culling (test-gated).
|
||||
- **VISUAL GATE 3:** no stable bluish void anywhere at the cottage (boundary, cellar, exiting); the cellar still seals; no new flap.
|
||||
|
||||
### Task 3.1: Diagnose the residual void (no code change)
|
||||
|
||||
- [ ] **Step 1.** Launch with `ACDREAM_PROBE_FLAP=1`. At the cottage doorway and in the cellar, capture `[flap]`/`[flap-sweep]` lines. Identify any portal still showing `proj=0` while its neighbour should be visible. Record the eye position + cell relative to that portal.
|
||||
- [ ] **Step 2.** Decide:
|
||||
- **(A) No residual void** → go to Task 3.2 (revert the band-aid).
|
||||
- **(B) Residual void at a close portal** → the eye is still reaching a degenerate position; first re-check Parts 1+2 at that spot (boom flat? cell sticky?). Only if the eye is legitimately close to a portal it must see through, port the InitCell side-test dead-band (Task 3.3) before reverting the band-aid.
|
||||
|
||||
### Task 3.2: Reassess / revert the `9f95252` eye-in-portal flood band-aid
|
||||
|
||||
**Files:**
|
||||
- Modify: `src/AcDream.App/Rendering/PortalVisibilityBuilder.cs`
|
||||
|
||||
- [ ] **Step 1: Write a guard test first.** In `tests/AcDream.App.Tests/Rendering/PortalVisibilityBuilderTests.cs` (or the nearest existing builder test file — locate with a glob), add/keep a test that a normal interior doorway floods its neighbour through the projected+clipped region WITHOUT relying on `EyeInsidePortalOpening` (i.e. with the eye a normal distance back). Run it to confirm it passes BEFORE removing the band-aid.
|
||||
- [ ] **Step 2: Remove the band-aid.** Delete the `clippedRegion.Count == 0` → `EyeInsidePortalOpening` flood block (`PortalVisibilityBuilder.cs:171-177`) and replace with the plain cull:
|
||||
|
||||
```csharp
|
||||
if (clippedRegion.Count == 0)
|
||||
continue; // portal not visible through this chain
|
||||
```
|
||||
|
||||
Then delete the now-unused `EyeInsidePortalOpening`, `PointInPoly2D`, and `EyeStandingPerpDist` members (`PortalVisibilityBuilder.cs:415-474`) — confirm no other references with a grep before deleting.
|
||||
|
||||
- [ ] **Step 3: Build + App suite.** Run: `dotnet build` then `dotnet test tests\AcDream.App.Tests\AcDream.App.Tests.csproj`. Expected: green, no regression.
|
||||
|
||||
- [ ] **Step 4: VISUAL CHECK (mini-gate).** Launch; confirm the cellar **ceiling** (the thing `9f95252` originally fixed) is still sealed with the band-aid removed (Parts 1+2 should now keep it sealed via the stable viewer cell). If the ceiling drops, the band-aid was load-bearing → **restore it** (`git revert` the removal) and record that the boom/dead-zone did not fully subsume it; keep it and move on. Either outcome is a valid, documented result.
|
||||
|
||||
### Task 3.3 (conditional): Port the InitCell side-test dead-band
|
||||
|
||||
Only if Task 3.1 chose path (B). Retail `PView::InitCell` (`0x005a4b70`, pc:432896-432936) treats a viewer within **±0.000199999995 m** of a portal plane as the front (positive) side. acdream's `CameraOnInteriorSide` (`PortalVisibilityBuilder.cs:326-333`) uses `PortalSideEpsilon = 0.01f`.
|
||||
|
||||
- [ ] **Step 1.** Add a test in the builder test file: a camera exactly on a portal plane is treated as interior-side (traverses). A camera 1 cm clearly behind is culled.
|
||||
- [ ] **Step 2.** Only change `PortalSideEpsilon` if the test + visual gate confirm it does not re-introduce a flap (tightening can cull a portal the eye is slightly behind). If it regresses, leave `0.01f` and note the divergence. Retail-faithfulness here is secondary to not re-opening the flap.
|
||||
|
||||
### Task 3.4: Commit Part 3 + VISUAL GATE 3
|
||||
|
||||
- [ ] **Step 1: Commit** whatever Part 3 landed (band-aid removed, or kept-and-documented, ± side-band):
|
||||
|
||||
```bash
|
||||
git add -A
|
||||
git commit -m "fix(render): Part 3 — reassess eye-in-portal flood after boom+dead-zone stabilise the viewer cell
|
||||
|
||||
<describe the actual outcome: band-aid 9f95252 removed as redundant / kept as
|
||||
load-bearing; ± InitCell side-test dead-band>. The stable bluish void is gone
|
||||
because Parts 1+2 keep the eye out of the degenerate in-portal-plane position;
|
||||
MinW=0.05 clip-space Sutherland-Hodgman (5f596f2) is kept (region-clip needs w>0).
|
||||
|
||||
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>"
|
||||
```
|
||||
|
||||
- [ ] **Step 2: VISUAL GATE 3 — STOP.** Full cottage tour (outside → doorway → room → cellar → back). Ask the user to confirm: no stable bluish void, cellar seals, no flicker, no new flap. This closes the flicker/void fix.
|
||||
|
||||
---
|
||||
|
||||
## Post-completion
|
||||
|
||||
- [ ] Update `docs/research/2026-06-05-viewer-cell-flicker-rootcause-and-fix-plan-handoff.md` status (or write a short ship note) and the `reference_render_pipeline_state.md` memory: flicker + void fixed via the 3-part port; note which of Parts 2/3 (band-aid) ended up load-bearing.
|
||||
- [ ] Update `docs/plans/2026-05-12-milestones.md` M1.5 narrative if the indoor world now "feels right."
|
||||
- [ ] If a residual remains (e.g. #78 outdoor terrain gating), file/refresh the issue — do not fold it into this fix.
|
||||
|
||||
---
|
||||
|
||||
## Self-Review
|
||||
|
||||
**Spec coverage (handoff §4):**
|
||||
- Part 1 (camera boom stability) → Tasks 1.1–1.4. ✔ Snap ported; firewall already present (verified, noted).
|
||||
- Part 2 (viewer-cell dead-zone) → Tasks 2.1–2.4. ✔ ±0.000199999995 ported into the shared point-in-cell test.
|
||||
- Part 3 (w-space clip + reassess `9f95252`) → Tasks 3.1–3.4. ✔ Diagnose-then-decide; band-aid reassessment explicit; MinW correction documented.
|
||||
- Per-part acceptance + visual gates → every part ends in a VISUAL GATE. ✔
|
||||
- Order Part 1 → gate → Part 2 → gate → Part 3 → gate. ✔
|
||||
|
||||
**Placeholder scan:** Part 1 and Part 2 have complete code. Part 3 is intentionally diagnostic (its concrete change depends on the Part 1+2 visual outcome) — the conditional branches and the exact files/lines are specified, and the "no MinW→0" correction is concrete. This is honest given the gating, not a placeholder.
|
||||
|
||||
**Type consistency:** `ApplyConvergenceSnap(Vector3, Vector3, Vector3, Vector3) → (Vector3, Vector3, bool)` used identically in test and wiring. `SnapEpsilon`/`RotCloseEpsilon` consts referenced in helper + comment. `CellBspPlaneEpsilon` local to `PointInsideCellBsp`. `CellBSPNode { Type=…, SplittingPlane=new Plane(Vector3,float), PosNode=… }` matches `SphereIntersectsCellBspTests`. `CameraDiagnostics` statics saved/restored (they leak between tests) as the existing tests do.
|
||||
|
||||
**Risk notes:** Part 2 changes a Core primitive shared with physics — the FULL Core suite is the gate, and the change is bounded to a ±0.2 mm band (retail-exact). Part 1's integration test is float-ULP-sensitive; frame count (120) + displacement (~7 m) were chosen so the un-snapped step (~5e-5 m) is tens of ULP above zero — a real RED — while still below SnapEpsilon (snapped GREEN).
|
||||
Loading…
Add table
Add a link
Reference in a new issue