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

Support PMv2 DPI awareness, add SDL_HINT_WINDOWS_DPI_AWARENESS #5393

Closed
wants to merge 8 commits into from

Conversation

ericwa
Copy link
Contributor

@ericwa ericwa commented Mar 10, 2022

Description

  1. Detect if per-monitor V2 DPI awareness is enabled, and calculate window frame sizes as required by the Windows docs (calling AdjustWindowRectExForDpi). Fixes "infinite window growth" bugs that occurred when per-monitor V2 DPI awareness was enabled without having proper support for it in SDL (Monitor DPI transition on latest Windows 10 causes infinite window growth #3286 , Windows bug: window size continues to increase or shrink across monitors of different magnifications #4712).

  2. The SDL_HINT_WINDOWS_DPI_AWARENESS hint allows setting a specific DPI awareness ("unaware", "system", "permonitor", "permonitorv2") on Windows.

Note, the implementation of DPI awareness here maintains the window client size in pixels; it doesn't attempt to maintain apparent size when dragging between monitors with different scale factors. My plan is to implement that in a later PR with a separate hint, along with virtualizing the SDL coordinate system to be DPI scaled rather than raw pixels. This hint SDL_HINT_WINDOWS_DPI_AWARENESS is just equivalent to calling the Windows API's directly and doesn't otherwise change SDL behavoiur. (This hint could be useful for SDL apps that want 1 SDL unit = 1 pixel, though.)

See also the discussion in: #2119 ).

Existing Issue(s)

#3286
#4712

The hint allows setting a specific DPI awareness ("unaware", "system", "permonitor", "permonitorv2").

This is the first part of High-DPI support on Windows ( libsdl-org#2119 ).
It doesn't implement a virtualized SDL coordinate system, which will be
addressed in a later commit. (This hint could be useful for SDL apps
that want 1 SDL unit = 1 pixel, though.)

Detecting and behaving correctly under per-monitor V2
(calling AdjustWindowRectExForDpi where needed) should fix the
following issues:

libsdl-org#3286
libsdl-org#4712
@kg
Copy link
Contributor

kg commented Mar 10, 2022

Hi, the person who reported #3286 originally here - I don't consume SDL directly so I'm not immediately familiar enough to know: Given that you currently preserve the client size in pixels, does that mean I could just detect the DPI change on my end in my game and adjust the requested framebuffer size and everything should work? Is the DPI value you query exposed to the end user right now?

My pre-SDL2 codebase was already doing DPI stuff manually by querying window/monitor handles and monitoring for DPI changes, so it wouldn't be too bad for me to pull that forward and combine it with your fixes.

The hint looks like a good change, because right now I have to embed a manifest and that's kind of awkward. Being able to flip it between states per-run to debug is great. Didn't see anything wrong with the code in this PR while looking at it.

@deltabeard
Copy link
Contributor

... does that mean I could just detect the DPI change on my end in my game and adjust the requested framebuffer size and everything should work? Is the DPI value you query exposed to the end user right now?

I currently check for the SDL_WINDOWEVENT_MOVED event, which when triggered, I call SDL_GetDisplayDPI() to obtain the DPI of the screen that the window is on. This gets me the new DPI if the window moved to a screen with a different DPI, and I can then resize textures, etc.

Instead of using a manifest, I use the following code:

// Public Domain License
#ifdef _WIN32
# define WIN32_LEAN_AND_MEAN
# include <Windows.h>

# if (_WIN32_WINNT >= 0x0603)
#  include <shellscalingapi.h>
# endif

void set_dpi_awareness(void)
{
# if (_WIN32_WINNT >= 0x0605)
	SetProcessDpiAwarenessContext(DPI_AWARENESS_CONTEXT_PER_MONITOR_AWARE_V2);
# elif (_WIN32_WINNT >= 0x0603)
	SetProcessDpiAwareness(PROCESS_PER_MONITOR_DPI_AWARE);
# elif (_WIN32_WINNT >= 0x0600)
	SetProcessDPIAware();
# elif defined(__MINGW64__)
	SetProcessDPIAware();
# endif
	return;
}
#endif

Although Microsoft recommends using a manifest file.

@ericwa
Copy link
Contributor Author

ericwa commented Mar 11, 2022

... does that mean I could just detect the DPI change on my end in my game and adjust the requested framebuffer size and everything should work? Is the DPI value you query exposed to the end user right now?

I currently check for the SDL_WINDOWEVENT_MOVED event, which when triggered, I call SDL_GetDisplayDPI() to obtain the DPI of the screen that the window is on. This gets me the new DPI if the window moved to a screen with a different DPI, and I can then resize textures, etc.

Thanks for confirming, I was thinking along those lines but hadn't tested it.

That might be the best solution for now.. it's not ideal - the MS docs want you to implement WM_GETDPISCALEDSIZE so Windows can ask the app how it's going to resize, before committing to a DPI change. They warn that "recursive resize loops" can happen if this isn't done: https://docs.microsoft.com/en-us/windows/win32/hidpi/high-dpi-desktop-application-development-on-windows#common-pitfalls-win32

My hope is to finish a second PR soon (more detail #2119 (comment)) that will let SDL automatically do this resizing when dragging between monitors with different scale factors, but it's a bit of a bigger change because it changes most of the SDL coordinates to be scaled points (like SDL on Wayland/macOS).

@slime73
Copy link
Contributor

slime73 commented Apr 26, 2022

@slouken @icculus now that 2.0.22 is out do you think you could give this a closer look? I'd love it if 2.0.24 shipped with improved (or, even better, complete) highdpi support for Windows

@ericwa
Copy link
Contributor Author

ericwa commented May 29, 2022

Found a bug which I'm currently looking into:

  • launch testwm2 with the SDL_WINDOWS_DPI_AWARENESS=permonitorv2 environment variable
  • Windows 10 21H2 (OS Build 19044.1706)
  • Left (primary) monitor: 3840x2160, 125% scaling
  • Right (secondary) monitor: 2560x1440, 100% scaling

Expected: the window launches on the primary with a 640x480 pixel client area, and 640x480 window size reported by SDL.
Dragging it to the secondary monitor, the client area remains 640x480.
Pressing Alt+Right/Alt+Left also keeps the window 640x480.

Observed: Alt+Right/Alt+Left cause the window to shrink to 510x383 when it moves from the 125% monitor to the 100% one.

What seems to be happening is, dragging interactively delivers WM_GETDPISCALEDSIZE but programmatically moving the window with SDL_SetWindowPosition doesn't, and the WM_DPICHANGED in this PR is built under the assumption that WM_GETDPISCALEDSIZE is always called.

Fix should be to adjust WM_DPICHANGED to not assume WM_GETDPISCALEDSIZE has been called.

ericwa added 7 commits June 2, 2022 01:33
If the move results in a DPI change, we need to allow the window to resize (e.g. AdjustWindowRectExForDpi frame sizes are different).

- WM_DPICHANGED: Don't assume WM_GETDPISCALEDSIZE is always called for PMv2 awareness - it's only called during interactive dragging.

- WIN_AdjustWindowRectWithStyle: always calculate final window size including frame based on the destination rect,
not based on the current window DPI.

- Update wmmsg.h to include WM_GETDPISCALEDSIZE (for WMMSG_DEBUG)

- WIN_AdjustWindowRectWithStyle: add optional logging

- WM_GETMINMAXINFO: add optional HIGHDPI_DEBUG logging

- WM_DPICHANGED: fix potentially clobbering data->expected_resize

Together these changes fix the following scenario:
- launch testwm2 with the SDL_WINDOWS_DPI_AWARENESS=permonitorv2 environment variable
- Windows 10 21H2 (OS Build 19044.1706)
- Left (primary) monitor: 3840x2160, 125% scaling
- Right (secondary) monitor: 2560x1440, 100% scaling

- Alt+Enter, Alt+Enter (to enter + leave desktop fullscreen), Alt+Right (to move window to right monitor). Ensure the window client area stays 640x480. Drag the window back to the 125% monitor, ensure client area stays 640x480.
…tartup if defined

document some additional quirks
@ericwa
Copy link
Contributor Author

ericwa commented Jun 9, 2022

The issue with unwanted window resizing when doing Alt+Right/Alt+Left to move between displays is fixed now, and I made the follow up PR #5778 with the rest of High-DPI support (resizing the window client area size in pixels to maintain the same size in points during DPI changes).

So if it's easier to review/test all as one big PR, this one could be ignored in favour of #5778

@ericwa
Copy link
Contributor Author

ericwa commented Jun 11, 2022

Closing, as this has been merged in along with #5778

@ericwa ericwa closed this Jun 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants