[PATCH v6 0/1] MR6708: win32u: Sync cursor position on window position change.
-- v6: win32u: Sync cursor position on window position change. https://gitlab.winehq.org/wine/wine/-/merge_requests/6708
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/user32/tests/msg.c | 51 ++++++++++++++++++++++++++++++++++++++++- server/queue.c | 9 ++++++++ server/user.h | 1 + server/window.c | 10 ++++++-- 4 files changed, 68 insertions(+), 3 deletions(-) diff --git a/dlls/user32/tests/msg.c b/dlls/user32/tests/msg.c index 0347f0d73b0..590f50aa6b7 100644 --- a/dlls/user32/tests/msg.c +++ b/dlls/user32/tests/msg.c @@ -73,6 +73,8 @@ #define ARCH "none" #endif +static void pump_msg_loop(HWND hwnd, HACCEL hAccel); + /* encoded DRAWITEMSTRUCT into an LPARAM */ typedef struct { @@ -108,6 +110,7 @@ typedef struct static BOOL test_DestroyWindow_flag; static BOOL test_context_menu; +static BOOL ignore_mouse_messages = TRUE; static HWINEVENTHOOK hEvent_hook; static HHOOK hKBD_hook; static HHOOK hCBT_hook; @@ -6160,6 +6163,30 @@ static const struct message WmFrameChanged_move[] = { { 0 } }; +static const struct message WmMove_mouse[] = { + { WM_WINDOWPOSCHANGING, sent|wparam, SWP_NOACTIVATE|SWP_NOSIZE }, + { WM_WINDOWPOSCHANGED, sent|wparam, SWP_NOACTIVATE|SWP_NOACTIVATE|SWP_NOSIZE|SWP_NOCLIENTSIZE }, + { WM_MOVE, sent|defwinproc, 0 }, + { WM_GETTEXT, sent|optional }, + { EVENT_OBJECT_LOCATIONCHANGE, winevent_hook|wparam|lparam, 0, 0 }, + { EVENT_OBJECT_LOCATIONCHANGE, winevent_hook|wparam|lparam|optional, 0, 0 }, /* Win7 seems to send this twice. */ + { WM_SETCURSOR, sent|lparam, 0, HTCLIENT | (WM_MOUSEMOVE << 16) }, + { WM_MOUSEMOVE, sent }, + /* WM_MOUSEMOVE with accompanying WM_SETCURSOR may sometimes appear a few extra times on Windows 7 with the + * same parameters. */ + { WM_SETCURSOR, sent|lparam|optional, 0, HTCLIENT | (WM_MOUSEMOVE << 16) }, + { WM_MOUSEMOVE, sent|optional }, + { WM_SETCURSOR, sent|lparam|optional, 0, HTCLIENT | (WM_MOUSEMOVE << 16) }, + { WM_MOUSEMOVE, sent|optional }, + { WM_SETCURSOR, sent|lparam|optional, 0, HTCLIENT | (WM_MOUSEMOVE << 16) }, + { WM_MOUSEMOVE, sent|optional }, + { WM_SETCURSOR, sent|lparam|optional, 0, HTCLIENT | (WM_MOUSEMOVE << 16) }, + { WM_MOUSEMOVE, sent|optional }, + { WM_SETCURSOR, sent|lparam|optional, 0, HTCLIENT | (WM_MOUSEMOVE << 16) }, + { WM_MOUSEMOVE, sent|optional }, + { 0 } +}; + static void test_setwindowpos(void) { HWND hwnd; @@ -6198,6 +6225,7 @@ static void test_setwindowpos(void) expect(sysX + X, rc.right); expect(winY + Y, rc.bottom); + GetWindowRect(hwnd, &rc); res = SetWindowPos( hwnd, 0, 0, 0, 0, 0, SWP_NOSIZE | SWP_NOZORDER | SWP_NOACTIVATE | SWP_FRAMECHANGED); ok_sequence(WmFrameChanged_move, "FrameChanged", FALSE); @@ -6207,6 +6235,26 @@ static void test_setwindowpos(void) expect(sysX, rc.right); expect(winY, rc.bottom); + /* get away from possible menu bar to avoid spurious position changed induced by WM. */ + res = SetWindowPos( hwnd, HWND_TOPMOST, 200, 200, 200, 200, SWP_SHOWWINDOW ); + ok(res == TRUE, "SetWindowPos expected TRUE, got %Id.\n", res); + SetForegroundWindow( hwnd ); + SetActiveWindow( hwnd ); + flush_events(); + pump_msg_loop( hwnd, 0 ); + flush_sequence(); + GetWindowRect( hwnd, &rc ); + SetCursorPos( rc.left + 100, rc.top + 100 ); + flush_events(); + pump_msg_loop( hwnd, 0 ); + flush_sequence(); + ignore_mouse_messages = FALSE; + res = SetWindowPos( hwnd, 0, 205, 205, 0, 0, SWP_NOSIZE | SWP_NOZORDER | SWP_NOACTIVATE ); + ok(res == TRUE, "SetWindowPos expected TRUE, got %Id.\n", res); + flush_events(); + ok_sequence(WmMove_mouse, "MouseMove", FALSE); + ignore_mouse_messages = TRUE; + DestroyWindow(hwnd); } @@ -10853,7 +10901,8 @@ static LRESULT MsgCheckProc (BOOL unicode, HWND hwnd, UINT message, case WM_NCMOUSEMOVE: case WM_SETCURSOR: case WM_IME_SELECT: - return 0; + if (ignore_mouse_messages) return 0; + break; } msg.hwnd = hwnd; diff --git a/server/queue.c b/server/queue.c index c99c8d8661b..767b595e650 100644 --- a/server/queue.c +++ b/server/queue.c @@ -584,6 +584,15 @@ static void set_cursor_pos( struct desktop *desktop, int x, int y ) queue_hardware_message( desktop, msg, 1 ); } +/* sync cursor position after window change */ +void update_cursor_pos( struct desktop *desktop ) +{ + const desktop_shm_t *desktop_shm; + + desktop_shm = desktop->shared; + set_cursor_pos( desktop, desktop_shm->cursor.x, desktop_shm->cursor.y ); +} + /* retrieve default position and time for synthesized messages */ static void get_message_defaults( struct msg_queue *queue, int *x, int *y, unsigned int *time ) { diff --git a/server/user.h b/server/user.h index 6f9612ffb93..c9547fb390f 100644 --- a/server/user.h +++ b/server/user.h @@ -127,6 +127,7 @@ extern int attach_thread_input( struct thread *thread_from, struct thread *threa extern void detach_thread_input( struct thread *thread_from ); extern void set_clip_rectangle( struct desktop *desktop, const rectangle_t *rect, unsigned int flags, int reset ); +extern void update_cursor_pos( struct desktop *desktop ); extern void post_message( user_handle_t win, unsigned int message, lparam_t wparam, lparam_t lparam ); extern void send_notify_message( user_handle_t win, unsigned int message, diff --git a/server/window.c b/server/window.c index 412592fbc71..3196fc49582 100644 --- a/server/window.c +++ b/server/window.c @@ -2401,11 +2401,11 @@ 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 ); - unsigned int flags = req->swp_flags; + unsigned int flags = req->swp_flags, old_style; if (!win) return; if (!win->parent) flags |= SWP_NOZORDER; /* no Z order for the desktop */ @@ -2470,8 +2470,14 @@ DECL_HANDLER(set_window_pos) if (win->paint_flags & PAINT_HAS_PIXEL_FORMAT) update_pixel_format_flags( win ); win->monitor_dpi = req->monitor_dpi; + old_window = win->window_rect; + old_client = win->client_rect; + old_style = win->style; set_window_pos( win, previous, flags, &window_rect, &client_rect, &visible_rect, &surface_rect, &valid_rect ); + 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/6708
v6: - also check for window rect change; - rename orig_style -> old_style. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6708#note_85972
On Thu Oct 24 17:05:18 2024 +0000, Paul Gofman wrote:
changed this line in [version 6 of the diff](/wine/wine/-/merge_requests/6708/diffs?diff_id=139986&start_sha=92df851be5033bc4dcafed475f7f254fa4b59c29#3dde8c1935f0e74922f8b34a2af10931253453d9_2477_2478) I designed a test which does SetWindowPos, changes window rect and ends up not changing client rect (which strictly speaking doesn't require mouse sync as relative position is unchanged), and indeed Windows still sends mouse move. I've updated the patch and attaching my ad-hoc change to test I used to confirm that.
[0001-test1.patch](/uploads/0421ed64e9cfc7ed3e50d19e79148461/0001-test1.patch) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6708#note_85973
On Thu Oct 24 17:07:22 2024 +0000, Paul Gofman wrote:
I designed a test which does SetWindowPos, changes window rect and ends up not changing client rect (which strictly speaking doesn't require mouse sync as relative position is unchanged), and indeed Windows still sends mouse move. I've updated the patch and attaching my ad-hoc change to test I used to confirm that. [0001-test1.patch](/uploads/0421ed64e9cfc7ed3e50d19e79148461/0001-test1.patch) You probably don't need to bother checking the rects at all?
```suggestion:-0+1 if (win->style & orig_style & WS_VISIBLE) update_cursor_pos( win->desktop ); ``` -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6708#note_86013
On Wed Oct 23 18:26:36 2024 +0000, Paul Gofman wrote:
changed this line in [version 4 of the diff](/wine/wine/-/merge_requests/6708/diffs?diff_id=139818&start_sha=0075df7571a2f3ea9926632ad391eb38ed298718#14b74bf2d7f44f52a31e4ee1baa51371f14df769_575_575) Well, Windows doesn't send WM_MOUSEMOVE if nothing really changed. Maybe checking actual sizes may be replaced with checking flags, but it seems to be conditions through flags will be more convoluted?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6708#note_86014
On Thu Oct 24 22:18:33 2024 +0000, Paul Gofman wrote:
Well, Windows doesn't send WM_MOUSEMOVE if nothing really changed. Maybe checking actual sizes may be replaced with checking flags, but it seems to be conditions through flags will be more convoluted? I didn't check each possible condition when it is sent or not sent, but IMO erroring on the side not sending the message when it is functionally not needed while Windows would send it (while we don't send it at all now) is better than sending extra.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6708#note_86015
On Thu Oct 24 22:20:00 2024 +0000, Paul Gofman wrote:
I didn't check each possible condition when it is sent or not sent, but IMO erroring on the side not sending the message when it is functionally not needed while Windows would send it (while we don't send it at all now) is better than sending extra. I don't think it's worth it, adding more checks makes the code more complicated for no good reason. The mouse may be moved at any time by the user, a couple of possible spurious messages in a very corner case shouldn't make a difference.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6708#note_86036
participants (3)
-
Paul Gofman -
Paul Gofman (＠gofman)
-
Rémi Bernon