[PATCH v3 0/2] MR10725: win32u: Check config again before setting win32 window position.
For example, Homeworld Remastered Collection (244160) Homeworld 2 Classic changes the display mode after NtUserSetActiveWindow(), then the following NtUserSetInternalWindowPos() call sets the window rect to the window rect before the display mode change. Thus, the game window can end up having an unexpected rect. There is a similar problem when LOWORD(state_cmd) == 0. Fix a regression from be60f77909fc9ce2c9b6f1abd005e3b4aabaafec. -- v3: win32u: Check window_rect again later if set_foreground_window() or NtUserSetActiveWindow() is called. win32u: Set the normal window position without going through NtUserSetInternalWindowPos(). https://gitlab.winehq.org/wine/wine/-/merge_requests/10725
From: Zhiyi Zhang <zzhang@codeweavers.com> Fix a regression from be60f77909fc9ce2c9b6f1abd005e3b4aabaafec, the intent of which was to restore the win32 window position to the current host window position. However, NtUserSetInternalWindowPos() eventually calls NtUserSetWindowPos() and NtUserShowWindow(). We don't want that because they might generate window position change messages that applications might not expect. Fix Homeworld Remastered Collection (244160) Homeworld 2 Classic black screen after restoring from the minimized state in fullscreen mode due to an unexpected window size. --- dlls/win32u/message.c | 14 +++++++++++++- dlls/win32u/win32u_private.h | 1 + dlls/win32u/window.c | 2 +- 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/dlls/win32u/message.c b/dlls/win32u/message.c index 81e7fde1212..02f94837cb6 100644 --- a/dlls/win32u/message.c +++ b/dlls/win32u/message.c @@ -2234,9 +2234,21 @@ static LRESULT handle_internal_message( HWND hwnd, UINT msg, WPARAM wparam, LPAR switch (LOWORD(state_cmd)) { case SC_RESTORE: + { + WND *win; + if (HIWORD(state_cmd)) NtUserSetActiveWindow( hwnd ); - NtUserSetInternalWindowPos( hwnd, SW_SHOW, &window_rect, NULL ); + + /* make the win32 window restore to the current host window config */ + win = get_win_ptr( hwnd ); + if (win && win != WND_OTHER_PROCESS && win != WND_DESKTOP) + { + make_rect_onscreen( &window_rect ); + win->normal_rect = rect_thread_to_win_dpi( hwnd, window_rect ); + release_win_ptr( win ); + } /* fallthrough */ + } default: send_message( hwnd, WM_SYSCOMMAND, LOWORD(state_cmd), 0 ); break; diff --git a/dlls/win32u/win32u_private.h b/dlls/win32u/win32u_private.h index 3bc6765d35e..b8f9fd14406 100644 --- a/dlls/win32u/win32u_private.h +++ b/dlls/win32u/win32u_private.h @@ -183,6 +183,7 @@ extern struct window_rects map_dpi_window_rects( struct window_rects rects, UINT extern RECT map_rect_raw_to_virt( RECT rect, UINT dpi_to ); extern RECT map_rect_virt_to_raw( RECT rect, UINT dpi_from ); extern struct window_rects map_window_rects_virt_to_raw( struct window_rects rects, UINT dpi_from ); +extern void make_rect_onscreen( RECT *rect ); extern POINT point_phys_to_win_dpi( HWND hwnd, POINT pt ); extern POINT point_thread_to_win_dpi( HWND hwnd, POINT pt ); extern RECT rect_thread_to_win_dpi( HWND hwnd, RECT rect ); diff --git a/dlls/win32u/window.c b/dlls/win32u/window.c index 682a9bad5a2..49ce499101b 100644 --- a/dlls/win32u/window.c +++ b/dlls/win32u/window.c @@ -3023,7 +3023,7 @@ BOOL WINAPI NtUserGetWindowPlacement( HWND hwnd, WINDOWPLACEMENT *placement ) } /* make sure the specified rect is visible on screen */ -static void make_rect_onscreen( RECT *rect ) +void make_rect_onscreen( RECT *rect ) { MONITORINFO info = monitor_info_from_rect( *rect, get_thread_dpi() ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10725
From: Zhiyi Zhang <zzhang@codeweavers.com> set_foreground_window() and NtUserSetActiveWindow() send messages to applications and might cause the window rect to change. Thus, the value in window_rect can be stale and we should wait for the next WM_WINE_WINDOW_STATE_CHANGED message to get the latest window_rect. A WM_WINE_WINDOW_STATE_CHANGED is posted in case there is no more WM_WINE_WINDOW_STATE_CHANGED in the message queue. --- dlls/win32u/message.c | 17 +++++++++++++++-- dlls/winex11.drv/window.c | 2 +- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/dlls/win32u/message.c b/dlls/win32u/message.c index 02f94837cb6..95494df6905 100644 --- a/dlls/win32u/message.c +++ b/dlls/win32u/message.c @@ -2230,14 +2230,27 @@ static LRESULT handle_internal_message( HWND hwnd, UINT msg, WPARAM wparam, LPAR if (!user_driver->pGetWindowStateUpdates( hwnd, &state_cmd, &swp_flags, &window_rect, &foreground )) return 0; window_rect = map_rect_raw_to_virt( window_rect, get_thread_dpi() ); - if (foreground) set_foreground_window( foreground, FALSE, TRUE ); + if (foreground) + { + set_foreground_window( foreground, FALSE, TRUE ); + /* set_foreground_window() may have changed window_rect, check again later */ + NtUserPostMessage( hwnd, WM_WINE_WINDOW_STATE_CHANGED, 0, 0 ); + return 0; + } + switch (LOWORD(state_cmd)) { case SC_RESTORE: { WND *win; - if (HIWORD(state_cmd)) NtUserSetActiveWindow( hwnd ); + if (HIWORD(state_cmd)) + { + NtUserSetActiveWindow( hwnd ); + /* NtUserSetActiveWindow() may have changed window_rect, check again later */ + NtUserPostMessage( hwnd, WM_WINE_WINDOW_STATE_CHANGED, 0, 0 ); + return 0; + } /* make the win32 window restore to the current host window config */ win = get_win_ptr( hwnd ); diff --git a/dlls/winex11.drv/window.c b/dlls/winex11.drv/window.c index d2d53a40be3..7714fe6bc2b 100644 --- a/dlls/winex11.drv/window.c +++ b/dlls/winex11.drv/window.c @@ -1700,7 +1700,7 @@ static UINT window_update_client_state( struct x11drv_win_data *data ) } else if (old_style & (WS_MINIMIZE | WS_MAXIMIZE)) { - BOOL activate = (old_style & (WS_MINIMIZE | WS_VISIBLE)) == (WS_MINIMIZE | WS_VISIBLE); + BOOL activate = ((old_style & (WS_MINIMIZE | WS_VISIBLE)) == (WS_MINIMIZE | WS_VISIBLE)) && get_active_window() != data->hwnd; TRACE( "restoring win %p/%lx\n", data->hwnd, data->whole_window ); return MAKELONG(SC_RESTORE, activate); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10725
Rémi Bernon (@rbernon) commented about dlls/winex11.drv/window.c:
} else if (old_style & (WS_MINIMIZE | WS_MAXIMIZE)) { - BOOL activate = (old_style & (WS_MINIMIZE | WS_VISIBLE)) == (WS_MINIMIZE | WS_VISIBLE); + BOOL activate = ((old_style & (WS_MINIMIZE | WS_VISIBLE)) == (WS_MINIMIZE | WS_VISIBLE)) && get_active_window() != data->hwnd; TRACE( "restoring win %p/%lx\n", data->hwnd, data->whole_window ); return MAKELONG(SC_RESTORE, activate);
Hmm, I don't think this belongs to winex11, also why is this necessary? How can the window be active already? Also, I guess this has been restored from the revert, but I think the decision to activate the window is also probably not right. Any window that is restored by the host WM is likely going to activate and if it is important that it gets activated before being restored we should do it unconditionally. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10725#note_137718
Hmm, I don't think this belongs to winex11, also why is this necessary? How can the window be active already?
With this MR, after calling NtUserSetActiveWindow(), another WM_WINE_WINDOW_STATE_CHANGED is posted to the message queue (or there is an existing WM_WINE_WINDOW_STATE_CHANGED), then the current WM_WINE_WINDOW_STATE_CHANGED handling is done. When handling the next WM_WINE_WINDOW_STATE_CHANGED, the window would have been activated so we shouldn't activate it again. This is also needed to avoid infinite WM_WINE_WINDOW_STATE_CHANGED messages.
if it is important that it gets activated before being restored we should do it unconditionally.
We need to be careful and not introduce duplicate activate message sequences. On Windows, when restoring a window, WM_ACTIVATE is sent only once before WM_SYSCOMMAND SC_RESTORE. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10725#note_137720
participants (3)
-
Rémi Bernon (＠rbernon) -
Zhiyi Zhang
-
Zhiyi Zhang (＠zhiyi)