Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

On Windows, a window's ICON_SMALL is not set #11569

Closed
TomEdwardsEnscape opened this issue May 30, 2023 · 4 comments · Fixed by #14564
Closed

On Windows, a window's ICON_SMALL is not set #11569

TomEdwardsEnscape opened this issue May 30, 2023 · 4 comments · Fixed by #14564

Comments

@TomEdwardsEnscape
Copy link
Contributor

When Avalonia sets a window icon on Win32, it only sets the "big" icon. If the image being set is an ICO file, this results in the OS selecting a large (>=48px) image from within and downsizing it, with low-quality results.

The existing call that Avalonia makes should be duplicated, with the wParam changed to Icons.ICON_SMALL. This prompts the OS to select an appropriate image from the same ICO file.

A comparison gif:

small_icon

To Reproduce
Steps to reproduce the behavior:

  1. Create a .ico file with both small (16px, 24px...) icons, and visually different large (48px+) icons
  2. Use the .ico file as a window icon
  3. Start your application

You will see the large icon used where the small icon would be expected.

Expected behavior
Windows selects the appropriate small icon from a .ico file.

Desktop (please complete the following information):

  • OS: Windows
  • Version 11 preview 8
@timunie
Copy link
Contributor

timunie commented May 31, 2023

@tomenscape great finding. Do you think you're able to provide a PR to fix it?

@TomEdwardsEnscape
Copy link
Contributor Author

TomEdwardsEnscape commented May 31, 2023

I started making a branch but I have discovered that things are far more broken than I thought.

Avalonia.Win32.IconImpl accepts a System.Drawing.Icon object. The code is already broken at this point, because an Icon contains only one image. The resolution of this image is decided when the icon is loaded from the stream.

This currently happens in Win32Platform, where a constructor without an icon size is used. According to Microsoft's docs, this constructor loads the lowest resolution image from the icon.

Avalonia.Win32.IconImpl should be doing this:

  1. Accept a Stream and take a copy of it
  2. Use GetSystemMetricsForDpi to determine the size of a "small" (SM_CXSMICON/SM_CYSMICON) and "large" (SM_CXICON/SM_CYICON) icon for the target window
  3. Create Icon objects using the copied stream and the integer values returned by GetSystemMetricsForDpi
  4. Send the platform the correct icon size, or both, for tray/menu/window icons, as the platform APIs dictate
  5. When a window's DPI changes, call GetSystemMetricsForDpi again to get new icon resolutions, create new Icon objects, and update the window with them

(I'm not sure how to update tray icons when DPI changes. That would require Avalonia to know which screen they are being displayed on...and can't they be displayed on multiple?)

Setting both the big and small icons to the same image does improve the scaling quality, as shown above, but remains incorrect behaviour. Since community contributions are frozen and this change is much bigger than I anticipated, I won't open a PR right now.

More reading:

https://blog.barthe.ph/2009/07/17/wmseticon/
https://learn.microsoft.com/en-us/windows/win32/api/commctrl/nf-commctrl-loadiconmetric

@TomEdwardsEnscape
Copy link
Contributor Author

GetSystemMetricsForDpi is a fairly recent addition which requires Windows 10. Maybe we can use UnmanagedMethods.GetSystemMetrics (already wrapped) and multiply that by the window's DPI ourselves.

@timunie
Copy link
Contributor

timunie commented May 31, 2023

@tomenscape fyi win 7 will not be supported officially anymore. We are glad if it works, but there is no guarantee and no testing on this anymore.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants