[PATCH 0/1] MR6812: server: Don't update cursor pos in (set_window_pos) if windows wasn't moved.

[Paul Gofman (＠gofman)]

Fixes a regression introduced by commit db9a4bc66a5ff550a0a25899b6646117f66c50df.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/6812

[PATCH 1/1] server: Don't update cursor pos in (set_window_pos) if windows wasn't moved.

[Paul Gofman]

From: Paul Gofman <pgofman(a)codeweavers.com>

Fixes a regression introduced by commit db9a4bc66a5ff550a0a25899b6646117f66c50df.
---
 dlls/user32/tests/msg.c | 12 ++++++++++++
 server/window.c         |  8 ++++++--
 2 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/dlls/user32/tests/msg.c b/dlls/user32/tests/msg.c
index 590f50aa6b7..45e91af517d 100644
--- a/dlls/user32/tests/msg.c
+++ b/dlls/user32/tests/msg.c
@@ -6187,6 +6187,13 @@ static const struct message WmMove_mouse[] = {
     { 0 }
 };
 
+static const struct message WmMove_mouse2[] = {
+    { WM_WINDOWPOSCHANGING, sent|wparam, SWP_NOACTIVATE },
+    { WM_GETTEXT, sent|optional },
+    { WM_GETMINMAXINFO, sent|defwinproc },
+    { 0 }
+};
+
 static void test_setwindowpos(void)
 {
     HWND hwnd;
@@ -6253,6 +6260,11 @@ static void test_setwindowpos(void)
     ok(res == TRUE, "SetWindowPos expected TRUE, got %Id.\n", res);
     flush_events();
     ok_sequence(WmMove_mouse, "MouseMove", FALSE);
+    /* if the window and client rects were not changed WM_MOUSEMOVE is not sent. */
+    res = SetWindowPos( hwnd, 0, 205, 205, 200, 200, SWP_NOZORDER | SWP_NOACTIVATE );
+    ok(res == TRUE, "SetWindowPos expected TRUE, got %Id.\n", res);
+    flush_events();
+    ok_sequence(WmMove_mouse2, "MouseMove2", FALSE);
     ignore_mouse_messages = TRUE;
 
     DestroyWindow(hwnd);
diff --git a/server/window.c b/server/window.c
index 46377513719..8a55c66dea6 100644
--- a/server/window.c
+++ b/server/window.c
@@ -2488,7 +2488,7 @@ DECL_HANDLER(get_window_tree)
 /* set the position and Z order of a window */
 DECL_HANDLER(set_window_pos)
 {
-    rectangle_t window_rect, client_rect, visible_rect, surface_rect, valid_rect;
+    rectangle_t window_rect, client_rect, visible_rect, surface_rect, valid_rect, old_window, old_client;
     const rectangle_t *extra_rects = get_req_data();
     struct window *previous = NULL;
     struct window *top, *win = get_window( req->handle );
@@ -2558,9 +2558,13 @@ DECL_HANDLER(set_window_pos)
 
     win->monitor_dpi = req->monitor_dpi;
     old_style = win->style;
+    old_window = win->window_rect;
+    old_client = win->client_rect;
     set_window_pos( win, previous, flags, &window_rect, &client_rect,
                     &visible_rect, &surface_rect, &valid_rect );
-    if (win->style & old_style & WS_VISIBLE) update_cursor_pos( win->desktop );
+    if ((win->style & old_style & WS_VISIBLE) && (memcmp( &old_client, &win->client_rect, sizeof(old_client) )
+        || memcmp( &old_window, &win->window_rect, sizeof(old_window) )))
+        update_cursor_pos( win->desktop );
 
     if (win->paint_flags & SET_WINPOS_LAYERED_WINDOW) validate_whole_window( win );
 
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/6812

[PATCH 1/1] server: Don't update cursor pos in (set_window_pos) if windows wasn't moved.

[Marvin]

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=149654

Your paranoid android.


=== debian11b (64 bit WoW report) ===

kernel32:
comm.c:1586: Test failed: Unexpected time 1000, expected around 500

user32:
input.c:4305: Test succeeded inside todo block: button_down_hwnd_todo 1: got MSG_TEST_WIN hwnd 0000000002BC00EA, msg WM_LBUTTONDOWN, wparam 0x1, lparam 0x320032

[Paul Gofman (＠gofman)]

The known regression happens in the The Elder Scrolls IV: Oblivion and Fallout: New Vegas launchers. The buttons in the launchers are not clickable.

What happens there (I only debugged The Elder Scrolls IV: Oblivion) is that the launcher, upon receiving WM_MOUSEMOVE, sends STM_SETIMAGE message (from its code and not through some other API; that happens after returning from window proc for WM_MOUSEMOVE). Handling of STM_SETIMAGE in user32/static involves calling SetWindowPos() (without actual size change in this case), so this results in auto-inducing loop.

FWIW v6 of the original patch in https://gitlab.winehq.org/wine/wine/-/merge_requests/6708 actually had the check I am adding here, but it was considered not needed on the basis that WM_MOUSEMOVE can happen anytime regardless.

This specific manifestation is in theory might be possible to fix in static control implementation, but it doesn't look right: the test shows that Windows doesn't send mousemove if position is unchanged and it looks important to fix it on the lower level; some apps in principle may react on that in a similar way themselves without our controls involved.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/6812#note_87455

[PATCH 0/1] MR6812: server: Don't update cursor pos in (set_window_pos) if windows wasn't moved. - approved

[Rémi Bernon]

This merge request was approved by Rémi Bernon.
-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/6812