wxWidgets icon indicating copy to clipboard operation
wxWidgets copied to clipboard

SVG icons and child windows (controls) are scaled incorrectly under Windows on HiDPI displays

Open hatelamers opened this issue 1 year ago • 10 comments

Description

Bug description:

When a wxWidgets app is compiled with #define wxUSE_DPI_AWARE_MANIFEST 2 and run on HiDPI display with custom scaling (f.i. 200%) all SVG icons loaded from XRC have wrong sizes in menus, data views, on buttons, tabs etc. Also all child windows (resizeable or restored with wxPersistenceManager::Get().RegisterAndRestore(control)) have wrong relative dimensions comparing to 100% scale. As result the GUI looks broken.

Expected behaviour:

Relative dimensions and proportions of GUI elements on HiDPI monitor with custom scaling should not be different to those on monitors scaled to 100%. Example of "correct" view on 100% scaled display: app-100-percent-scale

Observed behaviour:

Apart from fonts and main window size other GUI elements are scaled incorrectly on display with 200% scaling: app-200-percent-scale

To Reproduce:

  1. Construct a minimal wxWidgets app with one main window loaded from XRC.
  2. Add a menu to the XRC with a couple of items with SVG icons (default size: 16)
  3. Add wxSplitterWindow containing 2 wxDataViewCtrl elements (or wxDataViewListCtrl for simplicity) to the XRC.
  4. Add wxPersistenceManager::Get().RegisterAndRestore(splitter); in initialization code block of the main frame.
  5. Insert some items into data views (wxDataViewCtrl::AppendIconTextColumn(...);)
  6. Compile
  7. Run the app on a HiDPI display scaled to 200%.
  8. See icons having wrong sizes.
  9. Move the splitter.
  10. Close the app.
  11. Run it again (with 200% scaling)
  12. See the splitter on different position from where you moved it previously.

Use attached sample code: svgtest.zip

Platform and version information

  • wxWidgets version: 3.2.5
  • wxWidgets port: wxMSW (compiled from sources)
  • OS: Windows 10, 11

hatelamers avatar Jul 12 '24 16:07 hatelamers

@hatelamers , Can you make a minimal possible patch to one of the XRC samples and attach it to the ticket?

Thx.

oneeyeman1 avatar Jul 12 '24 16:07 oneeyeman1

@hatelamers , Can you make a minimal possible patch to one of the XRC samples and attach it to the ticket?

can do but it will take couple of days (much to much work)

if it helps, the code of the app in the screenshots is available here, the codebase is quite large but it is one-click-compile (VSCode workspace with CMake-presets)

hatelamers avatar Jul 12 '24 17:07 hatelamers

This is definitely not supposed to happen and doesn't happen in the other applications, including the xrc sample, for example, so it must be due to something done in the application code, but I have no idea what could it be, sorry. Trying to reproduce the problem in a minimal example should find the crucial difference, but it would indeed take time — which I, unfortunately, also don't have right now.

Is the application really DPI-aware? This can be easily checked by looking at the "DPI Awareness" column in the "Process Explorer", for example.

vadz avatar Jul 15 '24 00:07 vadz

Is the application really DPI-aware? This can be easily checked by looking at the "DPI Awareness" column in the "Process Explorer", for example.

Yes, it is: Process Explorer shows "Per-Monitor Aware"

BTW, if compiled with #undef wxUSE_DPI_AWARE_MANIFEST (i.e. is "Unaware") the app appears under 200% monitor scale a bit blurry but dimensions are all Ok.

hatelamers avatar Jul 15 '24 13:07 hatelamers

Is the application really DPI-aware? This can be easily checked by looking at the "DPI Awareness" column in the "Process Explorer", for example.

Yes, it is: Process Explorer shows "Per-Monitor Aware"

Thanks for confirming this. Unfortunately I really have no idea why do these controls misbehave in your application then. They should work correctly in e.g. treelist sample.

BTW, if compiled with #undef wxUSE_DPI_AWARE_MANIFEST (i.e. is "Unaware") the app appears under 200% monitor scale a bit blurry but dimensions are all Ok.

Yes, this is normal, for non-DPI-aware applications the OS just scales up everything by a constant factor.

vadz avatar Jul 15 '24 13:07 vadz

Yes, it is: Process Explorer shows "Per-Monitor Aware"

Maybe you just wrote it incompletely, but it should be "Per-Monitor (v2)": the v2 part is AFAIK generally rather important.

PBfordev avatar Jul 15 '24 17:07 PBfordev

I finally managed to craft a minimalistic test app concentrated around the use cases in question (s. updated issue description).

