[PATCH v2 0/1] MR11659: server: Consider _MOVE_NOCOALESCE and _ABSOLUTE flags when merging mouse events.
-- v2: server: Consider _MOVE_NOCOALESCE and _ABSOLUTE flags when merging mouse events. https://gitlab.winehq.org/wine/wine/-/merge_requests/11659
From: Paul Gofman <pgofman@codeweavers.com> --- dlls/user32/tests/input.c | 200 +++++++++++++++++++++++++++++++++++++- server/protocol.def | 2 +- server/queue.c | 16 ++- 3 files changed, 211 insertions(+), 7 deletions(-) diff --git a/dlls/user32/tests/input.c b/dlls/user32/tests/input.c index 360a5b6b725..9c3cba51020 100644 --- a/dlls/user32/tests/input.c +++ b/dlls/user32/tests/input.c @@ -4147,6 +4147,27 @@ static BOOL accept_mouse_messages_nomove( UINT msg ) return is_mouse_message( msg ) && msg != WM_MOUSEMOVE; } +static BOOL peek_mouse_move( UINT *x, UINT *y, LPARAM *extra_info ) +{ + MSG msg; + + *x = *y = 0xdeadbeef; + *extra_info = 0xdeadbeef; + while (PeekMessageA( &msg, NULL, 0, 0, PM_REMOVE )) + { + if (msg.message == WM_MOUSEMOVE) + { + *x = LOWORD(msg.lParam); + *y = HIWORD(msg.lParam); + *extra_info = GetMessageExtraInfo(); + return TRUE; + } + DispatchMessageA( &msg ); + if (msg.message == WM_MOUSEMOVE) return TRUE; + } + return FALSE; +} + static void test_SendInput_mouse_messages(void) { #define WIN_MSG(m, h, w, l, ...) {.func = MSG_TEST_WIN, .message = {.msg = m, .hwnd = h, .wparam = w, .lparam = l}, ## __VA_ARGS__} @@ -4216,10 +4237,121 @@ static void test_SendInput_mouse_messages(void) {.color = RGB(50, 100, 255), .alpha = 1, .flags = LWA_COLORKEY | LWA_ALPHA, .expect_click = TRUE}, }; +#define BROKEN_INFO(val) ((val) << 16) +#define BROKEN_OPTIONAL(val) ((val) | 0x80000000) + static const struct coalesce_test + { + unsigned int line; + struct + { + BOOL absolute, no_coalesce, increment_pos, end; + } + inputs[8]; + LPARAM expected_info[8]; + } + coalesce_tests[] = + { + /* Relative only. */ + { + __LINE__, {{ FALSE, FALSE, TRUE }, { FALSE, FALSE, TRUE }, { .end = TRUE }}, + { 2 | BROKEN_INFO(1) }, + }, + { + __LINE__, {{ FALSE, FALSE, TRUE }, { FALSE, TRUE, TRUE }, { .end = TRUE }}, + { 2 | BROKEN_INFO(1) }, + }, + { + __LINE__, {{ FALSE, TRUE, TRUE }, { FALSE, FALSE, TRUE }, { .end = TRUE }}, + { 1, BROKEN_OPTIONAL( 2 ) }, + }, + { + __LINE__, {{ FALSE, TRUE, TRUE }, { FALSE, TRUE, TRUE }, { .end = TRUE }}, + { 1, BROKEN_OPTIONAL( 2 ) }, + }, + /* Absolute only, only no_coalesce flag of already queued message matters, a new message can be coalesced + * regardless of the flag on it. If the new position is the same the message is skipped regardless + * of no_coalesce flag. extra_info from newer event replaces the old one. */ + { + __LINE__, {{ TRUE, FALSE, TRUE }, { TRUE, FALSE, TRUE }, { .end = TRUE }}, + { 2 }, + }, + { + __LINE__, {{ TRUE, FALSE, TRUE }, { TRUE, FALSE, TRUE }, { .end = TRUE }}, + { 2 }, + }, + { + __LINE__, {{ TRUE, TRUE, TRUE }, { TRUE, TRUE, TRUE }, { .end = TRUE }}, + { 1, 2 }, + }, + { + __LINE__, {{ TRUE, FALSE, TRUE }, { TRUE, TRUE, TRUE }, { .end = TRUE }}, + { 2 }, + }, + { + __LINE__, {{ TRUE, TRUE, TRUE }, { TRUE, FALSE, TRUE }, { .end = TRUE }}, + { 1, 2 }, + }, + { + __LINE__, {{ TRUE, TRUE, FALSE }, { TRUE, FALSE, TRUE }, { .end = TRUE }}, + { 2 | BROKEN_INFO(1), BROKEN_INFO(2) }, + }, + /* One relative, one absolute. Only no_coalesce on the first queued message affects result. + * extra_info from newer event replaces the old one. */ + { + __LINE__, {{ TRUE, FALSE, TRUE }, { FALSE, FALSE, TRUE }, { .end = TRUE }}, + { 2 }, + }, + { + __LINE__, {{ TRUE, FALSE, TRUE }, { FALSE, TRUE, TRUE }, { .end = TRUE }}, + { 2 }, + }, + { + __LINE__, {{ TRUE, TRUE, TRUE }, { FALSE, FALSE, TRUE }, { .end = TRUE }}, + { 1, 2 }, + }, + { + __LINE__, {{ TRUE, TRUE, TRUE }, { FALSE, TRUE, TRUE }, { .end = TRUE }}, + { 1, 2 }, + }, + + { + __LINE__, {{ FALSE, FALSE, TRUE }, { TRUE, FALSE, TRUE }, { .end = TRUE }}, + { 2 }, + }, + { + __LINE__, {{ FALSE, FALSE, TRUE }, { TRUE, TRUE, TRUE }, { .end = TRUE }}, + { 2 }, + }, + { + __LINE__, {{ FALSE, TRUE, TRUE }, { TRUE, FALSE, TRUE }, { .end = TRUE }}, + { 1, 2 }, + }, + { + __LINE__, {{ FALSE, TRUE, TRUE }, { TRUE, TRUE, TRUE }, { .end = TRUE }}, + { 1, 2 }, + }, + /* Some random combinations of absolute and relative events. */ + { + __LINE__, {{ FALSE, FALSE, TRUE }, { TRUE, TRUE, TRUE }, { TRUE, TRUE, TRUE }, { .end = TRUE }}, + { 2, 3 }, + }, + { + __LINE__, {{ TRUE, TRUE, TRUE }, { FALSE, FALSE, TRUE }, { TRUE, TRUE, TRUE }, { .end = TRUE }}, + { 1, 3 }, + }, + { + __LINE__, {{ FALSE, TRUE, TRUE }, { TRUE, TRUE, TRUE }, { TRUE, TRUE, TRUE }, { .end = TRUE }}, + { 1, 2, 3 }, + }, + }; +#undef BROKEN_INFO +#undef BROKEN_OPTIONAL + struct create_transparent_window_params params = {0}; ULONG_PTR old_proc, old_other_proc; RECT clip_rect = {55, 55, 55, 55}; - UINT dblclk_time, i; + UINT dblclk_time, i, j, x, y, virt_cx, virt_cy; + LPARAM extra_info; HWND hwnd, other; DWORD thread_id; HANDLE thread; @@ -4648,6 +4780,72 @@ static void test_SendInput_mouse_messages(void) ok_ret( 1, SetDoubleClickTime( dblclk_time ) ); + + /* Test MOUSEEVENTF_MOVE_NOCOALESCE. */ + hwnd = CreateWindowW( L"static", NULL, WS_VISIBLE | WS_POPUP, 100, 100, 100, 100, NULL, NULL, NULL, NULL ); + ok_ne( NULL, hwnd, HWND, "%p" ); + wait_messages( 100, FALSE ); + current_sequence_len = 0; + + virt_cx = GetSystemMetrics( SM_CXVIRTUALSCREEN ); + virt_cy = GetSystemMetrics( SM_CYVIRTUALSCREEN ); + SetCapture( hwnd ); + for (i = 0; i < ARRAY_SIZE(coalesce_tests); ++i) + { + const struct coalesce_test *test = &coalesce_tests[i]; + UINT event_x, event_y; + BOOL bres; + + winetest_push_context( "test %u (at %u)", i, test->line ); + SetCursorPos( 130, 130 ); + wait_messages( 50, FALSE ); + + event_x = event_y = 130; + for (j = 0; !test->inputs[j].end; ++j) + { + DWORD flags = MOUSEEVENTF_MOVE; + + if (test->inputs[j].no_coalesce) flags |= MOUSEEVENTF_MOVE_NOCOALESCE; + if (test->inputs[j].absolute) + { + if (test->inputs[j].increment_pos) + { + event_x += 10; + event_y += 10; + } + flags |= MOUSEEVENTF_ABSOLUTE | MOUSEEVENTF_VIRTUALDESK; + mouse_event( flags, event_x * 65535 / (virt_cx - 1), event_y * 65535 / (virt_cy - 1), 0, j + 1 ); + } + else + { + mouse_event( flags, 20, 20, 0, j + 1 ); + } + } + /* Timing affects coalescing on Windows, regardless of the flags, let it settle. */ + Sleep( 10 ); + for (j = 0; test->expected_info[j] & 0xffff; ++j) + { + winetest_push_context( "msg %u", j ); + bres = peek_mouse_move( &x, &y, &extra_info ); + ok( bres || broken( !bres && test->expected_info[j] & 0x80000000 ), "missing WM_MOUSEMOVE.\n" ); + if (bres && extra_info) + { + ok( extra_info == (test->expected_info[j] & 0xffff) || broken( extra_info == (test->expected_info[j] >> 16)), + "got %Iu, expected %Iu.\n", extra_info, test->expected_info[j] ); + } + winetest_pop_context(); + } + bres = peek_mouse_move( &x, &y, &extra_info ); + /* Sometimes with real hardware mouse connected a real mouse move gets through after ours, + * ignore that if that is not our message. */ + ok( !bres || !extra_info || broken( (test->expected_info[j] >> 16) && extra_info == (test->expected_info[j] >> 16)), + "extra WM_MOUSEMOVE, extra_info %Iu.\n", extra_info ); + winetest_pop_context(); + } + + SetCapture( 0 ); + ok_ret( 1, DestroyWindow( hwnd ) ); + wait_messages( 0, FALSE ); } diff --git a/server/protocol.def b/server/protocol.def index 118bafd9c9b..4a38f7426fb 100644 --- a/server/protocol.def +++ b/server/protocol.def @@ -313,7 +313,7 @@ struct hardware_msg_data { lparam_t info; /* extra info */ data_size_t size; /* size of hardware message data */ - int __pad; + unsigned int mi_flags; /* mouse input flags */ unsigned int hw_id; /* unique id */ unsigned int flags; /* hook flags */ struct hw_msg_source source; /* message source */ diff --git a/server/queue.c b/server/queue.c index b89a0dc5c6e..743a1bdff68 100644 --- a/server/queue.c +++ b/server/queue.c @@ -841,17 +841,20 @@ static int merge_mousemove( struct thread_input *input, const struct message *ms if (!(prev = find_mouse_message( input, msg ))) return 0; - prev->wparam = msg->wparam; - prev->lparam = msg->lparam; - prev->x = msg->x; - prev->y = msg->y; - prev->time = msg->time; if (msg->type == MSG_HARDWARE && prev->data && msg->data) { struct hardware_msg_data *prev_data = prev->data; struct hardware_msg_data *msg_data = msg->data; + + if (prev_data->mi_flags & MOUSEEVENTF_MOVE_NOCOALESCE) return 0; prev_data->info = msg_data->info; + prev_data->mi_flags = msg_data->mi_flags; } + prev->wparam = msg->wparam; + prev->lparam = msg->lparam; + prev->x = msg->x; + prev->y = msg->y; + prev->time = msg->time; list_remove( &prev->entry ); list_add_tail( &input->msg_list, &prev->entry ); return 1; @@ -2187,6 +2190,7 @@ static int queue_mouse_message( struct desktop *desktop, user_handle_t win, cons lparam_t wparam = input->mouse.data << 16; const POINT *raw = &empty_raw; int wait = 0, x, y; + unsigned int mi_flags; static const unsigned int messages[] = { @@ -2252,6 +2256,7 @@ static int queue_mouse_message( struct desktop *desktop, user_handle_t win, cons x = desktop_shm->cursor.x; y = desktop_shm->cursor.y; } + mi_flags = flags; if ((foreground = get_foreground_thread( desktop, win ))) { @@ -2292,6 +2297,7 @@ static int queue_mouse_message( struct desktop *desktop, user_handle_t win, cons msg->lparam = 0; msg->x = x; msg->y = y; + msg_data->mi_flags = mi_flags; if (origin == IMO_INJECTED) msg_data->flags = LLMHF_INJECTED; /* specify a sender only when sending the last message */ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/11659
v2: - invert 'broken' in the relative / relative tests and adjust the functional patch accordingly. Those tests were only 'not broken' on my local Windows 11 machine, while also that logic wasn't making much sense, if the message is updated it would better have all the latest message parameters. Looks like with the consequent relative SendInput the second is just getting ignored here for some reason (not reproducible on Testbot) and that explains that results. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/11659#note_148801
Rémi Bernon (@rbernon) commented about server/protocol.def:
{ lparam_t info; /* extra info */ data_size_t size; /* size of hardware message data */ - int __pad; + unsigned int mi_flags; /* mouse input flags */ What about keeping this in the private structure, with a field in `struct message` that can be checked by the `find_mouse_message` helper?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/11659#note_149041
Rémi Bernon (@rbernon) commented about dlls/user32/tests/input.c:
+ { + unsigned int line; + struct + { + BOOL absolute, no_coalesce, increment_pos, end; + } + inputs[8]; + LPARAM expected_info[8]; + } + coalesce_tests[] = + { + /* Relative only. */ + { + __LINE__, {{ FALSE, FALSE, TRUE }, { FALSE, FALSE, TRUE }, { .end = TRUE }}, + { 2 | BROKEN_INFO(1) }, + }, These TRUE / FALSE array of arrays aren't very readable, can we make it better? Also, do you have any idea how reliable the tests are?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/11659#note_149042
On Mon Aug 17 08:07:05 2026 +0000, Rémi Bernon wrote:
These TRUE / FALSE array of arrays aren't very readable, can we make it better? Also, do you have any idea how reliable the tests are? Do you think adding designated initializers will do it?
WRT reliability, I am never sure when it comes to such sort of tests, but look stable locally and now over multiple Testbot runs at least. Maybe if (some of) those turns problematic we can remove those? It is also that when I started looking into this I had no idea what the actual logic is and need some matrix of test cases to extrapolate it, now that the change reduced to one line checking the flag in the stored message maybe these tests are excessive. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/11659#note_149070
On Mon Aug 17 08:07:04 2026 +0000, Rémi Bernon wrote:
What about keeping this in the private structure, with a field in `struct message` that can be checked by the `find_mouse_message` helper? Thanks, sure, that will be better.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/11659#note_149071
participants (3)
-
Paul Gofman -
Paul Gofman (＠gofman)
-
Rémi Bernon (＠rbernon)