From e56aa5115cc73d0e7e1eed37f8447c557c95e5dd Mon Sep 17 00:00:00 2001 From: Erik Date: Thu, 13 Aug 2026 17:49:17 +0200 Subject: [PATCH] fix #376+#388: real fullscreen mode switching, state-aware display apply Slice 5+6 of the display block, one coherent unit (they share the state machine the goal's dual review covers). GlfwDisplayModeSwitcher (#376) ports retail's fullscreen semantics - Device::ForceDisplayResolution @gmClient::Init 0x004047af is a REAL video-mode change - through native glfwSetWindowMonitor on the same IWindow.Native.Glfw handle path #348's cursor cache proved. Primary monitor (retail's primary display device); refresh = the monitor's highest for the picked WxH; the windowed placement is remembered for the exit path; every failure is a no-throw (bool, reason) result. SilkRuntimeDisplayWindowTarget.Apply (#388) becomes the state-aware machine: fullscreen target = validated native mode switch (mode must be in #391's DisplayModeCatalog - an offered mode is supported by construction, making the "Graphics mode not supported" crash class unreachable from the dropdown); windowed target while fullscreen = the native exit (which sets the client size itself); plain windowed pick = the proven #387 size write. A raw Size write NEVER happens against a fullscreen window - on GLFW that is a video-mode request, and an unsupported one was the exact unhandled-GlfwException that killed the user's 2026-08-13 session. The old Silk borderless WindowState path is deleted from the apply. New IWindowedSizeSurface narrows the window dependency so the machine is unit-testable (FakePacingSurface idiom). Live-verified on this machine (goal-sanctioned automated run): display: fullscreen mode switch 1920x1080@300 -> framebuffer resize event 1920x1080 -> vulkan: swapchain recreated 1920x1080 ok=True -> graceful close, desktop mode restored. Tests: 5 state-machine facts (validated switch/never-size-write, unoffered refusal, failed-switch usability, native exit, plain windowed write). App suite 4,972/3 skips. Gate script sections D4-D6 written (black-screen-risk steps flagged). Dual Opus review of the pair follows as its own round. Co-Authored-By: Claude Fable 5 --- docs/ISSUES.md | 30 +++- .../2026-08-13-display-block-test-script.md | 46 ++++- .../Settings/DisplayModeSwitching.cs | 163 ++++++++++++++++++ .../Settings/RuntimeSettingsTargets.cs | 106 ++++++++++-- .../RuntimeSettingsControllerTests.cs | 133 ++++++++++++++ 5 files changed, 451 insertions(+), 27 deletions(-) create mode 100644 src/AcDream.App/Settings/DisplayModeSwitching.cs diff --git a/docs/ISSUES.md b/docs/ISSUES.md index df963fc8..9fb720a9 100644 --- a/docs/ISSUES.md +++ b/docs/ISSUES.md @@ -124,7 +124,22 @@ until the port lands. ## #388 — CRASH: unhandled GlfwException "Failed to set video mode: Graphics mode not supported" during a fullscreen-state resolution/settings apply; and fullscreen/maximized windows silently ignore resolution picks -**Status:** OPEN — filed 2026-08-13 from the user's live gate session +**Status:** DONE 2026-08-13 (this commit, with #376) — display block slices +5+6, pending the user's physical-display gate and the pair's dual Opus +review. `SilkRuntimeDisplayWindowTarget.Apply` is now the state-aware +machine: a fullscreen target is a VALIDATED native mode switch +(`GlfwDisplayModeSwitcher.TryEnterFullscreen` — mode must be in the #391 +catalog, refresh = the monitor's highest for that WxH); a windowed target +while fullscreen leaves via the native exit (which sets the client size +itself); a raw `Size` write NEVER happens against a fullscreen window (on +GLFW that is a video-mode request — this crash's mechanism); every failure +is a logged no-throw with the window left usable. Live-verified on this +machine: `display: fullscreen mode switch 1920x1080@300` → +`vulkan: swapchain recreated 1920x1080` → graceful exit, desktop restored. +The old Silk borderless `WindowState` path is deleted from the apply. +Original filing below. + +**Original filing:** OPEN — filed 2026-08-13 from the user's live gate session (log: scratchpad `rdp-verify.log` / task b7rd8zkd4, exit 29 after an unhandled `Silk.NET.GLFW.GlfwException`). Two distinct facts from the same session, both in the #376/#377 fullscreen family: @@ -611,7 +626,18 @@ fixed). ## #376 — Fullscreen resolution picks cannot switch the display mode (Silk API limit; needs native glfwSetWindowMonitor) -**Status:** OPEN — filed 2026-08-11, split from #374's investigation. +**Status:** DONE 2026-08-13 (this commit, with #388) — display block slice +5, pending the user's physical-display gate. `GlfwDisplayModeSwitcher` +ports retail's `Device::ForceDisplayResolution` semantics through native +`glfwSetWindowMonitor` (the same `IWindow.Native.Glfw` handle path #348's +cursor cache proved; primary monitor, matching retail's primary display +device), validated against the #391 mode catalog before any attempt, with +the monitor's highest refresh rate for the picked WxH and the windowed +placement remembered for the exit path. Live-verified: a real +1920x1080@300 mode switch, swapchain following, graceful restore. +Original filing below. + +**Original filing:** OPEN — filed 2026-08-11, split from #374's investigation. While FULLSCREEN, the visible resolution is the display's video mode, and Silk's abstract windowing API cannot change it: `IViewProperties.VideoMode` is read-only, and Silk fullscreen is diff --git a/docs/research/2026-08-13-display-block-test-script.md b/docs/research/2026-08-13-display-block-test-script.md index 16c5d660..268c1b7d 100644 --- a/docs/research/2026-08-13-display-block-test-script.md +++ b/docs/research/2026-08-13-display-block-test-script.md @@ -67,14 +67,46 @@ saving anything until you save). immediately under the current resolution; the automatic clamp/reload on a resolution change never writes anything). -## §D4–§D6 — Fullscreen family (#377/#376/#388) — PENDING SLICES +## §D4 — Fullscreen startup (#377) -*(To be filled in when the fullscreen slices land. Will include: fullscreen -on/off via the Config checkbox, resolution picks while fullscreen becoming -REAL display-mode switches — **[BLACK-SCREEN RISK]** steps will be flagged: -have Alt+F4 / the physical power of patience ready — and the #388 crash -scenario surviving: a fullscreen-state settings apply must never take the -client down.)* +1. **F11 → Config → tick Full Screen, then close the client and relaunch.** + The client must come up fullscreen at your saved resolution, no crash + (the old startup access-violation is 3/3-clean on current code, plus one + automated fullscreen boot verified live). + +## §D5 — Real display-mode switching (#376) **[BLACK-SCREEN RISK]** + +Every step here changes your monitor's actual video mode — the panel will +blank/re-sync for a moment each time. If a switch ever leaves the screen +black for more than ~5 seconds, Alt+F4 closes the client gracefully and +the desktop mode restores. + +1. **Windowed at 1920x1080 → tick Full Screen.** The display itself + switches to 1920x1080 (at your monitor's highest refresh for that mode — + this panel reports 300 Hz): everything gets larger-grained, the monitor + re-syncs. This is retail's fullscreen, not a stretched borderless + window. Automated evidence for this step already exists + (`display: fullscreen mode switch 1920x1080@300` → + `vulkan: swapchain recreated 1920x1080`). +2. **While fullscreen, pick a different resolution** (e.g. 2560x1440). The + DISPLAY switches modes again — a real re-sync, crisp at the new mode, + never a scaled image. +3. **Untick Full Screen.** The desktop returns to its native mode and the + client becomes a normal window at your picked resolution, positioned + where it was before entering fullscreen. + +## §D6 — No settings apply may crash the client (#388) + +1. **Repeat your original crash recipe:** while fullscreen, change + resolution a few times quickly, toggle Full Screen off and on, drag + sliders in between. The client must never die — any refused/failed + switch logs a `display: ... failed/refused` line and leaves you in a + usable window instead. +2. The old crash's exact mechanism (a window-resize applied to a + fullscreen window becoming an unsupported video-mode request) is now + structurally impossible: fullscreen applies only validated modes from + the dropdown's own list, and size writes never touch a fullscreen + window. --- diff --git a/src/AcDream.App/Settings/DisplayModeSwitching.cs b/src/AcDream.App/Settings/DisplayModeSwitching.cs new file mode 100644 index 00000000..4776923d --- /dev/null +++ b/src/AcDream.App/Settings/DisplayModeSwitching.cs @@ -0,0 +1,163 @@ +using System; +using Silk.NET.GLFW; +using Silk.NET.Windowing; + +namespace AcDream.App.Settings; + +/// +/// #376/#388: the fullscreen display-mode seam. Retail's fullscreen +/// resolution switch is a REAL video-mode change +/// (Device::ForceDisplayResolution, gmClient::Init @0x004047af); +/// Silk's abstract fullscreen is desktop-mode borderless and its +/// VideoMode is read-only, so entering/leaving a mode goes through +/// native glfwSetWindowMonitor. The interface exists so the +/// state-aware apply in is +/// unit-testable with a fake. +/// +internal interface IDisplayModeSwitcher +{ + /// True while the window is a native fullscreen window (has a + /// monitor attached). + bool IsFullscreen { get; } + + /// Switches the window to exclusive fullscreen at the given + /// mode. False (with a reason) instead of throwing on any failure — + /// a settings apply must never crash the client (#388). + bool TryEnterFullscreen(int width, int height, out string? error); + + /// Returns to a windowed window at the given client size. + /// False (with a reason) instead of throwing. + bool TryLeaveFullscreen(int width, int height, out string? error); +} + +/// +/// The native GLFW implementation. Uses the SAME handle path the #348 +/// cursor cache proved (IWindow.Native.Glfw) and the primary +/// monitor — retail's ForceDisplayResolution likewise drove the +/// primary display device. The refresh rate for a mode is the highest the +/// monitor reports for that WxH (retail passed the device's mode as-is). +/// Callers validate the requested WxH against DisplayModeCatalog +/// BEFORE calling — an offered mode is supported by construction, which is +/// what makes the old "Graphics mode not supported" crash class +/// unreachable from the dropdown. +/// +internal sealed unsafe class GlfwDisplayModeSwitcher : IDisplayModeSwitcher +{ + private readonly IWindow _window; + private (int X, int Y) _windowedPosition = (60, 60); + + public GlfwDisplayModeSwitcher(IWindow window) + { + _window = window ?? throw new ArgumentNullException(nameof(window)); + } + + public bool IsFullscreen + { + get + { + WindowHandle* handle = Handle(); + if (handle is null) return false; + return Glfw.GetApi().GetWindowMonitor(handle) is not null; + } + } + + public bool TryEnterFullscreen(int width, int height, out string? error) + { + error = null; + WindowHandle* handle = Handle(); + if (handle is null) + { + error = "no native GLFW window handle"; + return false; + } + + try + { + Glfw glfw = Glfw.GetApi(); + Silk.NET.GLFW.Monitor* monitor = glfw.GetPrimaryMonitor(); + if (monitor is null) + { + error = "no primary monitor"; + return false; + } + + if (!TryFindRefreshRate(glfw, monitor, width, height, out int refresh)) + { + error = $"mode {width}x{height} is not in the monitor's mode list"; + return false; + } + + if (glfw.GetWindowMonitor(handle) is null) + { + // Remember the windowed placement so leaving fullscreen can + // restore it (GLFW does not remember it for us). + glfw.GetWindowPos(handle, out int x, out int y); + _windowedPosition = (x, y); + } + + glfw.SetWindowMonitor(handle, monitor, 0, 0, width, height, refresh); + Console.WriteLine( + $"display: fullscreen mode switch {width}x{height}@{refresh}"); + return true; + } + catch (GlfwException ex) + { + // #388: a failed switch is a logged failure the caller reverts + // from, never a crash. Validation makes this path exceptional. + error = ex.Message; + return false; + } + } + + public bool TryLeaveFullscreen(int width, int height, out string? error) + { + error = null; + WindowHandle* handle = Handle(); + if (handle is null) + { + error = "no native GLFW window handle"; + return false; + } + + try + { + Glfw glfw = Glfw.GetApi(); + if (glfw.GetWindowMonitor(handle) is null) + return true; // already windowed + glfw.SetWindowMonitor( + handle, null, + _windowedPosition.X, _windowedPosition.Y, + width, height, 0); + Console.WriteLine( + $"display: left fullscreen to windowed {width}x{height}"); + return true; + } + catch (GlfwException ex) + { + error = ex.Message; + return false; + } + } + + /// The monitor's highest refresh rate for an exact WxH, or + /// false when the monitor does not report the mode at all. + private static bool TryFindRefreshRate( + Glfw glfw, Silk.NET.GLFW.Monitor* monitor, int width, int height, out int refresh) + { + refresh = 0; + Silk.NET.GLFW.VideoMode* modes = glfw.GetVideoModes(monitor, out int count); + if (modes is null) return false; + for (int i = 0; i < count; i++) + { + if (modes[i].Width == width && modes[i].Height == height) + refresh = Math.Max(refresh, modes[i].RefreshRate); + } + return refresh > 0; + } + + private WindowHandle* Handle() + { + nint native = _window.Native?.Glfw ?? 0; + return native == 0 ? null : (WindowHandle*)native; + } +} diff --git a/src/AcDream.App/Settings/RuntimeSettingsTargets.cs b/src/AcDream.App/Settings/RuntimeSettingsTargets.cs index b69d459c..6683ba48 100644 --- a/src/AcDream.App/Settings/RuntimeSettingsTargets.cs +++ b/src/AcDream.App/Settings/RuntimeSettingsTargets.cs @@ -1,3 +1,4 @@ +using System.Linq; using AcDream.App.Audio; using AcDream.App.Net; using AcDream.App.Rendering; @@ -44,20 +45,105 @@ internal interface IRuntimeChatOpacityTarget void Apply(float defaultOpacity, float activeOpacity); } +/// The one window property the display apply touches — a narrow +/// seam so the #388 state machine is testable without faking all of +/// (same idiom as FakePacingSurface's +/// surface). +internal interface IWindowedSizeSurface +{ + Vector2D Size { get; set; } +} + +internal sealed class SilkWindowSizeSurface(IWindow window) : IWindowedSizeSurface +{ + private readonly IWindow _window = window + ?? throw new ArgumentNullException(nameof(window)); + + public Vector2D Size + { + get => _window.Size; + set => _window.Size = value; + } +} + internal sealed class SilkRuntimeDisplayWindowTarget : IRuntimeDisplayWindowTarget { - private readonly IWindow _window; + private readonly IWindowedSizeSurface _window; + private readonly IDisplayModeSwitcher _modeSwitcher; + private readonly Func _isOfferedMode; public SilkRuntimeDisplayWindowTarget(IWindow window) + : this( + new SilkWindowSizeSurface(window), + new GlfwDisplayModeSwitcher(window), + // #391's catalog is the validation source: an offered mode is + // supported by construction. With no catalog installed + // (fixture/uninitialised hosts) nothing is "offered", so + // fullscreen entry is refused rather than guessed. + spec => Rendering.DisplayModeCatalog.Resolutions?.Contains(spec) == true) { - _window = window ?? throw new ArgumentNullException(nameof(window)); } + internal SilkRuntimeDisplayWindowTarget( + IWindowedSizeSurface window, + IDisplayModeSwitcher modeSwitcher, + Func isOfferedMode) + { + _window = window ?? throw new ArgumentNullException(nameof(window)); + _modeSwitcher = modeSwitcher + ?? throw new ArgumentNullException(nameof(modeSwitcher)); + _isOfferedMode = isOfferedMode + ?? throw new ArgumentNullException(nameof(isOfferedMode)); + } + + /// + /// #388: the state-aware display apply. Windowed target = a window + /// resize (the proven #387 chain); fullscreen target = a validated + /// native display-mode switch (#376, retail's + /// Device::ForceDisplayResolution semantics via + /// glfwSetWindowMonitor). A raw Size write NEVER happens + /// against a fullscreen window — on GLFW that is a video-mode request + /// and an unsupported one killed the client mid-session (the 2026-08-13 + /// gate crash). Every failure path logs and leaves the window in a + /// usable state instead of throwing. + /// public void Apply(DisplaySettings display) { ArgumentNullException.ThrowIfNull(display); bool haveResolution = TryParseResolution(display.Resolution, out int width, out int height); + + if (display.Fullscreen) + { + if (!haveResolution) + return; + if (!_isOfferedMode.Invoke($"{width}x{height}")) + { + Console.WriteLine( + $"display: fullscreen {width}x{height} refused — not an offered mode"); + return; + } + if (!_modeSwitcher.TryEnterFullscreen(width, height, out string? error)) + Console.WriteLine( + $"display: fullscreen {width}x{height} failed ({error}) — staying windowed"); + return; + } + + // Windowed target: leave fullscreen first if needed (the native exit + // sets the client size itself), otherwise plain window resize. + if (_modeSwitcher.IsFullscreen) + { + if (!haveResolution) + { + width = _window.Size.X; + height = _window.Size.Y; + } + if (!_modeSwitcher.TryLeaveFullscreen(width, height, out string? error)) + Console.WriteLine( + $"display: leaving fullscreen failed ({error})"); + return; + } + if (haveResolution && (_window.Size.X != width || _window.Size.Y != height)) { // #387 evidence line (permanent): the resolution-pick write path. @@ -66,22 +152,6 @@ internal sealed class SilkRuntimeDisplayWindowTarget : IRuntimeDisplayWindowTarg $"(window was {_window.Size.X}x{_window.Size.Y})"); _window.Size = new Vector2D(width, height); } - - WindowState desired = display.Fullscreen - ? WindowState.Fullscreen - : WindowState.Normal; - - // #374 investigation note: while FULLSCREEN, the visible resolution - // is the display's video mode, and Silk's abstract windowing API - // cannot change it (IViewProperties.VideoMode is read-only; Silk - // fullscreen is desktop-mode borderless). The Size write above is - // therefore only visible in windowed mode — a resolution pick while - // fullscreen changes what a later return to windowed restores, not - // the fullscreen mode itself. Retail's own fullscreen resolution - // switch (Device::ForceDisplayResolution) needs a native - // glfwSetWindowMonitor port — issue #376. - if (_window.WindowState != desired) - _window.WindowState = desired; } internal static bool TryParseResolution( diff --git a/tests/AcDream.App.Tests/Settings/RuntimeSettingsControllerTests.cs b/tests/AcDream.App.Tests/Settings/RuntimeSettingsControllerTests.cs index 92c30c74..a955de6f 100644 --- a/tests/AcDream.App.Tests/Settings/RuntimeSettingsControllerTests.cs +++ b/tests/AcDream.App.Tests/Settings/RuntimeSettingsControllerTests.cs @@ -154,6 +154,139 @@ public sealed class RuntimeSettingsControllerTests Assert.Equal(expectedFov, cameras.Fly.FovY, precision: 5); } + // ── #376/#388: the state-aware display apply ──────────────────────── + + private sealed class FakeSizeSurface : IWindowedSizeSurface + { + public Silk.NET.Maths.Vector2D Size { get; set; } = new(1280, 720); + public int Writes { get; private set; } + Silk.NET.Maths.Vector2D IWindowedSizeSurface.Size + { + get => Size; + set { Size = value; Writes++; } + } + } + + private sealed class FakeModeSwitcher : IDisplayModeSwitcher + { + public bool IsFullscreen { get; set; } + public bool EnterSucceeds { get; set; } = true; + public List Calls { get; } = []; + + public bool TryEnterFullscreen(int width, int height, out string? error) + { + Calls.Add($"enter:{width}x{height}"); + error = EnterSucceeds ? null : "injected failure"; + if (EnterSucceeds) IsFullscreen = true; + return EnterSucceeds; + } + + public bool TryLeaveFullscreen(int width, int height, out string? error) + { + Calls.Add($"leave:{width}x{height}"); + error = null; + IsFullscreen = false; + return true; + } + } + + [Fact] + public void DisplayApply_FullscreenPick_IsAValidatedModeSwitch_NeverASizeWrite() + { + // #388: a raw Size write on a fullscreen GLFW window is a video-mode + // request — the crash class from the 2026-08-13 gate session. + var surface = new FakeSizeSurface(); + var switcher = new FakeModeSwitcher(); + var target = new SilkRuntimeDisplayWindowTarget( + surface, switcher, spec => spec == "1920x1080"); + + target.Apply(DisplaySettings.Default with + { + Fullscreen = true, + Resolution = "1920x1080", + }); + + Assert.Equal(["enter:1920x1080"], switcher.Calls); + Assert.Equal(0, surface.Writes); + } + + [Fact] + public void DisplayApply_UnofferedFullscreenMode_IsRefused_NotAttempted() + { + // #376: validation against the offered catalog makes "Graphics mode + // not supported" unreachable from the dropdown. + var surface = new FakeSizeSurface(); + var switcher = new FakeModeSwitcher(); + var target = new SilkRuntimeDisplayWindowTarget( + surface, switcher, _ => false); + + target.Apply(DisplaySettings.Default with + { + Fullscreen = true, + Resolution = "1234x777", + }); + + Assert.Empty(switcher.Calls); + Assert.Equal(0, surface.Writes); + } + + [Fact] + public void DisplayApply_FailedModeSwitch_LeavesTheWindowUsable() + { + // #388: a failed switch is logged and the client stays windowed — + // never a throw out of a settings apply. + var surface = new FakeSizeSurface(); + var switcher = new FakeModeSwitcher { EnterSucceeds = false }; + var target = new SilkRuntimeDisplayWindowTarget( + surface, switcher, _ => true); + + target.Apply(DisplaySettings.Default with + { + Fullscreen = true, + Resolution = "1920x1080", + }); + + Assert.False(switcher.IsFullscreen); + Assert.Equal(0, surface.Writes); + } + + [Fact] + public void DisplayApply_WindowedWhileFullscreen_LeavesViaTheSwitcher() + { + var surface = new FakeSizeSurface(); + var switcher = new FakeModeSwitcher { IsFullscreen = true }; + var target = new SilkRuntimeDisplayWindowTarget( + surface, switcher, _ => true); + + target.Apply(DisplaySettings.Default with + { + Fullscreen = false, + Resolution = "1600x900", + }); + + Assert.Equal(["leave:1600x900"], switcher.Calls); + Assert.Equal(0, surface.Writes); // the native exit sets the size itself + } + + [Fact] + public void DisplayApply_PlainWindowedPick_IsTheProvenSizeWrite() + { + var surface = new FakeSizeSurface(); + var switcher = new FakeModeSwitcher(); + var target = new SilkRuntimeDisplayWindowTarget( + surface, switcher, _ => true); + + target.Apply(DisplaySettings.Default with + { + Fullscreen = false, + Resolution = "1600x900", + }); + + Assert.Empty(switcher.Calls); + Assert.Equal(1, surface.Writes); + Assert.Equal(new Silk.NET.Maths.Vector2D(1600, 900), surface.Size); + } + [Fact] public void RuntimeTarget_ApplyDisplayWindowState_AppliesFieldOfViewLive() {