The test app misbehaves on monitor with scale greater than 100% in the same manner as the original one. It is definitely something in the XRC code: the icons loaded in the app by wxBitmapBundle::FromSVGFile and set in the GUI manually scale correctly.

hatelamers avatar Jul 24 '24 14:07 hatelamers

Yes, it is: Process Explorer shows "Per-Monitor Aware"

Maybe you just wrote it incompletely, but it should be "Per-Monitor (v2)": the v2 part is AFAIK generally rather important.

Process Explorer doesn't show "v2", Task Manager does

hatelamers avatar Jul 24 '24 14:07 hatelamers

Thanks for the minimal test, this makes it simple to reproduce and see what is going on.

ParseStringInPixels converts the default_size="16,16" in the XRC from DIP to logical size in https://github.com/wxWidgets/wxWidgets/blob/f63d759e3cffd273923fd3d007110bfa6d113940/src/xrc/xmlres.cpp#L2022

Is this supposed to be converted, or stay in DIP?

A quick hack reverting this, by adding svgDefaultSize = wxWindow::ToDIP(svgDefaultSize, nullptr); fixes the icon sizes. Not sure if this is the best fix, or if adding a 6th parameter to ParseStringInPixels is any better.

MaartenBent avatar Jul 24 '24 16:07 MaartenBent

Default size for SVGs should definitely be in DIPs, nothing else makes sense (it must be the same under all platforms and at any scale).

We probably need another parameter, but instead of adding a 6th one, I'd rather unify it with windowToUse, as this is not necessary when using DIPs, i.e. pass some struct to it.

Splitter position is something else, though... We need to add conversion to wxPersistenceManager classes too.

vadz avatar Jul 24 '24 17:07 vadz

Actually, to fix the SVG size there is an even simpler solution, please check #24840 which fixes the provided example for me (thanks for making it!).

vadz avatar Sep 26 '24 18:09 vadz

However I can't reproduce the problem with the splitter that you describe. I'm testing this with master, and not 3.2, but there have been no significant changes (I see only f24b3d5483 (Save last wxSplitterWindow position before it was unsplit, 2024-02-12), but this shouldn't affect anything as long as it's never unsplit) and I don't see why would it misbehave as long as the DPI doesn't change: it saves the position in physical pixels and also restores it in physical pixels.

I still think that it might be better to save it in logical pixels to account for DPI changing between the program executions, but if scaling doesn't change, it should still work.

What am I missing?

vadz avatar Sep 26 '24 18:09 vadz

Thanks for seeing into this issue, I will try the fix ASAP. The problem with splitter and other resizable controls may have been caused by playing around too much with different monitors (f.i. closing the app on the one and opening on the other), when I have the time, I will be describing the circumstances more exactly.

hatelamers avatar Sep 26 '24 18:09 hatelamers

There is definitely an issue with saving positions in one DPI and restoring them in a different DPI but there is already #18752 for this.

vadz avatar Sep 26 '24 19:09 vadz

Actually, to fix the SVG size there is an even simpler solution, please check #24840 which fixes the provided example for me (thanks for making it!).

Sorry for taking so long to test the fix.

The scaling of SVG looks correct now.

However, when you add an SVG with size given in DU to XRC, f.i. a menu item in my test app:

<object class="wxMenuItem" name="item3">
    <label>Item 3 from XRC (DU)</label>
    <bitmap default_size="16d,16d">./folder.svg</bitmap>
</object>

The app throws an error "XRC error: 19: cannot parse "16d,16d" as SVG size".

Since the docs https://docs.wxwidgets.org/latest/overview_xrcformat.html don't mention specific using pixels only for default_size, it looks like a bug in XRCConvertFromAbsValue or there's something missing in the fix.

hatelamers avatar Nov 09 '24 18:11 hatelamers

IMO it doesn't make sense to use dialog units, which are DPI-dependent, for the base size of SVG, which should be DPI-independent. Please let me know if I'm missing something here, and why do you want to specify the SVG base size in dialog units.

The documentation needs to be corrected, however.

vadz avatar Nov 10 '24 14:11 vadz

IMO it doesn't make sense to use dialog units, which are DPI-dependent, for the base size of SVG, which should be DPI-independent. Please let me know if I'm missing something here, and why do you want to specify the SVG base size in dialog units.

The documentation needs to be corrected, however.

I agree, DU aren't very useful in this context. I don't actually need this functionality (I just wanted to be thorough), however, it used to be possible before the fix, i.e. regression is in place.

hatelamers avatar Nov 10 '24 14:11 hatelamers

I'll just update the docs then, thanks.

vadz avatar Nov 10 '24 14:11 vadz