[PATCH v2 0/5] MR5322: server: Clear queue QS_RAWINPUT if get_rawinput_buffer got everything.
-- v2: server: Do not use QS_RAWINPUT for internal hardware messages. server: Clear queue QS_RAWINPUT if get_rawinput_buffer got everything. server: Process internal messages when checking queue status. server: Check for internal hardware messages before other messages. win32u: Use a structure to pass peek_message arguments. https://gitlab.winehq.org/wine/wine/-/merge_requests/5322
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/win32u/message.c | 49 ++++++++++++++++++++++++++++++++----------- 1 file changed, 37 insertions(+), 12 deletions(-) diff --git a/dlls/win32u/message.c b/dlls/win32u/message.c index f7f751ef883..6c2335dedee 100644 --- a/dlls/win32u/message.c +++ b/dlls/win32u/message.c @@ -2691,6 +2691,15 @@ static BOOL process_hardware_message( MSG *msg, UINT hw_id, const struct hardwar return ret; } +struct peek_message_filter +{ + HWND hwnd; + UINT first; + UINT last; + UINT mask; + UINT flags; +}; + /*********************************************************************** * peek_message * @@ -2698,9 +2707,11 @@ static BOOL process_hardware_message( MSG *msg, UINT hw_id, const struct hardwar * available; -1 on error. * All pending sent messages are processed before returning. */ -static int peek_message( MSG *msg, HWND hwnd, UINT first, UINT last, UINT flags, UINT changed_mask ) +static int peek_message( MSG *msg, const struct peek_message_filter *filter ) { LRESULT result; + HWND hwnd = filter->hwnd; + UINT first = filter->first, last = filter->last, flags = filter->flags; struct user_thread_info *thread_info = get_user_thread_info(); INPUT_MESSAGE_SOURCE prev_source = thread_info->client_info.msg_source; struct received_message_info info; @@ -2728,8 +2739,8 @@ static int peek_message( MSG *msg, HWND hwnd, UINT first, UINT last, UINT flags, req->get_first = first; req->get_last = last; req->hw_id = hw_id; - req->wake_mask = changed_mask & (QS_SENDMESSAGE | QS_SMRESULT); - req->changed_mask = changed_mask; + req->wake_mask = filter->mask & (QS_SENDMESSAGE | QS_SMRESULT); + req->changed_mask = filter->mask; wine_server_set_reply( req, buffer, buffer_size ); if (!(res = wine_server_call( req ))) { @@ -2754,8 +2765,8 @@ static int peek_message( MSG *msg, HWND hwnd, UINT first, UINT last, UINT flags, free( buffer ); if (res == STATUS_PENDING) { - thread_info->wake_mask = changed_mask & (QS_SENDMESSAGE | QS_SMRESULT); - thread_info->changed_mask = changed_mask; + thread_info->wake_mask = filter->mask & (QS_SENDMESSAGE | QS_SMRESULT); + thread_info->changed_mask = filter->mask; return 0; } if (res != STATUS_BUFFER_OVERFLOW) @@ -2900,8 +2911,17 @@ static int peek_message( MSG *msg, HWND hwnd, UINT first, UINT last, UINT flags, } } else - peek_message( msg, info.msg.hwnd, info.msg.message, - info.msg.message, flags | PM_REMOVE, changed_mask ); + { + struct peek_message_filter new_filter = + { + .hwnd = info.msg.hwnd, + .flags = flags | PM_REMOVE, + .first = info.msg.message, + .last = info.msg.message, + .mask = filter->mask, + }; + peek_message( msg, &new_filter ); + } continue; } if (info.msg.message >= WM_DDE_FIRST && info.msg.message <= WM_DDE_LAST) @@ -2952,7 +2972,7 @@ static int peek_message( MSG *msg, HWND hwnd, UINT first, UINT last, UINT flags, info.msg.wParam, info.msg.lParam ); /* if some PM_QS* flags were specified, only handle sent messages from now on */ - if (HIWORD(flags) && !changed_mask) flags = PM_QS_SENDMESSAGE | LOWORD(flags); + if (HIWORD(flags) && !filter->mask) flags = PM_QS_SENDMESSAGE | LOWORD(flags); } } @@ -2963,8 +2983,9 @@ static int peek_message( MSG *msg, HWND hwnd, UINT first, UINT last, UINT flags, */ static void process_sent_messages(void) { + struct peek_message_filter filter = {.flags = PM_REMOVE | PM_QS_SENDMESSAGE}; MSG msg; - peek_message( &msg, 0, 0, 0, PM_REMOVE | PM_QS_SENDMESSAGE, 0 ); + peek_message( &msg, &filter ); } /*********************************************************************** @@ -3187,13 +3208,14 @@ BOOL WINAPI NtUserWaitMessage(void) */ BOOL WINAPI NtUserPeekMessage( MSG *msg_out, HWND hwnd, UINT first, UINT last, UINT flags ) { + struct peek_message_filter filter = {.hwnd = hwnd, .first = first, .last = last, .flags = flags}; MSG msg; int ret; user_check_not_lock(); check_for_driver_events( 0 ); - ret = peek_message( &msg, hwnd, first, last, flags, 0 ); + ret = peek_message( &msg, &filter ); if (ret < 0) return FALSE; if (!ret) @@ -3201,7 +3223,7 @@ BOOL WINAPI NtUserPeekMessage( MSG *msg_out, HWND hwnd, UINT first, UINT last, U flush_window_surfaces( TRUE ); ret = wait_message( 0, NULL, 0, QS_ALLINPUT, 0 ); /* if we received driver events, check again for a pending message */ - if (ret == WAIT_TIMEOUT || peek_message( &msg, hwnd, first, last, flags, 0 ) <= 0) return FALSE; + if (ret == WAIT_TIMEOUT || peek_message( &msg, &filter ) <= 0) return FALSE; } check_for_driver_events( msg.message ); @@ -3224,6 +3246,7 @@ BOOL WINAPI NtUserPeekMessage( MSG *msg_out, HWND hwnd, UINT first, UINT last, U */ BOOL WINAPI NtUserGetMessage( MSG *msg, HWND hwnd, UINT first, UINT last ) { + struct peek_message_filter filter = {.hwnd = hwnd, .first = first, .last = last}; HANDLE server_queue = get_server_queue_handle(); unsigned int mask = QS_POSTMESSAGE | QS_SENDMESSAGE; /* Always selected */ int ret; @@ -3242,7 +3265,9 @@ BOOL WINAPI NtUserGetMessage( MSG *msg, HWND hwnd, UINT first, UINT last ) } else mask = QS_ALLINPUT; - while (!(ret = peek_message( msg, hwnd, first, last, PM_REMOVE | (mask << 16), mask ))) + filter.mask = mask; + filter.flags = PM_REMOVE | (mask << 16); + while (!(ret = peek_message( msg, &filter ))) { wait_objects( 1, &server_queue, INFINITE, mask & (QS_SENDMESSAGE | QS_SMRESULT), mask, 0 ); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5322
From: Rémi Bernon <rbernon(a)codeweavers.com> --- server/queue.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/server/queue.c b/server/queue.c index 0e8653bedf0..f355e8eb5ae 100644 --- a/server/queue.c +++ b/server/queue.c @@ -2983,6 +2983,12 @@ DECL_HANDLER(get_message) } if (!queue) return; + + /* check for any hardware internal message */ + if (get_hardware_message( current, req->hw_id, get_win, WM_WINE_FIRST_DRIVER_MSG, + WM_WINE_LAST_DRIVER_MSG, req->flags, reply )) + return; + queue->last_get_msg = current_time; if (!filter) filter = QS_ALLINPUT; @@ -3023,11 +3029,6 @@ DECL_HANDLER(get_message) get_hardware_message( current, req->hw_id, get_win, req->get_first, req->get_last, req->flags, reply )) return; - /* check for any internal driver message */ - if (get_hardware_message( current, req->hw_id, get_win, WM_WINE_FIRST_DRIVER_MSG, - WM_WINE_LAST_DRIVER_MSG, req->flags, reply )) - return; - /* now check for WM_PAINT */ if ((filter & QS_PAINT) && queue->paint_count && -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5322
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/win32u/input.c | 9 +++++++++ dlls/win32u/message.c | 13 +++---------- dlls/win32u/ntuser_private.h | 11 +++++++++++ server/protocol.def | 1 + server/queue.c | 6 ++++++ 5 files changed, 30 insertions(+), 10 deletions(-) diff --git a/dlls/win32u/input.c b/dlls/win32u/input.c index 4a7c04f66c5..1886ff979d7 100644 --- a/dlls/win32u/input.c +++ b/dlls/win32u/input.c @@ -791,8 +791,17 @@ BOOL WINAPI NtUserGetCursorInfo( CURSORINFO *info ) static void check_for_events( UINT flags ) { + struct peek_message_filter filter = + { + .internal = TRUE, + .flags = PM_REMOVE, + }; + MSG msg; + if (!user_driver->pProcessEvents( flags )) flush_window_surfaces( TRUE ); + + peek_message( &msg, &filter ); } /********************************************************************** diff --git a/dlls/win32u/message.c b/dlls/win32u/message.c index 6c2335dedee..84625f82964 100644 --- a/dlls/win32u/message.c +++ b/dlls/win32u/message.c @@ -2691,15 +2691,6 @@ static BOOL process_hardware_message( MSG *msg, UINT hw_id, const struct hardwar return ret; } -struct peek_message_filter -{ - HWND hwnd; - UINT first; - UINT last; - UINT mask; - UINT flags; -}; - /*********************************************************************** * peek_message * @@ -2707,7 +2698,7 @@ struct peek_message_filter * available; -1 on error. * All pending sent messages are processed before returning. */ -static int peek_message( MSG *msg, const struct peek_message_filter *filter ) +int peek_message( MSG *msg, const struct peek_message_filter *filter ) { LRESULT result; HWND hwnd = filter->hwnd; @@ -2734,6 +2725,7 @@ static int peek_message( MSG *msg, const struct peek_message_filter *filter ) SERVER_START_REQ( get_message ) { + req->internal = filter->internal; req->flags = flags; req->get_win = wine_server_user_handle( hwnd ); req->get_first = first; @@ -2919,6 +2911,7 @@ static int peek_message( MSG *msg, const struct peek_message_filter *filter ) .first = info.msg.message, .last = info.msg.message, .mask = filter->mask, + .internal = filter->internal, }; peek_message( msg, &new_filter ); } diff --git a/dlls/win32u/ntuser_private.h b/dlls/win32u/ntuser_private.h index 3b6cab5bdc9..bb2169998b6 100644 --- a/dlls/win32u/ntuser_private.h +++ b/dlls/win32u/ntuser_private.h @@ -238,6 +238,17 @@ extern void free_dce( struct dce *dce, HWND hwnd ); extern void invalidate_dce( WND *win, const RECT *extra_rect ); /* message.c */ +struct peek_message_filter +{ + HWND hwnd; + UINT first; + UINT last; + UINT mask; + UINT flags; + BOOL internal; +}; + +extern int peek_message( MSG *msg, const struct peek_message_filter *filter ); extern BOOL set_keyboard_auto_repeat( BOOL enable ); /* systray.c */ diff --git a/server/protocol.def b/server/protocol.def index 13aea96e796..d76390ce308 100644 --- a/server/protocol.def +++ b/server/protocol.def @@ -2149,6 +2149,7 @@ enum message_type unsigned int hw_id; /* id of the previous hardware message (or 0) */ unsigned int wake_mask; /* wakeup bits mask */ unsigned int changed_mask; /* changed bits mask */ + unsigned int internal; /* get internal messages only */ @REPLY user_handle_t win; /* window handle */ unsigned int msg; /* message code */ diff --git a/server/queue.c b/server/queue.c index f355e8eb5ae..27fcd74e996 100644 --- a/server/queue.c +++ b/server/queue.c @@ -2989,6 +2989,12 @@ DECL_HANDLER(get_message) WM_WINE_LAST_DRIVER_MSG, req->flags, reply )) return; + if (req->internal) /* check for internal messages only, leave queue flags unchanged */ + { + set_error( STATUS_PENDING ); + return; + } + queue->last_get_msg = current_time; if (!filter) filter = QS_ALLINPUT; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5322
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/dinput/tests/device8.c | 11 +++++++++++ server/queue.c | 7 +++++++ 2 files changed, 18 insertions(+) diff --git a/dlls/dinput/tests/device8.c b/dlls/dinput/tests/device8.c index 7fb0ef538e0..6bfc88f01cc 100644 --- a/dlls/dinput/tests/device8.c +++ b/dlls/dinput/tests/device8.c @@ -2038,6 +2038,9 @@ static void test_hid_touch_screen(void) ret = RegisterRawInputDevices( &rawdevice, 1, sizeof(RAWINPUTDEVICE) ); ok( ret, "RegisterRawInputDevices failed, error %lu\n", GetLastError() ); + res = GetQueueStatus( QS_RAWINPUT ); + ok( res == 0, "got res %#lx\n", res ); + bus_send_hid_input( file, &desc, &touch_multiple, sizeof(touch_multiple) ); bus_wait_hid_input( file, &desc, 5000 ); bus_send_hid_input( file, &desc, &touch_release, sizeof(touch_release) ); @@ -2046,6 +2049,10 @@ static void test_hid_touch_screen(void) res = MsgWaitForMultipleObjects( 0, NULL, FALSE, 500, QS_POINTER ); ok( !res, "MsgWaitForMultipleObjects returned %#lx\n", res ); + res = GetQueueStatus( QS_RAWINPUT ); + ok( LOWORD(res) == QS_RAWINPUT, "got res %#lx\n", res ); + ok( HIWORD(res) == QS_RAWINPUT, "got res %#lx\n", res ); + memset( rawbuffer, 0, sizeof(rawbuffer) ); rawinput = (RAWINPUT *)rawbuffer; rawbuffer_size = sizeof(rawbuffer); @@ -2064,6 +2071,10 @@ static void test_hid_touch_screen(void) ok( !memcmp( rawinput->data.hid.bRawData, touch_multiple.report_buf, desc.caps.InputReportByteLength ), "got unexpected report data\n" ); + res = GetQueueStatus( QS_RAWINPUT ); + ok( LOWORD(res) == 0, "got res %#lx\n", res ); + ok( HIWORD(res) == 0, "got res %#lx\n", res ); + rawdevice.dwFlags = RIDEV_REMOVE; rawdevice.hwndTarget = 0; ret = RegisterRawInputDevices( &rawdevice, 1, sizeof(RAWINPUTDEVICE) ); diff --git a/server/queue.c b/server/queue.c index 27fcd74e996..2eb8ece4dbc 100644 --- a/server/queue.c +++ b/server/queue.c @@ -3720,6 +3720,7 @@ DECL_HANDLER(get_rawinput_buffer) else if ((buffer = mem_alloc( buffer_size ))) { size_t total_size = 0, next_size = 0; + int clear = 1; reply->next_size = get_reply_max_size(); @@ -3734,6 +3735,7 @@ DECL_HANDLER(get_rawinput_buffer) if (total_size + sizeof(RAWINPUTHEADER) + data_size > buffer_size) { next_size = sizeof(RAWINPUTHEADER) + data_size; + clear = 0; break; } @@ -3748,6 +3750,10 @@ DECL_HANDLER(get_rawinput_buffer) free_message( msg ); count++; } + else if (get_hardware_msg_bit( msg->msg ) == QS_RAWINPUT) + { + clear = 0; + } } if (!next_size) @@ -3764,6 +3770,7 @@ DECL_HANDLER(get_rawinput_buffer) reply->count = count; set_reply_data_ptr( buffer, total_size ); + if (clear) clear_queue_bits( current->queue, QS_RAWINPUT ); } } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5322
From: Rémi Bernon <rbernon(a)codeweavers.com> --- server/queue.c | 29 ++++++++++++++++------------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/server/queue.c b/server/queue.c index 2eb8ece4dbc..cbe402f1fb9 100644 --- a/server/queue.c +++ b/server/queue.c @@ -47,6 +47,8 @@ #define WM_NCMOUSEFIRST WM_NCMOUSEMOVE #define WM_NCMOUSELAST (WM_NCMOUSEFIRST+(WM_MOUSELAST-WM_MOUSEFIRST)) +#define QS_INTERNAL_HARDWARE 0x80000000 + enum message_kind { SEND_MESSAGE, POST_MESSAGE }; #define NB_MSG_KINDS (POST_MESSAGE+1) @@ -581,7 +583,8 @@ void set_queue_hooks( struct thread *thread, struct hook_table *hooks ) /* check the queue status */ static inline int is_signaled( struct msg_queue *queue ) { - return ((queue->wake_bits & queue->wake_mask) || (queue->changed_bits & queue->changed_mask)); + return (queue->wake_bits & (queue->wake_mask | QS_INTERNAL_HARDWARE)) || + (queue->changed_bits & (queue->changed_mask | QS_INTERNAL_HARDWARE)); } /* set some queue bits */ @@ -630,6 +633,12 @@ static inline int filter_contains_hw_range( unsigned int first, unsigned int las return 1; } +/* is this message an internal driver notification message */ +static inline BOOL is_internal_hardware_message( unsigned int message ) +{ + return (message >= WM_WINE_FIRST_DRIVER_MSG && message <= WM_WINE_LAST_DRIVER_MSG); +} + /* get the QS_* bit corresponding to a given hardware message */ static inline int get_hardware_msg_bit( unsigned int message ) { @@ -637,8 +646,7 @@ static inline int get_hardware_msg_bit( unsigned int message ) if (message == WM_INPUT_DEVICE_CHANGE || message == WM_INPUT) return QS_RAWINPUT; if (message == WM_MOUSEMOVE || message == WM_NCMOUSEMOVE) return QS_MOUSEMOVE; if (message >= WM_KEYFIRST && message <= WM_KEYLAST) return QS_KEY; - if (message == WM_WINE_CLIPCURSOR) return QS_RAWINPUT; - if (message == WM_WINE_SETCURSOR) return QS_RAWINPUT; + if (is_internal_hardware_message( message )) return QS_INTERNAL_HARDWARE; return QS_MOUSEBUTTON; } @@ -1617,6 +1625,7 @@ static user_handle_t find_hardware_message_window( struct desktop *desktop, stru { case QS_POINTER: case QS_RAWINPUT: + case QS_INTERNAL_HARDWARE: if (!(win = msg->win) && input) win = input->focus; break; case QS_KEY: @@ -2411,12 +2420,6 @@ static int check_hw_message_filter( user_handle_t win, unsigned int msg_code, } } -/* is this message an internal driver notification message */ -static inline BOOL is_internal_hardware_message( unsigned int message ) -{ - return (message >= WM_WINE_FIRST_DRIVER_MSG && message <= WM_WINE_LAST_DRIVER_MSG); -} - /* find a hardware message for the given queue */ static int get_hardware_message( struct thread *thread, unsigned int hw_id, user_handle_t filter_win, unsigned int first, unsigned int last, unsigned int flags, @@ -2805,8 +2808,8 @@ DECL_HANDLER(set_queue_mask) { queue->wake_mask = req->wake_mask; queue->changed_mask = req->changed_mask; - reply->wake_bits = queue->wake_bits; - reply->changed_bits = queue->changed_bits; + reply->wake_bits = queue->wake_bits & ~QS_INTERNAL_HARDWARE; + reply->changed_bits = queue->changed_bits & ~QS_INTERNAL_HARDWARE; if (is_signaled( queue )) { /* if skip wait is set, do what would have been done in the subsequent wait */ @@ -2823,8 +2826,8 @@ DECL_HANDLER(get_queue_status) struct msg_queue *queue = current->queue; if (queue) { - reply->wake_bits = queue->wake_bits; - reply->changed_bits = queue->changed_bits; + reply->wake_bits = queue->wake_bits & ~QS_INTERNAL_HARDWARE; + reply->changed_bits = queue->changed_bits & ~QS_INTERNAL_HARDWARE; queue->changed_bits &= ~req->clear_bits; } else reply->wake_bits = reply->changed_bits = 0; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5322
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=144096 Your paranoid android. === w8 (32 bit report) === dinput: device8.c:2075: Test failed: got res 0x400 === debian11b (64 bit WoW report) === user32: input.c:3845: Test failed: WaitForSingleObject returned 258, error 0
Updated with a wineserver-internal queue bits for internal hardware messages instead of no bits at all. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5322#note_64897
Checking with Apex directly requires some non-trivial amount of work pulling these patches to Proton with some prior changes. Instead I made the test program which I believe rather closely reproduces what the game is doing which I am attaching. The patches fix the issue (never breaking out of raw input receiving loop). However, I am receiving almost no rawinput events while, e. g., moving mouse and pressing keyboard (as seen by printf output), that's not the case on Windows. I didn't debug why and whether that is related to the patches. [rawinput.c](/uploads/0202fc9dd26939dc66a0a807c60fce12/rawinput.c) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5322#note_64913
Upstream doesn't really support RIDEV_INPUTSINK yet, so basically you will only receive WM_INPUT messages when moving the mouse over a Wine window, maybe that's why you don't see so many messages? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5322#note_64915
That's the same when moving over the tests's window, also making sure that window is focused by clicking it and pressing keyboard also misses messages with this example. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5322#note_64916
@rbernon apex launches successfully with this patch, but compared to the hack from proton, a bug appears in apex that the cursor disappeared in the menu. I use native wayland wine(not xwayland). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5322#note_64956
@rbernon MR also cause regression in performance, when move the mouse in apex, the FPS drops by 2 times. There are no such problems with the Proton hack over vanilla git wine. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5322#note_64994
@h0tc0d3 Thanks, I'll have a better look. @gofman The missing rawinput messages comes from the difference in (upstream) Wine where X11 input is only checked when the application checks for messages. This means that rawinput messages are only queued when PeekMessage is called, and are then mostly immediately processed by the message loop and leave the rawinput buffer empty. On Windows (and in Proton), input comes from the background and will fill the rawinput buffer during the Sleep. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5322#note_65177
participants (4)
-
Grigory Vasilyev (＠h0tc0d3) -
Marvin -
Paul Gofman (＠gofman)
-
Rémi Bernon