From c254fea83d7af6e6deea8c2d96870f8805040fe1 Mon Sep 17 00:00:00 2001 From: Erik Date: Thu, 20 Aug 2026 15:13:29 +0200 Subject: [PATCH] fix(app): apply the window icon from Load, not beside Window.Create The client shipped with a PE icon Explorer showed and a window that did not: launched from the launcher it still drew the stock Windows application icon. Silk's Window.Create only builds the managed object. IWindow.Initialize is what, in Silk's own words, "creates the window on the underlying platform". Applying an icon before that throws: after Window.Create : IsInitialized = False SetWindowIcon BEFORE Initialize : THREW InvalidOperationException: Window should be initialized. after Initialize : IsInitialized = True SetWindowIcon AFTER Initialize : returned without throwing What made this quiet rather than obvious is the fallback. GLFW registers its window class against a resource named GLFW_ICON and, not finding one, uses IDI_APPLICATION - the generic Windows icon - rather than the executable's own. So the PE icon kept showing on the file while the live window lost it, which reads as a packaging problem and is nothing of the kind. The launcher was unaffected because Avalonia takes a different path entirely, and that asymmetry was the tell. Apply now happens in OnLoad, beside the other window-dependent startup work, and refuses with a message naming the ordering requirement if it is ever called on an uninitialized window - the previous generic catch reported "Window should be initialized" to a stderr nobody reads, which said nothing about icons. The regression guard reads the compiled call graph, because this is an ordering edge with no observable return value: OnLoad must call Apply, and no method that calls Window.Create may. Verified by reintroducing the bug and watching it fail, then restoring the fix and watching it pass. Solution builds clean; 14,408 tests pass on the standard hermetic lane filter, 0 failures. Co-Authored-By: Claude Opus 5 --- assets/icons/README.md | 15 ++++-- src/AcDream.App/Rendering/GameWindow.cs | 12 +++-- src/AcDream.App/Rendering/WindowIconLoader.cs | 21 ++++++++ .../Rendering/WindowIconLoaderTests.cs | 54 +++++++++++++++++++ 4 files changed, 94 insertions(+), 8 deletions(-) diff --git a/assets/icons/README.md b/assets/icons/README.md index 53622cff..9ff56a21 100644 --- a/assets/icons/README.md +++ b/assets/icons/README.md @@ -78,9 +78,16 @@ Neither icon is loaded from disk at runtime. - **PE icon** — `` in each `.csproj`, pointing at the `.ico` here. This is what Explorer and the taskbar shortcut show. - **Client window icon** — `AcDream.App.Rendering.WindowIconLoader` hands GLFW - four sizes at startup. The PNGs are *embedded resources* linked from this - directory, so there is one source of truth for the art and no missing-file - case at runtime. `WindowIconLoaderTests` guards the resource names, which are - otherwise coupled to `LogicalName` in the csproj by string only. + four sizes **from the `Load` callback**. That timing is load-bearing: Silk's + `Window.Create` only builds the managed object, and `IWindow.Initialize` is + what creates the native window, so applying an icon any earlier throws + "Window should be initialized". The failure is quiet and misleading — GLFW + falls back to the stock Windows application icon rather than the + executable's, so Explorer shows the mark and the running window does not. + The PNGs are *embedded resources* linked from this directory, so there is one + source of truth for the art and no missing-file case at runtime. + `WindowIconLoaderTests` guards both the resource names, which are otherwise + coupled to `LogicalName` in the csproj by string only, and the call-site + ordering. - **Launcher window icon** — `AvaloniaResource` linked from here, referenced as `avares://acdream-launcher/Assets/acdream-launcher.png`. diff --git a/src/AcDream.App/Rendering/GameWindow.cs b/src/AcDream.App/Rendering/GameWindow.cs index bfdacb1e..b6de89db 100644 --- a/src/AcDream.App/Rendering/GameWindow.cs +++ b/src/AcDream.App/Rendering/GameWindow.cs @@ -801,10 +801,6 @@ public sealed class GameWindow : _startupQuality = startup.Quality; _window = Window.Create(options); - // Before any callback binding: the icon is pure window decoration and - // has no ordering relationship with the render loop, so it belongs at - // the earliest point the native window exists. - WindowIconLoader.Apply(_window); IWindow window = _window; _lifetime.PublishNativeWindow( window, @@ -1301,6 +1297,14 @@ public sealed class GameWindow : // windowing thread, before any Options-panel mount reads it. DisplayModeCatalog.InstallFromWindow(_window!); + // Must be here, not next to Window.Create: Silk's Window.Create only + // builds the managed object, and IWindow.Initialize is what "creates + // the window on the underlying platform". Setting an icon before that + // throws "Window should be initialized" and leaves the window with + // GLFW's fallback — which is the generic Windows application icon, not + // the executable's, so the loss is visible on every surface. + WindowIconLoader.Apply(_window!); + GameWindowCompositionPipeline.Run< GameWindowPlatformResult, HostInputCameraResult, diff --git a/src/AcDream.App/Rendering/WindowIconLoader.cs b/src/AcDream.App/Rendering/WindowIconLoader.cs index b40aa3e8..6764bae5 100644 --- a/src/AcDream.App/Rendering/WindowIconLoader.cs +++ b/src/AcDream.App/Rendering/WindowIconLoader.cs @@ -43,10 +43,31 @@ internal static class WindowIconLoader /// /// Decode the embedded icon set and hand it to the window. /// + /// + /// A window that has already been initialized. Silk's Window.Create + /// returns an uninitialized object — IWindow.Initialize is what + /// "creates the window on the underlying platform" — so this must be called + /// from Load or later. + /// public static void Apply(IWindow window) { ArgumentNullException.ThrowIfNull(window); + // Named explicitly rather than left to the catch below. Calling too + // early throws a generic InvalidOperationException whose message says + // nothing about icons, and the symptom — GLFW falling back to the + // stock Windows application icon while the executable's own PE icon + // still shows in Explorer — looks like a packaging problem rather than + // an ordering one. That misdirection already cost one shipped build. + if (!window.IsInitialized) + { + Console.Error.WriteLine( + "window icon: refusing to set an icon on an uninitialized window — " + + "call WindowIconLoader.Apply from the Load callback, not next to " + + "Window.Create."); + return; + } + RawImage[] images; try { diff --git a/tests/AcDream.App.Tests/Rendering/WindowIconLoaderTests.cs b/tests/AcDream.App.Tests/Rendering/WindowIconLoaderTests.cs index f8d88a39..3dae8745 100644 --- a/tests/AcDream.App.Tests/Rendering/WindowIconLoaderTests.cs +++ b/tests/AcDream.App.Tests/Rendering/WindowIconLoaderTests.cs @@ -1,5 +1,6 @@ using System.Reflection; using AcDream.App.Rendering; +using AcDream.App.Tests.Architecture; using SixLabors.ImageSharp; using SixLabors.ImageSharp.PixelFormats; using Xunit; @@ -76,4 +77,57 @@ public class WindowIconLoaderTests Assert.Contains(16, sizes); Assert.True(sizes.Max() >= 128, "icon set has no large size for Alt-Tab/taskbar"); } + + /// + /// Regression guard for the ordering bug that shipped once already. + /// + /// + /// Silk's Window.Create only builds the managed object; + /// IWindow.Initialize is what creates the native window. Applying an + /// icon before that throws "Window should be initialized", and GLFW then + /// falls back to the stock Windows application icon rather than the + /// executable's own — so the client shipped with a PE icon that Explorer + /// showed and the running window did not. The contract is an ordering edge + /// with no observable return value, which is exactly what + /// exists for. + /// + [Fact] + public void IconIsAppliedFromLoad_NotFromWindowConstruction() + { + var gameWindow = typeof(GameWindow); + MethodInfo? onLoad = gameWindow.GetMethod( + "OnLoad", BindingFlags.NonPublic | BindingFlags.Instance); + Assert.NotNull(onLoad); + + bool CallsApply(MethodBase method) => + CompiledCallGraph.Read(method).Any( + c => c.Target.DeclaringType == typeof(WindowIconLoader) + && c.Target.Name == nameof(WindowIconLoader.Apply)); + + Assert.True( + CallsApply(onLoad!), + "GameWindow.OnLoad must apply the window icon: it is the first point " + + "at which the native window exists."); + + // And nowhere that runs before initialization may call it. + foreach (MethodInfo candidate in gameWindow.GetMethods( + BindingFlags.NonPublic | BindingFlags.Public + | BindingFlags.Instance | BindingFlags.Static)) + { + if (candidate == onLoad || candidate.IsAbstract || candidate.ContainsGenericParameters) + continue; + + bool createsWindow = CompiledCallGraph.Read(candidate).Any( + c => c.Target.Name == "Create" + && c.Target.DeclaringType?.FullName == "Silk.NET.Windowing.Window"); + if (!createsWindow) + continue; + + Assert.False( + CallsApply(candidate), + $"{candidate.Name} calls Window.Create and applies the window icon in the " + + "same method. The window is not initialized there, so the icon is " + + "silently lost — apply it from OnLoad instead."); + } + } }