[PATCH 0/2] MR5774: win32u: Move desktop resize on WM_DISPLAYCHANGE of the drivers.
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winemac.drv/macdrv.h | 1 - dlls/winemac.drv/window.c | 6 +----- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/dlls/winemac.drv/macdrv.h b/dlls/winemac.drv/macdrv.h index a7905050f16..ab5b9a2667b 100644 --- a/dlls/winemac.drv/macdrv.h +++ b/dlls/winemac.drv/macdrv.h @@ -95,7 +95,6 @@ static inline RECT rect_from_cgrect(CGRect cgrect) enum macdrv_window_messages { WM_MACDRV_SET_WIN_REGION = WM_WINE_FIRST_DRIVER_MSG, - WM_MACDRV_RESET_DEVICE_METRICS, WM_MACDRV_DISPLAYCHANGE, WM_MACDRV_ACTIVATE_ON_FOLLOWING_FOCUS, }; diff --git a/dlls/winemac.drv/window.c b/dlls/winemac.drv/window.c index 84668b44c51..35cf9255279 100644 --- a/dlls/winemac.drv/window.c +++ b/dlls/winemac.drv/window.c @@ -1586,8 +1586,6 @@ void macdrv_resize_desktop(void) if (!NtUserGetWindowRect(hwnd, ¤t_desktop_rect) || !CGRectEqualToRect(cgrect_from_rect(current_desktop_rect), new_desktop_rect)) { - send_message_timeout(HWND_BROADCAST, WM_MACDRV_RESET_DEVICE_METRICS, 0, 0, - SMTO_ABORTIFHUNG, 2000, NULL); NtUserSetWindowPos(hwnd, 0, CGRectGetMinX(new_desktop_rect), CGRectGetMinY(new_desktop_rect), CGRectGetWidth(new_desktop_rect), CGRectGetHeight(new_desktop_rect), SWP_NOZORDER | SWP_NOACTIVATE | SWP_DEFERERASE); @@ -2020,10 +2018,8 @@ LRESULT macdrv_WindowMessage(HWND hwnd, UINT msg, WPARAM wp, LPARAM lp) release_win_data(data); } return 0; - case WM_MACDRV_RESET_DEVICE_METRICS: - macdrv_reset_device_metrics(); - return 0; case WM_MACDRV_DISPLAYCHANGE: + macdrv_reset_device_metrics(); macdrv_reassert_window_position(hwnd); return 0; case WM_MACDRV_ACTIVATE_ON_FOLLOWING_FOCUS: -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5774
From: Rémi Bernon <rbernon(a)codeweavers.com> Changing the thread dpi awareness context before moving the desktop window, like wineandroid was doing, shouldn't be necessary, assuming that the desktop window is always per-monitor aware. --- dlls/win32u/defwnd.c | 17 ++++++++++++++++- dlls/wineandroid.drv/window.c | 5 ----- dlls/winemac.drv/display.c | 3 --- dlls/winemac.drv/macdrv.h | 2 -- dlls/winemac.drv/window.c | 24 +----------------------- dlls/winewayland.drv/window.c | 18 ------------------ dlls/winex11.drv/desktop.c | 22 ---------------------- dlls/winex11.drv/window.c | 5 +---- dlls/winex11.drv/x11drv.h | 2 -- dlls/winex11.drv/xrandr.c | 3 --- include/ntuser.h | 1 + 11 files changed, 19 insertions(+), 83 deletions(-) diff --git a/dlls/win32u/defwnd.c b/dlls/win32u/defwnd.c index 43d31bc4f54..6cfb379a506 100644 --- a/dlls/win32u/defwnd.c +++ b/dlls/win32u/defwnd.c @@ -2979,7 +2979,22 @@ LRESULT desktop_window_proc( HWND hwnd, UINT msg, WPARAM wparam, LPARAM lparam ) case WM_NCCALCSIZE: return 0; case WM_DISPLAYCHANGE: - return user_driver->pDesktopWindowProc( hwnd, msg, wparam, lparam ); + { + static RECT old_rect; + RECT new_rect = NtUserGetVirtualScreenRect(); + + TRACE( "desktop %p change from %s to %s\n", hwnd, wine_dbgstr_rect( &old_rect ), wine_dbgstr_rect( &new_rect ) ); + + if (old_rect.left == new_rect.left && old_rect.top == new_rect.top) return TRUE; + old_rect = new_rect; + + NtUserSetWindowPos( hwnd, 0, new_rect.left, new_rect.top, + new_rect.right - new_rect.left, new_rect.bottom - new_rect.top, + SWP_NOZORDER | SWP_NOACTIVATE | SWP_DEFERERASE ); + send_message_timeout( HWND_BROADCAST, WM_WINE_DESKTOP_RESIZED, old_rect.left, + old_rect.top, SMTO_ABORTIFHUNG, 2000, FALSE ); + return TRUE; + } default: if (msg >= WM_USER && hwnd == get_desktop_window()) return user_driver->pDesktopWindowProc( hwnd, msg, wparam, lparam ); diff --git a/dlls/wineandroid.drv/window.c b/dlls/wineandroid.drv/window.c index de1de330181..f06e1701631 100644 --- a/dlls/wineandroid.drv/window.c +++ b/dlls/wineandroid.drv/window.c @@ -416,7 +416,6 @@ static void pull_events(void) */ static int process_events( DWORD mask ) { - UINT context; struct java_event *event, *next, *previous; unsigned int count = 0; @@ -457,13 +456,9 @@ static int process_events( DWORD mask ) { case DESKTOP_CHANGED: TRACE( "DESKTOP_CHANGED %ux%u\n", event->data.desktop.width, event->data.desktop.height ); - context = NtUserSetThreadDpiAwarenessContext( NTUSER_DPI_PER_MONITOR_AWARE ); screen_width = event->data.desktop.width; screen_height = event->data.desktop.height; init_monitors( screen_width, screen_height ); - NtUserSetWindowPos( NtUserGetDesktopWindow(), 0, 0, 0, screen_width, screen_height, - SWP_NOZORDER | SWP_NOACTIVATE | SWP_NOREDRAW ); - NtUserSetThreadDpiAwarenessContext( context ); break; case CONFIG_CHANGED: diff --git a/dlls/winemac.drv/display.c b/dlls/winemac.drv/display.c index 49ea924e5cb..31e4280f609 100644 --- a/dlls/winemac.drv/display.c +++ b/dlls/winemac.drv/display.c @@ -1112,10 +1112,7 @@ void macdrv_displays_changed(const macdrv_event *event) process it (by sending it to the desktop window). */ if (event->displays_changed.activating || NtUserGetWindowThread(hwnd, NULL) == GetCurrentThreadId()) - { NtUserCallNoParam(NtUserCallNoParam_UpdateDisplayCache); - macdrv_resize_desktop(); - } } UINT macdrv_UpdateDisplayDevices(const struct gdi_device_manager *device_manager, void *param) diff --git a/dlls/winemac.drv/macdrv.h b/dlls/winemac.drv/macdrv.h index ab5b9a2667b..68e20bbf5db 100644 --- a/dlls/winemac.drv/macdrv.h +++ b/dlls/winemac.drv/macdrv.h @@ -95,7 +95,6 @@ static inline RECT rect_from_cgrect(CGRect cgrect) enum macdrv_window_messages { WM_MACDRV_SET_WIN_REGION = WM_WINE_FIRST_DRIVER_MSG, - WM_MACDRV_DISPLAYCHANGE, WM_MACDRV_ACTIVATE_ON_FOLLOWING_FOCUS, }; @@ -268,7 +267,6 @@ extern CGImageRef create_cgimage_from_icon_bitmaps(HDC hdc, HANDLE icon, HBITMAP extern void macdrv_status_item_mouse_move(const macdrv_event *event); extern void check_retina_status(void); -extern void macdrv_resize_desktop(void); extern void init_user_driver(void); /* unixlib interface */ diff --git a/dlls/winemac.drv/window.c b/dlls/winemac.drv/window.c index 35cf9255279..0bc3bf62add 100644 --- a/dlls/winemac.drv/window.c +++ b/dlls/winemac.drv/window.c @@ -1575,25 +1575,6 @@ void macdrv_SetDesktopWindow(HWND hwnd) set_app_icon(); } -void macdrv_resize_desktop(void) -{ - HWND hwnd = NtUserGetDesktopWindow(); - CGRect new_desktop_rect; - RECT current_desktop_rect; - - macdrv_reset_device_metrics(); - new_desktop_rect = macdrv_get_desktop_rect(); - if (!NtUserGetWindowRect(hwnd, ¤t_desktop_rect) || - !CGRectEqualToRect(cgrect_from_rect(current_desktop_rect), new_desktop_rect)) - { - NtUserSetWindowPos(hwnd, 0, CGRectGetMinX(new_desktop_rect), CGRectGetMinY(new_desktop_rect), - CGRectGetWidth(new_desktop_rect), CGRectGetHeight(new_desktop_rect), - SWP_NOZORDER | SWP_NOACTIVATE | SWP_DEFERERASE); - send_message_timeout(HWND_BROADCAST, WM_MACDRV_DISPLAYCHANGE, 0, 0, - SMTO_ABORTIFHUNG, 2000, NULL); - } -} - #define WM_WINE_NOTIFY_ACTIVITY WM_USER LRESULT macdrv_DesktopWindowProc(HWND hwnd, UINT msg, WPARAM wp, LPARAM lp) @@ -1614,9 +1595,6 @@ LRESULT macdrv_DesktopWindowProc(HWND hwnd, UINT msg, WPARAM wp, LPARAM lp) #pragma clang diagnostic pop break; } - case WM_DISPLAYCHANGE: - macdrv_resize_desktop(); - break; } return NtUserMessageCall(hwnd, msg, wp, lp, 0, NtUserDefWindowProc, FALSE); } @@ -2018,7 +1996,7 @@ LRESULT macdrv_WindowMessage(HWND hwnd, UINT msg, WPARAM wp, LPARAM lp) release_win_data(data); } return 0; - case WM_MACDRV_DISPLAYCHANGE: + case WM_WINE_DESKTOP_RESIZED: macdrv_reset_device_metrics(); macdrv_reassert_window_position(hwnd); return 0; diff --git a/dlls/winewayland.drv/window.c b/dlls/winewayland.drv/window.c index 3706b6afb97..33d816e5f79 100644 --- a/dlls/winewayland.drv/window.c +++ b/dlls/winewayland.drv/window.c @@ -502,16 +502,6 @@ void WAYLAND_WindowPosChanged(HWND hwnd, HWND insert_after, UINT swp_flags, wayland_win_data_release(data); } -static void wayland_resize_desktop(void) -{ - RECT virtual_rect = NtUserGetVirtualScreenRect(); - NtUserSetWindowPos(NtUserGetDesktopWindow(), 0, - virtual_rect.left, virtual_rect.top, - virtual_rect.right - virtual_rect.left, - virtual_rect.bottom - virtual_rect.top, - SWP_NOZORDER | SWP_NOACTIVATE | SWP_DEFERERASE); -} - static void wayland_configure_window(HWND hwnd) { struct wayland_surface *surface; @@ -634,7 +624,6 @@ LRESULT WAYLAND_WindowMessage(HWND hwnd, UINT msg, WPARAM wp, LPARAM lp) { case WM_WAYLAND_INIT_DISPLAY_DEVICES: NtUserCallNoParam(NtUserCallNoParam_UpdateDisplayCache); - wayland_resize_desktop(); return 0; case WM_WAYLAND_CONFIGURE: wayland_configure_window(hwnd); @@ -653,13 +642,6 @@ LRESULT WAYLAND_WindowMessage(HWND hwnd, UINT msg, WPARAM wp, LPARAM lp) */ LRESULT WAYLAND_DesktopWindowProc(HWND hwnd, UINT msg, WPARAM wp, LPARAM lp) { - switch (msg) - { - case WM_DISPLAYCHANGE: - wayland_resize_desktop(); - break; - } - return NtUserMessageCall(hwnd, msg, wp, lp, 0, NtUserDefWindowProc, FALSE); } diff --git a/dlls/winex11.drv/desktop.c b/dlls/winex11.drv/desktop.c index cb119c9a7be..dae0d652737 100644 --- a/dlls/winex11.drv/desktop.c +++ b/dlls/winex11.drv/desktop.c @@ -100,25 +100,3 @@ BOOL is_desktop_fullscreen(void) return (primary_rect.right - primary_rect.left == host_primary_rect.right - host_primary_rect.left && primary_rect.bottom - primary_rect.top == host_primary_rect.bottom - host_primary_rect.top); } - -/*********************************************************************** - * X11DRV_resize_desktop - */ -void X11DRV_resize_desktop(void) -{ - static RECT old_virtual_rect; - - RECT virtual_rect = NtUserGetVirtualScreenRect(); - HWND hwnd = NtUserGetDesktopWindow(); - INT width = virtual_rect.right - virtual_rect.left, height = virtual_rect.bottom - virtual_rect.top; - - TRACE( "desktop %p change to (%dx%d)\n", hwnd, width, height ); - NtUserSetWindowPos( hwnd, 0, virtual_rect.left, virtual_rect.top, width, height, - SWP_NOZORDER | SWP_NOACTIVATE | SWP_DEFERERASE ); - - if (old_virtual_rect.left != virtual_rect.left || old_virtual_rect.top != virtual_rect.top) - send_message_timeout( HWND_BROADCAST, WM_X11DRV_DESKTOP_RESIZED, old_virtual_rect.left, - old_virtual_rect.top, SMTO_ABORTIFHUNG, 2000, FALSE ); - - old_virtual_rect = virtual_rect; -} diff --git a/dlls/winex11.drv/window.c b/dlls/winex11.drv/window.c index f975986a9d9..f2ffbebcc7c 100644 --- a/dlls/winex11.drv/window.c +++ b/dlls/winex11.drv/window.c @@ -2063,9 +2063,6 @@ LRESULT X11DRV_DesktopWindowProc( HWND hwnd, UINT msg, WPARAM wp, LPARAM lp ) case WM_WINE_ADD_TAB: send_notify_message( (HWND)wp, WM_X11DRV_ADD_TAB, 0, 0 ); break; - case WM_DISPLAYCHANGE: - X11DRV_resize_desktop(); - break; } return NtUserMessageCall( hwnd, msg, wp, lp, 0, NtUserDefWindowProc, FALSE ); } @@ -3111,7 +3108,7 @@ LRESULT X11DRV_WindowMessage( HWND hwnd, UINT msg, WPARAM wp, LPARAM lp ) release_win_data( data ); } return 0; - case WM_X11DRV_DESKTOP_RESIZED: + case WM_WINE_DESKTOP_RESIZED: if ((data = get_win_data( hwnd ))) { /* update the full screen state */ diff --git a/dlls/winex11.drv/x11drv.h b/dlls/winex11.drv/x11drv.h index 2a326b07a4c..06d76499db7 100644 --- a/dlls/winex11.drv/x11drv.h +++ b/dlls/winex11.drv/x11drv.h @@ -580,7 +580,6 @@ enum x11drv_window_messages { WM_X11DRV_UPDATE_CLIPBOARD = WM_WINE_FIRST_DRIVER_MSG, WM_X11DRV_SET_WIN_REGION, - WM_X11DRV_DESKTOP_RESIZED, WM_X11DRV_DELETE_TAB, WM_X11DRV_ADD_TAB }; @@ -751,7 +750,6 @@ struct x11drv_settings_handler extern void X11DRV_Settings_SetHandler(const struct x11drv_settings_handler *handler); extern void X11DRV_init_desktop( Window win, unsigned int width, unsigned int height ); -extern void X11DRV_resize_desktop(void); extern BOOL is_virtual_desktop(void); extern BOOL is_desktop_fullscreen(void); extern BOOL is_detached_mode(const DEVMODEW *); diff --git a/dlls/winex11.drv/xrandr.c b/dlls/winex11.drv/xrandr.c index 9a4284f493f..c35a4d8998d 100644 --- a/dlls/winex11.drv/xrandr.c +++ b/dlls/winex11.drv/xrandr.c @@ -1180,10 +1180,7 @@ static BOOL xrandr14_device_change_handler( HWND hwnd, XEvent *event ) xrandr14_invalidate_current_mode_cache(); if (hwnd == NtUserGetDesktopWindow() && NtUserGetWindowThread( hwnd, NULL ) == GetCurrentThreadId()) - { NtUserCallNoParam( NtUserCallNoParam_UpdateDisplayCache ); - X11DRV_resize_desktop(); - } /* Update xinerama monitors for xinerama_get_fullscreen_monitors() */ rect = get_host_primary_monitor_rect(); xinerama_init( rect.right - rect.left, rect.bottom - rect.top ); diff --git a/include/ntuser.h b/include/ntuser.h index ff45ffa2bc8..c81075b9225 100644 --- a/include/ntuser.h +++ b/include/ntuser.h @@ -504,6 +504,7 @@ enum wine_internal_message WM_WINE_KEYBOARD_LL_HOOK, WM_WINE_MOUSE_LL_HOOK, WM_WINE_UPDATEWINDOWSTATE, + WM_WINE_DESKTOP_RESIZED, WM_WINE_FIRST_DRIVER_MSG = 0x80001000, /* range of messages reserved for the USER driver */ WM_WINE_CLIPCURSOR = 0x80001ff0, /* internal driver notification messages */ WM_WINE_SETCURSOR, -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5774
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=146017 Your paranoid android. === debian11b (64 bit WoW report) === ddraw: ddraw1.c:14434: Test failed: Expect clip rect (0,0)-(640,480), got (0,0)-(1024,768). ddraw2.c:15446: Test failed: Expect clip rect (0,0)-(640,480), got (0,0)-(1024,768). ddraw4.c:18498: Test failed: Expect clip rect (0,0)-(640,480), got (0,0)-(1024,768). ddraw7.c:18892: Test failed: Expect clip rect (0,0)-(640,480), got (0,0)-(1024,768). dxgi: dxgi.c:6957: Test failed: Adapter 0: Output 0: Expect clip rect (0,0)-(640,480), got (0,0)-(1024,768).
Zhiyi Zhang (@zhiyi) commented about dlls/win32u/defwnd.c:
case WM_NCCALCSIZE: return 0;
Is the commit subject message missing an "out" -> "out of the drivers"? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5774#note_72178
Zhiyi Zhang (@zhiyi) commented about dlls/win32u/defwnd.c:
case WM_NCCALCSIZE: return 0; case WM_DISPLAYCHANGE: - return user_driver->pDesktopWindowProc( hwnd, msg, wparam, lparam ); + { + static RECT old_rect; + RECT new_rect = NtUserGetVirtualScreenRect(); + + TRACE( "desktop %p change from %s to %s\n", hwnd, wine_dbgstr_rect( &old_rect ), wine_dbgstr_rect( &new_rect ) ); + + if (old_rect.left == new_rect.left && old_rect.top == new_rect.top) return TRUE;
If there is only one monitor and I change the monitor from 4k to 1080p, the left and top of the virtual screen rectangle remain (0, 0). So I think you need to also check for size changes here. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5774#note_72179
Zhiyi Zhang (@zhiyi) commented about dlls/wineandroid.drv/window.c:
{ case DESKTOP_CHANGED: TRACE( "DESKTOP_CHANGED %ux%u\n", event->data.desktop.width, event->data.desktop.height ); - context = NtUserSetThreadDpiAwarenessContext( NTUSER_DPI_PER_MONITOR_AWARE ); Changing the thread dpi awareness context before moving the desktop
window, like wineandroid was doing, shouldn't be necessary, assuming that the desktop window is always per-monitor aware. I don't think this is correct. The explorer exe has "<dpiAware xmlns="http://schemas.microsoft.com/SMI/2005/WindowsSettings">true</dpiAware>", which is system aware instead of per-monitor aware according to https://learn.microsoft.com/en-us/windows/win32/hidpi/setting-the-default-dp... -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5774#note_72180
Zhiyi Zhang (@zhiyi) commented about dlls/wineandroid.drv/window.c:
{ case DESKTOP_CHANGED: TRACE( "DESKTOP_CHANGED %ux%u\n", event->data.desktop.width, event->data.desktop.height ); - context = NtUserSetThreadDpiAwarenessContext( NTUSER_DPI_PER_MONITOR_AWARE ); screen_width = event->data.desktop.width; screen_height = event->data.desktop.height; init_monitors( screen_width, screen_height ); - NtUserSetWindowPos( NtUserGetDesktopWindow(), 0, 0, 0, screen_width, screen_height,
If the resolution is changed by the host system instead of ChangeDisplaySettings() from Wine, I think the desktop window won't be resized anymore after this patch, right? Because you're not sending a WM_DISPLAYCHANGE in this case. Same for other platforms like x11 and Mac. And it was working before this patch. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5774#note_72181
On Mon Jun 3 09:58:36 2024 +0000, Zhiyi Zhang wrote:
Changing the thread dpi awareness context before moving the desktop window, like wineandroid was doing, shouldn't be necessary, assuming that the desktop window is always per-monitor aware. I don't think this is correct. The explorer exe has "<dpiAware xmlns="http://schemas.microsoft.com/SMI/2005/WindowsSettings">true</dpiAware>", which is system aware instead of per-monitor aware according to https://learn.microsoft.com/en-us/windows/win32/hidpi/setting-the-default-dp... Well, I don't know if we need to replicate that?
In win32u we explicitly return per-monitor aware for the desktop window. We could set the context here to guard against any future changes on the desktop window DPI awareness but it's also unnecessary right now. We could also make explorer process explicitly change its awareness on startup, and maybe that's something we should do regardless? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5774#note_72187
On Mon Jun 3 09:58:36 2024 +0000, Zhiyi Zhang wrote:
If the resolution is changed by the host system instead of ChangeDisplaySettings() from Wine, I think the desktop window won't be resized anymore after this patch, right? Because you're not sending a WM_DISPLAYCHANGE in this case. Same for other platforms like x11 and Mac. And it was working before this patch. Hmm, I think you're right. I was thinking that it would end up in a call to `apply_display_settings` but it's probably only going to refresh the display devices.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5774#note_72188
Well, I don't know if we need to replicate that?
It's probably added because Android's onSizeChanged() always reports physical size. And the desktop window might be running with a different DPI and DPI awareness context.
We could also make explorer process explicitly change its awareness on startup, and maybe that's something we should do regardless?
NtUserSetWindowPos() calls map_dpi_winpos() to map DPI, and map_dpi_winpos() in turn calls get_dpi_for_window(), which uses get_monitor_dpi() and get_monitor_dpi() always uses the primary monitor DPI. So the desktop window is not per-monitor aware and we should probably keep the `NtUserSetThreadDpiAwarenessContext` call. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5774#note_72190
On Mon Jun 3 10:45:03 2024 +0000, Zhiyi Zhang wrote:
Well, I don't know if we need to replicate that? It's probably added because Android's onSizeChanged() always reports physical size. And the desktop window might be running with a different DPI and DPI awareness context. We could also make explorer process explicitly change its awareness on startup, and maybe that's something we should do regardless? NtUserSetWindowPos() calls map_dpi_winpos() to map DPI, and map_dpi_winpos() in turn calls get_dpi_for_window(), which uses get_monitor_dpi() and get_monitor_dpi() always uses the primary monitor DPI. So the desktop window is not per-monitor aware and we should probably keep the `NtUserSetThreadDpiAwarenessContext` call. Right, looks like there's some inconsistencies between `get_window_dpi_awareness_context` and `get_dpi_for_window`.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5774#note_72194
participants (3)
-
Marvin -
Rémi Bernon -
Zhiyi Zhang (＠zhiyi)