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 <noreply@anthropic.com>
This commit is contained in:
parent
400e7c766f
commit
c254fea83d
4 changed files with 94 additions and 8 deletions
|
|
@ -78,9 +78,16 @@ Neither icon is loaded from disk at runtime.
|
|||
- **PE icon** — `<ApplicationIcon>` 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`.
|
||||
|
|
|
|||
|
|
@ -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<GameWindowGraphics, IInputContext>,
|
||||
HostInputCameraResult,
|
||||
|
|
|
|||
|
|
@ -43,10 +43,31 @@ internal static class WindowIconLoader
|
|||
/// <summary>
|
||||
/// Decode the embedded icon set and hand it to the window.
|
||||
/// </summary>
|
||||
/// <param name="window">
|
||||
/// A window that has already been initialized. Silk's <c>Window.Create</c>
|
||||
/// returns an uninitialized object — <c>IWindow.Initialize</c> is what
|
||||
/// "creates the window on the underlying platform" — so this must be called
|
||||
/// from <c>Load</c> or later.
|
||||
/// </param>
|
||||
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
|
||||
{
|
||||
|
|
|
|||
|
|
@ -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");
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Regression guard for the ordering bug that shipped once already.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// Silk's <c>Window.Create</c> only builds the managed object;
|
||||
/// <c>IWindow.Initialize</c> 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
|
||||
/// <see cref="CompiledCallGraph"/> exists for.
|
||||
/// </remarks>
|
||||
[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.");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue