[PATCH v5 0/8] MR10364: dinput: Add support for generating consistent guidInstance values for devices.
This MR adds support for creating `guidInstance` values for joystick devices in a way that matches native behavior. Our current method of creating a `guidInstance` value uses the device's rawinput handle, which changes each time a controller is plugged in. Fixes a bug in Rocket League where a controller plug/unplug cycle results in a new controller being detected each time. The process is like this: - Controller is connected, the game gets its `guidInstance` value from `EnumDevices()`, stores it and associates it with player 1. - Controller is disconnected. - The same controller is reconnected, the game calls `EnumDevices()`, and finds a new `guidInstance` value. - It then assumes this is a new controller, creates a new player, and behaves as though player 1's controller is unplugged. This MR implements `guidInstance` in a way that matches native, and ensures that the `guidInstance` value remains consistent. Doing things this way also makes it easier to add support for `DIPROP_JOYSTICKID` and `GUID_Joystick` in a future MR. I've implemented those in a branch [here](https://gitlab.winehq.org/cmcadams/wine/-/tree/WIP/dinput-instance-guid-v08) using this system. -- v5: dinput: Refresh joystick devices when an unknown GUID is used to create a device. dinput: Return E_FAIL when opening a joystick device that is currently disconnected. dinput: Create a version 1 UUID for dinput joystick devices. dinput: Create registry entries for HID joysticks. dinput: Use joystick cache in hid_joystick_create_device(). dinput: Use joystick cache in hid_joystick_enum_device(). dinput: Create a cache of currently connected HID joysticks. dinput: Only pass instance name string to device_instance_is_disabled(). https://gitlab.winehq.org/wine/wine/-/merge_requests/10364
From: Connor McAdams <cmcadams@codeweavers.com> Signed-off-by: Connor McAdams <cmcadams@codeweavers.com> --- dlls/dinput/device.c | 16 +++++++--------- dlls/dinput/device_private.h | 2 +- dlls/dinput/joystick_hid.c | 2 +- 3 files changed, 9 insertions(+), 11 deletions(-) diff --git a/dlls/dinput/device.c b/dlls/dinput/device.c index 35518dd91dd..142a9908b29 100644 --- a/dlls/dinput/device.c +++ b/dlls/dinput/device.c @@ -145,11 +145,9 @@ DWORD get_config_key( HKEY defkey, HKEY appkey, const WCHAR *name, WCHAR *buffer return ERROR_FILE_NOT_FOUND; } -BOOL device_instance_is_disabled( DIDEVICEINSTANCEW *instance, BOOL *override ) +BOOL device_instance_is_disabled( const WCHAR *instance, BOOL *override ) { - static const WCHAR disabled_str[] = {'d', 'i', 's', 'a', 'b', 'l', 'e', 'd', 0}; - static const WCHAR override_str[] = {'o', 'v', 'e', 'r', 'r', 'i', 'd', 'e', 0}; - static const WCHAR joystick_key[] = {'J', 'o', 'y', 's', 't', 'i', 'c', 'k', 's', 0}; + static const WCHAR *joystick_key = L"Joysticks"; WCHAR buffer[MAX_PATH]; HKEY hkey, appkey, temp; BOOL disable = FALSE; @@ -173,16 +171,16 @@ BOOL device_instance_is_disabled( DIDEVICEINSTANCEW *instance, BOOL *override ) } /* Look for the "controllername"="disabled" key */ - if (!get_config_key( hkey, appkey, instance->tszInstanceName, buffer, sizeof(buffer) )) + if (!get_config_key( hkey, appkey, instance, buffer, sizeof(buffer) )) { - if (!wcscmp( disabled_str, buffer )) + if (!wcscmp( L"disabled", buffer )) { - TRACE( "Disabling joystick '%s' based on registry key.\n", debugstr_w(instance->tszInstanceName) ); + TRACE( "Disabling joystick '%s' based on registry key.\n", debugstr_w(instance) ); disable = TRUE; } - else if (override && !wcscmp( override_str, buffer )) + else if (override && !wcscmp( L"override", buffer )) { - TRACE( "Force enabling joystick '%s' based on registry key.\n", debugstr_w(instance->tszInstanceName) ); + TRACE( "Force enabling joystick '%s' based on registry key.\n", debugstr_w(instance) ); *override = TRUE; } } diff --git a/dlls/dinput/device_private.h b/dlls/dinput/device_private.h index fa791f08a9d..fcab99779c5 100644 --- a/dlls/dinput/device_private.h +++ b/dlls/dinput/device_private.h @@ -134,7 +134,7 @@ extern BOOL device_object_matches_semantic( const DIDEVICEINSTANCEW *instance, c extern BOOL get_app_key(HKEY*, HKEY*); extern DWORD get_config_key( HKEY, HKEY, const WCHAR *, WCHAR *, DWORD ); -extern BOOL device_instance_is_disabled( DIDEVICEINSTANCEW *instance, BOOL *override ); +extern BOOL device_instance_is_disabled( const WCHAR *instance, BOOL *override ); extern void queue_event( IDirectInputDevice8W *iface, int index, DWORD data, DWORD time, DWORD seq ); extern const GUID dinput_pidvid_guid; diff --git a/dlls/dinput/joystick_hid.c b/dlls/dinput/joystick_hid.c index 22709c4bfd9..c2954553805 100644 --- a/dlls/dinput/joystick_hid.c +++ b/dlls/dinput/joystick_hid.c @@ -1605,7 +1605,7 @@ static HRESULT hid_joystick_device_open( int index, const GUID *guid, DIDEVICEIN attrs, caps, instance, version ))) continue; - if (device_instance_is_disabled( instance, &override )) + if (device_instance_is_disabled( instance->tszInstanceName, &override )) goto next; if (override && SetupDiGetDeviceInstanceIdW( set, &devinfo, device_id, MAX_PATH, NULL ) && -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10364
From: Connor McAdams <cmcadams@codeweavers.com> --- dlls/dinput/dinput.c | 1 + dlls/dinput/dinput_main.c | 1 + dlls/dinput/dinput_private.h | 2 + dlls/dinput/joystick_hid.c | 149 ++++++++++++++++++++++++++++++++++- 4 files changed, 152 insertions(+), 1 deletion(-) diff --git a/dlls/dinput/dinput.c b/dlls/dinput/dinput.c index f851e17a244..1695221d3a6 100644 --- a/dlls/dinput/dinput.c +++ b/dlls/dinput/dinput.c @@ -372,6 +372,7 @@ static HRESULT WINAPI dinput8_EnumDevices( IDirectInput8W *iface, DWORD type, LP if (device_class == DI8DEVCLASS_ALL || device_class == DI8DEVCLASS_GAMECTRL) { + if (FAILED(hid_joystick_refresh_devices())) WARN( "Failed to refresh HID joystick devices.\n" ); do { hr = hid_joystick_enum_device( type, flags, &instance, impl->dwVersion, i++ ); diff --git a/dlls/dinput/dinput_main.c b/dlls/dinput/dinput_main.c index e4bc1f7961d..b2dfaf76a92 100644 --- a/dlls/dinput/dinput_main.c +++ b/dlls/dinput/dinput_main.c @@ -523,6 +523,7 @@ BOOL WINAPI DllMain( HINSTANCE inst, DWORD reason, void *reserved ) case DLL_PROCESS_DETACH: if (reserved) break; unregister_di_em_win_class(); + hid_joystick_cleanup(); break; } return TRUE; diff --git a/dlls/dinput/dinput_private.h b/dlls/dinput/dinput_private.h index 0609b816b3b..f2d2ec1484c 100644 --- a/dlls/dinput/dinput_private.h +++ b/dlls/dinput/dinput_private.h @@ -56,6 +56,8 @@ extern HRESULT keyboard_enum_device( DWORD type, DWORD flags, DIDEVICEINSTANCEW extern HRESULT keyboard_create_device( struct dinput *dinput, const GUID *guid, IDirectInputDevice8W **out ); extern HRESULT hid_joystick_enum_device( DWORD type, DWORD flags, DIDEVICEINSTANCEW *instance, DWORD version, int index ); extern HRESULT hid_joystick_create_device( struct dinput *dinput, const GUID *guid, IDirectInputDevice8W **out ); +extern HRESULT hid_joystick_refresh_devices( void ); +extern void hid_joystick_cleanup( void ); struct DevicePlayer { GUID instance_guid; diff --git a/dlls/dinput/joystick_hid.c b/dlls/dinput/joystick_hid.c index c2954553805..8c6a752c020 100644 --- a/dlls/dinput/joystick_hid.c +++ b/dlls/dinput/joystick_hid.c @@ -36,7 +36,7 @@ #include "setupapi.h" #include "devguid.h" #include "dinput.h" -#include "setupapi.h" +#include "cfgmgr32.h" #include "dinput_private.h" #include "device_private.h" @@ -52,6 +52,42 @@ DEFINE_GUID( GUID_DEVINTERFACE_WINEXINPUT,0x6c53d5fd,0x6480,0x440f,0xb6,0x18,0x4 DEFINE_GUID( hid_joystick_guid, 0x9e573edb, 0x7734, 0x11d2, 0x8d, 0x4a, 0x23, 0x90, 0x3f, 0xb6, 0xbd, 0xf7 ); DEFINE_GUID( device_path_guid, 0x00000000, 0x0000, 0x0000, 0x8d, 0x4a, 0x23, 0x90, 0x3f, 0xb6, 0xbd, 0xf8 ); +static CRITICAL_SECTION joystick_cache_cs; +static CRITICAL_SECTION_DEBUG joystick_cache_cs_debug = +{ + 0, 0, &joystick_cache_cs, + { &joystick_cache_cs_debug.ProcessLocksList, &joystick_cache_cs_debug.ProcessLocksList }, + 0, 0, { (DWORD_PTR)(__FILE__ ": joystick_cache_cs") } +}; +static CRITICAL_SECTION joystick_cache_cs = { &joystick_cache_cs_debug, -1, 0, 0, 0, 0 }; + +/* Need to enter the CS to access this list. */ +static struct list joystick_cache = LIST_INIT(joystick_cache); + +struct joystick_instance +{ + struct list entry; + WORD vid, pid; + + WCHAR dev_path[MAX_PATH]; +}; + +static void joystick_cache_free( void ) +{ + struct joystick_instance *cur, *cur2; + + LIST_FOR_EACH_ENTRY_SAFE( cur, cur2, &joystick_cache, struct joystick_instance, entry ) + { + list_remove( &cur->entry ); + free(cur); + } +} + +void hid_joystick_cleanup( void ) +{ + joystick_cache_free(); +} + struct pid_control_report { BYTE id; @@ -1572,6 +1608,117 @@ failed: return DIERR_DEVICENOTREG; } +static HRESULT hid_joystick_get_class_dev_iface_list( const GUID *class, WCHAR **list ) +{ + WCHAR *ifaces = NULL; + GUID guid = *class; + ULONG size = 0; + CONFIGRET ret; + + *list = NULL; + while (1) + { + if ((ret = CM_Get_Device_Interface_List_SizeW( &size, &guid, NULL, CM_GET_DEVICE_INTERFACE_LIST_PRESENT ))) + WARN( "Got unexpected return value %lu.\n", ret ); + if (ret || size == 1) return DIERR_DEVICENOTREG; + if (!(ifaces = calloc( size, sizeof(*ifaces) ))) return E_OUTOFMEMORY; + + if (!(ret = CM_Get_Device_Interface_ListW( &guid, NULL, ifaces, size, CM_GET_DEVICE_INTERFACE_LIST_PRESENT ))) + break; + free( ifaces ); + if (ret != CR_BUFFER_SMALL) return E_FAIL; + } + + *list = ifaces; + return DI_OK; +} + +static BOOL hid_device_is_valid_joystick( const WCHAR *path, WORD *vid, WORD *pid ) +{ + PHIDP_PREPARSED_DATA preparsed = NULL; + HIDD_ATTRIBUTES attrs = { 0 }; + HANDLE device_file = NULL; + WCHAR product[MAX_PATH]; + BOOL ret_val = FALSE; + HIDP_CAPS caps; + + device_file = CreateFileW( path, GENERIC_READ | GENERIC_WRITE, FILE_SHARE_READ | FILE_SHARE_WRITE, + NULL, OPEN_EXISTING, FILE_FLAG_OVERLAPPED | FILE_FLAG_NO_BUFFERING, 0 ); + if (device_file == INVALID_HANDLE_VALUE) return FALSE; + + if (!HidD_GetPreparsedData( device_file, &preparsed )) goto exit; + if (!HidD_GetAttributes( device_file, &attrs )) goto exit; + if (HidP_GetCaps( preparsed, &caps ) != HIDP_STATUS_SUCCESS) goto exit; + + switch (MAKELONG( caps.Usage, caps.UsagePage )) + { + case MAKELONG( HID_USAGE_GENERIC_GAMEPAD, HID_USAGE_PAGE_GENERIC ): break; + case MAKELONG( HID_USAGE_GENERIC_JOYSTICK, HID_USAGE_PAGE_GENERIC ): break; + case MAKELONG( HID_USAGE_GENERIC_MOUSE, HID_USAGE_PAGE_GENERIC ): goto exit; + case MAKELONG( HID_USAGE_GENERIC_KEYBOARD, HID_USAGE_PAGE_GENERIC ): goto exit; + default: FIXME( "device usage %04x:%04x not implemented!\n", caps.UsagePage, caps.Usage); goto exit; + } + + if (!HidD_GetProductString( device_file, product, sizeof(product) )) goto exit; + if (device_instance_is_disabled( product, NULL )) goto exit; + *vid = attrs.VendorID; + *pid = attrs.ProductID; + ret_val = TRUE; + +exit: + HidD_FreePreparsedData( preparsed ); + CloseHandle( device_file ); + return ret_val; +} + +HRESULT hid_joystick_refresh_devices( void ) +{ + WCHAR *hid_list, *tmp; + HRESULT hr; + GUID hid; + + TRACE( "\n" ); + + hid_list = NULL; + HidD_GetHidGuid( &hid ); + hr = hid_joystick_get_class_dev_iface_list( &hid, &hid_list ); + if (FAILED(hr)) return hr; + + /* + * Iterate over connected HID devices, find all joysticks, and add them to + * the joystick local cache list. + */ + EnterCriticalSection( &joystick_cache_cs ); + joystick_cache_free(); + for (tmp = hid_list; *tmp; tmp = tmp + wcslen( tmp ) + 1) + { + struct joystick_instance *instance = NULL; + WORD vid, pid; + + if (!hid_device_is_valid_joystick( tmp, &vid, &pid )) continue; + if (!(instance = malloc( sizeof(*instance) ))) + { + hr = E_OUTOFMEMORY; + goto exit; + } + instance->vid = vid; + instance->pid = pid; + wcscpy( instance->dev_path, tmp ); + /* + * SetupAPI returns paths in lowercase, cfgmgr32 does not. joy.cpl + * depends on this being lowercase. + */ + CharLowerW( instance->dev_path ); + list_add_tail( &joystick_cache, &instance->entry ); + } + +exit: + if (FAILED(hr)) joystick_cache_free(); + free( hid_list ); + LeaveCriticalSection( &joystick_cache_cs ); + return hr; +} + static HRESULT hid_joystick_device_open( int index, const GUID *guid, DIDEVICEINSTANCEW *instance, WCHAR *device_path, HANDLE *device, PHIDP_PREPARSED_DATA *preparsed, HIDD_ATTRIBUTES *attrs, HIDP_CAPS *caps, DWORD version ) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10364
From: Connor McAdams <cmcadams@codeweavers.com> --- dlls/dinput/Makefile.in | 2 +- dlls/dinput/joystick_hid.c | 80 +++++++++++++++++++++++++++++++++----- dlls/dinput8/Makefile.in | 2 +- 3 files changed, 72 insertions(+), 12 deletions(-) diff --git a/dlls/dinput/Makefile.in b/dlls/dinput/Makefile.in index 70aa575a837..883639195ba 100644 --- a/dlls/dinput/Makefile.in +++ b/dlls/dinput/Makefile.in @@ -1,6 +1,6 @@ MODULE = dinput.dll IMPORTLIB = dinput -IMPORTS = dinput dxguid uuid comctl32 ole32 user32 advapi32 hid setupapi +IMPORTS = dinput dxguid uuid comctl32 ole32 user32 advapi32 hid setupapi cfgmgr32 EXTRADEFS = -DDIRECTINPUT_VERSION=0x0700 VER_FILEDESCRIPTION_STR = "Wine DirectInput" diff --git a/dlls/dinput/joystick_hid.c b/dlls/dinput/joystick_hid.c index 8c6a752c020..b475eb64000 100644 --- a/dlls/dinput/joystick_hid.c +++ b/dlls/dinput/joystick_hid.c @@ -42,6 +42,8 @@ #include "device_private.h" #include "initguid.h" +#include "devpkey.h" +#include "propkey.h" #include "wine/debug.h" #include "wine/hid.h" @@ -67,6 +69,7 @@ static struct list joystick_cache = LIST_INIT(joystick_cache); struct joystick_instance { struct list entry; + BOOL override; WORD vid, pid; WCHAR dev_path[MAX_PATH]; @@ -83,6 +86,21 @@ static void joystick_cache_free( void ) } } +static struct joystick_instance *joystick_cache_get_instance_from_index( unsigned int index ) +{ + struct joystick_instance *inst = NULL; + unsigned int i = 0; + + if (index >= list_count( &joystick_cache )) return NULL; + + LIST_FOR_EACH_ENTRY( inst, &joystick_cache, struct joystick_instance, entry ) + { + if (index == i++) break; + } + + return inst; +} + void hid_joystick_cleanup( void ) { joystick_cache_free(); @@ -1608,7 +1626,7 @@ failed: return DIERR_DEVICENOTREG; } -static HRESULT hid_joystick_get_class_dev_iface_list( const GUID *class, WCHAR **list ) +static HRESULT hid_joystick_get_class_dev_iface_list( const GUID *class, const DEVINSTID_W instance_id, WCHAR **list ) { WCHAR *ifaces = NULL; GUID guid = *class; @@ -1618,12 +1636,12 @@ static HRESULT hid_joystick_get_class_dev_iface_list( const GUID *class, WCHAR * *list = NULL; while (1) { - if ((ret = CM_Get_Device_Interface_List_SizeW( &size, &guid, NULL, CM_GET_DEVICE_INTERFACE_LIST_PRESENT ))) + if ((ret = CM_Get_Device_Interface_List_SizeW( &size, &guid, instance_id, CM_GET_DEVICE_INTERFACE_LIST_PRESENT ))) WARN( "Got unexpected return value %lu.\n", ret ); if (ret || size == 1) return DIERR_DEVICENOTREG; if (!(ifaces = calloc( size, sizeof(*ifaces) ))) return E_OUTOFMEMORY; - if (!(ret = CM_Get_Device_Interface_ListW( &guid, NULL, ifaces, size, CM_GET_DEVICE_INTERFACE_LIST_PRESENT ))) + if (!(ret = CM_Get_Device_Interface_ListW( &guid, instance_id, ifaces, size, CM_GET_DEVICE_INTERFACE_LIST_PRESENT ))) break; free( ifaces ); if (ret != CR_BUFFER_SMALL) return E_FAIL; @@ -1633,7 +1651,7 @@ static HRESULT hid_joystick_get_class_dev_iface_list( const GUID *class, WCHAR * return DI_OK; } -static BOOL hid_device_is_valid_joystick( const WCHAR *path, WORD *vid, WORD *pid ) +static BOOL hid_device_is_valid_joystick( const WCHAR *path, WORD *vid, WORD *pid, BOOL *override ) { PHIDP_PREPARSED_DATA preparsed = NULL; HIDD_ATTRIBUTES attrs = { 0 }; @@ -1660,7 +1678,7 @@ static BOOL hid_device_is_valid_joystick( const WCHAR *path, WORD *vid, WORD *pi } if (!HidD_GetProductString( device_file, product, sizeof(product) )) goto exit; - if (device_instance_is_disabled( product, NULL )) goto exit; + if (device_instance_is_disabled( product, override )) goto exit; *vid = attrs.VendorID; *pid = attrs.ProductID; ret_val = TRUE; @@ -1681,7 +1699,7 @@ HRESULT hid_joystick_refresh_devices( void ) hid_list = NULL; HidD_GetHidGuid( &hid ); - hr = hid_joystick_get_class_dev_iface_list( &hid, &hid_list ); + hr = hid_joystick_get_class_dev_iface_list( &hid, NULL, &hid_list ); if (FAILED(hr)) return hr; /* @@ -1693,9 +1711,10 @@ HRESULT hid_joystick_refresh_devices( void ) for (tmp = hid_list; *tmp; tmp = tmp + wcslen( tmp ) + 1) { struct joystick_instance *instance = NULL; + BOOL override; WORD vid, pid; - if (!hid_device_is_valid_joystick( tmp, &vid, &pid )) continue; + if (!hid_device_is_valid_joystick( tmp, &vid, &pid, &override )) continue; if (!(instance = malloc( sizeof(*instance) ))) { hr = E_OUTOFMEMORY; @@ -1703,6 +1722,7 @@ HRESULT hid_joystick_refresh_devices( void ) } instance->vid = vid; instance->pid = pid; + instance->override = override; wcscpy( instance->dev_path, tmp ); /* * SetupAPI returns paths in lowercase, cfgmgr32 does not. joy.cpl @@ -1794,20 +1814,60 @@ static HRESULT hid_joystick_device_open( int index, const GUID *guid, DIDEVICEIN return DI_OK; } +static HRESULT hid_joystick_instance_try_open( struct joystick_instance *instance, HANDLE *device, + PHIDP_PREPARSED_DATA *preparsed, HIDD_ATTRIBUTES *attrs, HIDP_CAPS *caps, + DIDEVICEINSTANCEW *di_instance, WCHAR *device_path, DWORD version ) +{ + WCHAR *path = instance->dev_path; + HRESULT hr; + + if (instance->override) + { + WCHAR instance_id[MAX_DEVICE_ID_LEN] = { 0 }; + DEVPROPTYPE type; + CONFIGRET ret; + WCHAR *tmp; + ULONG size; + + size = sizeof(instance_id); + ret = CM_Get_Device_Interface_PropertyW( instance->dev_path, &DEVPKEY_Device_InstanceId, &type, + (BYTE *)instance_id, &size, 0 ); + if (ret || !(tmp = wcsstr( instance_id, L"&IG_" ))) + { + WARN( "Failed to get valid instance id for overridden device.\n" ); + return E_FAIL; + } + + memcpy( tmp, L"&XI_", sizeof(L"&XI_") - sizeof(WCHAR) ); + if (FAILED(hid_joystick_get_class_dev_iface_list( &GUID_DEVINTERFACE_WINEXINPUT, instance_id, &path ))) + return E_FAIL; + } + + hr = hid_joystick_device_try_open( path, device, preparsed, attrs, caps, di_instance, version ); + if (SUCCEEDED(hr)) wcscpy( device_path, path ); + if (path != instance->dev_path) free( path ); + return hr; +} + + HRESULT hid_joystick_enum_device( DWORD type, DWORD flags, DIDEVICEINSTANCEW *instance, DWORD version, int index ) { HIDD_ATTRIBUTES attrs = {.Size = sizeof(attrs)}; PHIDP_PREPARSED_DATA preparsed; + struct joystick_instance *inst; WCHAR device_path[MAX_PATH]; - GUID guid = GUID_NULL; HIDP_CAPS caps; HANDLE device; HRESULT hr; TRACE( "type %#lx, flags %#lx, instance %p, version %#lx, index %d\n", type, flags, instance, version, index ); - hr = hid_joystick_device_open( index, &guid, instance, device_path, &device, &preparsed, - &attrs, &caps, version ); + EnterCriticalSection( &joystick_cache_cs ); + if ((inst = joystick_cache_get_instance_from_index( index ))) + hr = hid_joystick_instance_try_open( inst, &device, &preparsed, &attrs, &caps, instance, device_path, version ); + else + hr = DIERR_DEVICENOTREG; + LeaveCriticalSection( &joystick_cache_cs ); if (hr != DI_OK) return hr; HidD_FreePreparsedData( preparsed ); diff --git a/dlls/dinput8/Makefile.in b/dlls/dinput8/Makefile.in index 1b288a1dce2..bdb50f9fa05 100644 --- a/dlls/dinput8/Makefile.in +++ b/dlls/dinput8/Makefile.in @@ -1,6 +1,6 @@ MODULE = dinput8.dll IMPORTLIB = dinput8 -IMPORTS = dinput8 dxguid uuid comctl32 ole32 user32 advapi32 hid setupapi +IMPORTS = dinput8 dxguid uuid comctl32 ole32 user32 advapi32 hid setupapi cfgmgr32 EXTRADEFS = -DDIRECTINPUT_VERSION=0x0800 PARENTSRC = ../dinput -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10364
From: Connor McAdams <cmcadams@codeweavers.com> --- dlls/dinput/joystick_hid.c | 136 ++++++++++++---------------------- dlls/dinput/tests/joystick8.c | 2 +- 2 files changed, 47 insertions(+), 91 deletions(-) diff --git a/dlls/dinput/joystick_hid.c b/dlls/dinput/joystick_hid.c index b475eb64000..64639d3cc6a 100644 --- a/dlls/dinput/joystick_hid.c +++ b/dlls/dinput/joystick_hid.c @@ -71,6 +71,7 @@ struct joystick_instance struct list entry; BOOL override; WORD vid, pid; + GUID guid; WCHAR dev_path[MAX_PATH]; }; @@ -101,6 +102,20 @@ static struct joystick_instance *joystick_cache_get_instance_from_index( unsigne return inst; } +static struct joystick_instance *joystick_cache_get_instance_from_guid( const GUID *guid ) +{ + GUID guid_product = dinput_pidvid_guid; + struct joystick_instance *inst = NULL; + + LIST_FOR_EACH_ENTRY( inst, &joystick_cache, struct joystick_instance, entry ) + { + guid_product.Data1 = MAKELONG( inst->vid, inst->pid); + if (IsEqualGUID( &inst->guid, guid ) || IsEqualGUID( &guid_product, guid )) return inst; + } + + return NULL; +} + void hid_joystick_cleanup( void ) { joystick_cache_free(); @@ -1475,13 +1490,12 @@ static HRESULT hid_joystick_device_try_open( const WCHAR *path, HANDLE *device, BOOL has_accelerator, has_brake, has_clutch, has_z, has_pov; PHIDP_PREPARSED_DATA preparsed_data = NULL; HIDP_LINK_COLLECTION_NODE nodes[256]; - DWORD type, size, button_count = 0; + DWORD type, button_count = 0; HIDP_BUTTON_CAPS buttons[10]; HIDP_VALUE_CAPS value; HANDLE device_file; ULONG node_count; NTSTATUS status; - UINT32 handle; USHORT count; device_file = CreateFileW( path, GENERIC_READ | GENERIC_WRITE, FILE_SHARE_READ | FILE_SHARE_WRITE, @@ -1504,14 +1518,7 @@ static HRESULT hid_joystick_device_try_open( const WCHAR *path, HANDLE *device, if (!HidD_GetProductString( device_file, instance->tszInstanceName, MAX_PATH * sizeof(WCHAR) )) goto failed; if (!HidD_GetProductString( device_file, instance->tszProductName, MAX_PATH * sizeof(WCHAR) )) goto failed; - if (!DeviceIoControl( device_file, IOCTL_HID_GET_WINE_RAWINPUT_HANDLE, NULL, 0, &handle, sizeof(handle), &size, NULL )) - { - ERR( "failed to get raw input handle, error %lu\n", GetLastError() ); - goto failed; - } - - instance->guidInstance = hid_joystick_guid; - instance->guidInstance.Data1 ^= handle; + instance->guidInstance = GUID_NULL; instance->guidProduct = dinput_pidvid_guid; instance->guidProduct.Data1 = MAKELONG( attrs->VendorID, attrs->ProductID ); instance->guidFFDriver = GUID_NULL; @@ -1651,7 +1658,7 @@ static HRESULT hid_joystick_get_class_dev_iface_list( const GUID *class, const D return DI_OK; } -static BOOL hid_device_is_valid_joystick( const WCHAR *path, WORD *vid, WORD *pid, BOOL *override ) +static BOOL hid_device_is_valid_joystick( const WCHAR *path, WORD *vid, WORD *pid, BOOL *override, GUID *guid_instance ) { PHIDP_PREPARSED_DATA preparsed = NULL; HIDD_ATTRIBUTES attrs = { 0 }; @@ -1659,6 +1666,8 @@ static BOOL hid_device_is_valid_joystick( const WCHAR *path, WORD *vid, WORD *pi WCHAR product[MAX_PATH]; BOOL ret_val = FALSE; HIDP_CAPS caps; + UINT32 handle; + DWORD size; device_file = CreateFileW( path, GENERIC_READ | GENERIC_WRITE, FILE_SHARE_READ | FILE_SHARE_WRITE, NULL, OPEN_EXISTING, FILE_FLAG_OVERLAPPED | FILE_FLAG_NO_BUFFERING, 0 ); @@ -1679,6 +1688,13 @@ static BOOL hid_device_is_valid_joystick( const WCHAR *path, WORD *vid, WORD *pi if (!HidD_GetProductString( device_file, product, sizeof(product) )) goto exit; if (device_instance_is_disabled( product, override )) goto exit; + if (!DeviceIoControl( device_file, IOCTL_HID_GET_WINE_RAWINPUT_HANDLE, NULL, 0, &handle, sizeof(handle), &size, NULL )) + { + ERR( "failed to get raw input handle, error %lu\n", GetLastError() ); + goto exit; + } + *guid_instance = hid_joystick_guid; + guid_instance->Data1 ^= handle; *vid = attrs.VendorID; *pid = attrs.ProductID; ret_val = TRUE; @@ -1713,8 +1729,9 @@ HRESULT hid_joystick_refresh_devices( void ) struct joystick_instance *instance = NULL; BOOL override; WORD vid, pid; + GUID guid; - if (!hid_device_is_valid_joystick( tmp, &vid, &pid, &override )) continue; + if (!hid_device_is_valid_joystick( tmp, &vid, &pid, &override, &guid )) continue; if (!(instance = malloc( sizeof(*instance) ))) { hr = E_OUTOFMEMORY; @@ -1723,6 +1740,7 @@ HRESULT hid_joystick_refresh_devices( void ) instance->vid = vid; instance->pid = pid; instance->override = override; + instance->guid = guid; wcscpy( instance->dev_path, tmp ); /* * SetupAPI returns paths in lowercase, cfgmgr32 does not. joy.cpl @@ -1739,81 +1757,6 @@ exit: return hr; } -static HRESULT hid_joystick_device_open( int index, const GUID *guid, DIDEVICEINSTANCEW *instance, - WCHAR *device_path, HANDLE *device, PHIDP_PREPARSED_DATA *preparsed, - HIDD_ATTRIBUTES *attrs, HIDP_CAPS *caps, DWORD version ) -{ - char buffer[sizeof(SP_DEVICE_INTERFACE_DETAIL_DATA_W) + MAX_PATH * sizeof(WCHAR)]; - SP_DEVICE_INTERFACE_DETAIL_DATA_W *detail = (void *)buffer; - SP_DEVICE_INTERFACE_DATA iface = {.cbSize = sizeof(iface)}; - SP_DEVINFO_DATA devinfo = {.cbSize = sizeof(devinfo)}; - WCHAR device_id[MAX_PATH], *tmp; - HDEVINFO set, xi_set; - BOOL override; - UINT32 i = 0; - GUID hid; - - TRACE( "index %d, guid %s\n", index, debugstr_guid( guid ) ); - - HidD_GetHidGuid( &hid ); - - set = SetupDiGetClassDevsW( &hid, NULL, NULL, DIGCF_DEVICEINTERFACE | DIGCF_PRESENT ); - if (set == INVALID_HANDLE_VALUE) return DIERR_DEVICENOTREG; - xi_set = SetupDiGetClassDevsW( &GUID_DEVINTERFACE_WINEXINPUT, NULL, NULL, DIGCF_DEVICEINTERFACE | DIGCF_PRESENT ); - - *device = NULL; - *preparsed = NULL; - while (SetupDiEnumDeviceInterfaces( set, NULL, &hid, i++, &iface )) - { - detail->cbSize = sizeof(SP_DEVICE_INTERFACE_DETAIL_DATA_W); - if (!SetupDiGetDeviceInterfaceDetailW( set, &iface, detail, sizeof(buffer), NULL, &devinfo )) - continue; - if (FAILED(hid_joystick_device_try_open( detail->DevicePath, device, preparsed, - attrs, caps, instance, version ))) - continue; - - if (device_instance_is_disabled( instance->tszInstanceName, &override )) - goto next; - - if (override && SetupDiGetDeviceInstanceIdW( set, &devinfo, device_id, MAX_PATH, NULL ) && - (tmp = wcsstr( device_id, L"&IG_" ))) - { - memcpy( tmp, L"&XI_", sizeof(L"&XI_") - sizeof(WCHAR) ); - if (!SetupDiOpenDeviceInfoW( xi_set, device_id, NULL, 0, &devinfo )) - goto next; - if (!SetupDiEnumDeviceInterfaces( xi_set, &devinfo, &GUID_DEVINTERFACE_WINEXINPUT, 0, &iface )) - goto next; - if (!SetupDiGetDeviceInterfaceDetailW( xi_set, &iface, detail, sizeof(buffer), NULL, &devinfo )) - goto next; - - CloseHandle( *device ); - HidD_FreePreparsedData( *preparsed ); - if (FAILED(hid_joystick_device_try_open( detail->DevicePath, device, preparsed, - attrs, caps, instance, version ))) - continue; - } - - /* enumerate device by GUID */ - if (IsEqualGUID( guid, &instance->guidProduct ) || IsEqualGUID( guid, &instance->guidInstance )) break; - - /* enumerate all devices */ - if (index >= 0 && !index--) break; - - next: - CloseHandle( *device ); - HidD_FreePreparsedData( *preparsed ); - *device = NULL; - *preparsed = NULL; - } - - if (xi_set != INVALID_HANDLE_VALUE) SetupDiDestroyDeviceInfoList( xi_set ); - SetupDiDestroyDeviceInfoList( set ); - if (!*device || !*preparsed) return DIERR_DEVICENOTREG; - - lstrcpynW( device_path, detail->DevicePath, MAX_PATH ); - return DI_OK; -} - static HRESULT hid_joystick_instance_try_open( struct joystick_instance *instance, HANDLE *device, PHIDP_PREPARSED_DATA *preparsed, HIDD_ATTRIBUTES *attrs, HIDP_CAPS *caps, DIDEVICEINSTANCEW *di_instance, WCHAR *device_path, DWORD version ) @@ -1844,7 +1787,11 @@ static HRESULT hid_joystick_instance_try_open( struct joystick_instance *instanc } hr = hid_joystick_device_try_open( path, device, preparsed, attrs, caps, di_instance, version ); - if (SUCCEEDED(hr)) wcscpy( device_path, path ); + if (SUCCEEDED(hr)) + { + wcscpy( device_path, path ); + di_instance->guidInstance = instance->guid; + } if (path != instance->dev_path) free( path ); return hr; } @@ -2248,8 +2195,17 @@ HRESULT hid_joystick_create_device( struct dinput *dinput, const GUID *guid, IDi impl->base.read_event = CreateEventW( NULL, TRUE, FALSE, NULL ); if (memcmp( device_path_guid.Data4, guid->Data4, sizeof(device_path_guid.Data4) )) - hr = hid_joystick_device_open( -1, guid, &impl->base.instance, impl->device_path, &impl->device, &impl->preparsed, - &attrs, &impl->caps, dinput->dwVersion ); + { + struct joystick_instance *inst; + + EnterCriticalSection( &joystick_cache_cs ); + if ((inst = joystick_cache_get_instance_from_guid( guid ))) + hr = hid_joystick_instance_try_open( inst, &impl->device, &impl->preparsed, &attrs, &impl->caps, + &impl->base.instance, impl->device_path, dinput->dwVersion ); + else + hr = DIERR_DEVICENOTREG; + LeaveCriticalSection( &joystick_cache_cs ); + } else { wcscpy( impl->device_path, *(const WCHAR **)guid ); diff --git a/dlls/dinput/tests/joystick8.c b/dlls/dinput/tests/joystick8.c index 5b2ca8fe66d..cafc43d8b72 100644 --- a/dlls/dinput/tests/joystick8.c +++ b/dlls/dinput/tests/joystick8.c @@ -6161,7 +6161,7 @@ static void test_joystick_instance_guid( DWORD version ) ok( hr == DI_OK, "Unexpected hr %#lx.\n", hr ); hr = dinput_create_device( &di2, &expect_instances[0], &device ); - if (version == 0x800) todo_wine ok( hr == DIERR_DEVICENOTREG, "Unexpected hr %#lx.\n", hr ); + if (version == 0x800) ok( hr == DIERR_DEVICENOTREG, "Unexpected hr %#lx.\n", hr ); else ok( hr == DI_OK, "Unexpected hr %#lx.\n", hr ); if (SUCCEEDED(hr)) { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10364
From: Connor McAdams <cmcadams@codeweavers.com> --- dlls/dinput/joystick_hid.c | 217 ++++++++++++++++++++++++++++++++++ dlls/dinput/tests/hotplug.c | 2 +- dlls/dinput/tests/joystick8.c | 5 +- 3 files changed, 220 insertions(+), 4 deletions(-) diff --git a/dlls/dinput/joystick_hid.c b/dlls/dinput/joystick_hid.c index 64639d3cc6a..2d75194117e 100644 --- a/dlls/dinput/joystick_hid.c +++ b/dlls/dinput/joystick_hid.c @@ -62,6 +62,7 @@ static CRITICAL_SECTION_DEBUG joystick_cache_cs_debug = 0, 0, { (DWORD_PTR)(__FILE__ ": joystick_cache_cs") } }; static CRITICAL_SECTION joystick_cache_cs = { &joystick_cache_cs_debug, -1, 0, 0, 0, 0 }; +static HANDLE dinput_reg_mutex; /* Need to enter the CS to access this list. */ static struct list joystick_cache = LIST_INIT(joystick_cache); @@ -118,9 +119,143 @@ static struct joystick_instance *joystick_cache_get_instance_from_guid( const GU void hid_joystick_cleanup( void ) { + if (dinput_reg_mutex) CloseHandle( dinput_reg_mutex ); joystick_cache_free(); } +struct joystick_instance_id +{ + struct list entry; + + unsigned int idx; + BOOL assigned; + WORD vid, pid; + GUID guid; +}; + +static void joystick_instance_id_list_free( struct list *list ) +{ + struct joystick_instance_id *cur, *cur2; + + LIST_FOR_EACH_ENTRY_SAFE( cur, cur2, list, struct joystick_instance_id, entry ) + { + list_remove( &cur->entry ); + free( cur ); + } +} + +static HRESULT add_joystick_instance_id_to_list( WORD vid, WORD pid, unsigned int idx, const GUID *guid, + struct list *list, struct joystick_instance_id **out_id ) +{ + struct joystick_instance_id *instance_id = calloc( 1, sizeof(*instance_id) ); + + if (!instance_id) return E_OUTOFMEMORY; + + instance_id->vid = vid; + instance_id->pid = pid; + instance_id->idx = idx; + instance_id->guid = *guid; + list_add_tail( list, &instance_id->entry ); + if (out_id) *out_id = instance_id; + return DI_OK; +} + +/* Joystick instance ID registry access functions. */ +static BOOL WINAPI dinput_init_reg_mutex( INIT_ONCE *once, void *param, void **ctx ) +{ + HANDLE mutex = CreateMutexW( NULL, FALSE, L"__wine_dinput_reg_mutex" ); + + if (mutex) dinput_reg_mutex = mutex; + + return !!mutex; +} + +static void get_dinput_reg_mutex( void ) +{ + static INIT_ONCE once = INIT_ONCE_STATIC_INIT; + + if (!dinput_reg_mutex) InitOnceExecuteOnce( &once, dinput_init_reg_mutex, NULL, NULL ); + WaitForSingleObject( dinput_reg_mutex, INFINITE ); +} + +static void release_dinput_reg_mutex( void ) +{ + assert( dinput_reg_mutex ); + ReleaseMutex( dinput_reg_mutex ); +} + +static const WCHAR *dinput_path = L"System\\CurrentControlSet\\Control\\MediaProperties\\" + "PrivateProperties\\DirectInput"; +static void set_joystick_instance_id_in_registry( struct joystick_instance_id *inst_id ) +{ + WCHAR buf[MAX_PATH]; + LSTATUS ret; + HKEY hkey; + + swprintf( buf, ARRAY_SIZE(buf), L"%s\\VID_%04X&PID_%04X\\Calibration\\%d", dinput_path, inst_id->vid, inst_id->pid, + inst_id->idx ); + if (RegCreateKeyExW( HKEY_CURRENT_USER, buf, 0, NULL, 0, KEY_ALL_ACCESS, NULL, &hkey, NULL )) + { + ERR( "Failed to create key.\n" ); + return; + } + + if ((ret = RegSetValueExW( hkey, L"GUID", 0, REG_BINARY, (const BYTE *)&inst_id->guid, + sizeof(inst_id->guid) ))) + WARN( "Failed to set instance GUID value in registry with error %#lx.\n", ret ); + + RegCloseKey( hkey ); +} + +/* + * Read the current joystick instances from the registry and return the + * values in a list. + */ +static HRESULT hid_joystick_get_instance_ids( struct list *out_list ) +{ + WCHAR buf[MAX_PATH]; + HRESULT hr = S_OK; + HKEY root_key; + + if (RegCreateKeyExW( HKEY_CURRENT_USER, dinput_path, 0, NULL, 0, KEY_ALL_ACCESS, NULL, &root_key, NULL )) return hr; + + for (UINT i = 0; !RegEnumKeyW( root_key, i, buf, ARRAY_SIZE(buf) ); i++) + { + WORD vid, pid; + HKEY dev_key; + + if (swscanf( buf, L"VID_%04X&PID_%04X", &vid, &pid ) != 2) continue; + + wcscat( buf, L"\\Calibration" ); + if (RegOpenKeyExW( root_key, buf, 0, KEY_ALL_ACCESS, &dev_key )) continue; + + for (UINT j = 0; !RegEnumKeyW( dev_key, j, buf, ARRAY_SIZE(buf) ); j++) + { + unsigned int idx; + DWORD len; + GUID guid; + + if (swscanf( buf, L"%d", &idx ) != 1) continue; + + len = sizeof(guid); + if (RegGetValueW( dev_key, buf, L"GUID", RRF_RT_REG_BINARY, NULL, &guid, &len )) + { + WARN( "Failed to get GUID key from %s in registry, expect problems.\n", debugstr_w( buf ) ); + continue; + } + + hr = add_joystick_instance_id_to_list( vid, pid, idx, &guid, out_list, NULL ); + if (FAILED(hr)) break; + } + RegCloseKey( dev_key ); + if (FAILED(hr)) break; + } + + if (FAILED(hr)) joystick_instance_id_list_free( out_list ); + RegCloseKey( root_key ); + return hr; +} + struct pid_control_report { BYTE id; @@ -1705,14 +1840,57 @@ exit: return ret_val; } +static HRESULT get_joystick_instance_id_for_joystick( WORD vid, WORD pid, struct list *cur_ids, + struct list *new_ids, struct joystick_instance_id **out_id ) +{ + struct joystick_instance_id *instance_id_prev, *tmp; + + /* First, try to find an unassigned ID from the existing registry entries. */ + instance_id_prev = tmp = NULL; + LIST_FOR_EACH_ENTRY( tmp, cur_ids, struct joystick_instance_id, entry ) + { + if (tmp->vid == vid && tmp->pid == pid) + { + if (!tmp->assigned) + { + *out_id = tmp; + return DI_OK; + } + instance_id_prev = tmp; + } + } + + /* + * There were no unassigned entries in the current registry list, need to + * check if we've added any new entries for this vid/pid pair already to + * get the proper index. + */ + LIST_FOR_EACH_ENTRY( tmp, new_ids, struct joystick_instance_id, entry ) + { + if (tmp->vid == vid && tmp->pid == pid) + { + if (instance_id_prev) assert( tmp->idx > instance_id_prev->idx ); + instance_id_prev = tmp; + } + } + + return add_joystick_instance_id_to_list( vid, pid, instance_id_prev ? instance_id_prev->idx + 1 : 0, + &GUID_NULL, new_ids, out_id ); +} + HRESULT hid_joystick_refresh_devices( void ) { + struct list cur_instance_ids, new_instance_ids; + struct joystick_instance_id *cur_id; + struct joystick_instance *cur; WCHAR *hid_list, *tmp; HRESULT hr; GUID hid; TRACE( "\n" ); + list_init( &cur_instance_ids ); + list_init( &new_instance_ids ); hid_list = NULL; HidD_GetHidGuid( &hid ); hr = hid_joystick_get_class_dev_iface_list( &hid, NULL, &hid_list ); @@ -1750,7 +1928,46 @@ HRESULT hid_joystick_refresh_devices( void ) list_add_tail( &joystick_cache, &instance->entry ); } + /* + * Pull all guidInstance values that currently exist in the registry, and + * store them in cur_instance_ids. + */ + get_dinput_reg_mutex(); + hr = hid_joystick_get_instance_ids( &cur_instance_ids ); + if (FAILED(hr)) + { + release_dinput_reg_mutex(); + goto exit; + } + + /* + * Now, go through our list of joystick devices and assign them IDs from the + * instance ID list. If we run out of IDs, generate new ones. + */ + LIST_FOR_EACH_ENTRY( cur, &joystick_cache, struct joystick_instance, entry ) + { + struct joystick_instance_id *id; + + hr = get_joystick_instance_id_for_joystick( cur->vid, cur->pid, &cur_instance_ids, &new_instance_ids, &id ); + if (FAILED(hr)) + { + release_dinput_reg_mutex(); + goto exit; + } + + id->assigned = TRUE; + if (IsEqualGUID( &id->guid, &GUID_NULL )) id->guid = cur->guid; + cur->guid = id->guid; + } + + /* New IDs to write into the registry. */ + LIST_FOR_EACH_ENTRY( cur_id, &new_instance_ids, struct joystick_instance_id, entry ) + set_joystick_instance_id_in_registry(cur_id); + release_dinput_reg_mutex(); + exit: + joystick_instance_id_list_free( &cur_instance_ids ); + joystick_instance_id_list_free( &new_instance_ids ); if (FAILED(hr)) joystick_cache_free(); free( hid_list ); LeaveCriticalSection( &joystick_cache_cs ); diff --git a/dlls/dinput/tests/hotplug.c b/dlls/dinput/tests/hotplug.c index 10a3051e8ce..e4147797652 100644 --- a/dlls/dinput/tests/hotplug.c +++ b/dlls/dinput/tests/hotplug.c @@ -222,7 +222,7 @@ static BOOL test_input_lost( DWORD version ) hr = dinput_test_create_device( version, &devinst2, &device2 ); ok( hr == DI_OK, "Unexpected hr %#lx.\n", hr ); ok( !!device2, "device2 is NULL.\n" ); - todo_wine ok( !memcmp( &devinst.guidInstance, &devinst2.guidInstance, sizeof(GUID) ), + ok( !memcmp( &devinst.guidInstance, &devinst2.guidInstance, sizeof(GUID) ), "Unexpected guidInstance.\n" ); ref = IDirectInputDevice8_Release( device2 ); ok( ref == 0, "Release returned %ld\n", ref ); diff --git a/dlls/dinput/tests/joystick8.c b/dlls/dinput/tests/joystick8.c index cafc43d8b72..2c395cb4462 100644 --- a/dlls/dinput/tests/joystick8.c +++ b/dlls/dinput/tests/joystick8.c @@ -6311,7 +6311,7 @@ static void test_joystick_instance_guid( DWORD version ) winetest_push_context( "device %d", i ); - todo_wine ok( IsEqualGUID( &expect_instances[expected_joystick - 1], &instances[i] ), + ok( IsEqualGUID( &expect_instances[expected_joystick - 1], &instances[i] ), "Unexpected instance %s.\n", debugstr_guid( &instances[i] ) ); hr = dinput_create_device( &di, &instances[i], &device ); ok( hr == DI_OK, "Unexpected hr %#lx.\n", hr ); @@ -6335,7 +6335,7 @@ static void test_joystick_instance_guid( DWORD version ) winetest_push_context( "device %d", i ); - todo_wine ok( IsEqualGUID( &expect_instances[expected_joystick - 1], &instances[i] ), + ok( IsEqualGUID( &expect_instances[expected_joystick - 1], &instances[i] ), "Unexpected instance %s.\n", debugstr_guid( &instances[i] ) ); hr = dinput_create_device( &di, &instances[i], &device ); ok( hr == DI_OK, "Unexpected hr %#lx.\n", hr ); @@ -6356,7 +6356,6 @@ static void test_joystick_instance_guid( DWORD version ) { winetest_push_context( "device %d", i ); - todo_wine_if( !(i & 0x1) ) ok( IsEqualGUID( &expect_instances[i], &instances[i] ), "Unexpected guidInstance %s.\n", debugstr_guid( &instances[i] ) ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10364
From: Connor McAdams <cmcadams@codeweavers.com> --- dlls/dinput/joystick_hid.c | 54 ++++++++++++++++++++++++----------- dlls/dinput/tests/joystick8.c | 4 +-- 2 files changed, 40 insertions(+), 18 deletions(-) diff --git a/dlls/dinput/joystick_hid.c b/dlls/dinput/joystick_hid.c index 2d75194117e..5d21e993d10 100644 --- a/dlls/dinput/joystick_hid.c +++ b/dlls/dinput/joystick_hid.c @@ -51,9 +51,41 @@ WINE_DEFAULT_DEBUG_CHANNEL(dinput); DEFINE_GUID( GUID_DEVINTERFACE_WINEXINPUT,0x6c53d5fd,0x6480,0x440f,0xb6,0x18,0x47,0x67,0x50,0xc5,0xe1,0xa6 ); -DEFINE_GUID( hid_joystick_guid, 0x9e573edb, 0x7734, 0x11d2, 0x8d, 0x4a, 0x23, 0x90, 0x3f, 0xb6, 0xbd, 0xf7 ); DEFINE_GUID( device_path_guid, 0x00000000, 0x0000, 0x0000, 0x8d, 0x4a, 0x23, 0x90, 0x3f, 0xb6, 0xbd, 0xf8 ); +/* + * Version 1 UUID timestamps are a count of the number of 100 nanosecond + * intervals since 00:00:00.00, 15 October 1582 (the date of Gregorian + * reform to the Christian calendar). FILETIME is a count of the number of 100 + * nanosecond intervals since 00:00:00.00, 1 January 1601. In order to convert + * a FILETIME value to a UUID timestamp, we need to add: + * - 17 days in October 1582. + * - 30 days in November 1582. + * - 31 days in December 1582. + * - 18 years between January 1583 and January 1601. + * - 5 leap days in those 18 years. + */ +#define UUID_TIME_TO_SYSTEM_TIME_DAYS ((ULONG64)((365 * 18) + 5 + 17 + 30 + 31)) +#define UUID_TIME_TO_SYSTEM_TIME_NS_DIFFERENCE ((UUID_TIME_TO_SYSTEM_TIME_DAYS) * 24 * 60 * 60 * 10000000) +DEFINE_GUID( dinput_joystick_uuid_init, 0x00000000, 0x0000, 0x1000, 0x80, 0x00, 0x00, 0x00, 'D', 'E', 'S', 'T' ); +static void create_v1_uuid( GUID *guid ) +{ + GUID tmp = dinput_joystick_uuid_init; + static LONG clock_seq; + ULARGE_INTEGER time; + ULONG cur_seq; + + GetSystemTimeAsFileTime( (FILETIME *)&time ); + time.QuadPart += UUID_TIME_TO_SYSTEM_TIME_NS_DIFFERENCE; + tmp.Data1 = (time.QuadPart & 0xffffffff); + tmp.Data2 = ((time.QuadPart >> 32) & 0xffff); + tmp.Data3 |= ((time.QuadPart >> 48) & 0x0fff); + cur_seq = InterlockedIncrement( &clock_seq ); + tmp.Data4[1] |= (cur_seq & 0xff); + tmp.Data4[0] |= ((cur_seq & 0x3f00) >> 8); + *guid = tmp; +} + static CRITICAL_SECTION joystick_cache_cs; static CRITICAL_SECTION_DEBUG joystick_cache_cs_debug = { @@ -1793,7 +1825,7 @@ static HRESULT hid_joystick_get_class_dev_iface_list( const GUID *class, const D return DI_OK; } -static BOOL hid_device_is_valid_joystick( const WCHAR *path, WORD *vid, WORD *pid, BOOL *override, GUID *guid_instance ) +static BOOL hid_device_is_valid_joystick( const WCHAR *path, WORD *vid, WORD *pid, BOOL *override ) { PHIDP_PREPARSED_DATA preparsed = NULL; HIDD_ATTRIBUTES attrs = { 0 }; @@ -1801,8 +1833,6 @@ static BOOL hid_device_is_valid_joystick( const WCHAR *path, WORD *vid, WORD *pi WCHAR product[MAX_PATH]; BOOL ret_val = FALSE; HIDP_CAPS caps; - UINT32 handle; - DWORD size; device_file = CreateFileW( path, GENERIC_READ | GENERIC_WRITE, FILE_SHARE_READ | FILE_SHARE_WRITE, NULL, OPEN_EXISTING, FILE_FLAG_OVERLAPPED | FILE_FLAG_NO_BUFFERING, 0 ); @@ -1823,13 +1853,6 @@ static BOOL hid_device_is_valid_joystick( const WCHAR *path, WORD *vid, WORD *pi if (!HidD_GetProductString( device_file, product, sizeof(product) )) goto exit; if (device_instance_is_disabled( product, override )) goto exit; - if (!DeviceIoControl( device_file, IOCTL_HID_GET_WINE_RAWINPUT_HANDLE, NULL, 0, &handle, sizeof(handle), &size, NULL )) - { - ERR( "failed to get raw input handle, error %lu\n", GetLastError() ); - goto exit; - } - *guid_instance = hid_joystick_guid; - guid_instance->Data1 ^= handle; *vid = attrs.VendorID; *pid = attrs.ProductID; ret_val = TRUE; @@ -1844,6 +1867,7 @@ static HRESULT get_joystick_instance_id_for_joystick( WORD vid, WORD pid, struct struct list *new_ids, struct joystick_instance_id **out_id ) { struct joystick_instance_id *instance_id_prev, *tmp; + GUID uuid; /* First, try to find an unassigned ID from the existing registry entries. */ instance_id_prev = tmp = NULL; @@ -1874,8 +1898,9 @@ static HRESULT get_joystick_instance_id_for_joystick( WORD vid, WORD pid, struct } } + create_v1_uuid( &uuid ); return add_joystick_instance_id_to_list( vid, pid, instance_id_prev ? instance_id_prev->idx + 1 : 0, - &GUID_NULL, new_ids, out_id ); + &uuid, new_ids, out_id ); } HRESULT hid_joystick_refresh_devices( void ) @@ -1907,9 +1932,8 @@ HRESULT hid_joystick_refresh_devices( void ) struct joystick_instance *instance = NULL; BOOL override; WORD vid, pid; - GUID guid; - if (!hid_device_is_valid_joystick( tmp, &vid, &pid, &override, &guid )) continue; + if (!hid_device_is_valid_joystick( tmp, &vid, &pid, &override )) continue; if (!(instance = malloc( sizeof(*instance) ))) { hr = E_OUTOFMEMORY; @@ -1918,7 +1942,6 @@ HRESULT hid_joystick_refresh_devices( void ) instance->vid = vid; instance->pid = pid; instance->override = override; - instance->guid = guid; wcscpy( instance->dev_path, tmp ); /* * SetupAPI returns paths in lowercase, cfgmgr32 does not. joy.cpl @@ -1956,7 +1979,6 @@ HRESULT hid_joystick_refresh_devices( void ) } id->assigned = TRUE; - if (IsEqualGUID( &id->guid, &GUID_NULL )) id->guid = cur->guid; cur->guid = id->guid; } diff --git a/dlls/dinput/tests/joystick8.c b/dlls/dinput/tests/joystick8.c index 2c395cb4462..f2b9c490054 100644 --- a/dlls/dinput/tests/joystick8.c +++ b/dlls/dinput/tests/joystick8.c @@ -6191,12 +6191,12 @@ static void test_joystick_instance_guid( DWORD version ) { winetest_push_context( "device %d", i ); - todo_wine ok( !IsEqualGUID( &expect_instances[i], &instances[i] ), + ok( !IsEqualGUID( &expect_instances[i], &instances[i] ), "Unexpected instance %s.\n", debugstr_guid( &instances[i] ) ); /* Old guidInstance no longer works. */ hr = dinput_create_device( &di, &expect_instances[i], &device ); - todo_wine ok( hr == DIERR_DEVICENOTREG, "Unexpected hr %#lx.\n", hr ); + ok( hr == DIERR_DEVICENOTREG, "Unexpected hr %#lx.\n", hr ); if (SUCCEEDED(hr)) IDirectInputDevice8_Release( device ); expect_instances[i] = instances[i]; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10364
From: Connor McAdams <cmcadams@codeweavers.com> --- dlls/dinput/joystick_hid.c | 2 +- dlls/dinput/tests/joystick8.c | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/dlls/dinput/joystick_hid.c b/dlls/dinput/joystick_hid.c index 5d21e993d10..7e49eefab37 100644 --- a/dlls/dinput/joystick_hid.c +++ b/dlls/dinput/joystick_hid.c @@ -1667,7 +1667,7 @@ static HRESULT hid_joystick_device_try_open( const WCHAR *path, HANDLE *device, device_file = CreateFileW( path, GENERIC_READ | GENERIC_WRITE, FILE_SHARE_READ | FILE_SHARE_WRITE, NULL, OPEN_EXISTING, FILE_FLAG_OVERLAPPED | FILE_FLAG_NO_BUFFERING, 0 ); - if (device_file == INVALID_HANDLE_VALUE) return DIERR_DEVICENOTREG; + if (device_file == INVALID_HANDLE_VALUE) return E_FAIL; if (!HidD_GetPreparsedData( device_file, &preparsed_data )) goto failed; if (!HidD_GetAttributes( device_file, attrs )) goto failed; diff --git a/dlls/dinput/tests/joystick8.c b/dlls/dinput/tests/joystick8.c index f2b9c490054..979e7585ead 100644 --- a/dlls/dinput/tests/joystick8.c +++ b/dlls/dinput/tests/joystick8.c @@ -6267,7 +6267,7 @@ static void test_joystick_instance_guid( DWORD version ) IDirectInputDevice8_Release( device ); hr = dinput_create_device( &di, &expect_instances[0], &device ); - todo_wine ok( hr == E_FAIL, "Unexpected hr %#lx.\n", hr ); + ok( hr == E_FAIL, "Unexpected hr %#lx.\n", hr ); if (SUCCEEDED(hr)) IDirectInputDevice8_Release( device ); hr = dinput_create_device( &di, &instances[1], &device ); @@ -6293,10 +6293,10 @@ static void test_joystick_instance_guid( DWORD version ) /* Stopped devices should return E_FAIL. */ hr = dinput_create_device( &di, &instances[0], &device ); - todo_wine ok( hr == E_FAIL, "Unexpected hr %#lx.\n", hr ); + ok( hr == E_FAIL, "Unexpected hr %#lx.\n", hr ); if (SUCCEEDED(hr)) IDirectInputDevice8_Release( device ); hr = dinput_create_device( &di, &instances[2], &device ); - todo_wine ok( hr == E_FAIL, "Unexpected hr %#lx.\n", hr ); + ok( hr == E_FAIL, "Unexpected hr %#lx.\n", hr ); if (SUCCEEDED(hr)) IDirectInputDevice8_Release( device ); /* After calling EnumDevices(), guidInstance values will be reassigned. */ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10364
From: Connor McAdams <cmcadams@codeweavers.com> --- dlls/dinput/dinput.c | 9 +++++++- dlls/dinput/tests/joystick8.c | 40 ++++++++++++----------------------- 2 files changed, 22 insertions(+), 27 deletions(-) diff --git a/dlls/dinput/dinput.c b/dlls/dinput/dinput.c index 1695221d3a6..1670f0421f0 100644 --- a/dlls/dinput/dinput.c +++ b/dlls/dinput/dinput.c @@ -282,7 +282,14 @@ static HRESULT WINAPI dinput7_CreateDeviceEx( IDirectInput7W *iface, const GUID if (IsEqualGUID( &GUID_SysKeyboard, guid )) hr = keyboard_create_device( impl, guid, &device ); else if (IsEqualGUID( &GUID_SysMouse, guid )) hr = mouse_create_device( impl, guid, &device ); - else hr = hid_joystick_create_device( impl, guid, &device ); + else + { + if ((hr = hid_joystick_create_device( impl, guid, &device )) == DIERR_DEVICENOTREG) + { + hid_joystick_refresh_devices(); + hr = hid_joystick_create_device( impl, guid, &device ); + } + } if (FAILED(hr)) return hr; diff --git a/dlls/dinput/tests/joystick8.c b/dlls/dinput/tests/joystick8.c index 979e7585ead..8cfe90c9fce 100644 --- a/dlls/dinput/tests/joystick8.c +++ b/dlls/dinput/tests/joystick8.c @@ -6224,7 +6224,7 @@ static void test_joystick_instance_guid( DWORD version ) ok( hr == DIERR_DEVICENOTREG, "Unexpected hr %#lx.\n", hr ); hr = dinput_create_device( &di, &expect_instances[0], &device ); - todo_wine ok( hr == DIERR_DEVICENOTREG, "Unexpected hr %#lx.\n", hr ); + ok( hr == DIERR_DEVICENOTREG, "Unexpected hr %#lx.\n", hr ); if (SUCCEEDED(hr)) IDirectInputDevice8_Release( device ); instances_end = instances; @@ -6245,7 +6245,7 @@ static void test_joystick_instance_guid( DWORD version ) ok( hr == DIERR_DEVICENOTREG, "Unexpected hr %#lx.\n", hr ); hr = dinput_create_device( &di, &expect_instances[0], &device ); - todo_wine ok( hr == DIERR_DEVICENOTREG, "Unexpected hr %#lx.\n", hr ); + ok( hr == DIERR_DEVICENOTREG, "Unexpected hr %#lx.\n", hr ); if (SUCCEEDED(hr)) IDirectInputDevice8_Release( device ); instances_end = instances; @@ -6396,17 +6396,13 @@ static void test_joystick_instance_guid( DWORD version ) hid_device_start( &descs[i], 1 ); hr = dinput_create_device( &di, &expect_instances[i], &device ); - todo_wine ok( hr == DI_OK, "Unexpected hr %#lx.\n", hr ); - if (SUCCEEDED(hr)) - { - check_device_hid_serial( device, descs[i].serial_str ); - IDirectInputDevice8_Release( device ); - } + ok( hr == DI_OK, "Unexpected hr %#lx.\n", hr ); + check_device_hid_serial( device, descs[i].serial_str ); + IDirectInputDevice8_Release( device ); hid_device_stop( &descs[i], 1 ); hr = dinput_create_device( &di, &expect_instances[i], &device ); - todo_wine ok( hr == E_FAIL, "Unexpected hr %#lx.\n", hr ); - if (SUCCEEDED(hr)) IDirectInputDevice8_Release( device ); + ok( hr == E_FAIL, "Unexpected hr %#lx.\n", hr ); hid_device_start( &descs[i], 1 ); winetest_pop_context(); @@ -6433,17 +6429,13 @@ static void test_joystick_instance_guid( DWORD version ) hid_device_start( &descs[0], 1 ); hr = dinput_create_device( &di, &expect_instances[0], &device ); - todo_wine ok( hr == DI_OK, "Unexpected hr %#lx.\n", hr ); - if (SUCCEEDED(hr)) - { - check_device_hid_serial( device, descs[0].serial_str ); - IDirectInputDevice8_Release( device ); - } + ok( hr == DI_OK, "Unexpected hr %#lx.\n", hr ); + check_device_hid_serial( device, descs[0].serial_str ); + IDirectInputDevice8_Release( device ); hid_device_stop( &descs[0], 1 ); hr = dinput_create_device( &di, &expect_instances[0], &device ); - todo_wine ok( hr == E_FAIL, "Unexpected hr %#lx.\n", hr ); - if (SUCCEEDED(hr)) IDirectInputDevice8_Release( device ); + ok( hr == E_FAIL, "Unexpected hr %#lx.\n", hr ); /* Start device 1. */ @@ -6454,8 +6446,7 @@ static void test_joystick_instance_guid( DWORD version ) * with device 0. */ hr = dinput_create_device( &di, &expect_instances[0], &device ); - todo_wine ok( hr == E_FAIL, "Unexpected hr %#lx.\n", hr ); - if (SUCCEEDED(hr)) IDirectInputDevice8_Release( device ); + ok( hr == E_FAIL, "Unexpected hr %#lx.\n", hr ); /* * expect_instances[1] is not currently associated with a device, which @@ -6467,12 +6458,9 @@ static void test_joystick_instance_guid( DWORD version ) ok( hr == DIERR_DEVICENOTREG, "Unexpected hr %#lx.\n", hr ); hr = dinput_create_device( &di, &expect_instances[0], &device ); - todo_wine ok( hr == DI_OK, "Unexpected hr %#lx.\n", hr ); - if (SUCCEEDED(hr)) - { - check_device_hid_serial( device, descs[1].serial_str ); - IDirectInputDevice8_Release( device ); - } + ok( hr == DI_OK, "Unexpected hr %#lx.\n", hr ); + check_device_hid_serial( device, descs[1].serial_str ); + IDirectInputDevice8_Release( device ); hid_device_stop( &descs[1], 1 ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10364
On Mon Apr 20 14:08:01 2026 +0000, Connor McAdams wrote:
changed this line in [version 5 of the diff](/wine/wine/-/merge_requests/10364/diffs?diff_id=261726&start_sha=9cef9159d5dad342cb04cc69bcf554ae48fc1ea5#4ad1ac26b8a52443a124ce8c55def4aa2a4f5075_162_138) I've reworked things in the current revision, removing `DevicePath` from the registry completely because I think it was just causing more confusion.
The way it is done now is: - All existing registry entries are read into a list. - Enumerated devices check this list to find an existing VID/PID/index, if one exists, it is reused. - If no VID/PID/index pairing exists, a new guidInstance is created, and added to a separate list. - After all devices have been assigned a guidInstance, we check if there are any entries in the new guidInstance list, and write them to the registry. I think this method is clearer, and I've also tried splitting it a bit to make review easier. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10364#note_137098
On Mon Apr 20 14:08:01 2026 +0000, Connor McAdams wrote:
changed this line in [version 5 of the diff](/wine/wine/-/merge_requests/10364/diffs?diff_id=261726&start_sha=9cef9159d5dad342cb04cc69bcf554ae48fc1ea5#4ad1ac26b8a52443a124ce8c55def4aa2a4f5075_2027_1936) I've changed this now to be checked for disablement/override when enumerating over the currently connected HID devices, but deferring the handling of overridden devices until `EnumDevices()` or `CreateDevice()` is called.
AFAICT, an application will either get one device or the other (the IG device or the overridden one), so IMO it's fine for them to share the same `guidInstance` value, given they have the same VID/PID/index in both cases. I've also tested the override behavior in joy.cpl to confirm this is working properly. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10364#note_137099
On Mon Apr 20 14:18:36 2026 +0000, Connor McAdams wrote:
I've changed this now to be checked for disablement/override when enumerating over the currently connected HID devices, but deferring the handling of overridden devices until `EnumDevices()` or `CreateDevice()` is called. AFAICT, an application will either get one device or the other (the IG device or the overridden one), so IMO it's fine for them to share the same `guidInstance` value, given they have the same VID/PID/index in both cases. I've also tested the override behavior in joy.cpl to confirm this is working properly. I still don't think we should do that here. It would be simpler to just cache the list of HID devices without doing any kind of filtering here. I don't think we need (or even should) to open the devices, we can just assume everything that is enumerated can be cached.
Another instance is the device_instance_is_disabled setting, this is also app-specific, but you take it into account when enumerating the devices it'll have some effect on the cache and potentially on other applications. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10364#note_137239
Rémi Bernon (@rbernon) commented about dlls/dinput/joystick_hid.c:
WCHAR device_path[MAX_PATH]; - GUID guid = GUID_NULL; HIDP_CAPS caps; HANDLE device; HRESULT hr;
TRACE( "type %#lx, flags %#lx, instance %p, version %#lx, index %d\n", type, flags, instance, version, index );
- hr = hid_joystick_device_open( index, &guid, instance, device_path, &device, &preparsed, - &attrs, &caps, version ); + EnterCriticalSection( &joystick_cache_cs ); + if ((inst = joystick_cache_get_instance_from_index( index ))) + hr = hid_joystick_instance_try_open( inst, &device, &preparsed, &attrs, &caps, instance, device_path, version ); + else + hr = DIERR_DEVICENOTREG; + LeaveCriticalSection( &joystick_cache_cs );
Won't this fail to find a new device unless EnumDevices is called to refresh the cache? I think you need to refresh the cache on failure too, it's only done later in the series and it needs to be done earlier. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10364#note_137240
I don't think we need (or even should) to open the devices, we can just assume everything that is enumerated can be cached.
Is there an issue with opening the devices outside of of not needing to? We need to get the VID/PID at some point. We could try to extract from device path, but that seems like it'd be more fragile. Opening it here also lets us filter out the wine HID mouse/keyboard. It also filters out devices we don't support opening early.
Another instance is the device_instance_is_disabled setting, this is also app-specific, but you take it into account when enumerating the devices it'll have some effect on the cache and potentially on other applications.
How would this have an effect on other applications? The cache is local to a process/app AFAIU. Unless different threads can have different enabled/disabled/overrides, I don't see how this would be a problem. The apps that do have the controller enabled will generate a `guidInstance`, there shouldn't be a conflict there. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10364#note_137331
On Tue Apr 21 18:20:33 2026 +0000, Rémi Bernon wrote:
Won't this fail to find a new device unless EnumDevices is called to refresh the cache? I think you need to refresh the cache on failure too, it's only done later in the series and it needs to be done earlier. This function you're commenting on (`hid_joystick_enum_device()`) is only called from `EnumDevices()`. I used to have refresh inside of that function in the original series, but it was moved to `dinput8_EnumDevices()` on your suggestion. We can move it back here if that's preferred.
I can unsplit things to have the cache refresh on failure to open the device, but that'd be done elsewhere. Even split, the behavior shouldn't be any worse than it already is without these patches. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10364#note_137332
On Wed Apr 22 12:03:55 2026 +0000, Connor McAdams wrote:
I don't think we need (or even should) to open the devices, we can just assume everything that is enumerated can be cached. Is there an issue with opening the devices outside of of not needing to? We need to get the VID/PID at some point. We could try to extract from device path, but that seems like it'd be more fragile. Opening it here also lets us filter out the wine HID mouse/keyboard. It also filters out devices we don't support opening early. Another instance is the device_instance_is_disabled setting, this is also app-specific, but you take it into account when enumerating the devices it'll have some effect on the cache and potentially on other applications. How would this have an effect on other applications? The cache is local to a process/app AFAIU. Unless different threads can have different enabled/disabled/overrides, I don't see how this would be a problem. The apps that do have the controller enabled will generate a `guidInstance`, there shouldn't be a conflict there. Yeah, you're probably right. I've made some modifications to keep the changes a bit more localized and opened https://gitlab.winehq.org/wine/wine/-/merge_requests/10719.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10364#note_137347
This merge request was closed by Rémi Bernon. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10364
participants (3)
-
Connor McAdams -
Connor McAdams (＠cmcadams)
-
Rémi Bernon (＠rbernon)