From: Arkadiusz Hiler ahiler@codeweavers.com
With this patch we start storing history of last 64 mouse positions on the server side which can be retrieved by the newly introduced get_cursor_history request.
The history is kept in reverse chronological order in an array that acts as circular buffer - this makes it easy to iterate over it in GetMouseMovePointsEx().
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=36873 Signed-off-by: Arkadiusz Hiler ahiler@codeweavers.com Signed-off-by: Rémi Bernon rbernon@codeweavers.com ---
v4: Fix a few remaining white spaces issues, sign it off. Supersedes: 192973
dlls/user32/input.c | 60 ++++++++++++++++++++++---- dlls/user32/tests/input.c | 88 +++++++++++++++++++++++++++++++++++++++ server/protocol.def | 22 ++++++++++ server/queue.c | 29 +++++++++++++ server/trace.c | 28 +++++++++++++ 5 files changed, 219 insertions(+), 8 deletions(-)
diff --git a/dlls/user32/input.c b/dlls/user32/input.c index 150b7de9704..75905beebce 100644 --- a/dlls/user32/input.c +++ b/dlls/user32/input.c @@ -1271,22 +1271,66 @@ TrackMouseEvent (TRACKMOUSEEVENT *ptme) * Success: count of point set in the buffer * Failure: -1 */ -int WINAPI GetMouseMovePointsEx(UINT size, LPMOUSEMOVEPOINT ptin, LPMOUSEMOVEPOINT ptout, int count, DWORD res) { +int WINAPI GetMouseMovePointsEx( UINT size, LPMOUSEMOVEPOINT ptin, LPMOUSEMOVEPOINT ptout, int count, DWORD resolution ) +{ + cursor_history_t history; + cursor_pos_t *pos; + int copied; + unsigned int i;
- if((size != sizeof(MOUSEMOVEPOINT)) || (count < 0) || (count > 64)) { - SetLastError(ERROR_INVALID_PARAMETER); + C_ASSERT( ARRAY_SIZE( history.positions ) == 64 ); + + TRACE( "%d, %p, %p, %d, %d\n", size, ptin, ptout, count, resolution ); + + if ((size != sizeof(MOUSEMOVEPOINT)) || (count < 0) || (count > ARRAY_SIZE( history.positions ))) + { + SetLastError( ERROR_INVALID_PARAMETER ); return -1; }
- if(!ptin || (!ptout && count)) { - SetLastError(ERROR_NOACCESS); + if (!ptin || (!ptout && count)) + { + SetLastError( ERROR_NOACCESS ); return -1; }
- FIXME("(%d %p %p %d %d) stub\n", size, ptin, ptout, count, res); + if (resolution != GMMP_USE_DISPLAY_POINTS) + { + FIXME( "only GMMP_USE_DISPLAY_POINTS is supported for now\n" ); + SetLastError( ERROR_POINT_NOT_FOUND ); + return -1; + } + + SERVER_START_REQ( get_cursor_history ) + { + wine_server_set_reply( req, &history, sizeof(history) ); + if (wine_server_call_err( req )) return -1; + } + SERVER_END_REQ; + + for (i = 0; i < ARRAY_SIZE( history.positions ); i++) + { + pos = &history.positions[(i + history.newest) % ARRAY_SIZE( history.positions )]; + if (ptin->x == pos->x && ptin->y == pos->y && (!ptin->time || ptin->time == pos->time)) + break; + } + + if (i == ARRAY_SIZE( history.positions )) + { + SetLastError( ERROR_POINT_NOT_FOUND ); + return -1; + } + + for (copied = 0; copied < count && i < ARRAY_SIZE( history.positions ); copied++, i++) + { + pos = &history.positions[(i + history.newest) % ARRAY_SIZE( history.positions )]; + ptout[copied].x = pos->x; + ptout[copied].y = pos->y; + ptout[copied].time = pos->time; + ptout[copied].dwExtraInfo = pos->info; + }
- SetLastError(ERROR_POINT_NOT_FOUND); - return -1; + return copied; }
/*********************************************************************** diff --git a/dlls/user32/tests/input.c b/dlls/user32/tests/input.c index ef6d6e1d430..c3363877b73 100644 --- a/dlls/user32/tests/input.c +++ b/dlls/user32/tests/input.c @@ -1481,6 +1481,7 @@ static void test_GetMouseMovePointsEx(void) MOUSEMOVEPOINT in; MOUSEMOVEPOINT out[200]; POINT point; + TEST_INPUT input;
/* Get a valid content for the input struct */ if(!GetCursorPos(&point)) { @@ -1605,6 +1606,93 @@ static void test_GetMouseMovePointsEx(void) ok(GetLastError() == ERROR_INVALID_PARAMETER || GetLastError() == MYERROR, "expected error ERROR_INVALID_PARAMETER, got %u\n", GetLastError());
+ /* more than 64 to be sure we wrap around */ + for (int i = 0; i < 67; i++) + { + in.x = i; + in.y = i*2; + SetCursorPos( in.x, in.y ); + } + + SetLastError( MYERROR ); + retval = pGetMouseMovePointsEx( sizeof(MOUSEMOVEPOINT), &in, out, BUFLIM, GMMP_USE_DISPLAY_POINTS ); + ok( retval == 64, "expected to get 64 mouse move points but got %d\n", retval ); + ok( GetLastError() == MYERROR, "expected error to stay %x, got %x\n", MYERROR, GetLastError() ); + + for (int i = 0; i < retval; i++) + { + ok( out[i].x == in.x && out[i].y == in.y, "wrong position %d, expected %dx%d got %dx%d\n", i, in.x, in.y, out[i].x, out[i].y ); + in.x--; + in.y -= 2; + } + + in.x = 1500; + in.y = 1500; + SetLastError( MYERROR ); + retval = pGetMouseMovePointsEx( sizeof(MOUSEMOVEPOINT), &in, out, BUFLIM, GMMP_USE_DISPLAY_POINTS ); + ok( retval == -1, "expected to get -1 but got %d\n", retval ); + ok( GetLastError() == ERROR_POINT_NOT_FOUND, "expected error to be set to %x, got %x\n", ERROR_POINT_NOT_FOUND, GetLastError() ); + + /* make sure there's no deduplication */ + in.x = 6; + in.y = 6; + SetCursorPos( in.x, in.y ); + SetCursorPos( in.x, in.y ); + retval = pGetMouseMovePointsEx( sizeof(MOUSEMOVEPOINT), &in, out, BUFLIM, GMMP_USE_DISPLAY_POINTS ); + ok( retval == 64, "expected to get 64 mouse move points but got %d\n", retval ); + ok( out[0].x == 6 && out[0].y == 6, "expected cursor position to be 6x6 but got %d %d\n", out[0].x, out[0].y ); + ok( out[1].x == 6 && out[1].y == 6, "expected cursor position to be 6x6 but got %d %d\n", out[1].x, out[1].y ); + + /* make sure 2 events are distinguishable by their timestamps */ + in.x = 150; + in.y = 75; + SetCursorPos( 30, 30 ); + SetCursorPos( in.x, in.y ); + SetCursorPos( 150, 150 ); + Sleep( 3 ); + SetCursorPos( in.x, in.y ); + + retval = pGetMouseMovePointsEx( sizeof(MOUSEMOVEPOINT), &in, out, BUFLIM, GMMP_USE_DISPLAY_POINTS ); + ok( retval == 64, "expected to get 64 mouse move points but got %d\n", retval ); + ok( out[0].x == 150 && out[0].y == 75, "expected cursor position to be 150x75 but got %d %d\n", out[0].x, out[0].y ); + ok( out[1].x == 150 && out[1].y == 150, "expected cursor position to be 150x150 but got %d %d\n", out[1].x, out[1].y ); + ok( out[2].x == 150 && out[2].y == 75, "expected cursor position to be 150x75 but got %d %d\n", out[2].x, out[2].y ); + + in.time = out[2].time; + retval = pGetMouseMovePointsEx( sizeof(MOUSEMOVEPOINT), &in, out, BUFLIM, GMMP_USE_DISPLAY_POINTS ); + ok( retval == 62, "expected to get 62 mouse move points but got %d\n", retval ); + ok( out[0].x == 150 && out[0].y == 75, "expected cursor position to be 150x75 but got %d %d\n", out[0].x, out[0].y ); + ok( out[1].x == 30 && out[1].y == 30, "expected cursor position to be 30x30 but got %d %d\n", out[1].x, out[1].y ); + + /* events created through other means should also be on the list with correct extra info */ + mouse_event( MOUSEEVENTF_MOVE, -13, 17, 0, 0xcafecafe ); + ok( GetCursorPos( &point ), "failed to get cursor position\n" ); + ok( in.x != point.x && in.y != point.y, "cursor didn't change position after mouse_event()\n" ); + in.time = 0; + in.x = point.x; + in.y = point.y; + retval = pGetMouseMovePointsEx( sizeof(MOUSEMOVEPOINT), &in, out, BUFLIM, GMMP_USE_DISPLAY_POINTS ); + ok( retval == 64, "expected to get 64 mouse move points but got %d\n", retval ); + ok( out[0].dwExtraInfo == 0xcafecafe, "wrong extra info, got 0x%lx expected 0xcafecafe\n", out[0].dwExtraInfo ); + + input.type = INPUT_MOUSE; + memset( &input, 0, sizeof(input) ); + input.u.mi.dwFlags = MOUSEEVENTF_MOVE; + input.u.mi.dwExtraInfo = 0xdeadbeef; + input.u.mi.dx = -17; + input.u.mi.dy = 13; + SendInput( 1, (INPUT *)&input, sizeof(INPUT) ); + ok( GetCursorPos( &point ), "failed to get cursor position\n" ); + ok( in.x != point.x && in.y != point.y, "cursor didn't change position after mouse_event()\n" ); + in.time = 0; + in.x = point.x; + in.y = point.y; + retval = pGetMouseMovePointsEx( sizeof(MOUSEMOVEPOINT), &in, out, BUFLIM, GMMP_USE_DISPLAY_POINTS ); + ok( retval == 64, "expected to get 64 mouse move points but got %d\n", retval ); + ok( out[0].dwExtraInfo == 0xdeadbeef, "wrong extra info, got 0x%lx expected 0xdeadbeef\n", out[0].dwExtraInfo ); + + retval = pGetMouseMovePointsEx( sizeof(MOUSEMOVEPOINT), &in, out, BUFLIM, GMMP_USE_HIGH_RESOLUTION_POINTS ); + todo_wine ok( retval == 64, "expected to get 64 high resolution mouse move points but got %d\n", retval ); #undef BUFLIM #undef MYERROR } diff --git a/server/protocol.def b/server/protocol.def index 860632ee127..d7d3061c043 100644 --- a/server/protocol.def +++ b/server/protocol.def @@ -801,6 +801,22 @@ struct rawinput_device user_handle_t target; };
+typedef struct +{ + int x; + int y; + unsigned int time; + int __pad; + lparam_t info; +} cursor_pos_t; + +typedef struct +{ + unsigned int newest; + int __pad; + cursor_pos_t positions[64]; +} cursor_history_t; + /****************************************************************/ /* Request declarations */
@@ -3640,6 +3656,12 @@ struct handle_info #define SET_CURSOR_CLIP 0x08 #define SET_CURSOR_NOCLIP 0x10
+/* Get the history of the 64 last cursor positions */ +@REQ(get_cursor_history) +@REPLY + VARARG(history,cursor_history); +@END +
/* Batch read rawinput message data */ @REQ(get_rawinput_buffer) diff --git a/server/queue.c b/server/queue.c index eba5de0972e..f29e8e45028 100644 --- a/server/queue.c +++ b/server/queue.c @@ -227,6 +227,8 @@ static const struct object_ops thread_input_ops = /* pointer to input structure of foreground thread */ static unsigned int last_input_time;
+static cursor_history_t cursor_history; + static void queue_hardware_message( struct desktop *desktop, struct message *msg, int always_queue ); static void free_message( struct message *msg );
@@ -1528,14 +1530,32 @@ static void update_rawinput_device(const struct rawinput_device *device) e->device.target = get_user_full_handle( e->device.target ); }
+static void prepend_cursor_history( int x, int y, unsigned int time, lparam_t info ) +{ + const size_t positions_len = ARRAY_SIZE( cursor_history.positions ); + cursor_pos_t *pos; + cursor_history.newest = (cursor_history.newest + positions_len - 1) % positions_len; + pos = &cursor_history.positions[cursor_history.newest]; + + pos->x = x; + pos->y = y; + pos->time = time; + pos->info = info; +} + /* queue a hardware message into a given thread input */ static void queue_hardware_message( struct desktop *desktop, struct message *msg, int always_queue ) { user_handle_t win; struct thread *thread; struct thread_input *input; + struct hardware_msg_data *msg_data; unsigned int msg_code;
+ msg_data = msg->data; + if (msg->msg == WM_MOUSEMOVE) + prepend_cursor_history( msg->x, msg->y, msg->time, msg_data->info ); + update_input_key_state( desktop, desktop->keystate, msg->msg, msg->wparam ); last_input_time = get_tick_count(); if (msg->msg != WM_MOUSEMOVE) always_queue = 1; @@ -3313,6 +3333,15 @@ DECL_HANDLER(set_cursor) reply->last_change = input->desktop->cursor.last_change; }
+/* Get the history of the 64 last cursor positions */ +DECL_HANDLER(get_cursor_history) +{ + if (sizeof(cursor_history) <= get_reply_max_size()) + set_reply_data( &cursor_history, min( sizeof(cursor_history), get_reply_max_size() ) ); + else + set_error( STATUS_BUFFER_TOO_SMALL ); +} + DECL_HANDLER(get_rawinput_buffer) { struct thread_input *input = current->queue->input; diff --git a/server/trace.c b/server/trace.c index 32ccad16684..b7a242cd41a 100644 --- a/server/trace.c +++ b/server/trace.c @@ -889,6 +889,34 @@ static void dump_varargs_rectangles( const char *prefix, data_size_t size ) remove_data( size ); }
+static void dump_varargs_cursor_history( const char *prefix, data_size_t size ) +{ + const cursor_history_t *history = cur_data; + const cursor_pos_t *pos; + data_size_t len; + if (size != sizeof(*history)) + { + fprintf( stderr, "%s{}", prefix ); + return; + } + + fprintf( stderr, "%s{", prefix ); + fprintf( stderr, "newest=%u,", history->newest ); + fprintf( stderr, "positions={" ); + len = ARRAY_SIZE( history->positions ); + pos = history->positions; + while (len > 0) + { + fprintf( stderr, "{x=%d,y=%d,time=%u", pos->x, pos->y, pos->time ); + dump_uint64( ",info=", &pos->info ); + fputc( '}', stderr ); + pos++; + if (--len) fputc( ',', stderr ); + } + fputc( '}', stderr ); + fputc( '}', stderr ); +} + static void dump_varargs_message_data( const char *prefix, data_size_t size ) { /* FIXME: dump the structured data */
Hi,
While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check?
Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=79116
Your paranoid android.
=== w10pro64_2scr (64 bit report) ===
user32: input.c:1302: Test failed: Wrong set pos: (99,100) input.c:1322: Test failed: GetCursorPos: (99,100)
=== debiant (32 bit report) ===
user32: monitor: Timeout
=== debiant (32 bit Chinese:China report) ===
user32: clipboard.c:833: Test failed: 2: gle 5 clipboard.c:838: Test failed: 2.0: got 0000 instead of 000d clipboard.c:868: Test failed: 2: gle 1418 clipboard.c:717: Test failed: 3: gle 5 clipboard.c:719: Test failed: 3: gle 1418 clipboard.c:746: Test failed: 3: count 5 clipboard.c:749: Test failed: 3: gle 1418 clipboard.c:755: Test failed: 3: 0003 not available clipboard.c:757: Test failed: 3: count 5 instead of 2 clipboard.c:760: Test failed: 3: gle 5 clipboard.c:765: Test failed: 3.0: got 0000 instead of 000e clipboard.c:805: Test failed: 3: gle 1418 clipboard.c:815: Test failed: 3: count 5 clipboard.c:818: Test failed: 3: gle 1418 clipboard.c:826: Test failed: 3: 0003 not available clipboard.c:828: Test failed: 3: count 5 instead of 2 clipboard.c:833: Test failed: 3: gle 5 clipboard.c:838: Test failed: 3.0: got 0000 instead of 000e clipboard.c:868: Test failed: 3: gle 1418 clipboard.c:717: Test failed: 4: gle 5 clipboard.c:719: Test failed: 4: gle 1418 clipboard.c:746: Test failed: 4: count 6 clipboard.c:749: Test failed: 4: gle 1418 clipboard.c:757: Test failed: 4: count 6 instead of 2 clipboard.c:760: Test failed: 4: gle 5 clipboard.c:765: Test failed: 4.0: got 0000 instead of 0003 clipboard.c:805: Test failed: 4: gle 1418 clipboard.c:815: Test failed: 4: count 6 clipboard.c:818: Test failed: 4: gle 1418 clipboard.c:828: Test failed: 4: count 6 instead of 2 clipboard.c:833: Test failed: 4: gle 5 clipboard.c:838: Test failed: 4.0: got 0000 instead of 0003 clipboard.c:868: Test failed: 4: gle 1418 clipboard.c:717: Test failed: 5: gle 5 clipboard.c:719: Test failed: 5: gle 1418 clipboard.c:746: Test failed: 5: count 7 clipboard.c:749: Test failed: 5: gle 1418 clipboard.c:755: Test failed: 5: 0008 not available clipboard.c:755: Test failed: 5: 0011 not available clipboard.c:757: Test failed: 5: count 7 instead of 3 clipboard.c:760: Test failed: 5: gle 5 clipboard.c:765: Test failed: 5.0: got 0000 instead of 0002 clipboard.c:805: Test failed: 5: gle 1418 clipboard.c:815: Test failed: 5: count 7 clipboard.c:818: Test failed: 5: gle 1418 clipboard.c:826: Test failed: 5: 0008 not available clipboard.c:826: Test failed: 5: 0011 not available clipboard.c:828: Test failed: 5: count 7 instead of 3 clipboard.c:833: Test failed: 5: gle 5 clipboard.c:838: Test failed: 5.0: got 0000 instead of 0002 clipboard.c:868: Test failed: 5: gle 1418 clipboard.c:717: Test failed: 6: gle 5 clipboard.c:719: Test failed: 6: gle 1418 clipboard.c:746: Test failed: 6: count 8 clipboard.c:749: Test failed: 6: gle 1418 clipboard.c:755: Test failed: 6: 0011 not available clipboard.c:757: Test failed: 6: count 8 instead of 3 clipboard.c:760: Test failed: 6: gle 5 clipboard.c:765: Test failed: 6.0: got 0000 instead of 0008 clipboard.c:805: Test failed: 6: gle 1418 clipboard.c:815: Test failed: 6: count 8 clipboard.c:818: Test failed: 6: gle 1418 clipboard.c:826: Test failed: 6: 0011 not available clipboard.c:828: Test failed: 6: count 8 instead of 3 clipboard.c:833: Test failed: 6: gle 5 clipboard.c:838: Test failed: 6.0: got 0000 instead of 0008 clipboard.c:868: Test failed: 6: gle 1418 clipboard.c:717: Test failed: 7: gle 5 clipboard.c:719: Test failed: 7: gle 1418 clipboard.c:746: Test failed: 7: count 9 clipboard.c:749: Test failed: 7: gle 1418 clipboard.c:757: Test failed: 7: count 9 instead of 3 clipboard.c:760: Test failed: 7: gle 5 clipboard.c:765: Test failed: 7.0: got 0000 instead of 0011 clipboard.c:805: Test failed: 7: gle 1418 clipboard.c:815: Test failed: 7: count 9 clipboard.c:818: Test failed: 7: gle 1418 clipboard.c:828: Test failed: 7: count 9 instead of 3 clipboard.c:833: Test failed: 7: gle 5 clipboard.c:838: Test failed: 7.0: got 0000 instead of 0011 clipboard.c:868: Test failed: 7: gle 1418 clipboard.c:874: Test failed: gle 5 clipboard.c:876: Test failed: gle 1418 clipboard.c:878: Test failed: gle 1418 clipboard.c:1089: Test failed: OpenClipboard failed: 5 clipboard.c:1094: Test failed: EmptyClipboard failed: 1418 clipboard.c:1096: Test failed: sequence diff 0 clipboard.c:1099: Test failed: EmptyClipboard failed: 1418 clipboard.c:1101: Test failed: sequence diff 0 clipboard.c:1101: Test failed: WM_DESTROYCLIPBOARD not received clipboard.c:1108: Test failed: sequence diff 0 clipboard.c:1112: Test failed: sequence diff 0 clipboard.c:1116: Test failed: sequence diff 0 clipboard.c:1120: Test failed: CF_OEMTEXT available clipboard.c:1124: Test failed: CloseClipboard failed: 1418 clipboard.c:1127: Test failed: sequence diff 0 clipboard.c:1017: Test failed: wait failed clipboard.c:1127: Test failed: WM_DRAWCLIPBOARD not received clipboard.c:1127: Test failed: WM_CLIPBOARDUPDATE not received clipboard.c:1138: Test failed: wrong owner 00000000 clipboard.c:1140: Test failed: got data for CF_UNICODETEXT clipboard.c:1142: Test failed: WM_RENDERFORMAT received 0000 clipboard.c:1148: Test failed: WM_RENDERFORMAT received 0000 clipboard.c:1174: Test failed: WM_DESTROYCLIPBOARD not received clipboard.c:1176: Test failed: wrong format count 0 on WM_DESTROYCLIPBOARD monitor: Timeout
=== debiant (build log) ===
Task: WineTest did not produce the wow32 report
Rémi Bernon rbernon@codeweavers.com writes:
+typedef struct +{
- unsigned int newest;
- int __pad;
- cursor_pos_t positions[64];
+} cursor_history_t;
I expect it would be cleaner without this structure, simply returning an array of points.
@@ -227,6 +227,8 @@ static const struct object_ops thread_input_ops = /* pointer to input structure of foreground thread */ static unsigned int last_input_time;
+static cursor_history_t cursor_history;
This most likely needs to be per-desktop.
On Tue, Sep 29, 2020 at 12:17:12PM +0200, Alexandre Julliard wrote:
Rémi Bernon rbernon@codeweavers.com writes:
+typedef struct +{
- unsigned int newest;
- int __pad;
- cursor_pos_t positions[64];
+} cursor_history_t;
I expect it would be cleaner without this structure, simply returning an array of points.
I will still have to keep track of "newest" on the server side... Or I can use memmove() when adding elements to history to keep the positions[] ordered. That may be too costly for something that get called this often though. Any preference?
@@ -227,6 +227,8 @@ static const struct object_ops thread_input_ops = /* pointer to input structure of foreground thread */ static unsigned int last_input_time;
+static cursor_history_t cursor_history;
This most likely needs to be per-desktop.
Turns out it's shared between Desktops, at least within the same WindowStation. I'll add tests for that.
Do we care about making sure it's not per-winstation too?
Arkadiusz Hiler ahiler@codeweavers.com writes:
On Tue, Sep 29, 2020 at 12:17:12PM +0200, Alexandre Julliard wrote:
Rémi Bernon rbernon@codeweavers.com writes:
+typedef struct +{
- unsigned int newest;
- int __pad;
- cursor_pos_t positions[64];
+} cursor_history_t;
I expect it would be cleaner without this structure, simply returning an array of points.
I will still have to keep track of "newest" on the server side... Or I can use memmove() when adding elements to history to keep the positions[] ordered. That may be too costly for something that get called this often though. Any preference?
You can keep a position on the server side, and return them in the correct order in the server reply data.
@@ -227,6 +227,8 @@ static const struct object_ops thread_input_ops = /* pointer to input structure of foreground thread */ static unsigned int last_input_time;
+static cursor_history_t cursor_history;
This most likely needs to be per-desktop.
Turns out it's shared between Desktops, at least within the same WindowStation. I'll add tests for that.
Do we care about making sure it's not per-winstation too?
Yes.