fix #389 review round: settings v3 FOV migration + live apply; AD-90
Dual-lens Opus review of 7e0c1303 (reports committed under
docs/research/). The law, gate, and vertical application are CONFIRMED
at instruction-byte level against the PDB-paired acclient.exe (the BN
text FPU-elides this whole area); the fix round addresses the findings:
- Blast MUST-FIX 1: real schema migration instead of a hand-edited dev
file. SettingsStore v2->v3: a pre-v3 display.fieldOfView was the
applied vertical FOV in degrees; v3 means retail's m_fGameFOV.
LoadDisplay migrates on read - the untouched old default 60 maps to
the retail default 90; a deliberate other value preserves its visible
16:9 framing (x (16/9 - 0.1)), clamped to the registered [10,160];
the next save stamps v3 and migration never reruns. The dev
settings.json hand-edit was reverted so the migration owns it.
- Blast MUST-FIX 2 / mechanism M2: the Field of View now applies LIVE on
Save (retail: Render::GRPCallback_OnRenderPreferenceChanged @0x0054d999
-> SmartBox::SetDefaultFov). RuntimeSettingsTargets gains the camera
graph and applies through ApplyDisplayWindowState - the update-phase
seam, deliberately NOT the render-phase preview path (the review's
WATCH-3 cull-vs-raster landmine).
- Mechanism M1 -> register row AD-90: retail's divisor aspect runs
through the Render.AspectRatio preference (ComputeAspectForViewport
@0x0054f150, (w/h) x pref x 0.75) - exactly raw w/h at the registered
default, which is what acdream assumes; retail's NaN-through-the-gate
quirk (M3) is folded into the same row as deliberately not reproduced.
- Docs: RetailFieldOfView now cites the decisive vertical proof
(D3DXMatrixPerspectiveFovLH fovy slot @0x0059ab71), the unconditional
SmartBox::RenderNormalMode site, and M4's exact horizontal numbers
(89.0/83.9/80.6 deg); the Config FOV row comment updated to LIVE.
- Blast WATCH 4 disposition: the 15 replay-harness PI/3 constants stay -
they are CAPTURE-TIME camera parameters for recorded fixtures, not
production framing; changing them would invalidate the replays.
Tests: +6 SettingsStore migration facts, +1 live-apply fact.
App suite 4,962/3 skips; UI.Abstractions 922.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Erik
parent
6b844c142f
commit
d13d63d0a5
11 changed files with 1032 additions and 21 deletions
File diff suppressed because one or more lines are too long
386
docs/research/2026-08-13-389-fov-blast-review.md
Normal file
386
docs/research/2026-08-13-389-fov-blast-review.md
Normal file
|
|
@ -0,0 +1,386 @@
|
|||
# Blast-radius review — commit `7e0c1303` (#389 SmartboxFOV port)
|
||||
|
||||
**Reviewer lens:** BLAST RADIUS (consumers of the changed surfaces), 2026-08-13.
|
||||
**Commit:** `7e0c1303` — "fix #389: port retail's SmartboxFOV law; retire AD-89 (display slice 1)".
|
||||
**Scope:** report only. No files edited outside this document.
|
||||
|
||||
## Verdict summary
|
||||
|
||||
The mechanism is right and the consumer sweep is largely clean — every camera
|
||||
computes `Projection` on demand, nothing caches a projection or a FOV across a
|
||||
`SetAspect`/`SetGameFov`, and the three "writes `.Aspect` directly" suspects are
|
||||
all private cameras outside the controller. Two real defects survive, both on the
|
||||
axis the commit did *not* traverse (the **stored value**, not the code):
|
||||
|
||||
| # | Verdict | Surface | One-line |
|
||||
|---|---------|---------|----------|
|
||||
| 1 | **MUST-FIX** | `SettingsStore.LoadDisplay` | The stored `fieldOfView` changed MEANING with no schema bump and no migration; existing users silently get 35.8° vertical where they had 60°. Migration machinery + precedent already exist and were not used. |
|
||||
| 2 | **MUST-FIX** | divergence register / Config apply | Retail's FOV preference is LIVE (`SmartBox::SetDefaultFov` from the preference callback, re-applied every render); acdream's is next-launch-only. AD-89 was retired without a replacement row for the half that did not land. |
|
||||
| 3 | **WATCH (conditional MUST-FIX)** | `WorldRenderFrameBuilder.Build` ordering | The per-frame preview path mutates camera FOV *after* the frame snapshotted its frustum. Dead in production today (`HasDraftPreview => false`); becomes a live cull-vs-raster mismatch the moment anyone makes the slider live — i.e. while fixing #2. |
|
||||
| 4 | WATCH | 15 test harness sites + 4 comments | Still pin `MathF.PI / 3f` as "the production / RetailChaseCamera projection". Production is now 0.9363 rad. |
|
||||
|
||||
Reference values for the rest of this document (law = `gameFOV / (aspect − 0.1)`,
|
||||
`gameFOV = π/2`):
|
||||
|
||||
| aspect | applied FovY | vs. old constant |
|
||||
|---|---|---|
|
||||
| 4:3 | 1.2736 rad = **72.97°** | +12.97° |
|
||||
| 16:9 | 0.9363 rad = **53.64°** | −6.36° |
|
||||
| 21:9 | 0.7033 rad = **40.30°** | −19.70° |
|
||||
| 16:9, stored `60` reinterpreted | 0.6242 rad = **35.76°** | −24.24° ← MUST-FIX 1 |
|
||||
|
||||
---
|
||||
|
||||
## MUST-FIX 1 — the persisted `fieldOfView` was reinterpreted with no migration
|
||||
|
||||
**Evidence**
|
||||
|
||||
- `src/AcDream.UI.Abstractions/Panels/Settings/SettingsStore.cs:73`
|
||||
```csharp
|
||||
FieldOfView: ReadFloat (disp, "fieldOfView", d.FieldOfView),
|
||||
```
|
||||
Straight read. No range check, no version check — `LoadDisplay` (`:56-95`)
|
||||
never inspects `root["version"]` at all.
|
||||
- `src/AcDream.UI.Abstractions/Panels/Settings/SettingsStore.cs:596` —
|
||||
`["fieldOfView"] = d.FieldOfView` is written unconditionally by
|
||||
`BuildDisplayObject`, so **every** `settings.json` that has ever had a
|
||||
`display` section carries the key. It is not an "only if the user touched the
|
||||
slider" case.
|
||||
- The schema-version machinery the fix needs already exists and is already used
|
||||
for exactly this class of change:
|
||||
- `SettingsStore.cs:42` — `private const int CurrentSchemaVersion = 2;`
|
||||
- `SettingsStore.cs:397` — `// Version-1 migration: radar stored only X/Y and had no resolution key.`
|
||||
The commit bumped neither the constant nor added a `display` migration.
|
||||
- The commit message acknowledges the problem and solves it for one machine:
|
||||
*"user settings.json migrated 60→90 by hand (stale pre-port default)"*.
|
||||
|
||||
**Consequence.** A pre-`7e0c1303` `settings.json` holds `fieldOfView: 60` (the
|
||||
old `DisplaySettings.Default`). Post-commit that 60 is read as retail
|
||||
`m_fGameFOV` degrees, so at 16:9 the applied vertical FOV is
|
||||
`60° / (1.7778 − 0.1) = 35.76°` — a 40 % reduction from the 60° the same file
|
||||
used to produce. The visible symptom is a hard zoom-in that survives restarts and
|
||||
looks nothing like either the old build or retail, and the user has no way to
|
||||
know the number in their file changed meaning. It is the exact inverse of the
|
||||
"squished" report that opened #389.
|
||||
|
||||
**Shape of the fix (not applied).** Bump `CurrentSchemaVersion` to 3 and, in
|
||||
`LoadDisplay`, when the file's `version` is `< 3` and a `display.fieldOfView`
|
||||
key is present, replace it with `DisplaySettings.Default.FieldOfView` (or map it
|
||||
through the inverse law if preserving the user's framing is preferred:
|
||||
`gameFOV = storedVerticalDegrees × (aspect − 0.1)` — but the honest choice is the
|
||||
retail registered default, since the old number never had a retail meaning to
|
||||
preserve). Note the round-trip preservation contract in the class doc
|
||||
(`SettingsStore.cs:31-37`): unknown top-level keys are carried forward, so the
|
||||
version bump must be written on save through the existing `root["version"] =
|
||||
CurrentSchemaVersion` sites (`:296`, `:339`, `:446`, `:500`, `:683`).
|
||||
|
||||
---
|
||||
|
||||
## MUST-FIX 2 — AD-89 retired while half its own required scope is still unported
|
||||
|
||||
AD-89's deleted text (from the commit's own diff) said the port *"must also
|
||||
re-map the Config Field of View slider to retail's degree semantics in the same
|
||||
change"*. The **semantics** landed. The **liveness** did not, and there is now no
|
||||
register row for the gap.
|
||||
|
||||
**Retail is live.** Three decomp facts, two of which the commit already cites:
|
||||
|
||||
- `docs/research/named-retail/acclient_2013_pseudo_c.txt:344363-344365`
|
||||
(`Render::GRPCallback_OnRenderPreferenceChanged` @ `0x0054d982`-`0x0054d999`):
|
||||
```
|
||||
0054d984 float FieldOfView = Render::m_RenderPrefs.FieldOfView;
|
||||
0054d98c Current_Render_FieldOfView = FieldOfView;
|
||||
0054d999 SmartBox::SetDefaultFov(SmartBox::smartbox, FieldOfView);
|
||||
```
|
||||
The preference-changed callback pushes the new value immediately.
|
||||
- `acclient_2013_pseudo_c.txt:90988` (`SmartBox::SetDefaultFov` @ `0x00451e60`)
|
||||
writes `this->m_fGameFOV` and nothing else.
|
||||
- `:91727` / `:92660` (`0x00452b2f` / `0x00453b14`) re-evaluate
|
||||
`m_fGameFOV / (m_ViewportAspectRatio − 0.1)` **inside the render path**, so the
|
||||
new `m_fGameFOV` takes effect on the very next frame. There is no restart.
|
||||
|
||||
**acdream is next-launch-only.**
|
||||
|
||||
- `src/AcDream.App/Settings/RuntimeSettingsController.cs:171-176` — `Startup` is
|
||||
snapshotted in the constructor.
|
||||
- `:208-218` — `ApplyStartup` calls `target.ApplyDisplay(Startup.Display)`, i.e.
|
||||
the ctor-time snapshot, and is guarded by `_startupApplied` so it runs once.
|
||||
- `src/AcDream.App/UI/Layout/ConfigOptionsPageController.cs:571` — the FOV row's
|
||||
`apply` only calls `bindings.SaveDisplay(...)`. Nothing on that path reaches
|
||||
`CameraController`.
|
||||
- `ConfigOptionsPageController.cs:563-566` documents this honestly
|
||||
(`// Field of View: NEXT-LAUNCH ...`), and `DisplaySettings.cs:27-31` repeats
|
||||
it — but neither is a register row, and a search of
|
||||
`docs/architecture/retail-divergence-register.md` for `next launch` /
|
||||
`NEXT-LAUNCH` / `next-launch` returns **zero** hits.
|
||||
|
||||
**Why this is not "pre-existing, therefore out of scope".** Before this commit
|
||||
the slider was a no-op on the camera the player actually looks through
|
||||
(`CameraDiagnostics.UseRetailChaseCamera` defaults **true** —
|
||||
`src/AcDream.Core/Rendering/CameraDiagnostics.cs:27-28` — and the old
|
||||
`ApplyFieldOfView` wrote only `Orbit`/`Fly`/`Chase`, never `RetailChase`). The
|
||||
setting was inert, so its latency was immaterial. This commit made the value
|
||||
matter, which is precisely when the register rule bites: the deviation is now
|
||||
observable. Either add the AD row, or make it live — the live fix is one call
|
||||
(`cameras.SetGameFov(value * MathF.PI / 180f)`) alongside the existing
|
||||
`SaveDisplay` in the apply lambda. **If you take the live route, read MUST-FIX 3
|
||||
first — do not route it through the per-frame preview seam.**
|
||||
|
||||
---
|
||||
|
||||
## MUST-FIX 3 (conditional) — the preview seam mutates cameras *after* the frame snapshot
|
||||
|
||||
`src/AcDream.App/Rendering/WorldRenderFrameBuilder.cs:162-164`:
|
||||
|
||||
```csharp
|
||||
WorldCameraFrame camera = _camera.Resolve(); // snapshots Projection/ViewProjection/Frustum
|
||||
_visibility.Begin(in camera, waitingForLogin);
|
||||
_settings.Apply(in camera); // ← calls ApplyFieldOfView → SetGameFov → ApplyProjection
|
||||
```
|
||||
|
||||
`Resolve()` (`:196-211`) freezes `Projection`, `ViewProjection` and `Frustum`.
|
||||
`RuntimeWorldFrameSettingsPreview.Apply` (`:368-380`) then mutates every camera's
|
||||
`FovY` **and now also `Aspect`**. Downstream:
|
||||
|
||||
- **Culling / visibility use the snapshot**: `_visibility.Begin` →
|
||||
`RetailSelectionScene.SetViewFrustum(camera.Frustum)` (`:335`),
|
||||
`_buildings.Gather(..., in frustum)` (`:170-173`),
|
||||
`_environmentFrustum.Update(camera.ViewProjection)` (`:346`).
|
||||
- **Rasterization re-reads the camera live**: `WorldScenePassExecutor.cs:128,
|
||||
158, 171, 186, 208, 218, 227, 250, 280` all pass `camera.Camera` (the live
|
||||
`ICamera`), and `TerrainModernRenderer.cs:207`, `Wb/WbDrawDispatcher.cs:2011`,
|
||||
`ParticleRenderer.cs:253`, `ParticleRenderer.Rhi.cs:296`,
|
||||
`Sky/SkyRenderer.cs:189` each recompute `camera.View * camera.Projection`.
|
||||
|
||||
So a FOV change applied at line 164 produces one frame culled at the old frustum
|
||||
and rasterized at the new one — pop-in / holes at the screen edge on every frame
|
||||
the value moves.
|
||||
|
||||
**Why this is only a WATCH today.** `RuntimeSettingsController.cs:202` —
|
||||
`public bool HasDraftPreview => false;`, with the OP9 comment at `:195-201`
|
||||
stating it is *"always false in production"* since `SettingsVM` was retired. The
|
||||
branch is unreachable outside tests.
|
||||
|
||||
**Why it is a landmine.** MUST-FIX 2's natural-looking fix is "make the preview
|
||||
path live". Doing that turns this dormant ordering bug into a live artifact on
|
||||
the retail chase camera. The safe fixes are (a) drive `SetGameFov` from the
|
||||
Config apply lambda, outside the frame graph entirely, or (b) move
|
||||
`_settings.Apply` **above** `_camera.Resolve()` in `Build`.
|
||||
|
||||
---
|
||||
|
||||
## WATCH 4 — harness constants and comments still pin the deleted 60°
|
||||
|
||||
These compile and pass (explicit initializers override the changed defaults), but
|
||||
they now describe a projection no production camera uses. Two of them are
|
||||
load-bearing *replay* harnesses whose whole point is to reproduce the production
|
||||
frustum, and a 60° test frustum is 6.4° **wider** than production at 16:9 — a
|
||||
flood/clip regression that only shows at the production frustum can hide here.
|
||||
|
||||
Constructed at `MathF.PI / 3f`:
|
||||
|
||||
| File | Lines |
|
||||
|---|---|
|
||||
| `tests/AcDream.App.Tests/Rendering/Issue177StairDescentCameraFloodTests.cs` | 194, 250, 272, 378, 423, 478 |
|
||||
| `tests/AcDream.App.Tests/Rendering/PortalProjectionTests.cs` | 121, 154, 266, 418 — each tagged `// RetailChaseCamera` |
|
||||
| `tests/AcDream.App.Tests/Rendering/HouseExitWalkReplayTests.cs` | 479 |
|
||||
| `tests/AcDream.App.Tests/Rendering/Issue181VisFlapReplayTests.cs` | 38 |
|
||||
| `tests/AcDream.App.Tests/Rendering/OutdoorCellNodeTests.cs` | 41 |
|
||||
| `tests/AcDream.App.Tests/Rendering/TerrainCullOrientationTests.cs` | 42 |
|
||||
| `tests/AcDream.App.Tests/Rendering/TowerAscentReplayTests.cs` | 352 |
|
||||
|
||||
Comment-only drift (claims a production FOV that no longer exists):
|
||||
|
||||
- `CornerFloodReplayTests.cs:174` — *"Production projection: ChaseCamera/FlyCamera use FovY ~1.2"*
|
||||
- `Issue130DoorwayStripTests.cs:49` — *"FovY 1.2 rad"*
|
||||
- `Issue95DungeonFloodDiagnosticTests.cs:35` — *"FovY ~1.2"*
|
||||
- `TerrainCullOrientationTests.cs:15` — cites `RetailChaseCamera.cs:203 / :52` for the projection convention (line numbers also stale after the edit)
|
||||
|
||||
Recommended: leave the numeric constants (changing them re-baselines flood
|
||||
fixtures, which is its own gate) but retag them as *"pinned pre-#389 constant,
|
||||
not the live production FOV"*, and point the replay harnesses at
|
||||
`RetailFieldOfView.DefaultAppliedFovY` where the value is not fixture-pinned.
|
||||
|
||||
---
|
||||
|
||||
## CLEAN — per-surface enumeration
|
||||
|
||||
### Axis 1: readers of `ChaseCamera` / `RetailChaseCamera` / `OrbitCamera` / `FlyCamera` `.FovY` / `.Aspect` / `.Projection`
|
||||
|
||||
**No projection or FOV is cached anywhere.** All four cameras compute
|
||||
`Projection` in a get-accessor from live `FovY`/`Aspect`:
|
||||
`ChaseCamera.cs:66`, `OrbitCamera.cs:29`, `FlyCamera.cs:38`,
|
||||
`RetailChaseCamera.cs:56-57`. A grep for cached-projection fields
|
||||
(`_cachedProjection` / `_lastProjection` / `Matrix4x4 _projection`) returns
|
||||
nothing in `src/AcDream.App`. There is no stale-matrix surface to fix.
|
||||
|
||||
The named suspects:
|
||||
|
||||
- **`PortalTunnelPresentation.cs:313`** — **CLEAN, and now more faithful.**
|
||||
`_camera` is a private `PortalTunnelCamera` (declared `:87`), not one of the
|
||||
four and never attached to `CameraController`; `ApplyProjection` cannot reach
|
||||
it and it cannot fight back. Its FOV comes from
|
||||
`UseSmartBoxFov(smartBoxProjection)` (`:314`), where the projection is
|
||||
`_camera.Active.Projection` read live at `PrivatePresentationRenderer.cs:128`.
|
||||
That is exactly retail's `CreatureMode::UseSmartboxFOV` reading the active
|
||||
SmartBox projection, so the tunnel now inherits the smartbox law for free —
|
||||
previously it inherited the invented 60°.
|
||||
- **`PrivateEntityViewportRenderer.cs:203`** — **CLEAN.** `_camera` is an
|
||||
`IPrivateEntityViewportCamera` (`:15`, `:73`); the implementations are
|
||||
`DollViewportCamera` (`DollCamera.cs:59`, wrapping a private `DollCamera` with
|
||||
its own `FovRadians = MathF.PI / 4f`, `DollCamera.cs:41`) and
|
||||
`CreatureAppraisalPresentation` (`:315`, `:343`). Both are paperdoll/portrait
|
||||
surfaces, outside the law by design, and `DollCamera`'s own doc already cites
|
||||
retail's `UseSharpMode` branch — which the decomp confirms at
|
||||
`acclient_2013_pseudo_c.txt:91718` (`Render::SetFOVRad(this->m_fFOVRadians)`,
|
||||
the *non*-smartbox arm of the same function that carries the smartbox site at
|
||||
`0x00452b2f`).
|
||||
- **`TeleportViewPlaneController.cs:145-146`** — **CLEAN, with a noted footgun.**
|
||||
`ProjectionOverrideCamera.Aspect` is a write-through into `_source.Aspect`, and
|
||||
`_source` *is* the active world camera (`ApplyTo(_cameras.Active)` via
|
||||
`WorldRenderFrameBuilder.cs:198`). Nothing in production writes it — the only
|
||||
writer anywhere is `tests/.../PortalTunnelAssetTests.cs:229`, against its own
|
||||
fixture source. Its `Apply` (`:94-118`) recovers aspect as
|
||||
`baseProjection.M22 / baseProjection.M11`, which for
|
||||
`CreatePerspectiveFieldOfView` is exactly `aspect` regardless of FOV — the
|
||||
changed FovY does not perturb it. *Footgun:* a future caller that sets `.Aspect`
|
||||
on the wrapper would be silently reverted by the next `ApplyProjection`, since
|
||||
the controller is now the sole aspect authority. Worth a one-line comment on
|
||||
the setter.
|
||||
- **Picking / `WorldSelectionQuery`** — **CLEAN.**
|
||||
`SelectionCameraSource.Snapshot()`
|
||||
(`Composition/InteractionUiRuntimeSources.cs:553-559`) reads
|
||||
`_camera.Active.Projection` **live at pick time**, and
|
||||
`WorldSelectionQuery.PickAt` (`:145-154`) feeds that same snapshot's `View` +
|
||||
`Projection` into `_selectionScene.Pick`. There is no cached pick projection,
|
||||
so the pick ray cannot desync from the camera. (Pre-existing, unchanged: the
|
||||
candidate frustum handed to `RetailSelectionScene.SetViewFrustum` is the
|
||||
previous frame's snapshot — a one-frame skew across a resize, not introduced
|
||||
here.)
|
||||
- **`FrustumCuller` / `FrustumPlanes`** — **CLEAN.**
|
||||
`FrustumPlanes.FromViewProjection` (`FrustumCuller.cs:34-66`) is a pure
|
||||
function of the passed matrix; the struct stores planes, never a FOV.
|
||||
- **`RetailChaseCamera` consumers (Phase W single-viewpoint)** — **CLEAN.**
|
||||
`ViewerCellId` (`:41`) is the single viewpoint and is untouched.
|
||||
`CameraFrameController.cs:75`, `MouseLookController.cs:184`,
|
||||
`CameraPointerInputController.cs:304, 399` and
|
||||
`WorldRenderFrameBuilder.cs:255-259` all read pose / cell / the diagnostics
|
||||
flag — none reads `FovY` or `Aspect`. The near plane (0.1 m) and its long
|
||||
correctness note (`RetailChaseCamera.cs:46-55`) are unaffected: the commit
|
||||
changed only the FOV argument to the same `CreatePerspectiveFieldOfView` call.
|
||||
- **Name-collision false positives** (unrelated `.Projection`, entity-placement
|
||||
domain): `Rendering/Scene/LiveRenderProjectionJournal.cs`,
|
||||
`Rendering/Scene/CurrentRenderSceneOracle.cs`,
|
||||
`Runtime/Physics/RuntimeSetPositionState.cs`,
|
||||
`Runtime/Entities/*`, `Headless/Hosting/*`. **CLEAN — not this surface.**
|
||||
|
||||
### Axis 2: readers of `DisplaySettings.FieldOfView`
|
||||
|
||||
- **`ConfigOptionsPageController.cs:567-573`** — **CLEAN, improved.**
|
||||
`min: 10f, max: 160f, defaultValue: 90.0f` already matched retail's registered
|
||||
row (`UIPreferences::SetPreferenceRange(&Render_FieldOfView, 10f, 160f)`
|
||||
@ `0x004043b2`, `acclient_2013_pseudo_c.txt:3261`). Under the *old* semantics
|
||||
the row's `defaultValue: 90` silently contradicted
|
||||
`DisplaySettings.Default.FieldOfView = 60` — clicking "Defaults" wrote a
|
||||
different number than a fresh install. The commit made them agree. No slider
|
||||
change is needed.
|
||||
- **`WorldRenderFrameBuilder.cs:370-379`** (preview path) — **WATCH**, see
|
||||
MUST-FIX 3.
|
||||
- **`RuntimeSettingsTargets.cs:130` / `:143-148`** (startup) — **CLEAN.** The
|
||||
degree→radian conversion matches retail's own option setter
|
||||
(`SmartBox::SetDefaultFov` @ `0x00451e6a`, literal
|
||||
`0.017453292519943295`).
|
||||
- **`RuntimeSettingsController.cs:165, 171-176, 208-218`** — **CLEAN as code**,
|
||||
but is the mechanism behind MUST-FIX 2 (ctor-snapshot + once-only apply).
|
||||
- **Headless / Runtime / Cli** — **CLEAN.** `grep DisplaySettings|SettingsStore`
|
||||
over `src/AcDream.Headless`, `src/AcDream.Cli`, `src/AcDream.Runtime` returns
|
||||
nothing. Headless has no FOV surface to reinterpret.
|
||||
- **Launcher / UI Studio / fixtures** — **CLEAN.** No launcher project exists
|
||||
(`src/` is App, Bake, Cli, Content, Core, Core.Net, Headless,
|
||||
Plugin.Abstractions, Plugins.Smoke, Runtime, UI.Abstractions). No `ui-studio`
|
||||
settings surface reads `FieldOfView`. No `settings.json` fixture is committed
|
||||
(the only two in the tree are `.claude/` and `.vscode/`). MUST-FIX 1's blast
|
||||
radius is therefore exactly "every real user's
|
||||
`%LOCALAPPDATA%\acdream\settings.json`" and nothing else.
|
||||
- **`tests/.../SettingsStoreTests.cs:45, 86`** — **CLEAN.** Uses `100f` and
|
||||
`DisplaySettings.Default.FieldOfView`; both semantics-neutral.
|
||||
|
||||
### Axis 3: `CameraController` lifecycle
|
||||
|
||||
- **Constructor (`CameraController.cs:57-62`)** — **CLEAN.** C# runs
|
||||
field/property initializers before the constructor body, so `GameFovRadians`
|
||||
(`:53`) and `_aspect` (`:55`) are both set when `ApplyProjection()` runs.
|
||||
- **Aspect seeding** — **CLEAN.** `HostInputCameraComposition.cs:317-321` binds
|
||||
the camera target and then immediately calls
|
||||
`FramebufferResize.Resize(InitialFramebufferSize)`, so `_aspect` is the real
|
||||
viewport before any camera is read. There is no window in which the controller's
|
||||
16:9 default could stomp a correct non-16:9 aspect (the concern raised by
|
||||
`FramebufferResizeController.cs:124` — *"Late binding never replays an earlier
|
||||
resize transition"* — is answered by that explicit initial `Resize`).
|
||||
- **`EnterChaseMode` (`:74-88`)** — **CLEAN.** `ApplyProjection()` runs *before*
|
||||
`_mode` is assigned and before `ModeChanged` fires, so no listener can observe
|
||||
a half-converged camera. Both construction sites call it immediately after
|
||||
`new`: `PlayerModeController.cs:350-357` (first entry) and `:185-190`
|
||||
(`ToggleFlyOrChase`'s `_chase.Retail ??= new ...`). **No camera escapes the
|
||||
law.** Note `:350` / `:353` still seed `Aspect = _viewport.Aspect` and
|
||||
`EnterChaseMode` immediately overwrites it from `CameraController._aspect`;
|
||||
harmless because `FramebufferResizeController.Resize:126-127` feeds both from
|
||||
the same width/height in the same method, but the seed is now dead code.
|
||||
- **`RestoreState` (`:148-161`)** — **CLEAN.** `ApplyProjection()` runs after the
|
||||
camera fields are assigned but before `_mode` and before `ModeChanged`, and it
|
||||
null-guards both chase slots (`:130-131`, `:137-138`), so restoring a
|
||||
`Chase = null` prior state is safe. Its only caller is the entry-failure
|
||||
rollback at `PlayerModeController.cs:385`;
|
||||
`CameraControllerTests.RestoreState_ReestablishesPriorCameraAfterNotificationFailure`
|
||||
(`:126-150`) still covers the throwing-listener path unchanged.
|
||||
- **`ExitChaseMode` (`:90-100`)** — **CLEAN.** Deliberately does not re-apply;
|
||||
`Orbit`/`Fly` are already converged and the cleared chase cameras are garbage.
|
||||
- **Re-entrancy** — **CLEAN.** `ApplyProjection` invokes no callbacks, so
|
||||
`SetAspect`/`SetGameFov` cannot re-enter.
|
||||
|
||||
### Bonus verification the commit did not cite: the divisor's aspect orientation
|
||||
|
||||
Worth recording because it is the one input that could have silently inverted the
|
||||
whole law. If retail's `m_ViewportAspectRatio` were height/width, then at 16:9 the
|
||||
divisor would be `0.5625 − 0.1 = 0.4625`, giving `1.5708 / 0.4625 = 3.396 rad`
|
||||
— **greater than π**, so `TryAppliedVerticalFov` would return `false` on every
|
||||
modern screen, the cameras would stay pinned at `DefaultAppliedFovY` forever, and
|
||||
the slider would be inert. The unit tests (which feed `16f/9f` directly) would
|
||||
not catch it.
|
||||
|
||||
It is width/height, proved by the consumer rather than by the decomp of the
|
||||
producer: `acclient_2013_pseudo_c.txt:423669` (`PrimD3DRender::SetFOVInternal`
|
||||
@ `0x0059ab71`) passes `m_ViewportAspectRatio` straight into
|
||||
`D3DXMatrixPerspectiveFovLH`'s `Aspect` parameter, which is width/height by the
|
||||
D3D contract. Note that the producer itself,
|
||||
`RenderDevice::ComputeAspectForViewport` @ `0x0054f150`
|
||||
(`acclient_2013_pseudo_c.txt:345715-345722`), decompiles to a bare
|
||||
`return arg4;` / `return arg5;` — the classic Binary Ninja FPU-elision artifact
|
||||
(see `memory/reference_pe_byte_decode.md`), so it cannot be read directly.
|
||||
**Recommend adding the `0x0059ab71` citation to `RetailFieldOfView`'s class
|
||||
doc** — it is the only evidence in the tree that the divisor is not inverted, and
|
||||
the next reader will otherwise land on the mangled `ComputeAspectForViewport` and
|
||||
have to redo this.
|
||||
|
||||
---
|
||||
|
||||
## Ranked action list
|
||||
|
||||
1. **MUST-FIX** — `SettingsStore`: bump `CurrentSchemaVersion` to 3 and migrate
|
||||
`display.fieldOfView` on load for files written at version ≤ 2.
|
||||
(`SettingsStore.cs:42, 56-95, 397, 596`)
|
||||
2. **MUST-FIX** — either make the Config FOV row live
|
||||
(`ConfigOptionsPageController.cs:571` → also `cameras.SetGameFov(...)`) to
|
||||
match `Render::GRPCallback_OnRenderPreferenceChanged @0x0054d999`, **or** file
|
||||
the AD row for the next-launch divergence. Retiring AD-89 without one leaves
|
||||
an unrowed deviation.
|
||||
3. **WATCH → MUST-FIX if #2 is taken live** — do not route the live apply through
|
||||
`WorldRenderFrameBuilder`'s preview seam; either call `SetGameFov` from the
|
||||
Config apply lambda or move `_settings.Apply` above `_camera.Resolve()`
|
||||
(`WorldRenderFrameBuilder.cs:162-164`).
|
||||
4. **WATCH** — retag the 15 `MathF.PI / 3f` harness sites and 4 stale comments as
|
||||
pinned pre-#389 constants (§WATCH 4 table).
|
||||
5. **WATCH (documentation)** — add the `0x0059ab71` D3DX citation to
|
||||
`RetailFieldOfView`'s class doc; add a one-line note on
|
||||
`TeleportViewPlaneController.ProjectionOverrideCamera.Aspect` that the
|
||||
controller is now the sole aspect authority and a write here will be reverted.
|
||||
437
docs/research/2026-08-13-389-fov-mechanism-review.md
Normal file
437
docs/research/2026-08-13-389-fov-mechanism-review.md
Normal file
|
|
@ -0,0 +1,437 @@
|
|||
# #389 SmartboxFOV — MECHANISM review of commit `7e0c1303`
|
||||
|
||||
**Date:** 2026-08-13
|
||||
**Reviewer lens:** MECHANISM (is the ported law the law retail actually runs?)
|
||||
**Subject:** `7e0c1303` — *fix #389: port retail's SmartboxFOV law; retire AD-89 (display slice 1)*
|
||||
**Oracle:** `docs/research/named-retail/acclient_2013_pseudo_c.txt` +
|
||||
`docs/research/named-retail/acclient.h`, **plus byte-level disassembly** of the
|
||||
PDB-paired binary `C:\Users\erikn\Downloads\acclient.exe`
|
||||
(`check_exe_pdb.py` → `MATCH`, GUID `9e847e2f-777c-4bd9-886c-22256bb87f32`,
|
||||
linker 2013-09-06). Every x87 comparison and division in this area is
|
||||
FPU-elided or ambiguous in the Binary Ninja pseudo-C, so **all polarity and
|
||||
operand-order claims below are decided from the instruction bytes**, not from
|
||||
BN's rendering. Raw disassembly is in the appendix.
|
||||
|
||||
Report only — no files under `src/` or `tests/` were modified.
|
||||
|
||||
---
|
||||
|
||||
## Verdict summary
|
||||
|
||||
| # | Question | Verdict |
|
||||
|---|---|---|
|
||||
| 1 | Formula transcription exact (operand order, 0.1, aspect variable, direct feed) | **CONFIRMED**, with one qualification: retail's `m_ViewportAspectRatio` is **not** raw `w/h` in general — see M1 |
|
||||
| 2 | What `m_bUseSmartboxFOV == 0` selects; does acdream apply the law where retail uses the fixed branch | **CONFIRMED** — no misapplication. Paperdoll/appraisal/char-gen = fixed branch (π/4, byte-verified); portal space = smartbox law, and acdream's portal camera derives from the world projection |
|
||||
| 3 | Does `0x00453b14` differ from the CreatureMode site | **CONFIRMED (differs, benignly)** — it is `SmartBox::RenderNormalMode`, a different class, with **no** `m_bUseSmartboxFOV` gate at all; the arithmetic is byte-identical |
|
||||
| 4 | Gate is open (0, π), silent keep-previous; `SetFOVInternal` applies it **vertically** | **CONFIRMED** — open on both ends, `return 0` without touching `Render::fov`; and the value reaches `D3DXMatrixPerspectiveFovLH`'s **fovy** slot. The port is **not** inverted. One NaN corner: M3 |
|
||||
| 5 | `0x00451e6a` is the FOV option's setter; `[10,160]`/default 90 registered | **CONFIRMED** — `SmartBox::SetDefaultFov`, fed by `Render::UpdateFromPreferences`; range and default byte-verified |
|
||||
| 6 | Other writers of `Render::fov` / other `SetFOVRad` callers the port ignores | **CONFIRMED (none ignored)** — only 3 `SetFOVRad` call sites exist; the one other FOV writer (`Render::set_vdst`, the teleport override) is already ported in `TeleportViewPlaneController`, and my byte-decode **vindicates** its `2·atan(1/d)` against BN's misleading `__fpatan(arg1, 1.0)` |
|
||||
|
||||
**No MUST-FIX defect was found in the law, the gate, or its application.** Three
|
||||
bookkeeping/doc items are ranked below; the highest (M1) is a divergence-register
|
||||
obligation created by this commit, not a wrong number at retail's defaults.
|
||||
|
||||
---
|
||||
|
||||
## Q1 — Is the formula transcription exact?
|
||||
|
||||
**CONFIRMED for the expression; QUALIFIED for what "the aspect" is.**
|
||||
|
||||
Both smartbox sites are byte-identical arithmetic. `CreatureMode::Render`
|
||||
(pseudo-C line 91727, `0x00452b0d`–`0x00452b2f`):
|
||||
|
||||
```
|
||||
00452b0d mov ecx, [0x86f330] ; RenderDevice::render_device
|
||||
00452b13 fld dword [ecx + 0xa8] ; ST0 = m_ViewportAspectRatio
|
||||
00452b1a fsub dword [0x7948cc] ; ST0 = aspect - 0.1f (0x3dcccccd)
|
||||
00452b20 fdivr dword [eax + 0xc0] ; ST0 = m_fGameFOV / ST0 <-- fdivr: mem / ST0
|
||||
00452b26 fstp dword [esp] ; float-rounded quotient
|
||||
00452b2f call 0x54b2d0 ; Render::SetFOVRad(quotient)
|
||||
```
|
||||
|
||||
- **Operand order:** `fdivr` is *reverse* divide — `ST0 = memory / ST0`, i.e.
|
||||
`m_fGameFOV / (aspect − 0.1)`. The numerator is the game FOV. **Correct in the
|
||||
port.** (This is exactly the operand-order trap the question was aimed at; BN's
|
||||
`((long double)m_fGameFOV) / (aspect - 0.1)` happens to agree, but only the
|
||||
bytes prove it.)
|
||||
- **The 0.1 literal:** `[0x7948cc] = 0x3dcccccd = 0.10000000149011612`. The port's
|
||||
`AspectBias = 0.100000001f` parses to that identical float. **Exact.**
|
||||
- **Direct feed:** the quotient is stored to the stack slot and immediately passed
|
||||
to `SetFOVRad`; no clamp, scale, or half-angle in between. `SetFOVRad` →
|
||||
`SetFOVInternal(arg)` → `D3DXMatrixPerspectiveFovLH(pOut, arg, aspect, zn, zf)`
|
||||
(`0x0059ab71`). **No further transformation. Correct in the port.**
|
||||
- Retail evaluates in x87 80-bit and stores one `float`; acdream evaluates in
|
||||
`float`. Sub-ULP; immaterial.
|
||||
|
||||
### The qualification (see M1): which variable holds the aspect
|
||||
|
||||
The divisor's left operand is `RenderDevice::m_ViewportAspectRatio` (offset
|
||||
`0xa8`, confirmed by the store in `RenderDevice::SetViewport` at
|
||||
`0054f22e fstp dword [esi+0xa8]`). That field is **not** `width/height` in the
|
||||
path the game view uses. `RenderDevice::ComputeAspectForViewport` (`0x0054f150`,
|
||||
pseudo-C line 345708) is FPU-elided in BN to a meaningless `return arg4 / arg5`.
|
||||
The bytes say:
|
||||
|
||||
```
|
||||
0054f150 mov al, [esp+0x14] ; arg6
|
||||
0054f154 fild dword [esp+0xc] ; width
|
||||
0054f15a je 0x54f181 ; arg6 == 0 -> second path
|
||||
; arg6 != 0 : ST0 = width / height <-- raw viewport aspect
|
||||
0054f17c fdivp st(1)
|
||||
0054f17e ret 0x14
|
||||
0054f181: ; arg6 == 0 :
|
||||
0054f1a1 fdivp st(1) ; ST0 = width / height
|
||||
0054f1a3 fmul dword [ecx+0xa4] ; * m_DisplayAspectRatio
|
||||
0054f1a9 fmul dword [0x79b6dc] ; * 0.75
|
||||
0054f1af ret 0x14
|
||||
```
|
||||
|
||||
`m_DisplayAspectRatio` (offset `0xa4`, confirmed by the stores at `0059fc19` /
|
||||
`0059fc24` / `0059fc33`) comes from the **`Render.AspectRatio` user preference**
|
||||
(`RenderDeviceD3D::SetupDisplayAspectRatio` `0x0059fbd0`, pseudo-C 428476):
|
||||
`1 → 1.33333337f`, `2 → 1.77777779f`, otherwise the display's own `w/h`.
|
||||
|
||||
The game view's viewport is installed with `arg6 = 0`
|
||||
(`gmSmartBoxUI::RecvNotice_UpdateGameView`, `0x004d62f5`), so in normal gameplay:
|
||||
|
||||
```
|
||||
m_ViewportAspectRatio = (viewportW / viewportH) × m_DisplayAspectRatio × 0.75
|
||||
```
|
||||
|
||||
At the registered default `AspectRatio = 1` (static initializer
|
||||
`Render::m_RenderPrefs.AspectRatio = 0x1` at `0x0081efa8`; also the safe-settings
|
||||
reset at `0054ef25`), `1.33333337f × 0.75f` rounds to **exactly 1.0f**, so
|
||||
`m_ViewportAspectRatio == w/h` and **the port matches retail bit-for-bit**.
|
||||
It diverges only if the user selects the "widescreen" preference (× 4/3, a
|
||||
noticeably narrower vertical FOV) — a preference acdream does not implement.
|
||||
That is the M1 bookkeeping item, not a wrong number today.
|
||||
|
||||
---
|
||||
|
||||
## Q2 — What `m_bUseSmartboxFOV == 0` selects, and does acdream misapply the law?
|
||||
|
||||
**CONFIRMED — the port does not apply the smartbox law anywhere retail uses the
|
||||
fixed branch.**
|
||||
|
||||
The flag lives on `CreatureMode` (`acclient.h:52561`), which is the base of
|
||||
`UIElement_Viewport` (`acclient.h:52574`) — i.e. **it only exists for embedded 3-D
|
||||
UI viewports**, never for the world. `CreatureMode::Render` `0x00452af0`:
|
||||
|
||||
```
|
||||
00452ae8 mov al, [esi+0x6c] ; m_bUseSmartboxFOV
|
||||
00452af0 je 0x452b2b ; == 0 -> fixed branch
|
||||
00452b2b mov edx, [esi+0x68] ; m_fFOVRadians
|
||||
00452b2f call SetFOVRad
|
||||
```
|
||||
|
||||
`m_fFOVRadians` (offset `0x68`) has exactly **one** read and **zero** writes in the
|
||||
whole binary — its value is the constructor default, byte-decoded at
|
||||
`004543de mov dword [esi+0x68], 0x3f490fdb` = **π/4 = 45°** (BN hid this inside a
|
||||
17-byte `memcpy` literal). So the fixed branch is a constant, aspect-independent
|
||||
45° vertical FOV.
|
||||
|
||||
Who takes which branch (every caller in the binary):
|
||||
|
||||
| Site | Caller | Branch |
|
||||
|---|---|---|
|
||||
| `0x004a5aa9` | `gmPaperDollUI` (paperdoll) — `UseSharpMode` | fixed 45° |
|
||||
| `0x004e0440` | char-gen 3-D view — `UseSharpMode` | fixed 45° |
|
||||
| `0x004ee9bc` | `gmCG3DView` — `UseSharpMode` | fixed 45° |
|
||||
| `0x004d6db3` | `gmSmartBoxUI::PostInit` → `m_pPortalSpace` — `UseSmartboxFOV` | **smartbox law** |
|
||||
|
||||
`CreatureMode::UseSmartboxFOV` (`0x00452380`, line 91348) is the *only* writer of
|
||||
the flag and only ever sets it to 1.
|
||||
|
||||
acdream's side, checked exhaustively (`grep FovY`, `grep CreatePerspectiveFieldOfView`):
|
||||
|
||||
- The four cameras driven by `CameraController` (`Orbit`, `Fly`, `Chase`,
|
||||
`RetailChase`) are all **world** cameras — retail's world render is
|
||||
`SmartBox::RenderNormalMode`, which is *unconditionally* on the smartbox law.
|
||||
Correct.
|
||||
- `DollCamera.FovRadians = MathF.PI / 4f` — matches the byte-decoded fixed-branch
|
||||
constant. The commit's "paperdoll is exempt by design" claim is not just
|
||||
plausible, it is **numerically right**, and it is outside `CameraController`, so
|
||||
the law can never reach it.
|
||||
- `CreatureAppraisalPresentation`'s camera is likewise `π/4` and outside the
|
||||
controller — the same fixed-branch class of viewport. Correct.
|
||||
- **Portal space:** retail's portal space *does* take the smartbox law
|
||||
(`0x004d6db3`). acdream's `PortalTunnelCamera.UseSmartBoxFov(smartBoxProjection)`
|
||||
recovers the FOV from the world camera's projection matrix
|
||||
(`fov = 2·atan(1/M22)`) each draw (`PortalTunnelPresentation.Draw`, line 314), so
|
||||
it now inherits the new law automatically. Correct — and the "read the SmartBox
|
||||
each draw" shape matches retail's per-frame re-application.
|
||||
- `VulkanRhiScene`'s hardcoded `MathF.PI / 3f` is the RHI smoke-test scene
|
||||
(quadrant markers, wobble backdrop), not a world path. Not a finding.
|
||||
|
||||
Startup ordering was also checked, because the law makes FOV aspect-dependent:
|
||||
`HostInputCameraCompositionPhase` calls
|
||||
`FramebufferResize.Resize(InitialFramebufferSize)` immediately after binding the
|
||||
camera target (`HostInputCameraComposition.cs:320`), so the real framebuffer
|
||||
aspect — not `DefaultAppliedFovY`'s assumed 16:9 — is in force before the first
|
||||
frame.
|
||||
|
||||
---
|
||||
|
||||
## Q3 — Does `0x00453b14` differ from the CreatureMode site?
|
||||
|
||||
**CONFIRMED — different class, one structural difference, identical arithmetic.**
|
||||
|
||||
`0x00453b14` is inside **`SmartBox::RenderNormalMode`** (`0x00453aa0`, pseudo-C
|
||||
line 92639) — the world render, not `CreatureMode`. Differences:
|
||||
|
||||
1. **No `m_bUseSmartboxFOV` gate exists there at all.** The function goes straight
|
||||
to the `m_bUseViewDistance` test (`00453ae6 mov cl,[esi+0xc8]`). The world is
|
||||
*always* on the smartbox law (modulo the teleport override). This is the
|
||||
stronger statement the port relies on, and it holds.
|
||||
2. `m_fGameFOV` is read from `this` (the SmartBox, `[esi+0xc0]`) rather than from
|
||||
the `SmartBox::smartbox` global (`[eax+0xc0]` after
|
||||
`a158ca8300 mov eax,[0x83ca58]`). Same object in practice — `CreatureMode::Render`
|
||||
loads the global explicitly because `this` is a viewport, not the SmartBox.
|
||||
3. Otherwise the instruction sequence is the same four ops
|
||||
(`fld [dev+0xa8]` / `fsub 0.1f` / `fdivr m_fGameFOV` / `fstp`), then
|
||||
`call SetFOVRad`.
|
||||
|
||||
Both sites share the second gate the commit message does not mention: if
|
||||
`m_bUseViewDistance != 0`, retail calls `Render::set_vdst(m_fViewDistFOV)`
|
||||
**instead** — see Q6.
|
||||
|
||||
---
|
||||
|
||||
## Q4 — The gate, and is the value vertical?
|
||||
|
||||
**CONFIRMED on both halves. The port is not inverted.**
|
||||
|
||||
### The acceptance interval — open (0, π), silent keep-previous
|
||||
|
||||
`Render::SetFOVRad` (`0x0054b2d0`, pseudo-C line 342158), bytes:
|
||||
|
||||
```
|
||||
0054b2d0 fld dword [esp+4]
|
||||
0054b2d4 fcomp dword [0x795344] ; 0x795344 = 0.0f
|
||||
0054b2da fnstsw ax
|
||||
0054b2dc test ah, 0x41 ; C0 (less) | C3 (equal)
|
||||
0054b2df jnp 0x54b312 ; PF=0 <=> exactly one of C0/C3 <=> arg<0 or arg==0 -> reject
|
||||
0054b2e1 fld dword [esp+4]
|
||||
0054b2e5 fcomp qword [0x7bdd30] ; 0x7bdd30 = 3.141592653589793
|
||||
0054b2ed test ah, 1 ; C0 (less)
|
||||
0054b2f0 je 0x54b312 ; not-less (>= pi) -> reject
|
||||
0054b2fc mov dword [0x81ec84], 0x3dcccccd ; Render::znear = 0.1f
|
||||
0054b306 call [eax+0x1c] ; SetFOVInternal(arg)
|
||||
0054b30c mov eax, 1
|
||||
0054b311 ret
|
||||
0054b312 xor eax, eax ; return 0 — no SetFOVInternal, Render::fov untouched
|
||||
0054b314 ret
|
||||
```
|
||||
|
||||
- `arg == 0` is **rejected** (C3 path) and `arg == π` is **rejected** (needs strict
|
||||
C0). Interval is **open on both ends**. Matches
|
||||
`fovY > 0f && fovY < MathF.PI`.
|
||||
- The reject path is `xor eax,eax; ret` — it never reaches `SetFOVInternal`, so
|
||||
`Render::fov`, `Render::vdst`, `Render::znear` and the view-to-clip matrix all
|
||||
keep their previous values, **silently** (the `int32_t` return is discarded at
|
||||
every call site). `CameraController.ApplyProjection`'s "return without touching
|
||||
`FovY`" is the exact contract.
|
||||
- Note the port also short-circuits `divisor <= 0`. Retail reaches the same verdict
|
||||
by arithmetic: a negative divisor yields a negative quotient (rejected by test 1),
|
||||
a zero divisor yields ±∞ (rejected by test 2). Behaviourally equivalent — except
|
||||
for NaN, see **M3**.
|
||||
|
||||
### Vertical, not horizontal — three independent proofs
|
||||
|
||||
1. **`Render::SetFOVInternal` (`0x0054b340`)**: `Render::vdst = ty / tan(arg × 0.5)`
|
||||
where `ty = (viewportHeight − 1) × 0.5 × yinvscale` — half the viewport
|
||||
**height** over a distance is `tan(half **vertical** FOV)`.
|
||||
2. **`PrimD3DRender::SetFOVInternal` (`0x0059ab40`)** passes the same `arg2`
|
||||
straight into `D3DXMatrixPerspectiveFovLH(&m, arg2, m_ViewportAspectRatio,
|
||||
znear, zfar)`. That API's second parameter is **fovy** and its third is
|
||||
width/height. Decisive.
|
||||
3. **`SmartBox::GetOverrideFovDistance` (`0x00451be0`)** halves the *same*
|
||||
smartbox expression before `fptan` and reciprocates it
|
||||
(`fmul 0.5` → `fptan` → `fdivr 1.0`), i.e. it returns `cot(fov/2)` — only
|
||||
meaningful if the expression is a full vertical angle. (It also proves
|
||||
`view-plane distance == M22` of a perspective matrix, which is what
|
||||
`TeleportViewPlaneController.Begin` assumes.)
|
||||
|
||||
acdream feeds the value to `Matrix4x4.CreatePerspectiveFieldOfView(FovY, Aspect, …)`
|
||||
— same slot, same meaning. **Not inverted.**
|
||||
|
||||
---
|
||||
|
||||
## Q5 — Is `0x00451e6a` the Field of View option's setter, with [10,160]/default 90?
|
||||
|
||||
**CONFIRMED on every part.**
|
||||
|
||||
- Enclosing function: **`SmartBox::SetDefaultFov(SmartBox* this, float degrees)`**
|
||||
at `0x00451e60` (pseudo-C line 90988) —
|
||||
`m_fGameFOV = degrees × 0.017453292519943295`.
|
||||
- Its **only** caller in the binary is `Render::UpdateFromPreferences()`
|
||||
(`0x0054d850`, call at `0x0054d999`, pseudo-C 344365), guarded by a change test
|
||||
against `Current_Render_FieldOfView` and fed from
|
||||
`Render::m_RenderPrefs.FieldOfView`. So the chain is
|
||||
option slider → preference → `UpdateFromPreferences` → `SetDefaultFov` → `m_fGameFOV`.
|
||||
- **Range:** `UIPreferences::SetPreferenceRange(&Render_FieldOfView, 10f, 160f)` at
|
||||
`0x004043b2`, inside **`gmClient::InitUIPreferences` (`0x004035b0`, pseudo-C line
|
||||
2740)** — exactly as the commit claims. Corroborated by the console command at
|
||||
`0x00455d20` which validates `>= 0xa && <= 0xa0` before
|
||||
`UIPreferences::ModifyPreference(&Render_FieldOfView, …)`.
|
||||
- **Default:** the static initializer `Render::m_RenderPrefs` at `0x0081efb8` reads
|
||||
`float FieldOfView = 90`. The UI row is registered via
|
||||
`PlayerOptionPage::AddSliderOption(&Render_FieldOfView, 1)` (`0x0049e53e`).
|
||||
- **Ctor default of `m_fGameFOV`:** `0x00454649` in `SmartBox::SmartBox` sets
|
||||
`1.57079637f` = π/2 = 90°, consistent with the degrees default. Also
|
||||
`m_fViewDistFOV = 0`, `m_bUseViewDistance = 0`.
|
||||
- acdream's `DisplaySettings.Default.FieldOfView = 90f` and the Config row
|
||||
(`ConfigOptionsPageController` `min: 10f, max: 160f, defaultValue: 90.0f`) now
|
||||
agree with each other and with retail. The pre-#389 mismatch (record default 60,
|
||||
slider default 90) is genuinely closed.
|
||||
|
||||
**One behavioural difference, pre-existing (see M4):** retail applies an FOV change
|
||||
**immediately** — `Render::UpdateFromPreferences` is reached from
|
||||
`SceneTool::PrepareGraphicsDevice` (`0x0043e4f0`), a per-frame device-prep step.
|
||||
acdream applies `DisplaySettings.FieldOfView` only through
|
||||
`RuntimeSettingsController.ApplyStartup` (next launch), except while a Config
|
||||
draft preview is open (`WorldRenderFrameBuilder.Apply` → `ApplyFieldOfView`).
|
||||
|
||||
---
|
||||
|
||||
## Q6 — Any other writer of `Render::fov` / `SetFOVRad` caller the port ignores?
|
||||
|
||||
**CONFIRMED — none ignored. The one "other law" is already ported, and my
|
||||
byte-decode corrects the decomp in the port's favour.**
|
||||
|
||||
`SetFOVRad` has exactly **three** call sites in the binary (grep of the full
|
||||
pseudo-C): `0x00452b2f` (CreatureMode, both branches converge on this one call)
|
||||
and `0x00453b14` (SmartBox::RenderNormalMode). Nothing else calls it.
|
||||
|
||||
`Render::fov` is written only inside `Render::SetFOVInternal`, reached from:
|
||||
|
||||
1. `SetFOVRad` — the ported path.
|
||||
2. **`Render::set_vdst(float)` (`0x0054b240`)** — the *view-distance override*.
|
||||
Selected by `m_bUseViewDistance != 0` at **both** smartbox sites, i.e. it
|
||||
*replaces* the smartbox law while armed. Armed/disarmed by
|
||||
`SmartBox::SetOverrideFovDistance` (`0x00451bc0`), whose only callers are the
|
||||
**teleport animation** in `gmSmartBoxUI`: `EndTeleportAnimation`
|
||||
(`0x004d65d5`, disarm) and `UseTime` (`0x004d71c4`, `0x004d725f`, `0x004d73ce`
|
||||
arm; `0x004d747f` disarm), interpolating `teleportCurVDist` between the game's
|
||||
value and `TRANSITION_VIEW_PLANE_DISTANCE` over `TELEPORT_ANIM_FADE_TIME`.
|
||||
3. `Render::set_zfar` (`0x0054b320`) — re-applies the *existing* `Render::fov`
|
||||
after changing the far plane. Not an independent FOV writer.
|
||||
|
||||
acdream already ports (2) as `TeleportViewPlaneController`, and it is correct:
|
||||
|
||||
```
|
||||
0054b240 fld qword [0x7928c0] ; 1.0
|
||||
0054b246 fld dword [esp+4] ; d (ST0=d, ST1=1.0)
|
||||
0054b24a fpatan ; atan(ST1/ST0) = atan(1/d)
|
||||
0054b24c fadd st(0), st(0) ; fov = 2*atan(1/d)
|
||||
0054b252 fcomp dword [0x7ca9f0] ; 0.4f
|
||||
0054b25d jp -> znear = 0.1f ; d < 0.4
|
||||
0054b263 fmul dword [0x7c8a04] ; else znear = d * 0.25
|
||||
; then clamp fov to [0.001 (0x794720), pi (0x7bdd30)] and call SetFOVInternal
|
||||
```
|
||||
|
||||
BN renders line 342123 as `__fpatan(arg1, 1.0)`, which reads as `atan(d)` and would
|
||||
have made the port's `2f * MathF.Atan(1f / distance)` look wrong. The bytes show
|
||||
x87 `fpatan` computes `atan(ST1/ST0)` = **`atan(1/d)`** — **the port is right and
|
||||
the decomp text is the misleading party.** `near = MathF.Max(0.1f, distance*0.25f)`
|
||||
is likewise equivalent to retail's `d < 0.4 ? 0.1 : d*0.25` at every input.
|
||||
|
||||
Residual (pre-existing, not #389): acdream's world cameras use a 5000 m far plane
|
||||
where `Render::zfar` defaults to 4000 (`0x0081ec88`); `PortalTunnelCamera` already
|
||||
documents 4000.
|
||||
|
||||
---
|
||||
|
||||
## Findings, ranked
|
||||
|
||||
### M1 — REGISTER/DOC (highest): "the viewport aspect" is a preference-scaled quantity
|
||||
|
||||
`RetailFieldOfView`'s class doc and the commit message both describe the divisor's
|
||||
left operand as the viewport aspect ratio. Retail's `m_ViewportAspectRatio` for the
|
||||
game view is `(w/h) × m_DisplayAspectRatio × 0.75`, where `m_DisplayAspectRatio` is
|
||||
driven by the `Render.AspectRatio` preference (`Normal`→4:3, widescreen→16:9,
|
||||
otherwise the display's own ratio). At the **registered default (1)** the factor is
|
||||
exactly 1.0f and acdream matches retail bit-for-bit; at the widescreen setting
|
||||
retail's effective aspect is 4/3 larger and the vertical FOV correspondingly
|
||||
narrower. acdream implements no aspect-ratio preference.
|
||||
|
||||
*Why it matters:* #389 is the commit that made this field load-bearing, and the
|
||||
project rule is that a commit introducing a deviation carries its register row.
|
||||
**Action:** add a divergence-register row ("acdream feeds the raw framebuffer
|
||||
aspect; retail scales it by the `Render.AspectRatio` preference — identical at
|
||||
retail's default, divergent if that preference is ever implemented") and correct
|
||||
the one-line description in `RetailFieldOfView`'s doc to name
|
||||
`ComputeAspectForViewport @0x0054f150` as the definition. No behaviour change.
|
||||
|
||||
### M2 — NOTE: FOV changes do not take effect until next launch
|
||||
|
||||
Retail's option applies within one frame (`SceneTool::PrepareGraphicsDevice` →
|
||||
`Render::UpdateFromPreferences` → `SetDefaultFov`; the smartbox re-reads
|
||||
`m_fGameFOV` every `RenderNormalMode`). acdream applies it via `ApplyStartup` only
|
||||
(live only during a Config draft preview). Pre-existing and documented in
|
||||
`DisplaySettings`/`ConfigOptionsPageController`, but the semantics change in this
|
||||
commit makes the lag more visible (the slider now changes the *shape* of the view,
|
||||
not just its zoom). Worth an issue or a register row; not a #389 defect.
|
||||
|
||||
### M3 — NOTE: NaN corner of the gate is inverted
|
||||
|
||||
Retail's first test is `test ah,0x41 / jnp`: an unordered compare sets both C0 and
|
||||
C3 → even parity → **not** taken, and the second test's `test ah,1` sees C0 set →
|
||||
**accepted**. So retail would pass a NaN FOV through to `SetFOVInternal`. acdream's
|
||||
`fovY > 0f && fovY < MathF.PI` rejects NaN. Reaching it requires `m_fGameFOV` NaN or
|
||||
an exact `0/0`, neither of which the option range permits. Record it, do not
|
||||
"fix" it — rejecting NaN is strictly safer and unreachable.
|
||||
|
||||
### M4 — DOC nit: the "~85–90° horizontal" claim
|
||||
|
||||
At the 90° default the law yields horizontal FOV ≈ **89.0°** (4:3), **83.9°**
|
||||
(16:9), **80.6°** (21:9). The class doc says "~85–90°"; the test asserts
|
||||
`[80°, 90°]` (correct). Suggest "~80–89°, near-constant" in the doc so the doc and
|
||||
the test agree.
|
||||
|
||||
### Verified-good (no action)
|
||||
|
||||
- `AspectBias = 0.100000001f` ≡ `0x3dcccccd`. Exact.
|
||||
- `DefaultGameFovRadians = 1.57079637f` ≡ the `0x00454649` ctor literal. Exact.
|
||||
- Golden test values recomputed from the bytes: 4:3 → 1.27362 rad, 16:9 → 0.93624 rad,
|
||||
21:9 → 0.70334 rad (test asserts 0.70327 at `precision: 4` — both round to 0.7033).
|
||||
- `DollCamera` / `CreatureAppraisalPresentation` at π/4 match the byte-decoded
|
||||
`CreatureMode::m_fFOVRadians` default `0x3f490fdb`.
|
||||
- Startup applies the real framebuffer aspect before the first frame.
|
||||
- `TeleportViewPlaneController`'s `2·atan(1/d)` and `max(0.1, 0.25d)` are correct
|
||||
against the bytes, contradicting the BN text.
|
||||
|
||||
---
|
||||
|
||||
## Appendix — primary citations
|
||||
|
||||
| Address | Pseudo-C line | Symbol / meaning |
|
||||
|---|---|---|
|
||||
| `0x00452af0`–`0x00452b2f` | 91717–91727 | `CreatureMode::Render` FOV branch (fixed vs smartbox vs vdst) |
|
||||
| `0x00453ae6`–`0x00453b14` | 92655–92660 | `SmartBox::RenderNormalMode` smartbox site (no smartbox-flag gate) |
|
||||
| `0x00452380` | 91348 | `CreatureMode::UseSmartboxFOV` — sets the flag to 1 (only writer) |
|
||||
| `0x00452390` | 91356 | `CreatureMode::UseSharpMode` |
|
||||
| `0x004a5aa9` / `0x004e0440` / `0x004ee9bc` | 175534 / 228563 / 241997 | paperdoll + char-gen viewports (fixed branch) |
|
||||
| `0x004d6db3` | 219378 | `gmSmartBoxUI::PostInit` — portal space takes the smartbox law |
|
||||
| `0x004543de` | (in 93102 ctor) | `m_fFOVRadians = 0x3f490fdb` = π/4 (byte-decoded) |
|
||||
| `0x00451be0`–`0x00451c18` | 90792–90806 | `SmartBox::GetOverrideFovDistance` = `cot(fov/2)` |
|
||||
| `0x00451bc0` | 90783 | `SmartBox::SetOverrideFovDistance` |
|
||||
| `0x00451e60`/`0x00451e6a` | 90988–90991 | `SmartBox::SetDefaultFov` — degrees → radians |
|
||||
| `0x00454649` | 93302 | `SmartBox` ctor `m_fGameFOV = 1.57079637f` |
|
||||
| `0x0054b240` | 342121 | `Render::set_vdst` — `fov = 2·atan(1/d)`, `znear` rule, `[0.001, π]` clamp |
|
||||
| `0x0054b2d0` | 342158 | `Render::SetFOVRad` — open `(0, π)` gate, silent reject |
|
||||
| `0x0054b340` | 342196 | `Render::SetFOVInternal` — `vdst = ty / tan(fov/2)` (vertical) |
|
||||
| `0x0059ab40` | 423666 | `PrimD3DRender::SetFOVInternal` → `D3DXMatrixPerspectiveFovLH(fovy, aspect, …)` |
|
||||
| `0x0054f150` | 345708 | `RenderDevice::ComputeAspectForViewport` — `(w/h)·DAR·0.75` when `arg6 == 0` |
|
||||
| `0x0054f1c0`/`0x0054f22e` | 345720/345749 | `RenderDevice::SetViewport` — writes `m_ViewportAspectRatio` (`+0xa8`) |
|
||||
| `0x0059fbd0` | 428476 | `RenderDeviceD3D::SetupDisplayAspectRatio` — preference → `m_DisplayAspectRatio` |
|
||||
| `0x0054d850`/`0x0054d999` | 344244/344365 | `Render::UpdateFromPreferences` → `SetDefaultFov` |
|
||||
| `0x0043e4f0` | 69105 | `SceneTool::PrepareGraphicsDevice` → `UpdateFromPreferences` (per-frame) |
|
||||
| `0x004035b0`/`0x004043b2` | 2740/3261 | `gmClient::InitUIPreferences` — `SetPreferenceRange(10, 160)` |
|
||||
| `0x0081efb8` / `0x0081efa8` | 1102208 ff. | `m_RenderPrefs.FieldOfView = 90`, `.AspectRatio = 1` |
|
||||
| `0x004d65d5`, `0x004d71c4`, `0x004d725f`, `0x004d73ce`, `0x004d747f` | 219002–219775 | teleport-animation FOV override arm/disarm |
|
||||
|
||||
Constants byte-read from the paired binary: `[0x7948cc] = 0.1f (0x3dcccccd)`,
|
||||
`[0x79b6dc] = 0.75f`, `[0x795344] = 0.0f`, `[0x7bdd30] = π (f64)`,
|
||||
`[0x794720] = 0.001 (f64)`, `[0x7928c0] = 1.0 (f64)`, `[0x7ca9f0] = 0.4f`,
|
||||
`[0x7c8a04] = 0.25f`.
|
||||
|
|
@ -341,6 +341,9 @@ internal sealed class SessionPlayerCompositionPhase
|
|||
// CH6c: null when no retained UI exists (e.g. a no-window host) — the
|
||||
// Chat tab's opacity sliders then apply through NullRuntimeChatOpacityTarget.
|
||||
chatOpacity: interaction.RetainedUi?.Runtime.WindowOpacity,
|
||||
// #389 blast MUST-FIX 2: the live Field-of-View apply target —
|
||||
// see RuntimeSettingsTargets' cameras ctor doc.
|
||||
cameras: host.CameraController,
|
||||
log: d.Log,
|
||||
// OP6: null on a no-audio/headless host — ApplyAudio then
|
||||
// silently no-ops, same shape as chatOpacity above.
|
||||
|
|
|
|||
|
|
@ -9,11 +9,28 @@ namespace AcDream.App.Rendering;
|
|||
/// <code>
|
||||
/// Render::SetFOVRad(m_fGameFOV / (RenderDevice::m_ViewportAspectRatio - 0.1))
|
||||
/// </code>
|
||||
/// (<c>CreatureMode</c> smartbox sites <c>0x00452b2f</c> and <c>0x00453b14</c>;
|
||||
/// the same expression feeds a <c>tan</c> at <c>0x00451c0e</c>). The net
|
||||
/// effect: the HORIZONTAL view stays roughly constant (~85–90° of world at the
|
||||
/// 90° default) across aspect ratios while wide screens trim the vertical
|
||||
/// slice — 4:3 ≈ 73° vertical, 16:9 ≈ 53.6°, 21:9 ≈ 40°.
|
||||
/// (<c>CreatureMode</c> smartbox site <c>0x00452b2f</c> and
|
||||
/// <c>SmartBox::RenderNormalMode @0x00453b14</c> — the latter has NO
|
||||
/// <c>m_bUseSmartboxFOV</c> gate at all, so the world view is unconditionally
|
||||
/// on this law; the same expression feeds a <c>tan</c> at <c>0x00451c0e</c>).
|
||||
/// The applied value is decisively the VERTICAL FOV: the projection consumer
|
||||
/// is <c>D3DXMatrixPerspectiveFovLH</c> with it in the fovy slot
|
||||
/// (<c>0x0059ab71</c>; instruction-byte-verified by the 2026-08-13 mechanism
|
||||
/// review — the BN text FPU-elides every comparison in this area). The net
|
||||
/// effect: the HORIZONTAL view stays roughly constant across aspect ratios
|
||||
/// (89.0° at 4:3, 83.9° at 16:9, 80.6° at 21:9, at the 90° default) while
|
||||
/// wide screens trim the vertical slice — 4:3 ≈ 73° vertical,
|
||||
/// 16:9 ≈ 53.6°, 21:9 ≈ 40°.
|
||||
///
|
||||
/// <para>Aspect nuance (mechanism review M1, register row AD-90): retail's
|
||||
/// divisor aspect is not raw width/height —
|
||||
/// <c>RenderDevice::ComputeAspectForViewport @0x0054f150</c> yields
|
||||
/// <c>(w/h) × m_DisplayAspectRatio × 0.75</c>, where
|
||||
/// <c>m_DisplayAspectRatio</c> comes from the registered
|
||||
/// <c>Render.AspectRatio</c> preference. At that preference's DEFAULT
|
||||
/// (4:3 → factor exactly 1.0f) the expression collapses to raw w/h, which is
|
||||
/// what acdream uses; acdream has no AspectRatio preference, so a retail
|
||||
/// user who changed it would see a framing this port does not reproduce.</para>
|
||||
///
|
||||
/// <para><c>m_fGameFOV</c> is the user-facing number: ctor default
|
||||
/// <c>1.57079637f</c> = π/2 = 90° (<c>0x00454649</c>), set in DEGREES by the
|
||||
|
|
|
|||
|
|
@ -286,6 +286,7 @@ internal sealed class RuntimeSettingsTargets : IRuntimeSettingsTargets
|
|||
private readonly ICommandBus _commands;
|
||||
private readonly Action<string> _log;
|
||||
private readonly OpenAlAudioEngine? _audio;
|
||||
private readonly CameraController? _cameras;
|
||||
|
||||
public RuntimeSettingsTargets(
|
||||
IRuntimeDisplayWindowTarget displayWindow,
|
||||
|
|
@ -301,7 +302,14 @@ internal sealed class RuntimeSettingsTargets : IRuntimeSettingsTargets
|
|||
// ApplyAudio's doc. Optional/trailing so every pre-existing
|
||||
// construction site keeps compiling unchanged (matches
|
||||
// chatOpacity/log's own optional-trailing shape).
|
||||
OpenAlAudioEngine? audio = null)
|
||||
OpenAlAudioEngine? audio = null,
|
||||
// #389 blast-review MUST-FIX 2: retail's FOV preference applies LIVE
|
||||
// (Render::GRPCallback_OnRenderPreferenceChanged @0x0054d999 →
|
||||
// SmartBox::SetDefaultFov → m_fGameFOV, re-read by the smartbox
|
||||
// sites every render) — the saved Field of View must reach the
|
||||
// cameras on Save, not on the next launch. Null on hosts with no
|
||||
// camera graph (headless / fixture callers).
|
||||
CameraController? cameras = null)
|
||||
: this(
|
||||
displayWindow,
|
||||
new RuntimeQualityApplicationTarget(
|
||||
|
|
@ -317,7 +325,8 @@ internal sealed class RuntimeSettingsTargets : IRuntimeSettingsTargets
|
|||
chatOpacity is null
|
||||
? NullRuntimeChatOpacityTarget.Instance
|
||||
: new RuntimeChatOpacityTarget(chatOpacity),
|
||||
audio)
|
||||
audio,
|
||||
cameras)
|
||||
{
|
||||
}
|
||||
|
||||
|
|
@ -328,7 +337,8 @@ internal sealed class RuntimeSettingsTargets : IRuntimeSettingsTargets
|
|||
ICommandBus commands,
|
||||
Action<string>? log = null,
|
||||
IRuntimeChatOpacityTarget? chatOpacity = null,
|
||||
OpenAlAudioEngine? audio = null)
|
||||
OpenAlAudioEngine? audio = null,
|
||||
CameraController? cameras = null)
|
||||
{
|
||||
_displayWindow = displayWindow
|
||||
?? throw new ArgumentNullException(nameof(displayWindow));
|
||||
|
|
@ -338,10 +348,20 @@ internal sealed class RuntimeSettingsTargets : IRuntimeSettingsTargets
|
|||
_commands = commands ?? throw new ArgumentNullException(nameof(commands));
|
||||
_log = log ?? Console.WriteLine;
|
||||
_audio = audio;
|
||||
_cameras = cameras;
|
||||
}
|
||||
|
||||
public void ApplyDisplayWindowState(DisplaySettings display) =>
|
||||
public void ApplyDisplayWindowState(DisplaySettings display)
|
||||
{
|
||||
_displayWindow.Apply(display);
|
||||
// #389 blast MUST-FIX 2 (see the ctor's cameras doc): the Field of
|
||||
// View applies live on Save, from the update-phase save handler —
|
||||
// deliberately NOT from the render-phase preview seam
|
||||
// (WorldRenderFrameBuilder.Apply), whose mid-frame camera mutation
|
||||
// is the review's WATCH-3 cull-vs-raster landmine.
|
||||
if (_cameras is not null)
|
||||
RuntimeSettingsStartupTargets.ApplyFieldOfView(_cameras, display.FieldOfView);
|
||||
}
|
||||
|
||||
/// <summary>Campaign OP slice OP6: reuses the SAME static helper the
|
||||
/// startup path (<see cref="RuntimeSettingsStartupTargets.ApplyAudio"/>)
|
||||
|
|
|
|||
|
|
@ -575,10 +575,11 @@ public static class ConfigOptionsPageController
|
|||
storeOnly: true, // TS-74
|
||||
rangeLowKey: "ID_Graphics_Value_Slow", rangeHighKey: "ID_Graphics_Value_Fast");
|
||||
|
||||
// Field of View: NEXT-LAUNCH via DisplaySettings.FieldOfView + the
|
||||
// existing RuntimeSettingsController.ApplyStartup path — matches
|
||||
// the pre-existing (dev-tools Settings panel era) behaviour, not a
|
||||
// new gap this slice introduces.
|
||||
// Field of View: LIVE as of the #389 fix round — the stored value is
|
||||
// retail's m_fGameFOV in degrees and applies on Save through
|
||||
// RuntimeSettingsTargets.ApplyDisplayWindowState (retail applies its
|
||||
// FOV preference live too: Render::GRPCallback_OnRenderPreferenceChanged
|
||||
// @0x0054d999 → SmartBox::SetDefaultFov).
|
||||
BuildSliderRow(
|
||||
listBox, RangedSliderTemplateIndex, "ID_Graphics_FieldOfView",
|
||||
min: 10f, max: 160f, defaultValue: 90.0f, page, resolveString,
|
||||
|
|
|
|||
|
|
@ -20,10 +20,14 @@ public readonly record struct UiWindowPosition(float X, float Y);
|
|||
/// <see cref="Input.KeyBindings.LoadOrDefault"/> path.
|
||||
///
|
||||
/// <para>
|
||||
/// Schema (current version 2):
|
||||
/// Schema (current version 3 — v3 changed the MEANING of
|
||||
/// <c>display.fieldOfView</c>: it was the applied vertical FOV in degrees,
|
||||
/// it is now retail's <c>m_fGameFOV</c> in degrees (#389, the SmartboxFOV
|
||||
/// port). <see cref="LoadDisplay"/> migrates pre-v3 values on read — see
|
||||
/// <see cref="MigrateLegacyVerticalFovDegrees"/>):
|
||||
/// <code>
|
||||
/// {
|
||||
/// "version": 2,
|
||||
/// "version": 3,
|
||||
/// "display": { "resolution": "1920x1080", "fullscreen": false, ... }
|
||||
/// "windowLayouts": { "Character": { "1920x1080": { "chat": { ... } } } }
|
||||
/// }
|
||||
|
|
@ -39,7 +43,7 @@ public readonly record struct UiWindowPosition(float X, float Y);
|
|||
/// </summary>
|
||||
public sealed class SettingsStore
|
||||
{
|
||||
private const int CurrentSchemaVersion = 2;
|
||||
private const int CurrentSchemaVersion = 3;
|
||||
private readonly string _path;
|
||||
|
||||
public SettingsStore(string path)
|
||||
|
|
@ -66,11 +70,18 @@ public sealed class SettingsStore
|
|||
return DisplaySettings.Default;
|
||||
|
||||
var d = DisplaySettings.Default;
|
||||
// #389 (schema v3): a pre-v3 file's stored fieldOfView was the
|
||||
// applied vertical FOV; the same number now means retail's
|
||||
// gameFOV. Migrate on read — an absent key needs none (it falls
|
||||
// back to the already-v3 default).
|
||||
float fieldOfView = ReadFloat(disp, "fieldOfView", d.FieldOfView);
|
||||
if (ReadSchemaVersion(root) < 3 && disp.TryGetProperty("fieldOfView", out _))
|
||||
fieldOfView = MigrateLegacyVerticalFovDegrees(fieldOfView);
|
||||
return new DisplaySettings(
|
||||
Resolution: ReadString (disp, "resolution", d.Resolution),
|
||||
Fullscreen: ReadBool (disp, "fullscreen", d.Fullscreen),
|
||||
VSync: ReadBool (disp, "vsync", d.VSync),
|
||||
FieldOfView: ReadFloat (disp, "fieldOfView", d.FieldOfView),
|
||||
FieldOfView: fieldOfView,
|
||||
Gamma: ReadFloat (disp, "gamma", d.Gamma),
|
||||
ShowFps: ReadBool (disp, "showFps", d.ShowFps),
|
||||
Quality: ReadQuality (disp, "quality", d.Quality),
|
||||
|
|
@ -94,6 +105,38 @@ public sealed class SettingsStore
|
|||
}
|
||||
}
|
||||
|
||||
/// <summary>The file's top-level schema version; a file predating the
|
||||
/// version key reads as 1 (the same convention the v1 radar-layout
|
||||
/// migration below already uses).</summary>
|
||||
private static int ReadSchemaVersion(JsonElement root) =>
|
||||
root.TryGetProperty("version", out var v) && v.ValueKind == JsonValueKind.Number
|
||||
? v.GetInt32()
|
||||
: 1;
|
||||
|
||||
/// <summary>
|
||||
/// #389 blast-review MUST-FIX 1 — the v3 fieldOfView migration. The
|
||||
/// pre-v3 number was the applied VERTICAL FOV in degrees (written
|
||||
/// straight onto the cameras); the v3 number is retail's
|
||||
/// <c>m_fGameFOV</c> in degrees, run through the smartbox law. Two
|
||||
/// cases:
|
||||
/// - exactly the old default 60 → the new default 90: 60 was acdream's
|
||||
/// own invented constant, not a user preference, and mapping it to
|
||||
/// retail's registered default gives every untouched file the retail
|
||||
/// experience;
|
||||
/// - anything else was a DELIBERATE choice → preserve the user's
|
||||
/// visible framing at the era's default 16:9 aspect:
|
||||
/// gameFOV = vFOV × (16/9 − 0.1) (the smartbox law inverted at that
|
||||
/// aspect), clamped into the slider's registered [10,160].
|
||||
/// Runs on READ for any pre-v3 file; the next save stamps version 3 and
|
||||
/// the value stops migrating.
|
||||
/// </summary>
|
||||
internal static float MigrateLegacyVerticalFovDegrees(float legacyVerticalFovDegrees)
|
||||
{
|
||||
if (legacyVerticalFovDegrees == 60f)
|
||||
return 90f;
|
||||
return Math.Clamp(legacyVerticalFovDegrees * (16f / 9f - 0.1f), 10f, 160f);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Save Display settings, preserving any other top-level keys the file
|
||||
/// already contains (e.g. an <c>audio</c> section written by a newer
|
||||
|
|
|
|||
|
|
@ -29,9 +29,11 @@ public sealed class RetailFieldOfViewTests
|
|||
[Fact]
|
||||
public void Law_HoldsTheHorizontalViewRoughlyConstant()
|
||||
{
|
||||
// The point of the smartbox shape: horizontal FOV stays ~85–90°
|
||||
// across every aspect at the 90° default, instead of ballooning on
|
||||
// wide screens the way a fixed vertical FOV does.
|
||||
// The point of the smartbox shape: horizontal FOV stays within
|
||||
// 80–90° across every aspect at the 90° default (89.0° at 4:3,
|
||||
// 83.9° at 16:9, 80.6° at 21:9 — mechanism review M4's exact
|
||||
// numbers), instead of ballooning on wide screens the way a fixed
|
||||
// vertical FOV does.
|
||||
foreach (float aspect in new[] { 4f / 3f, 16f / 9f, 21f / 9f })
|
||||
{
|
||||
Assert.True(RetailFieldOfView.TryAppliedVerticalFov(
|
||||
|
|
|
|||
|
|
@ -154,6 +154,31 @@ public sealed class RuntimeSettingsControllerTests
|
|||
Assert.Equal(expectedFov, cameras.Fly.FovY, precision: 5);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void RuntimeTarget_ApplyDisplayWindowState_AppliesFieldOfViewLive()
|
||||
{
|
||||
// #389 blast-review MUST-FIX 2: retail's FOV preference applies LIVE
|
||||
// (Render::GRPCallback_OnRenderPreferenceChanged @0x0054d999) — a
|
||||
// Save must reach the cameras through the same seam that resizes the
|
||||
// window, not wait for the next launch.
|
||||
var cameras = new CameraController(new OrbitCamera(), new FlyCamera());
|
||||
var target = new RuntimeSettingsTargets(
|
||||
new InspectingDisplayWindowTarget(static _ => { }),
|
||||
new RecordingQualityApplicationTarget([]),
|
||||
new RecordingUiLockTarget([]),
|
||||
NullCommandBus.Instance,
|
||||
static _ => { },
|
||||
cameras: cameras);
|
||||
|
||||
target.ApplyDisplayWindowState(
|
||||
DisplaySettings.Default with { FieldOfView = 120f });
|
||||
|
||||
Assert.Equal(120f * (MathF.PI / 180f), cameras.GameFovRadians, precision: 5);
|
||||
Assert.True(RetailFieldOfView.TryAppliedVerticalFov(
|
||||
120f * (MathF.PI / 180f), 16f / 9f, out float expectedFov));
|
||||
Assert.Equal(expectedFov, cameras.Orbit.FovY, precision: 5);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void ConcreteRuntimeTargetAppliesEveryQualityDimensionInOrder()
|
||||
{
|
||||
|
|
|
|||
|
|
@ -87,6 +87,82 @@ public sealed class SettingsStoreTests : System.IDisposable
|
|||
Assert.Equal(ParticleRange.Extended, loaded.ParticleRange);
|
||||
}
|
||||
|
||||
// ── #389 schema-v3 fieldOfView migration ────────────────────────────
|
||||
// Pre-v3 the stored number was the applied vertical FOV in degrees;
|
||||
// v3 makes it retail's m_fGameFOV. LoadDisplay migrates pre-v3 files
|
||||
// on read (blast-review MUST-FIX 1).
|
||||
|
||||
[Theory]
|
||||
// The untouched old default (acdream's invented 60) → the retail
|
||||
// registered default 90.
|
||||
[InlineData(60f, 90f)]
|
||||
// A deliberate old choice preserves its visible 16:9 framing:
|
||||
// gameFOV = vFOV × (16/9 − 0.1). 45 × 1.67778 = 75.5.
|
||||
[InlineData(45f, 75.5f)]
|
||||
// Clamped into the slider's registered [10,160]: 120 × 1.67778 = 201.3.
|
||||
[InlineData(120f, 160f)]
|
||||
public void LoadDisplay_migrates_pre_v3_fieldOfView(float stored, float expected)
|
||||
{
|
||||
File.WriteAllText(_tempPath, $$"""
|
||||
{
|
||||
"version": 2,
|
||||
"display": { "fieldOfView": {{stored}} }
|
||||
}
|
||||
""");
|
||||
var store = new SettingsStore(_tempPath);
|
||||
|
||||
Assert.Equal(expected, store.LoadDisplay().FieldOfView, precision: 1);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void LoadDisplay_does_not_migrate_a_v3_fieldOfView()
|
||||
{
|
||||
// A post-migration 60 is a deliberate gameFOV — it must survive.
|
||||
File.WriteAllText(_tempPath, """
|
||||
{
|
||||
"version": 3,
|
||||
"display": { "fieldOfView": 60 }
|
||||
}
|
||||
""");
|
||||
var store = new SettingsStore(_tempPath);
|
||||
|
||||
Assert.Equal(60f, store.LoadDisplay().FieldOfView);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void LoadDisplay_absent_fieldOfView_needs_no_migration()
|
||||
{
|
||||
// The per-field fallback is DisplaySettings.Default (already v3
|
||||
// semantics, 90) — migrating it would corrupt the default.
|
||||
File.WriteAllText(_tempPath, """
|
||||
{
|
||||
"version": 2,
|
||||
"display": { "resolution": "1920x1080" }
|
||||
}
|
||||
""");
|
||||
var store = new SettingsStore(_tempPath);
|
||||
|
||||
Assert.Equal(90f, store.LoadDisplay().FieldOfView);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void SaveDisplay_stamps_v3_so_the_migration_runs_once()
|
||||
{
|
||||
File.WriteAllText(_tempPath, """
|
||||
{
|
||||
"version": 2,
|
||||
"display": { "fieldOfView": 60 }
|
||||
}
|
||||
""");
|
||||
var store = new SettingsStore(_tempPath);
|
||||
|
||||
DisplaySettings migrated = store.LoadDisplay(); // 60 → 90
|
||||
store.SaveDisplay(migrated); // stamps version 3
|
||||
|
||||
// A second load must NOT re-migrate the already-migrated 90.
|
||||
Assert.Equal(90f, store.LoadDisplay().FieldOfView);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void LoadDisplay_invalid_particle_range_falls_back_to_default()
|
||||
{
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue