[PATCH v4 0/1] MR10725: win32u: Set the normal window position without going through NtUserSetInternalWindowPos().
Fix a regression from be60f779, 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. -- v4: 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
v4: Drop "win32u: Check window_rect again later if set_foreground_window() or NtUserSetActiveWindow() is called.". It causes an issue with RiME. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10725#note_137948
Fwiw I think various issues are coming from the fact that we update the Win32 state concurrently. For a given window receiving focus, and possibly getting restored, we have two different threads receiving events from X11 and possibly going to do some changes: 1) The currently foreground window thread (often the desktop if no Wine window is focused, but sometimes it is another non-minimized Wine window thread), which has the charge of tracking the host focus and which will call set_foreground_window when host active window changes. 2) Any window that gets restored, or which minimized/maximized state changes (but I think issues occur mostly when un-minimizing as it also involves activation), which is going (explicitly now but it was also the case before the revert from the SC_RESTORE command as well) to call NtUserSetActiveWindow, and to restore the window. Perhaps it would be better to try a mechanism so that we have only one thread handling both. I think the foreground thread could be in charge, but we can also consider moving everything into the desktop thread, although there is perhaps a reason I preferred the foreground thread, but I can't remember exactly. It would however be a bit tricky to detect window restorations, because right now we only track window states on their owner thread (or process) and we don't really have a global host state kept anywhere. This is an inherent issue with the distributed model we have, I'm not sure what to do about it. I think it would better to have a centralized host window integration but we're unlikely going to have that anytime soon. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10725#note_137949
Rémi Bernon (@rbernon) commented about dlls/win32u/message.c:
{ 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 ); + }
What about doing that in a helper next to `set_window_placement`, something like `set_window_normal_rect`, to keep related things together instead of open coding it? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10725#note_137950
On Tue Apr 28 10:30:20 2026 +0000, Rémi Bernon wrote:
Fwiw I think various issues are coming from the fact that we update the Win32 state concurrently. For a given window receiving focus, and possibly getting restored, we have two different threads receiving events from X11 and possibly going to do some changes: 1) The currently foreground window thread (often the desktop if no Wine window is focused, but sometimes it is another non-minimized Wine window thread), which has the charge of tracking the host focus and which will call set_foreground_window when host active window changes. 2) Any window that gets restored, or which minimized/maximized state changes (but I think issues occur mostly when un-minimizing as it also involves activation), which is going (explicitly now but it was also the case before the revert from the SC_RESTORE command as well) to call NtUserSetActiveWindow, and to restore the window. Perhaps it would be better to try a mechanism so that we have only one thread handling both. I think the foreground thread could be in charge, but we can also consider moving everything into the desktop thread, although there is perhaps a reason I preferred the foreground thread, but I can't remember exactly. It would however be a bit tricky to detect window restorations, because right now we only track window states on their owner thread (or process) and we don't really have a global host state kept anywhere. This is an inherent issue with the distributed model we have, I'm not sure what to do about it. I think it would better to have a centralized host window integration but we're unlikely going to have that anytime soon. Ah yeah I think using the foreground window thread mitigates race conditions with foreground window changes coming from the application. The desktop thread might otherwise change foreground while some other changes is already happening... or something like that.
For the restoration issue and concurrent activation, maybe we can delay any SC_RESTORE command until the window has been made foreground by the previous foreground window. Avoiding the NtUserActivateWindow as well, which would then become unnecessary? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10725#note_137952
Avoiding the NtUserActivateWindow as well, which would then become unnecessary?
Some applications expect a WM_ACTIVATE from NtUserActivateWindow(), so I don't think it can be removed. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10725#note_138007
On Tue Apr 28 15:23:37 2026 +0000, Zhiyi Zhang wrote:
Avoiding the NtUserActivateWindow as well, which would then become unnecessary? Some applications expect a WM_ACTIVATE from NtUserActivateWindow(), so I don't think it can be removed. But set_foreground_window should do that too, whenever it is called by the foreground thread giving up focus.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10725#note_138013
participants (3)
-
Rémi Bernon (＠rbernon) -
Zhiyi Zhang
-
Zhiyi Zhang (＠zhiyi)