[PATCH v9 0/2] MR419: win32u/window: Ignore changing WS_EX_TOPMOST for message-only windows.
-- v9: win32u/window: Ignore changing WS_EX_TOPMOST for message-only windows. user32/tests: Check that message-only windows ignore WS_EX_TOPMOST. https://gitlab.winehq.org/wine/wine/-/merge_requests/419
From: Giovanni Mascellani <gmascellani(a)codeweavers.com> libOVR brings a message window topmost claiming that this way it receives WM_DEVICECHANGE faster. Currently, in Wine, this causes a WM_KILLFOCUS to be sent to the actual topmost window, which in turn causes "Shantae: Risky's Revenge" to minimize. The effect is that the game automatically minimizes as soon as it is launched, which is incorrect. --- dlls/user32/tests/win.c | 146 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 146 insertions(+) diff --git a/dlls/user32/tests/win.c b/dlls/user32/tests/win.c index 775164e3e9f..7a012564499 100644 --- a/dlls/user32/tests/win.c +++ b/dlls/user32/tests/win.c @@ -8879,6 +8879,151 @@ static void test_hwnd_message(void) DestroyWindow(hwnd); } +static HWND message_window_topmost_hwnd_msg = NULL; +static BOOL message_window_topmost_received_killfocus; + +static LRESULT WINAPI message_window_topmost_proc(HWND hwnd, UINT msg, WPARAM wparam, LPARAM lparam) +{ + if (message_window_topmost_hwnd_msg) + { + /* On Windows no message at all is expected here. However that + * would be brittle on Wine, because asynchronous X11 events + * could arrive at any time for the regular window. To balance + * between sensibility and specificity, WM_KILLFOCUS is always + * considered bad, while all the other messages are considered + * bad only on the message-only widow. */ + todo_wine_if(msg == WM_KILLFOCUS) + ok(msg != WM_KILLFOCUS, "Received message WM_KILLFOCUS for window %p (message-only window is %p)\n", + hwnd, message_window_topmost_hwnd_msg); + todo_wine_if(hwnd == message_window_topmost_hwnd_msg) + ok(hwnd != message_window_topmost_hwnd_msg, "Received message %u for message-only window %p\n", + msg, hwnd); + } + + if (msg == WM_KILLFOCUS) + message_window_topmost_received_killfocus = TRUE; + + return DefWindowProcW(hwnd, msg, wparam, lparam); +} + +static void test_message_window_topmost(void) +{ + /* All SWP_* flags except SWP_NOZORDER, which has a different effect. */ + const UINT swp_flags = SWP_ASYNCWINDOWPOS | SWP_DEFERERASE | SWP_DRAWFRAME | SWP_FRAMECHANGED + | SWP_HIDEWINDOW | SWP_NOACTIVATE | SWP_NOCOPYBITS | SWP_NOMOVE | SWP_NOOWNERZORDER + | SWP_NOREDRAW | SWP_NOSENDCHANGING | SWP_NOSIZE | SWP_SHOWWINDOW; + /* Same as above, except the flags that cause + * ERROR_INVALID_PARAMETER to be returned by DeferWindowPos(). */ + const UINT dwp_flags = SWP_DRAWFRAME | SWP_FRAMECHANGED + | SWP_HIDEWINDOW | SWP_NOACTIVATE | SWP_NOCOPYBITS | SWP_NOMOVE | SWP_NOOWNERZORDER + | SWP_NOREDRAW | SWP_NOSIZE | SWP_SHOWWINDOW; + WNDCLASSW cls = { 0 }; + HWND hwnd, hwnd_msg; + HDWP hdwp; + ATOM atom; + RECT rect; + BOOL ret; + MSG msg; + + cls.lpfnWndProc = message_window_topmost_proc; + cls.hInstance = GetModuleHandleW(NULL); + cls.lpszClassName = L"test_message_window_topmost"; + atom = RegisterClassW(&cls); + ok(!!atom, "Cannot register window class\n"); + + hwnd = CreateWindowW(L"test_message_window_topmost", L"main window", WS_OVERLAPPEDWINDOW | WS_VISIBLE, + 210, 211, 212, 213, NULL, NULL, GetModuleHandleW(NULL), NULL); + ok(!!hwnd, "Cannot create main window\n"); + + hwnd_msg = CreateWindowW(L"test_message_window_topmost", L"message window", 0, + 220, 221, 222, 223, HWND_MESSAGE, NULL, GetModuleHandleW(NULL), NULL); + ok(!!hwnd_msg, "Cannot create message window\n"); + + ret = GetWindowRect(hwnd_msg, &rect); + ok(ret, "Unexpected failure when calling GetWindowRect()\n"); + ok(rect.left == 220 && rect.top == 221 && rect.right == 220 + 222 && rect.bottom == 221 + 223, + "Unexpected rectangle %s\n", wine_dbgstr_rect(&rect)); + + while (PeekMessageW(&msg, NULL, 0, 0, PM_REMOVE)) + { + TranslateMessage(&msg); + DispatchMessageW(&msg); + } + + message_window_topmost_hwnd_msg = hwnd_msg; + + SetLastError(0xdeadbeef); + + ret = SetWindowPos(hwnd_msg, HWND_TOPMOST, 230, 231, 232, 233, 0); + ok(ret, "Unexpected failure when calling SetWindowPos()\n"); + + ret = SetWindowPos(hwnd_msg, HWND_TOPMOST, 234, 235, 236, 237, swp_flags); + ok(ret, "Unexpected failure when calling SetWindowPos()\n"); + + ret = SetWindowPos(hwnd_msg, HWND_NOTOPMOST, 240, 241, 242, 243, 0); + ok(ret, "Unexpected failure when calling SetWindowPos()\n"); + + ret = SetWindowPos(hwnd_msg, HWND_NOTOPMOST, 244, 245, 246, 247, swp_flags); + ok(ret, "Unexpected failure when calling SetWindowPos()\n"); + + hdwp = BeginDeferWindowPos(4); + ok(!!hdwp, "Unexpected failure when calling BeginDeferWindowPos()\n"); + + hdwp = DeferWindowPos(hdwp, hwnd_msg, HWND_TOPMOST, 250, 251, 252, 253, 0); + ok(!!hdwp, "Unexpected failure when calling DeferWindowPos()\n"); + + hdwp = DeferWindowPos(hdwp, hwnd_msg, HWND_TOPMOST, 254, 255, 256, 257, dwp_flags); + ok(!!hdwp, "Unexpected failure when calling DeferWindowPos()\n"); + + hdwp = DeferWindowPos(hdwp, hwnd_msg, HWND_NOTOPMOST, 260, 261, 262, 263, 0); + ok(!!hdwp, "Unexpected failure when calling DeferWindowPos()\n"); + + hdwp = DeferWindowPos(hdwp, hwnd_msg, HWND_NOTOPMOST, 264, 265, 266, 267, dwp_flags); + ok(!!hdwp, "Unexpected failure when calling DeferWindowPos()\n"); + + ret = EndDeferWindowPos(hdwp); + ok(ret, "Unexpected failure when calling EndDeferWindowPos()\n"); + + todo_wine ok(GetLastError() == 0xdeadbeef, "Last error unexpectedly set to %#lx\n", GetLastError()); + + while (PeekMessageW(&msg, NULL, 0, 0, PM_REMOVE)) + { + TranslateMessage(&msg); + DispatchMessageW(&msg); + } + + message_window_topmost_hwnd_msg = NULL; + + ret = GetWindowRect(hwnd_msg, &rect); + ok(ret, "Unexpected failure when calling GetWindowRect()\n"); + todo_wine ok(rect.left == 220 && rect.top == 221 && rect.right == 220 + 222 && rect.bottom == 221 + 223, + "Unexpected rectangle %s\n", wine_dbgstr_rect(&rect)); + + message_window_topmost_received_killfocus = FALSE; + + ret = SetWindowPos(hwnd_msg, HWND_TOPMOST, 230, 231, 232, 233, SWP_NOZORDER); + ok(ret, "Unexpected failure when calling SetWindowPos()\n"); + + while (PeekMessageW(&msg, NULL, 0, 0, PM_REMOVE)) + { + TranslateMessage(&msg); + DispatchMessageW(&msg); + } + + ok(message_window_topmost_received_killfocus, "Did not receive WM_KILLFOCUS\n"); + + ret = GetWindowRect(hwnd_msg, &rect); + ok(ret, "Unexpected failure when calling GetWindowRect()\n"); + ok(rect.left == 230 && rect.top == 231 && rect.right == 230 + 232 && rect.bottom == 231 + 233, + "Unexpected rectangle %s\n", wine_dbgstr_rect(&rect)); + + ok(DestroyWindow(hwnd_msg), "Cannot destroy main window\n"); + ok(DestroyWindow(hwnd), "Cannot destroy message window\n"); + + ok(UnregisterClassW(L"test_message_window_topmost", GetModuleHandleW(NULL)), "Cannot unregister class\n"); +} + + static void test_layered_window(void) { HWND hwnd, child; @@ -13237,6 +13382,7 @@ START_TEST(win) test_thick_child_size(hwndMain); test_fullscreen(); test_hwnd_message(); + test_message_window_topmost(); test_nonclient_area(hwndMain); test_params(); test_GetWindowModuleFileName(); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/419
From: Giovanni Mascellani <gmascellani(a)codeweavers.com> --- dlls/user32/tests/win.c | 6 ++---- dlls/win32u/window.c | 11 +++++++++++ 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/dlls/user32/tests/win.c b/dlls/user32/tests/win.c index 7a012564499..38b5c4bc005 100644 --- a/dlls/user32/tests/win.c +++ b/dlls/user32/tests/win.c @@ -8892,10 +8892,8 @@ static LRESULT WINAPI message_window_topmost_proc(HWND hwnd, UINT msg, WPARAM wp * between sensibility and specificity, WM_KILLFOCUS is always * considered bad, while all the other messages are considered * bad only on the message-only widow. */ - todo_wine_if(msg == WM_KILLFOCUS) ok(msg != WM_KILLFOCUS, "Received message WM_KILLFOCUS for window %p (message-only window is %p)\n", hwnd, message_window_topmost_hwnd_msg); - todo_wine_if(hwnd == message_window_topmost_hwnd_msg) ok(hwnd != message_window_topmost_hwnd_msg, "Received message %u for message-only window %p\n", msg, hwnd); } @@ -8984,7 +8982,7 @@ static void test_message_window_topmost(void) ret = EndDeferWindowPos(hdwp); ok(ret, "Unexpected failure when calling EndDeferWindowPos()\n"); - todo_wine ok(GetLastError() == 0xdeadbeef, "Last error unexpectedly set to %#lx\n", GetLastError()); + ok(GetLastError() == 0xdeadbeef, "Last error unexpectedly set to %#lx\n", GetLastError()); while (PeekMessageW(&msg, NULL, 0, 0, PM_REMOVE)) { @@ -8996,7 +8994,7 @@ static void test_message_window_topmost(void) ret = GetWindowRect(hwnd_msg, &rect); ok(ret, "Unexpected failure when calling GetWindowRect()\n"); - todo_wine ok(rect.left == 220 && rect.top == 221 && rect.right == 220 + 222 && rect.bottom == 221 + 223, + ok(rect.left == 220 && rect.top == 221 && rect.right == 220 + 222 && rect.bottom == 221 + 223, "Unexpected rectangle %s\n", wine_dbgstr_rect(&rect)); message_window_topmost_received_killfocus = FALSE; diff --git a/dlls/win32u/window.c b/dlls/win32u/window.c index 4499e226a7b..7901dc1be6e 100644 --- a/dlls/win32u/window.c +++ b/dlls/win32u/window.c @@ -3493,6 +3493,17 @@ BOOL set_window_pos( WINDOWPOS *winpos, int parent_x, int parent_y ) UINT orig_flags, context; BOOL ret = FALSE; + if (!(winpos->flags & SWP_NOZORDER) + && (winpos->hwndInsertAfter == HWND_TOPMOST || winpos->hwndInsertAfter == HWND_NOTOPMOST)) + { + HWND root; + + root = NtUserGetAncestor(winpos->hwnd, GA_ROOT); + root = NtUserGetAncestor(root, GA_PARENT); + if (root == get_hwnd_message_parent()) + return TRUE; + } + orig_flags = winpos->flags; /* First, check z-order arguments. */ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/419
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=147461 Your paranoid android. === w1064v1507 (32 bit report) === user32: win.c:13205: Test failed: window is minimized. === w10pro64 (32 bit report) === user32: win.c:3818: Test failed: GetForegroundWindow returned 000202CC win.c:3749: Test failed: SetForegroundWindow failed, error 0 win.c:3752: Test failed: GetForegroundWindow returned 000202CC win.c:3789: Test failed: GetForegroundWindow returned 000202CC win.c:3877: Test failed: GetActiveWindow() = 0003017E win.c:3881: Test failed: GetFocus() = 00000000 win.c:3884: Test failed: GetFocus() = 00000000 === w1064v1809 (64 bit report) === user32: win.c:13205: Test failed: window is minimized. === w10pro64 (64 bit report) === user32: win.c:190: Test failed: didn't get start_event win.c:3818: Test failed: GetForegroundWindow returned 00000000000202A0 win.c:3749: Test failed: SetForegroundWindow failed, error 0 win.c:3752: Test failed: GetForegroundWindow returned 00000000000202A0 win.c:3789: Test failed: GetForegroundWindow returned 00000000000202A0 win.c:3877: Test failed: GetActiveWindow() = 00000000000301D6 win.c:3881: Test failed: GetFocus() = 0000000000000000 win.c:3884: Test failed: GetFocus() = 0000000000000000 === debian11 (32 bit report) === user32: win.c:9404: Test failed: Expect width 100, got 1030. win.c:9405: Test failed: Expect height 100, got 774. === debian11 (32 bit ar:MA report) === user32: win.c:9011: Test failed: Did not receive WM_KILLFOCUS === debian11b (32 bit WoW report) === user32: win.c:9011: Test failed: Did not receive WM_KILLFOCUS === debian11b (64 bit WoW report) === kernel32: comm.c:1586: Test failed: Unexpected time 1001, expected around 500 user32: win.c:9011: Test failed: Did not receive WM_KILLFOCUS
Rémi Bernon (@rbernon) commented about dlls/win32u/window.c:
if (!insertafter_parent) return FALSE; if (insertafter_parent != parent) return TRUE; } }
```suggestion:-32+0 orig_flags = winpos->flags; /* First, check z-order arguments. */ if (!(winpos->flags & SWP_NOZORDER)) { /* fix sign extension */ if (winpos->hwndInsertAfter == (HWND)0xffff) winpos->hwndInsertAfter = HWND_TOPMOST; else if (winpos->hwndInsertAfter == (HWND)0xfffe) winpos->hwndInsertAfter = HWND_NOTOPMOST; if (winpos->hwndInsertAfter == HWND_TOPMOST || winpos->hwndInsertAfter == HWND_NOTOPMOST) { HWND parent = NtUserGetAncestor( NtUserGetAncestor( winpos->hwnd, GA_ROOT ), GA_PARENT ); if (parent == get_hwnd_message_parent()) return TRUE; } else if (winpos->hwndInsertAfter != HWND_TOP && winpos->hwndInsertAfter != HWND_BOTTOM)) { HWND parent = NtUserGetAncestor( winpos->hwnd, GA_PARENT ); HWND insertafter_parent = NtUserGetAncestor( winpos->hwndInsertAfter, GA_PARENT ); /* hwndInsertAfter must be a sibling of the window */ if (!insertafter_parent) return FALSE; if (insertafter_parent != parent) return TRUE; } } ``` -- https://gitlab.winehq.org/wine/wine/-/merge_requests/419#note_77977
Rémi Bernon (@rbernon) commented about dlls/user32/tests/win.c:
+ todo_wine ok(GetLastError() == 0xdeadbeef, "Last error unexpectedly set to %#lx\n", GetLastError()); + + while (PeekMessageW(&msg, NULL, 0, 0, PM_REMOVE)) + { + TranslateMessage(&msg); + DispatchMessageW(&msg); + } + + message_window_topmost_hwnd_msg = NULL; + + ret = GetWindowRect(hwnd_msg, &rect); + ok(ret, "Unexpected failure when calling GetWindowRect()\n"); + todo_wine ok(rect.left == 220 && rect.top == 221 && rect.right == 220 + 222 && rect.bottom == 221 + 223, + "Unexpected rectangle %s\n", wine_dbgstr_rect(&rect)); + + message_window_topmost_received_killfocus = FALSE;
ok(!message_window_topmost_received_killfocus, "Received WM_KILLFOCUS\n");
message_window_topmost_received_killfocus = FALSE;
And the window proc would be something simple like: ``` static BOOL message_window_topmost_received_killfocus; static LRESULT WINAPI message_window_topmost_proc(HWND hwnd, UINT msg, WPARAM wparam, LPARAM lparam) { if (msg == WM_KILLFOCUS) message_window_topmost_received_killfocus = TRUE; return DefWindowProcW(hwnd, msg, wparam, lparam); } ``` -- https://gitlab.winehq.org/wine/wine/-/merge_requests/419#note_77979
Rémi Bernon (@rbernon) commented about dlls/user32/tests/win.c:
+ BOOL ret; + MSG msg; + + cls.lpfnWndProc = message_window_topmost_proc; + cls.hInstance = GetModuleHandleW(NULL); + cls.lpszClassName = L"test_message_window_topmost"; + atom = RegisterClassW(&cls); + ok(!!atom, "Cannot register window class\n"); + + hwnd = CreateWindowW(L"test_message_window_topmost", L"main window", WS_OVERLAPPEDWINDOW | WS_VISIBLE, + 210, 211, 212, 213, NULL, NULL, GetModuleHandleW(NULL), NULL); + ok(!!hwnd, "Cannot create main window\n"); + + hwnd_msg = CreateWindowW(L"test_message_window_topmost", L"message window", 0, + 220, 221, 222, 223, HWND_MESSAGE, NULL, GetModuleHandleW(NULL), NULL); + ok(!!hwnd_msg, "Cannot create message window\n"); What about doing this instead?
```suggestion:-6+0 hwnd = CreateWindowW(L"static", L"main window", WS_OVERLAPPEDWINDOW | WS_VISIBLE, 210, 211, 212, 213, NULL, NULL, GetModuleHandleW(NULL), NULL); ok(!!hwnd, "Cannot create main window\n"); flush_events(TRUE); SetWindowLongPtrW(hwnd, GWLP_WNDPROC, (LONG_PTR)message_window_topmost_proc); hwnd_msg = CreateWindowW(L"static", L"message window", 0, 220, 221, 222, 223, HWND_MESSAGE, NULL, GetModuleHandleW(NULL), NULL); ok(!!hwnd_msg, "Cannot create message window\n"); flush_events(TRUE); SetWindowLongPtrW(hwnd_msg, GWLP_WNDPROC, (LONG_PTR)message_window_topmost_proc); ``` Then you wouldn't need a custom window class, or to workaround winex11 events (they should have been flushed before changing the window proc). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/419#note_77978
Note that attempts to set _TOPMOST should also be the same way ignored for child windows (see https://gitlab.winehq.org/wine/wine/-/merge_requests/667). I am not sure if that should necessarily be fixed all at once but maybe worth considering satisfying both cases? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/419#note_78023
Because, well, if neither MR fixes both issues then it might be indication that the condition to ignore SetWindowPos requesting to set _TOPMOST is a bit off in both. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/419#note_78027
participants (5)
-
Giovanni Mascellani -
Giovanni Mascellani (＠giomasce)
-
Marvin -
Paul Gofman (＠gofman)
-
Rémi Bernon