acdream/docs/research/2026-08-13-376-388-blast-review.md
Erik ec2a7b0cce fix #376/#388 review round: post-condition truth, idempotence, one
position memory, unified monitor, maximized restore; AD-92

Dual-lens Opus review of e56aa511 (reports committed under
docs/research/). The consolidated corrections:

- Mechanism M1 (load-bearing): on Windows, Silk's GLFW error callback
  QUEUES exceptions on a static list instead of throwing - they detonate
  later at window close, which is exactly #388's original two-stage
  crash shape. catch(GlfwException) was dead code here and a failed
  SetWindowMonitor "succeeded". Success is now judged by the NATIVE
  POST-CONDITION (GetWindowMonitor after the call) on both enter and
  exit; the catches remain only for the throwing platforms.
- M2 (both lenses): same-mode fullscreen re-apply is a no-op BEFORE any
  native work (new IDisplayModeSwitcher.CurrentFullscreenMode). Every
  Display-backed Config row applies per change - sliders per DRAG TICK -
  so without this every tick while fullscreen re-issued a real
  display-mode change.
- M3/M5 (both): the remembered windowed placement is process state (two
  target instances exist - startup and live-save); a fullscreen boot now
  exits through either instance to the real placement, not the (60,60)
  literal.
- M4 (both): the switcher resolves the WINDOW'S monitor (attached
  monitor when fullscreen, else IWindow.Monitor's index into the GLFW
  array - the same monitor DisplayModeCatalog enumerated), primary only
  as a last resort; the offered-list/switch-target mismatch is gone.
- Blast M2b: the offered-mode validator falls back to the SAME static
  ladder the dropdown falls back to - Full Screen is no longer a
  permanent silent no-op on catalog-less hosts (the switcher's own
  monitor-mode-list check remains the hard guard).
- Blast M3: a windowed pick on a MAXIMIZED window restores it first
  (Size writes are silently ignored while maximized; the deleted
  WindowState=Normal write used to do this incidentally). New
  IWindowedSizeSurface.IsMaximized/Restore.
- Mechanism M5: no silent bail-outs - the unparseable-resolution
  fullscreen path logs, and the failure line no longer claims "staying
  windowed" when the state is unchanged (#392 noted inline).
- Q1 nit: one cached Glfw wrapper (per-call GetApi allocated + took a
  native refcount); IsFullscreen/CurrentFullscreenMode guarded.
- AD-92: highest-refresh-for-WxH + refuse-and-log versus retail's
  pass-through-and-error ForceDisplayResolution.

Known-open tail, filed not hidden: #392 (persisted-flag divergence on a
refused enter - needs an apply-result seam); the mechanism report's
pacing-refresh WATCH rides the same seam.

Tests: +3 (same-mode no-op, unparseable-while-fullscreen refusal,
maximized restore-before-write). App suite 4,975/3 skips.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-08-13 18:10:07 +02:00

497 lines
26 KiB
Markdown
Raw Permalink Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

