[PATCH 0/3] MR3024: win32u: Introduce a new rawinput dedicated thread.
Currently only used to keep rawinput devices updated without iterating the entire list on every change. This thread will then let us solve several problems: * Solve issues with input not being polled if/when applications aren't checking their messages. This removes the need for polling messages in DInput, which is known to confuse and break DOSBOX. * Solve some issues with ll-hook deadlocks between hooking threads and the thread that is currently polling for inputs. * Useful to process internal messages and implement some user driver mechanisms. It can for instance already process WM_WINE_CLIPCURSOR messages, instead of relying on the foreground thread. * Similarly, for cursor icon changes, wineserver can notify it on cursor window changes and request the cursor icon to be updated from the user driver, instead of having to check the window the cursor is located in to change its icon on every mouse move. * Ultimately make it possible to drop the __wine_send_input PE export, the thread can read HID reports from devices that a process has registered for, and send WM_INPUT messages accordingly. This has some drawbacks though, as reading NT devices currently involves much more wineserver roundtrips than `__wine_send_input`. There's evidence that Windows uses a similar thread to process hardware events (as described in https://devblogs.microsoft.com/oldnewthing/20140213-00/?p=1773), although it's probably a system-wide kernel thread. I considered reading X11 input from a per-prefix dedicated thread (for instance in explorer.exe) or dedicated process, but having one thread per-process has several advantages. The rawinput device registration state is done per-process and according to https://gitlab.winehq.org/wine/wine/-/merge_requests/2120 it also needs to post notifications that are process specific. It also keeps the guarantee that each process will read the inputs from the window it has created, and not from some other windows, although the message dispatch may decide to send the message to another process later, it avoids having to listen to input on other processes windows. It can then be seen as an intermediate step towards having a per-prefix thread, if that's something we end up needing. Moving from a per-process thread to a per-prefix thread should be easier. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3024
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/user32/class.c | 3 +++ dlls/user32/input.c | 20 ++++++++++++++++++++ dlls/user32/user_private.h | 2 ++ dlls/win32u/rawinput.c | 10 ++++++++++ dlls/win32u/win32u_private.h | 1 + dlls/win32u/window.c | 3 +++ include/ntuser.h | 1 + 7 files changed, 40 insertions(+) diff --git a/dlls/user32/class.c b/dlls/user32/class.c index 0cfdf552a5d..9b717204b66 100644 --- a/dlls/user32/class.c +++ b/dlls/user32/class.c @@ -276,6 +276,9 @@ BOOL WINAPI User32InitBuiltinClasses( const struct win_hook_params *params, ULON /* Load uxtheme.dll so that standard scrollbars and dialogs are hooked for theming support */ load_uxtheme(); + + /* Initialize rawinput thread now that the desktop window is created */ + rawinput_init(); return TRUE; } diff --git a/dlls/user32/input.c b/dlls/user32/input.c index 8f3cd8acae7..8f37ae6a50f 100644 --- a/dlls/user32/input.c +++ b/dlls/user32/input.c @@ -31,6 +31,7 @@ WINE_DEFAULT_DEBUG_CHANNEL(win); WINE_DECLARE_DEBUG_CHANNEL(keyboard); +WINE_DECLARE_DEBUG_CHANNEL(rawinput); /*********************************************************************** * get_locale_kbd_layout @@ -890,3 +891,22 @@ HWND WINAPI GetTaskmanWindow(void) { return NtUserGetTaskmanWindow(); } + +static DWORD CALLBACK rawinput_thread( void *arg ) +{ + HWND hwnd = 0; + + TRACE_(rawinput)( "Starting rawinput thread\n" ); + + SetThreadDescription( GetCurrentThread(), L"wine_win32u_rawinput" ); + + /* wait for the desktop thread to fully initialize */ + SendMessageW( GetDesktopWindow(), WM_NULL, 0, 0 ); + + return NtUserCallHwndParam( hwnd, 0, NtUserCallHwndParam_RawInputThread ); +} + +void rawinput_init(void) +{ + CloseHandle( CreateThread( NULL, 0, rawinput_thread, NULL, 0, NULL ) ); +} diff --git a/dlls/user32/user_private.h b/dlls/user32/user_private.h index c3f877758c1..898a7496cae 100644 --- a/dlls/user32/user_private.h +++ b/dlls/user32/user_private.h @@ -45,6 +45,8 @@ struct wm_char_mapping_data extern HMODULE user32_module DECLSPEC_HIDDEN; +extern void rawinput_init(void); + extern BOOL post_dde_message( HWND hwnd, UINT msg, WPARAM wparam, LPARAM lparam, DWORD dest_tid, DWORD type ) DECLSPEC_HIDDEN; extern BOOL unpack_dde_message( HWND hwnd, UINT message, WPARAM *wparam, LPARAM *lparam, diff --git a/dlls/win32u/rawinput.c b/dlls/win32u/rawinput.c index bd2e00a31c4..466814cce2d 100644 --- a/dlls/win32u/rawinput.c +++ b/dlls/win32u/rawinput.c @@ -472,6 +472,16 @@ BOOL rawinput_device_get_usages( HANDLE handle, USAGE *usage_page, USAGE *usage return *usage_page || *usage; } +DWORD rawinput_thread( HWND hwnd, DWORD_PTR param ) +{ + for (;;) + { + NtUserMsgWaitForMultipleObjectsEx( 0, NULL, INFINITE, QS_ALLINPUT, 0 ); + } + + return 0; +} + /********************************************************************** * NtUserGetRawInputDeviceList (win32u.@) */ diff --git a/dlls/win32u/win32u_private.h b/dlls/win32u/win32u_private.h index 45e58093ba4..80c488dcdd9 100644 --- a/dlls/win32u/win32u_private.h +++ b/dlls/win32u/win32u_private.h @@ -138,6 +138,7 @@ extern LRESULT send_message_timeout( HWND hwnd, UINT msg, WPARAM wparam, LPARAM /* rawinput.c */ extern BOOL process_rawinput_message( MSG *msg, UINT hw_id, const struct hardware_msg_data *msg_data ) DECLSPEC_HIDDEN; extern BOOL rawinput_device_get_usages( HANDLE handle, USHORT *usage_page, USHORT *usage ) DECLSPEC_HIDDEN; +extern DWORD rawinput_thread( HWND hwnd, DWORD_PTR param ) DECLSPEC_HIDDEN; /* scroll.c */ extern void draw_nc_scrollbar( HWND hwnd, HDC hdc, BOOL draw_horizontal, BOOL draw_vertical ) DECLSPEC_HIDDEN; diff --git a/dlls/win32u/window.c b/dlls/win32u/window.c index 871227645a7..4bff0d85c83 100644 --- a/dlls/win32u/window.c +++ b/dlls/win32u/window.c @@ -5577,6 +5577,9 @@ ULONG_PTR WINAPI NtUserCallHwndParam( HWND hwnd, DWORD_PTR param, DWORD code ) case NtUserCallHwndParam_ShowOwnedPopups: return show_owned_popups( hwnd, param ); + case NtUserCallHwndParam_RawInputThread: + return rawinput_thread( hwnd, param ); + /* temporary exports */ case NtUserSetWindowStyle: { diff --git a/include/ntuser.h b/include/ntuser.h index 0fed9d72ec2..bae31657a5f 100644 --- a/include/ntuser.h +++ b/include/ntuser.h @@ -1360,6 +1360,7 @@ enum NtUserCallHwndParam_SetMDIClientInfo, NtUserCallHwndParam_SetWindowContextHelpId, NtUserCallHwndParam_ShowOwnedPopups, + NtUserCallHwndParam_RawInputThread, /* temporary exports */ NtUserSetWindowStyle, }; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3024
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/user32/input.c | 22 +++++++++++++++++++++- dlls/win32u/rawinput.c | 17 +++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/dlls/user32/input.c b/dlls/user32/input.c index 8f37ae6a50f..5b96a1d81d9 100644 --- a/dlls/user32/input.c +++ b/dlls/user32/input.c @@ -26,6 +26,10 @@ #include "user_private.h" #include "dbt.h" + +#include "initguid.h" +#include "ddk/hidclass.h" + #include "wine/server.h" #include "wine/debug.h" @@ -894,7 +898,19 @@ HWND WINAPI GetTaskmanWindow(void) static DWORD CALLBACK rawinput_thread( void *arg ) { - HWND hwnd = 0; + DEV_BROADCAST_DEVICEINTERFACE_W filter = + { + .dbcc_size = sizeof(DEV_BROADCAST_DEVICEINTERFACE_W), + .dbcc_devicetype = DBT_DEVTYP_DEVICEINTERFACE, + .dbcc_classguid = GUID_DEVINTERFACE_HID, + }; + WNDCLASSEXW wndclass = + { + .cbSize = sizeof(WNDCLASSEXW), + .lpszClassName = L"__wine_rawinput_devnotify", + .lpfnWndProc = DefWindowProcW, /* changed later from win32u rawinput_thread */ + }; + HWND hwnd; TRACE_(rawinput)( "Starting rawinput thread\n" ); @@ -903,6 +919,10 @@ static DWORD CALLBACK rawinput_thread( void *arg ) /* wait for the desktop thread to fully initialize */ SendMessageW( GetDesktopWindow(), WM_NULL, 0, 0 ); + RegisterClassExW( &wndclass ); + hwnd = CreateWindowExW( 0, wndclass.lpszClassName, NULL, 0, 0, 0, 0, 0, HWND_MESSAGE, NULL, NULL, NULL ); + + RegisterDeviceNotificationW( hwnd, &filter, DEVICE_NOTIFY_WINDOW_HANDLE ); return NtUserCallHwndParam( hwnd, 0, NtUserCallHwndParam_RawInputThread ); } diff --git a/dlls/win32u/rawinput.c b/dlls/win32u/rawinput.c index 466814cce2d..013a55bcc2a 100644 --- a/dlls/win32u/rawinput.c +++ b/dlls/win32u/rawinput.c @@ -472,11 +472,28 @@ BOOL rawinput_device_get_usages( HANDLE handle, USAGE *usage_page, USAGE *usage return *usage_page || *usage; } +static LRESULT CALLBACK rawinput_window_proc( HWND hwnd, UINT msg, WPARAM wparam, LPARAM lparam ) +{ + TRACE( "hwnd %p, msg %#x, wparam %#zx, lparam %#zx\n", hwnd, msg, (size_t)wparam, (size_t)lparam ); + + return NtUserMessageCall( hwnd, msg, wparam, lparam, 0, NtUserDefWindowProc, FALSE ); +} + DWORD rawinput_thread( HWND hwnd, DWORD_PTR param ) { + NtUserSetWindowLongPtr( hwnd, GWLP_WNDPROC, (LONG_PTR)rawinput_window_proc, FALSE ); + for (;;) { + MSG msg; + NtUserMsgWaitForMultipleObjectsEx( 0, NULL, INFINITE, QS_ALLINPUT, 0 ); + + while (NtUserPeekMessage( &msg, 0, 0, 0, PM_REMOVE )) + { + NtUserTranslateMessage( &msg, 0 ); + NtUserDispatchMessage( &msg ); + } } return 0; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3024
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/win32u/rawinput.c | 131 +++++++++++++++++++++++------------------ 1 file changed, 75 insertions(+), 56 deletions(-) diff --git a/dlls/win32u/rawinput.c b/dlls/win32u/rawinput.c index 013a55bcc2a..e495f3743ba 100644 --- a/dlls/win32u/rawinput.c +++ b/dlls/win32u/rawinput.c @@ -31,6 +31,8 @@ #define WIN32_NO_STATUS #include "winioctl.h" #include "ddk/hidclass.h" +#include "dbt.h" + #include "wine/hid.h" #include "wine/server.h" #include "wine/debug.h" @@ -209,11 +211,8 @@ static unsigned int registered_device_count; static struct list devices = LIST_INIT( devices ); static pthread_mutex_t rawinput_mutex = PTHREAD_MUTEX_INITIALIZER; -static struct device *add_device( HKEY key, DWORD type ) +static struct device *device_create( WCHAR *path, UINT type ) { - static const WCHAR symbolic_linkW[] = {'S','y','m','b','o','l','i','c','L','i','n','k',0}; - char value_buffer[offsetof(KEY_VALUE_PARTIAL_INFORMATION, Data[MAX_PATH * sizeof(WCHAR)])]; - KEY_VALUE_PARTIAL_INFORMATION *value = (KEY_VALUE_PARTIAL_INFORMATION *)value_buffer; static const RID_DEVICE_INFO_KEYBOARD keyboard_info = {0, 0, 1, 12, 3, 101}; static const RID_DEVICE_INFO_MOUSE mouse_info = {1, 5, 0, FALSE}; struct hid_preparsed_data *preparsed = NULL; @@ -228,18 +227,8 @@ static struct device *add_device( HKEY key, DWORD type ) void *buffer; SIZE_T size; HANDLE file; - WCHAR *path; - if (!query_reg_value( key, symbolic_linkW, value, sizeof(value_buffer) - sizeof(WCHAR) )) - { - ERR( "failed to get symbolic link value\n" ); - return NULL; - } - memset( value->Data + value->DataLength, 0, sizeof(WCHAR) ); - - /* upper case everything but the GUID */ - for (path = (WCHAR *)value->Data; *path && *path != '{'; path++) *path = towupper( *path ); - path = (WCHAR *)value->Data; + TRACE( "path %s type %#x\n", debugstr_w(path), type ); /* path is in DOS format and begins with \\?\ prefix */ path[1] = '?'; @@ -345,8 +334,6 @@ static struct device *add_device( HKEY key, DWORD type ) device->handle = ULongToHandle(handle); device->info = info; device->data = preparsed; - list_add_tail( &devices, &device->entry ); - return device; fail: @@ -355,6 +342,48 @@ fail: return NULL; } +static void device_remove( struct device *device ) +{ + list_remove( &device->entry ); + NtClose( device->file ); + free( device->data ); + free( device ); +} + +static struct device *find_device_from_path( WCHAR *path ) +{ + struct device *device; + + LIST_FOR_EACH_ENTRY( device, &devices, struct device, entry ) + if (!wcscmp( device->path, path )) return device; + + WARN( "Failed to find device with path %s.\n", debugstr_w(path) ); + return NULL; +} + +static void add_device( HKEY key, DWORD type ) +{ + static const WCHAR symbolic_linkW[] = {'S','y','m','b','o','l','i','c','L','i','n','k',0}; + char value_buffer[offsetof(KEY_VALUE_PARTIAL_INFORMATION, Data[MAX_PATH * sizeof(WCHAR)])]; + KEY_VALUE_PARTIAL_INFORMATION *value = (KEY_VALUE_PARTIAL_INFORMATION *)value_buffer; + struct device *device; + WCHAR *path; + + if (!query_reg_value( key, symbolic_linkW, value, sizeof(value_buffer) - sizeof(WCHAR) )) + { + ERR( "failed to get symbolic link value\n" ); + return; + } + memset( value->Data + value->DataLength, 0, sizeof(WCHAR) ); + + /* upper case everything but the GUID */ + for (path = (WCHAR *)value->Data; *path && *path != '{'; path++) *path = towupper( *path ); + path = (WCHAR *)value->Data; + + if ((device = device_create( path, type ))) + list_add_tail( &devices, &device->entry ); +} + static const WCHAR device_classesW[] = { '\\','R','e','g','i','s','t','r','y', @@ -419,25 +448,6 @@ static void enumerate_devices( DWORD type, const WCHAR *class ) NtClose( class_key ); } -static void rawinput_update_device_list(void) -{ - struct device *device, *next; - - TRACE( "\n" ); - - LIST_FOR_EACH_ENTRY_SAFE( device, next, &devices, struct device, entry ) - { - list_remove( &device->entry ); - NtClose( device->file ); - free( device->data ); - free( device ); - } - - enumerate_devices( RIM_TYPEMOUSE, guid_devinterface_mouseW ); - enumerate_devices( RIM_TYPEKEYBOARD, guid_devinterface_keyboardW ); - enumerate_devices( RIM_TYPEHID, guid_devinterface_hidW ); -} - static struct device *find_device_from_handle( HANDLE handle ) { struct device *device; @@ -445,11 +455,7 @@ static struct device *find_device_from_handle( HANDLE handle ) LIST_FOR_EACH_ENTRY( device, &devices, struct device, entry ) if (device->handle == handle) return device; - rawinput_update_device_list(); - - LIST_FOR_EACH_ENTRY( device, &devices, struct device, entry ) - if (device->handle == handle) return device; - + WARN( "Failed to find device with handle %p.\n", handle ); return NULL; } @@ -476,11 +482,37 @@ static LRESULT CALLBACK rawinput_window_proc( HWND hwnd, UINT msg, WPARAM wparam { TRACE( "hwnd %p, msg %#x, wparam %#zx, lparam %#zx\n", hwnd, msg, (size_t)wparam, (size_t)lparam ); + if (msg == WM_DEVICECHANGE) + { + DEV_BROADCAST_DEVICEINTERFACE_W *iface = (DEV_BROADCAST_DEVICEINTERFACE_W *)lparam; + struct device *device; + + switch (wparam) + { + case DBT_DEVICEARRIVAL: + pthread_mutex_lock( &rawinput_mutex ); + if ((device = device_create( iface->dbcc_name, RIM_TYPEHID ))) + list_add_tail( &devices, &device->entry ); + pthread_mutex_unlock( &rawinput_mutex ); + break; + case DBT_DEVICEREMOVECOMPLETE: + pthread_mutex_lock( &rawinput_mutex ); + if ((device = find_device_from_path( iface->dbcc_name ))) + device_remove( device ); + pthread_mutex_unlock( &rawinput_mutex ); + break; + } + } + return NtUserMessageCall( hwnd, msg, wparam, lparam, 0, NtUserDefWindowProc, FALSE ); } DWORD rawinput_thread( HWND hwnd, DWORD_PTR param ) { + enumerate_devices( RIM_TYPEMOUSE, guid_devinterface_mouseW ); + enumerate_devices( RIM_TYPEKEYBOARD, guid_devinterface_keyboardW ); + enumerate_devices( RIM_TYPEHID, guid_devinterface_hidW ); + NtUserSetWindowLongPtr( hwnd, GWLP_WNDPROC, (LONG_PTR)rawinput_window_proc, FALSE ); for (;;) @@ -504,8 +536,7 @@ DWORD rawinput_thread( HWND hwnd, DWORD_PTR param ) */ UINT WINAPI NtUserGetRawInputDeviceList( RAWINPUTDEVICELIST *device_list, UINT *device_count, UINT size ) { - unsigned int count = 0, ticks = NtGetTickCount(); - static unsigned int last_check; + unsigned int count = 0; struct device *device; TRACE( "device_list %p, device_count %p, size %u.\n", device_list, device_count, size ); @@ -524,12 +555,6 @@ UINT WINAPI NtUserGetRawInputDeviceList( RAWINPUTDEVICELIST *device_list, UINT * pthread_mutex_lock( &rawinput_mutex ); - if (ticks - last_check > 2000) - { - last_check = ticks; - rawinput_update_device_list(); - } - LIST_FOR_EACH_ENTRY( device, &devices, struct device, entry ) { if (*device_count < ++count || !device_list) continue; @@ -804,13 +829,7 @@ BOOL process_rawinput_message( MSG *msg, UINT hw_id, const struct hardware_msg_d if (!(thread_data = get_rawinput_thread_data())) return FALSE; - if (msg->message == WM_INPUT_DEVICE_CHANGE) - { - pthread_mutex_lock( &rawinput_mutex ); - rawinput_update_device_list(); - pthread_mutex_unlock( &rawinput_mutex ); - } - else + if (msg->message == WM_INPUT) { thread_data->buffer->header.dwSize = RAWINPUT_BUFFER_SIZE; if (!rawinput_from_hardware_message( thread_data->buffer, msg_data )) return FALSE; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3024
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=133590 Your paranoid android. === debian11 (32 bit report) === ntdll: rtl.c:2307: Test failed: got 2 rtl.c:2308: Test failed: got 0 win32u: win32u.c:410: Test failed: size = 3 win32u.c:413: Test failed: buf[0] = 000B0378 win32u.c:414: Test failed: buf[1] = 0010042C win32u.c:350: Test succeeded inside todo block: size = 3 win32u.c:357: Test succeeded inside todo block: buf[1] = 0010042C win32u.c:359: Test succeeded inside todo block: buf[2] = 0070015C === debian11 (32 bit zh:CN report) === win32u: win32u.c:410: Test failed: size = 3 win32u.c:350: Test succeeded inside todo block: size = 3 === debian11b (64 bit WoW report) === win32u: win32u.c:410: Test failed: size = 3 win32u.c:350: Test succeeded inside todo block: size = 3
On Thu Jun 8 15:01:34 2023 +0000, **** wrote:
Marvin replied on the mailing list: ``` 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=133590 Your paranoid android. === debian11 (32 bit report) === ntdll: rtl.c:2307: Test failed: got 2 rtl.c:2308: Test failed: got 0 win32u: win32u.c:410: Test failed: size = 3 win32u.c:413: Test failed: buf[0] = 000B0378 win32u.c:414: Test failed: buf[1] = 0010042C win32u.c:350: Test succeeded inside todo block: size = 3 win32u.c:357: Test succeeded inside todo block: buf[1] = 0010042C win32u.c:359: Test succeeded inside todo block: buf[2] = 0070015C === debian11 (32 bit zh:CN report) === win32u: win32u.c:410: Test failed: size = 3 win32u.c:350: Test succeeded inside todo block: size = 3 === debian11b (64 bit WoW report) === win32u: win32u.c:410: Test failed: size = 3 win32u.c:350: Test succeeded inside todo block: size = 3 ``` Will try to find a way to fix these.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3024#note_35172
participants (2)
-
Marvin -
Rémi Bernon