Stop scaling icons loaded from resources in wxMSW (PR #26370)
VZ
This is not backwards compatible but looks like the right thing to do because old behaviour was inconsistent: using LoadIcon() meant that we always loaded icons of the given (system default size) which meant that some icons were upscaled while the others were not, e.g. in a system using 200% scaling 16x16 icons were loaded as 32x32 ones, but so were 32x32 icons too.
Use LoadImage() to load the image in its actual size, where "actual" means the size of the first icon for icons with multiple bitmaps. This might not be the best thing to do and maybe we should load the default icon size if the icon provides it, which could be done in a way similar to what we do in MSW-specific wxIconBundle::AddIcon().
Not scaling the icons means that they always use scale of 1, which makes things behave correctly for DPI-unaware applications that were broken before, which seems better even if it breaks DPI-aware applications: but those should be using wxBitmapBundle, which still works correctly.
Closes #26369.
I'd really like to get some testing/feedback for this one because it's clearly backwards incompatible. But this seems the right thing to do — or, at least, using LoadIcon() as we did before is very clearly a wrong thing to do, for the reasons explained above.
Please let me know what does this break and let's see if we can repair it without going to the old behaviour. TIA!
You can view, comment on, or merge this pull request online at:
https://github.com/wxWidgets/wxWidgets/pull/26370
Commit Summary
- 35257e2 Stop scaling icons loaded from resources in wxMSW
File Changes
(1 file)
- M src/msw/gdiimage.cpp (66)
Patch Links:
- https://github.com/wxWidgets/wxWidgets/pull/26370.patch
- https://github.com/wxWidgets/wxWidgets/pull/26370.diff
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#26370)Well, no comments since 2 weeks, so I'll merge this and see what it breaks.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#26370)Applied to master, see the commit referenced above.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
VZ
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Maarten
MaartenBent left a comment (wxWidgets/wxWidgets#26370)After the second fix (b45b02c), I get debug asserts in wxIcon::InitFromHICON with wxDEBUG_LEVEL=2. Because width and height are 0 instead of the determined icon size.
wxICOResourceHandler::LoadIcon does not determine the correct size anymore. I think size should always be determined, and scale only when iconMayBeScaled.
This happens in every sample, by SetIcon(wxICON(sample));.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#26370)Oops, thanks for noticing this, fixed now in 42c12b2
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.