# Blast-radius review — `e56aa511` (#376 + #388: real fullscreen mode switching, state-aware display apply)
**Reviewer lens:** BLAST RADIUS (who else touches the changed surfaces, and who
read the values they used to write). Report only — no edits made.
**Date:** 2026-08-13
**Base:** `e56aa511`, worktree `.claude/worktrees/eloquent-hugle-42119e`,
branch `claude/latest-commits-cb0c8f`.
The commit is correct about the thing it set out to fix: a raw `Size` write
against a fullscreen GLFW window is genuinely gone, the mode switch is genuinely
native, and the failure paths are genuinely no-throw. Everything below is about
the *other* edges those two changes now sit on.
---
## Verdict summary
| # | Surface | Verdict |
|---|---|---|
| M1 | Every Config-tab display row re-runs the fullscreen switch; enter has no idempotence guard; sliders apply per drag tick | **MUST-FIX** |
| M2 | Validation source ≠ switch target (catalog monitor vs primary monitor; UI fallback vs validator fallback) | **MUST-FIX** |
| M3 | #388's *maximized* half is unfixed, and the only un-maximize in the codebase was deleted — but the issue is marked DONE | **MUST-FIX** |
| M4 | Refused/failed enter leaves `fullscreen: true` persisted + checkbox ticked, with no surface but a console line | **MUST-FIX** |
| M5 | Two `GlfwDisplayModeSwitcher` instances → the windowed-position memory is dead on the boot-fullscreen exit path (gate §D5.3 asserts the opposite) | **MUST-FIX** |
| M6 | No divergence-register row for the deviations this commit introduces | **MUST-FIX (process)** |
| W1 | Pacing monitor-refresh read happens *before* the switch, and `StateChanged` (the signal the deleted `WindowState` write used to fire) is now dead | WATCH |
| W2 | `IsFullscreen` is the one unguarded native call in a "never throws" class | WATCH |
| W3 | Windowed exit with no parseable resolution falls back to the *fullscreen* size | WATCH |
| W4 | `glfwSetWindowMonitor` dispatches Silk callbacks synchronously, inside the quiescence lock, inside the update phase | WATCH (verified safe, undocumented) |
| W5 | Hard-kill while fullscreen now leaves a real display-mode change to Windows to unwind | WATCH |
| W6 | `DisplayModeCatalog.InstallFromWindow` is unguarded, and a silent non-install disables fullscreen entirely | WATCH |
| A1 | Startup `ApplyDisplay` ordering vs `InstallFromWindow` | **CLEAN** |
| A2 | Readers of `_window.WindowState` across `src/` | **CLEAN** (there are none) |
| A3 | `RetailWindowLayoutPersistence` resolution key shared fullscreen/windowed | **CLEAN** (correct: same client extent) |
| A4 | #390 edge-detector sequence for enter and exit | **CLEAN** |
| A5 | Tests / `InspectingDisplayWindowTarget` | **CLEAN** |
| A6 | Headless / no-window hosts | **CLEAN** (never reach this code) |
---
## Axis 1 — callers of `SilkRuntimeDisplayWindowTarget` / `IRuntimeDisplayWindowTarget`
Exactly **two production construction sites**, both in `AcDream.App`, both
inside `GameWindow.OnLoad`'s single composition pipeline run:
| Site | Path | Instance lifetime |
|---|---|---|
| `src/AcDream.App/Rendering/GameWindow.cs:1296` | startup — `RuntimeSettingsStartupTargets``ApplyDisplay` | process |
| `src/AcDream.App/Composition/SessionPlayerComposition.cs:334` | live save — `RuntimeSettingsTargets``ApplyDisplayWindowState` | process (composition runs once at `OnLoad`, not per generation) |
Consumers of the interface: `RuntimeSettingsStartupTargets.ApplyDisplay`
(`RuntimeSettingsTargets.cs:194`) and
`RuntimeSettingsTargets.ApplyDisplayWindowState` (`RuntimeSettingsTargets.cs:424`).
No headless / no-window host reaches either — `AcDream.Headless` references only
`AcDream.Runtime`, and `SessionPlayerDependencies.Window` is a non-nullable
`IWindow` supplied only by `GameWindow` (`SessionPlayerComposition.cs:41`,
`GameWindow.cs:1445`). **CLEAN.**
### Callers relying on the OLD behaviour
- *"`Size` write while fullscreen persists a windowed size for later."* No
consumer depended on this. A resolution pick while fullscreen is now a real
mode switch, and the later windowed exit takes its size from
`display.Resolution` (`RuntimeSettingsTargets.cs:141`), which is the same
value the old `Size` write would have parked. **CLEAN.**
- *"`WindowState.Fullscreen` borderless."* Nothing in `src/` reads window state
(see Axis 2). **CLEAN** — with one exception, the *maximized* case, which is
M3 below.
### M5 — two switcher instances, one live position memory
`GlfwDisplayModeSwitcher._windowedPosition` (`DisplayModeSwitching.cs:47`) is
per-instance and defaults to `(60, 60)`. It is written only inside
`TryEnterFullscreen` (`DisplayModeSwitching.cs:90-96`).
Boot-fullscreen flow:
1. Phase 3 (`SettingsDevToolsComposition.cs:53``ApplyStartup`
`ApplyDisplay`) enters fullscreen through the **startup** target's switcher.
That instance captures the real windowed position.
2. Phase 8 (`SessionPlayerComposition.cs:334`) builds the **live** target with a
fresh switcher whose `_windowedPosition` is still `(60, 60)`.
3. User unticks Full Screen → live path → `TryLeaveFullscreen` restores to
`(60, 60)` at the fullscreen resolution.
On a 1920×1080 desktop that is a 1920×1080 client area placed at (60,60) — title
bar and bottom edge off-screen. The gate script's own acceptance text for §D5
step 3 says the client must return "positioned where it was before entering
fullscreen"; for the §D4 boot-fullscreen path it will not.
Cheapest correct fix: one shared switcher (or one shared placement record) rather
than one per target instance.
---
## Axis 2 — readers of `_window.WindowState`
`grep '\.WindowState|WindowState\.'` over `src/**/*.cs` returns **zero** reads of
`IWindow.WindowState` outside the deleted lines. Nothing keys window chrome,
screenshots, input mapping, or the L1 capability probes off it:
- Screenshots: `PrivatePresentationRenderer.cs:81` takes `input.ViewportWidth/Height`,
seeded from `_window.Size` at `GameWindow.cs:1575`. Size-driven, not state-driven.
- Input: cursor mode is driven by camera mode only
(`CameraPointerInputController.cs:445-447`); nothing consults fullscreen.
- #348's native cursor cache takes the GLFW window handle
(`GameWindow.cs:1026`), which `glfwSetWindowMonitor` does not change.
- L1 probes (`GraphicalHostPlatformServices`, the bindless/MDI/SSBO gates) read
no window state.
**CLEAN** on the read side. The one consumer of the *event*:
### W1 — the `StateChanged` signal is now dead, and pacing reads the monitor too early
`SilkWindowCallbackBinding.cs:153` wires `IWindow.StateChanged`
`DisplayFramePacingController.OnWindowStateChanged`
(`DisplayFramePacingController.cs:132`) → `RefreshActiveMonitor()`. Silk raises
`StateChanged` from its own `WindowState` setter and its iconify/maximize
callbacks; a native `glfwSetWindowMonitor` bypasses all of them. So the deleted
`_window.WindowState = Fullscreen` write was, incidentally, the thing that
re-read the active monitor's refresh rate after a fullscreen transition. That is
gone.
Compounding it, the startup path refreshes the monitor **before** the switch:
```
RuntimeSettingsTargets.cs:194-201
_pacing.RefreshActiveMonitor(); // desktop mode, e.g. 60 Hz
_pacing.ApplyPreference(display.VSync);
_displayWindow.Apply(display); // switches the display to 1920x1080@300
```
and the live path (`ApplyDisplayWindowState`, `RuntimeSettingsTargets.cs:424-434`)
never touches pacing at all. With VSync **off** and capped rendering,
`FramePacingPolicy.Resolve` (`FramePacingPolicy.cs:26-29`) caps the software
pacer at the cached Hz — which after a fullscreen enter is the pre-switch desktop
refresh. The only thing that rescues it is the incidental `Move` callback
(`SilkWindowCallbackBinding.cs:152`) firing because the window jumped to the
monitor origin — which does not fire if the window was already at that position.
Rated WATCH, not MUST-FIX, because VSync defaults on (`DisplaySettings.cs:92`)
and the VSync branch ignores the refresh value. Fix is one line: refresh the
active monitor *after* `_displayWindow.Apply`, on both paths.
### W2 — `IsFullscreen` can throw out of a "never throws" class
`DisplayModeSwitching.cs:54-62` calls `Glfw.GetApi().GetWindowMonitor(handle)`
with no `try`, while both `Try*` methods around it are wrapped. The class doc
promises "every failure is a no-throw (bool, reason) result". `Apply` calls
`IsFullscreen` on the windowed branch (`RuntimeSettingsTargets.cs:134`), which is
the branch that runs on every single Config-tab save. Low probability
(`glfwGetWindowMonitor` on a live handle does not error), but the contract is
stated and not honoured.
---
## Axis 3 — `DisplaySettings.Fullscreen` consumers
Complete reader set (`grep Fullscreen --include=*.cs src`):
| Reader | Site |
|---|---|
| Startup apply | `RuntimeSettingsTargets.cs:116` (via `ApplyDisplay`, `:199`) |
| Live apply | `RuntimeSettingsTargets.cs:116` (via `ApplyDisplayWindowState`, `:426`) |
| Config tab "Full Screen" row | `ConfigOptionsPageController.cs:650-651` |
| Persistence | `SettingsStore.cs:82` (read), `:640` (write) |
`RetailWindowLayoutPersistence` does **not** read it — see A3.
### Fullscreen boot path, end-to-end
1. `GameWindow.Run` creates the window unconditionally at 1280×720
(`GameWindow.cs:752-754`).
2. `OnLoad``DisplayModeCatalog.InstallFromWindow(_window!)`
(`GameWindow.cs:1246`).
3. Composition phase 3 builds `SilkRuntimeDisplayWindowTarget(_window!)`
(`GameWindow.cs:1296`) and `SettingsDevToolsCompositionPhase.Compose` runs
`ApplyStartup` (`SettingsDevToolsComposition.cs:53`) →
`RuntimeSettingsController.ApplyStartup:216``ApplyDisplay`
`Apply(display)`.
4. Fullscreen branch → catalog check → `TryEnterFullscreen`
`glfwSetWindowMonitor`.
5. GLFW dispatches the framebuffer-size callback synchronously; the viewport
target arms `VulkanGraphicsContext.RequestRecreate`
(`VulkanHostInputCameraCompositionFactory.cs:136`) and the camera aspect
updates (`FramebufferResizeController.cs:125-127`). Both targets are already
bound by phase 2, so the mid-`OnLoad` event lands on live owners.
Functionally sound. The problem is what happens when step 4 says no.
### M4 — flag/reality divergence has no surface
The Config toggle persists first, applies second
(`ConfigOptionsPageController.cs:651``RuntimeSettingsController.cs:386-392`).
Both refusal paths (`RuntimeSettingsTargets.cs:120-125` "not an offered mode",
`:126-128` "failed") only `Console.WriteLine`. Consequences:
- `settings.json` keeps `fullscreen: true` while the client is windowed.
- The checkbox reads `bindings.LoadDisplay().Fullscreen`
(`ConfigOptionsPageController.cs:650`) → renders **ticked** against a windowed
client.
- Every subsequent launch refuses again. The client boots windowed forever with
the box ticked and no visible explanation. The user's only feedback is a
console line they will not see in a normal launch.
This matters because the refusal is *reachable in ordinary use*, not just in
pathological setups — see M2. At minimum the refusal should reset the persisted
flag (or the row should read live state rather than the stored flag) so that what
the panel shows is what the client is.
Note the interaction with #377's documented recovery ("hand-edit settings.json
back to false"): the new refusal path re-creates exactly that stuck-flag shape,
minus the crash.
---
## Axis 4 — the #390 interaction (`2153bee2`)
`RetailUiRuntime.Draw` (`RetailUiRuntime.cs:620-652`) is a two-step edge detector
on the per-frame screen size. That size is `_window.Size`, captured once per
render at `GameWindow.cs:1575` and threaded through
`RenderFrameInput.ViewportWidth/Height`
`PrivatePresentationRenderer.cs:72-76``RetainedGameplayUiFrame.Render`
`Draw`.
**Enter (1280×720 → 1920×1080).** The toggle is handled in the *update* phase
(this is the same seam the #389 review deliberately chose over the render-phase
preview seam — `RuntimeSettingsTargets.cs:427-433`), so the native switch
completes before the frame's render:
- frame N render: `_window.Size` = 1920×1080 ≠ `_lastScreenSize` → one
`ClampAllToScreen()` (`RetailWindowLayoutPersistence.cs:102-129`, no store I/O,
`_restoring` suppresses per-move saves), `_screenSizeSettling = true`.
- frame N+1: size repeats → one `RestoreAll(saveBack: false)` for key
`"1920x1080"`.
One clamp, one reload, no transient size (the intermediate 1280×720 frame never
renders — the switch is synchronous within the update). **CLEAN.**
**Exit.** `TryLeaveFullscreen` is called with the *same* `display.Resolution`
(`RuntimeSettingsTargets.cs:141`) — unticking Full Screen does not change the
resolution setting. So the screen size does not change, the edge detector sees
nothing, and there is no clamp/reload. Correct, and consistent: the layout key is
unchanged. **CLEAN.**
**A3 — the shared layout key.** `ResolutionKey` is `"{W}x{H}"` with no fullscreen
discriminator (`RetailWindowLayoutPersistence.cs:304-305`). Fullscreen 1920×1080
and windowed 1920×1080 have the *same client extent*, so sharing the key is the
right answer, not a collision. **CLEAN.**
**No storm risk from the detector itself** — but see M1, which can produce a
native-call storm upstream of it.
---
## Axis 5 — tests
- No test in `tests/` asserts the old `WindowState` write. The only `WindowState`
hits are `ChatWindowState`, `RetainedWindowState`, `CreateWindowState`, and
`DisplayFramePacingControllerTests.cs:65`'s `OnWindowStateChanged(default)`
smoke call — none coupled to the display apply.
- `InspectingDisplayWindowTarget`
(`tests/AcDream.App.Tests/Settings/RuntimeSettingsControllerTests.cs:1072`) is a
fake `IRuntimeDisplayWindowTarget`; the interface is unchanged, so all 7 uses
stay valid.
- `GameWindowSlice8BoundaryTests.cs:298/331` asserts on `GameWindow.cs` source
text only — unaffected.
- The 5 new state-machine facts
(`RuntimeSettingsControllerTests.cs:191-283`) cover exactly the five branches
and are honest tests (they assert `surface.Writes == 0` on the fullscreen paths,
which is the real invariant).
**CLEAN.** Two gaps worth noting rather than fixing-as-bugs:
- No test pins the **idempotence** of a repeated fullscreen apply (M1). A
`FakeModeSwitcher` already records `Calls`; asserting `["enter:1920x1080"]`
after two identical applies would have caught it.
- No test covers `Fullscreen = true` + unparseable resolution
(`RuntimeSettingsTargets.cs:118-119`, silent return).
---
## Axis 6 — the catalog dependency and construction order
**A1 — ordering: CLEAN.** Two facts make it safe:
1. The offered-mode check is a **lazy** lambda, not a captured snapshot:
`spec => Rendering.DisplayModeCatalog.Resolutions?.Contains(spec) == true`
(`RuntimeSettingsTargets.cs:83`). Construction order relative to
`InstallFromWindow` is therefore irrelevant; only *apply* order matters.
2. `DisplayModeCatalog.InstallFromWindow(_window!)` is at `GameWindow.cs:1246`,
before `GameWindowCompositionPipeline.Run` at `:1249`; the startup apply is
phase 3 (`SettingsDevToolsComposition.cs:53`). Install strictly precedes apply.
A legitimate fullscreen boot is **not** refused for ordering reasons. But it can
be refused for two other reasons, which is the real finding:
### M2 — "an offered mode is supported by construction" is false along two axes
The commit's central safety claim is that validating against `DisplayModeCatalog`
makes the unsupported-mode crash class unreachable. That holds only if the
offered set is a subset of the *switch target's* mode list. Two ways it isn't:
**(a) Different monitors.** The catalog is curated from `window.Monitor` — the
monitor the window currently sits on (`DisplayModeCatalog.cs:53`). The switch
targets `glfwGetPrimaryMonitor()` (`DisplayModeSwitching.cs:77`), and the refresh
lookup enumerates *that* monitor's modes (`DisplayModeSwitching.cs:144-156`). On a
multi-monitor machine with the client on a secondary display, a mode that is
offered (secondary supports it) can be absent from the primary's list →
`"mode WxH is not in the monitor's mode list"` → silent refusal (M4). It fails
safe rather than crashing, but the invariant the design leans on is not there.
**(b) Different fallbacks at the two seams.** When no catalog installs
(`window.Monitor` null, or `Curate` returns empty — `DisplayModeCatalog.cs:54,68`):
- the dropdown falls back to the 6-entry static ladder
(`RetailUiRuntime.cs:2429``ConfigOptionsPageController.cs:641`
`availableResolutions ?? DisplaySettings.AvailableResolutions`), so the user
sees six selectable resolutions;
- the validator falls back to **nothing offered**
(`RuntimeSettingsTargets.cs:83`, `Resolutions?.Contains(...) == true` on null →
`false`).
So on any host where the catalog does not install, the Full Screen checkbox is a
permanent silent no-op while the dropdown still advertises six modes. The two
seams must share one fallback.
**(c) The likely everyday case.** `DisplaySettings.Default.Resolution` is
`"1280x720"` (`DisplaySettings.cs:90`). If the monitor does not enumerate a
1280×720 mode (common on high-refresh panels that expose a short mode list), a
fresh user who simply ticks Full Screen at the default resolution gets nothing —
no fullscreen, no message. Likewise any settings.json carrying a pre-#391
resolution (`800x600`, `1024x768` — both selectable in earlier builds, per the
OP6 note at `ConfigOptionsPageController.cs:625-633`) now boots windowed forever
with the flag ticked.
---
## M1 — the fullscreen enter is not idempotent, and every Config row re-runs it
This is the highest-blast-radius finding, because it comes in through a surface
the commit never looked at.
`storeOnly` on the Config rows is **only a caption-dim marker**
(`ConfigOptionsPageController.cs:136-152`) — it does not suppress the apply.
Every Graphics row's `apply:` lambda calls `bindings.SaveDisplay(...)`
(`ConfigOptionsPageController.cs:587, 643, 651, 659, 671, 678, 685, 693, 738,
746, 754, 766, 774, 780`), which routes
`RuntimeSettingsController.SaveDisplay:386-392`
`ApplyDisplayWindowState``SilkRuntimeDisplayWindowTarget.Apply`.
And `Apply`'s fullscreen branch has **no idempotence guard**
(`RuntimeSettingsTargets.cs:116-129`) — unlike the windowed branch, which still
has `if (haveResolution && (_window.Size.X != width || ...))` at `:147`. The old
code had `if (_window.WindowState != desired)` at the deleted `:83`; that guard
was removed with the rest of the block and nothing replaced it.
So, while `fullscreen: true`:
- toggling *any* store-only graphics row (MultiPassAlpha, BuildingDetailTextures,
LandscapeDrawDistance, TextureFiltering, …) fires a full
`glfwSetWindowMonitor(handle, primary, 0, 0, W, H, refresh)`;
- **dragging a slider** does it *per drag tick*. `FloatOptionRow.SetCurrentValue`
is documented as "the drag-tick entry point… applies it live IMMEDIATELY"
(`OptionPageModel.cs:232-240`), and `slider.ScalarChanged` calls it directly
(`ConfigOptionsPageController.cs:1044`). Field of View (`:587`), Screen
Brightness (`:671`) and Degrade Distance (`:693`) are all sliders on this tab.
Per drag tick that means: a full `glfwGetVideoModes` enumeration
(`DisplayModeSwitching.cs:148`), a `glfwSetWindowMonitor`, and one
`display: fullscreen mode switch WxH@R` console line
(`DisplayModeSwitching.cs:99-100`) — a permanent evidence line turned into a
per-frame log storm.
What saves it from being a visible mode-change storm on Windows is GLFW's
*internal* early-out (`setVideoModeWin32` returns before `ChangeDisplaySettingsEx`
when the chosen mode equals the current one). Correctness of a user-visible
behaviour should not rest on an undocumented internal of a third-party library —
especially on the exact path whose failure mode was "the client dies mid-session".
The gate script's own §D6 step 1 tells the user to "drag sliders in between"
while fullscreen, so this is directly in the acceptance path.
**Fix shape:** guard the enter the way the windowed branch guards its write —
remember the last applied `(W, H, monitor)` or query the current window monitor +
mode, and return early when the request already holds.
---
## M3 — #388's maximized half is unfixed, and the only un-maximize was deleted
#388's own title and filing include the maximized case: *"fullscreen/maximized
windows silently ignore resolution picks"*, with the evidence being five
consecutive picks producing no framebuffer event at `window was 2056x1290`
(`docs/ISSUES.md:126, 158-165`). The commit marks #388 **DONE**
(`docs/ISSUES.md:127`) having addressed only the fullscreen half.
Worse, the deleted block was the codebase's *only* write of `WindowState`
(Axis 2: there is now zero). Under the new `Apply`, a maximized window takes the
`_modeSwitcher.IsFullscreen == false` path (a maximized window has no attached
monitor) and receives a plain `_window.Size = ...`
(`RuntimeSettingsTargets.cs:147-154`) — precisely the write the issue documents
as silently ignored. Nothing anywhere restores the window to `Normal` first.
So: resolution picks on a maximized window are now permanently a no-op, with the
issue closed. Either restore-then-resize in the windowed branch, or reopen the
maximized half of #388 with the evidence carried forward.
---
## M6 — missing divergence-register rows (process)
`git show e56aa511 --stat` touches no register file. CLAUDE.md's rule is binding:
*"Any commit that introduces a deviation adds its register row IN THE SAME
COMMIT."* At least two deviations ship here, both acknowledged in the code's own
comments:
1. **Refresh-rate selection.** `TryFindRefreshRate` picks the monitor's
*highest* refresh for the requested WxH (`DisplayModeSwitching.cs:142-156`),
while the class doc states "retail passed the device's mode as-is"
(`DisplayModeSwitching.cs:37-38`). Entering fullscreen therefore changes the
panel's refresh rate as a side effect of a *resolution* pick (the live run
went to 300 Hz).
2. **Silent refusal.** Retail surfaced "Graphics mode not supported"; acdream
refuses to a console line and leaves the preference inconsistent (M4).
Arguably a third: primary-monitor-only targeting (`DisplayModeSwitching.cs:77`),
which the doc justifies against retail's primary display device — that one is
probably a citation rather than a deviation, but it is the assumption M2(a)
breaks, so it deserves to be written down somewhere auditable.
IA-22 (#391's curated list) is the natural neighbour row; these are separate
deviations and need their own.
---
## Remaining WATCH items
**W3 — windowed exit with no parseable resolution uses the fullscreen size.**
`RuntimeSettingsTargets.cs:136-140`: when `display.Resolution` does not parse, the
exit falls back to `_window.Size` — which, while fullscreen, *is* the fullscreen
extent. The window then leaves fullscreen as a desktop-sized window at
`(60, 60)`. Narrow (requires a corrupt/blank resolution string), but the fallback
reads "current windowed size" and means "current fullscreen size".
**W4 — synchronous native dispatch inside the quiescence lock.** On Win32,
`glfwSetWindowMonitor` dispatches `WM_SIZE`/`WM_MOVE` synchronously, so
`FramebufferResize` and `Move` re-enter `HostQuiescenceGate.Invoke`
(`SilkWindowCallbackBinding.cs:152-154`) on the same thread while the update
callback already holds `_sync`. This is **safe**`Monitor` is re-entrant
(`HostQuiescenceGate.cs:36-54`) — and the Vulkan side only *arms* recreation
(`VulkanHostInputCameraCompositionFactory.cs:136`), never recreating mid-frame.
Both are load-bearing invariants that nothing records; a comment at the
`SetWindowMonitor` call site would keep a future change from breaking them.
Also thread-affinity: `glfwSetWindowMonitor` and `glfwGetVideoModes` are
main-thread-only. Both call paths (composition `OnLoad`, update-phase save) are
main-thread today; nothing enforces it.
**W5 — hard kill while fullscreen.** The project's documented launch workflow
falls back to `Stop-Process -Force` when `CloseMainWindow` times out (CLAUDE.md,
"Logout-before-reconnect"). Previously that left a borderless window behind;
now it abandons a real `ChangeDisplaySettingsEx` mode change for Windows to
unwind. Low risk (CDS_FULLSCREEN changes are process-scoped and revert on exit),
but worth one line in the gate script's black-screen-risk note.
**W6 — `InstallFromWindow` is unguarded and fails silently.**
`GameWindow.cs:1246` has no `try`, while `SilkDisplayFramePacingSurface` already
documents that `window.Monitor` can throw `GlfwException` during an RDP/console
display-topology handoff (`DisplayFramePacingController.cs:42-49`). A throw there
kills `OnLoad`; a silent early-return (`DisplayModeCatalog.cs:54-69`) now also
silently disables fullscreen entirely (M2b). The two failure modes deserve
different treatment, and neither is currently distinguishable in a log.
---
## Ranked must-fixes
1. **M1** — guard the fullscreen enter against a no-change request.
`src/AcDream.App/Settings/RuntimeSettingsTargets.cs:116-129`; drivers at
`src/AcDream.App/UI/Layout/OptionPageModel.cs:232-240` and
`src/AcDream.App/UI/Layout/ConfigOptionsPageController.cs:587,671,693`.
2. **M2** — make the offered set and the switch target agree: same monitor
(`DisplayModeCatalog.cs:53` vs `DisplayModeSwitching.cs:77`) and same fallback
(`RuntimeSettingsTargets.cs:83` vs `RetailUiRuntime.cs:2429`
`ConfigOptionsPageController.cs:641`).
3. **M3** — either fix the maximized half of #388 (restore-to-`Normal` before the
windowed size write, `RuntimeSettingsTargets.cs:147-154`) or reopen it;
`docs/ISSUES.md:126-127` currently claims it closed.
4. **M4** — give a refused/failed enter a surface: reset the persisted flag, or
have the Config row read live state.
`RuntimeSettingsTargets.cs:120-128`, `ConfigOptionsPageController.cs:650-651`.
5. **M5** — share one windowed-placement record between the startup and live
switchers. `DisplayModeSwitching.cs:47`, `GameWindow.cs:1296`,
`SessionPlayerComposition.cs:334`.
6. **M6** — add the divergence-register rows (refresh-rate selection; silent
refusal), `docs/architecture/retail-divergence-register.md`.