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."); + } + } }