[PATCH v4 0/3] 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. -- v4: dinput: Properly implement guidInstance for joystick devices. dinput: Create registry entries for 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> A local cache of joystick instances is also maintained. Signed-off-by: Connor McAdams <cmcadams@codeweavers.com> --- dlls/dinput/Makefile.in | 2 +- dlls/dinput/dinput.c | 1 + dlls/dinput/dinput_main.c | 1 + dlls/dinput/dinput_private.h | 2 + dlls/dinput/joystick_hid.c | 468 ++++++++++++++++++++++++++++++++++- dlls/dinput8/Makefile.in | 2 +- 6 files changed, 473 insertions(+), 3 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/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..aabad4ff97a 100644 --- a/dlls/dinput/dinput_main.c +++ b/dlls/dinput/dinput_main.c @@ -522,6 +522,7 @@ BOOL WINAPI DllMain( HINSTANCE inst, DWORD reason, void *reserved ) break; case DLL_PROCESS_DETACH: if (reserved) break; + hid_joystick_cleanup(); unregister_di_em_win_class(); break; } 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..e297f393a60 100644 --- a/dlls/dinput/joystick_hid.c +++ b/dlls/dinput/joystick_hid.c @@ -36,12 +36,14 @@ #include "setupapi.h" #include "devguid.h" #include "dinput.h" -#include "setupapi.h" +#include "cfgmgr32.h" #include "dinput_private.h" #include "device_private.h" #include "initguid.h" +#include "devpkey.h" +#include "propkey.h" #include "wine/debug.h" #include "wine/hid.h" @@ -52,6 +54,294 @@ 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 ); +/* + * 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 HANDLE dinput_reg_mutex; +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 ); +} + +struct joystick_instance +{ + struct list entry; + WORD vid, pid; + GUID guid; + + WCHAR dev_path[MAX_PATH]; +}; + +static struct joystick_instance *joystick_instance_create( WORD vid, WORD pid, const GUID *guid, const WCHAR *path ) +{ + struct joystick_instance *inst = calloc( 1, sizeof(*inst) ); + + if (inst) + { + inst->vid = vid; + inst->pid = pid; + if (guid) inst->guid = *guid; + wcscpy( inst->dev_path, path ); + } + + return inst; +} + +/* Need to enter the mutex prior to accessing these registry entries. */ +static const WCHAR *dinput_path = L"System\\CurrentControlSet\\Control\\MediaProperties\\" + "PrivateProperties\\DirectInput"; +static void reset_joystick_instances_in_registry(void) +{ + WCHAR buf[MAX_PATH]; + HKEY root_key; + + if (RegCreateKeyExW( HKEY_CURRENT_USER, dinput_path, 0, NULL, 0, KEY_ALL_ACCESS, NULL, &root_key, NULL )) return; + + for (UINT i = 0; !RegEnumKeyW( root_key, i, buf, ARRAY_SIZE(buf) ); i++) + { + HKEY dev_key; + + 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++) + RegDeleteKeyValueW( dev_key, buf, L"DevicePath" ); + + RegCloseKey( dev_key ); + } + RegCloseKey( root_key ); +} + +static HKEY get_empty_joystick_instance_in_registry(WORD vid, WORD pid) +{ + HKEY root_key, dev_key = NULL; + WCHAR buf[MAX_PATH]; + LSTATUS ret; + DWORD len; + UINT i; + + swprintf( buf, ARRAY_SIZE(buf), L"%s\\VID_%04X&PID_%04X\\Calibration", dinput_path, vid, pid ); + if (RegCreateKeyExW( HKEY_CURRENT_USER, buf, 0, NULL, 0, KEY_ALL_ACCESS, NULL, &root_key, NULL )) + { + ERR( "Failed to create key.\n" ); + return NULL; + } + + for (i = 0; !RegEnumKeyW( root_key, i, buf, ARRAY_SIZE(buf) ); i++) + { + if ((ret = RegGetValueW( root_key, buf, L"DevicePath", RRF_RT_REG_SZ, NULL, NULL, &len ))) + { + if (ret != ERROR_FILE_NOT_FOUND) WARN( "Got unexpected error %#lx.\n", ret ); + if (!RegOpenKeyExW( root_key, buf, 0, KEY_ALL_ACCESS, &dev_key )) break; + } + } + + if (!dev_key) + { + swprintf( buf, ARRAY_SIZE(buf), L"%s\\VID_%04X&PID_%04X\\Calibration\\%d", dinput_path, vid, pid, i ); + if ((ret = RegCreateKeyExW( HKEY_CURRENT_USER, buf, 0, NULL, 0, KEY_ALL_ACCESS, NULL, &dev_key, NULL ))) + WARN( "Got unexpected error %#lx when creating key.\n", ret ); + } + RegCloseKey( root_key ); + return dev_key; +} + +static void set_joystick_instance_in_registry( struct joystick_instance *instance ) +{ + HKEY dev_key = get_empty_joystick_instance_in_registry( instance->vid, instance->pid ); + DWORD len = sizeof(instance->guid); + LSTATUS ret; + + if (!dev_key) + { + ERR( "Failed to get device instance in registry.\n" ); + return; + } + + /* If the instance GUID doesn't exist already, create it. */ + if ((ret = RegGetValueW( dev_key, NULL, L"GUID", RRF_RT_REG_BINARY, NULL, (BYTE *)&instance->guid, &len )) + || (len != sizeof(instance->guid))) + { + if (ret && ret != ERROR_FILE_NOT_FOUND) + WARN( "Got unexpected error %#lx.\n", ret ); + if (!ret && (len != sizeof(instance->guid))) + WARN( "GUID key value was invalid size %#lx, recreating value.\n", len ); + + create_v1_uuid( &instance->guid ); + if ((ret = RegSetValueExW( dev_key, L"GUID", 0, REG_BINARY, (const BYTE *)&instance->guid, + sizeof(instance->guid) ))) + WARN( "Failed to set instance GUID value in registry with error %#lx.\n", ret ); + } + + if ((ret = RegSetValueExW( dev_key, L"DevicePath", 0, REG_SZ, (const BYTE *)instance->dev_path, + (wcslen(instance->dev_path) + 1) * sizeof(WCHAR) ))) + WARN( "Failed to set device path in registry with error %#lx.\n", ret ); + + RegCloseKey( dev_key ); +} + +/* Need to enter the mutex to access this list. */ +static struct list joystick_cache = LIST_INIT(joystick_cache); + +/* + * Read current joystick instances in the registry, update any instances + * already in the local cache, and remove any instances in the local cache + * that no longer exist in the registry. + */ +static void hid_joystick_cache_update_from_registry( void ) +{ + struct joystick_instance *cur, *cur2; + struct list cur_joysticks; + WCHAR buf[MAX_PATH]; + HKEY root_key; + + if (RegCreateKeyExW( HKEY_CURRENT_USER, dinput_path, 0, NULL, 0, KEY_ALL_ACCESS, NULL, &root_key, NULL )) return; + + /* + * Store the current cache entries in a new list and remove them as we + * find them in the registry. + */ + list_init( &cur_joysticks ); + LIST_FOR_EACH_ENTRY_SAFE( cur, cur2, &joystick_cache, struct joystick_instance, entry ) + { + list_remove( &cur->entry ); + list_add_tail( &cur_joysticks, &cur->entry ); + } + + 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++) + { + struct joystick_instance *instance = NULL; + WCHAR dev_path[MAX_PATH] = { 0 }; + DWORD len; + GUID guid; + + 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; + } + + len = sizeof(dev_path); + RegGetValueW( dev_key, buf, L"DevicePath", RRF_RT_REG_SZ, NULL, dev_path, &len ); + /* + * No device path value in registry, should be removed from local + * cache. + */ + if (!wcslen( dev_path )) continue; + + /* + * Attempt to find an entry from the local cache that matches this + * registry entry. + */ + LIST_FOR_EACH_ENTRY_SAFE( cur, cur2, &cur_joysticks, struct joystick_instance, entry ) + { + if (IsEqualGUID( &cur->guid, &guid )) + { + wcscpy( cur->dev_path, dev_path ); + list_remove( &cur->entry ); + instance = cur; + break; + } + } + + /* No preexisting instance, create one. */ + if (!instance) + { + if (!(instance = joystick_instance_create( vid, pid, &guid, dev_path ))) + { + ERR( "Failed to allocate memory for device instance, expect problems.\n" ); + continue; + } + } + + list_add_tail( &joystick_cache, &instance->entry ); + } + RegCloseKey( dev_key ); + } + + /* + * Anything left in this list was in the local cache but not the registry, + * which means we should remove it. + */ + LIST_FOR_EACH_ENTRY_SAFE( cur, cur2, &cur_joysticks, struct joystick_instance, entry ) + { + list_remove( &cur->entry ); + free( cur ); + } + RegCloseKey( root_key ); +} + +void hid_joystick_cleanup( void ) +{ + struct joystick_instance *cur, *cur2; + + if (dinput_reg_mutex) CloseHandle( dinput_reg_mutex ); + LIST_FOR_EACH_ENTRY_SAFE( cur, cur2, &joystick_cache, struct joystick_instance, entry ) + { + list_remove( &cur->entry ); + free( cur ); + } +} + struct pid_control_report { BYTE id; @@ -1572,6 +1862,182 @@ failed: return DIERR_DEVICENOTREG; } +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; + ULONG size = 0; + CONFIGRET ret; + + *list = NULL; + while (1) + { + 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, instance_id, 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_joystick_validate( const WCHAR *path, WORD *vid, WORD *pid, WCHAR *instance_name ) +{ + 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 (vid) *vid = attrs.VendorID; + if (pid) *pid = attrs.ProductID; + if (instance_name) wcscpy( instance_name, product ); + ret_val = TRUE; + +exit: + HidD_FreePreparsedData( preparsed ); + CloseHandle( device_file ); + return ret_val; +} + +HRESULT hid_joystick_refresh_devices( void ) +{ + struct joystick_instance *cur, *cur2; + BOOL cache_changed = FALSE; + struct list cur_joysticks; + 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, NULL, &hid_list ); + if (FAILED(hr)) return hr; + + get_dinput_reg_mutex(); + hid_joystick_cache_update_from_registry(); + + list_init( &cur_joysticks ); + LIST_FOR_EACH_ENTRY_SAFE( cur, cur2, &joystick_cache, struct joystick_instance, entry ) + { + list_remove( &cur->entry ); + list_add_tail( &cur_joysticks, &cur->entry ); + } + + /* + * Iterate over connected HID devices and check them against the joystick + * cache list. + * If the local cache matches the currently present HID devices, nothing + * is changed. + * If the local cache does not match, missing devices are removed, any new + * devices are added, and the registry will be updated. + */ + for (tmp = hid_list; *tmp; tmp = tmp + wcslen( tmp ) + 1) + { + WCHAR instance_id[MAX_DEVICE_ID_LEN], product[MAX_PATH], path[MAX_PATH]; + struct joystick_instance *instance = NULL; + BOOL override = FALSE; + DEVPROPTYPE type; + CONFIGRET ret; + WORD vid, pid; + WCHAR *tmp2; + ULONG size; + + wcscpy( path, tmp ); + memset( product, 0, sizeof(product) ); + if (!hid_joystick_validate( path, &vid, &pid, product )) continue; + if (device_instance_is_disabled( product, &override )) continue; + + ret = CM_Get_Device_Interface_PropertyW( path, &DEVPKEY_Device_InstanceId, &type, (BYTE *)instance_id, &size, 0 ); + if (!ret && override && (tmp2 = wcsstr( instance_id, L"&IG_" ))) + { + WCHAR *xi_iface; + + memcpy( tmp2, L"&XI_", sizeof(L"&XI_") - sizeof(WCHAR) ); + if (SUCCEEDED(hid_joystick_get_class_dev_iface_list( &GUID_DEVINTERFACE_WINEXINPUT, instance_id, &xi_iface ))) + { + wcscpy( path, xi_iface ); + free( xi_iface ); + if (!hid_joystick_validate( path, NULL, NULL, NULL )) continue; + } + } + + /* Try to find a preexisting joystick instance for this device. */ + LIST_FOR_EACH_ENTRY_SAFE( cur, cur2, &cur_joysticks, struct joystick_instance, entry ) + { + if ((cur->vid == vid) && (cur->pid == pid)) + { + list_remove( &cur->entry ); + instance = cur; + break; + } + } + + /* No preexisting instance for this device, create one. */ + if (!instance) + { + if (!(instance = joystick_instance_create( vid, pid, NULL, path ))) + { + hr = E_OUTOFMEMORY; + goto exit; + } + cache_changed = TRUE; + } + else if (wcscmp(instance->dev_path, path)) + { + cache_changed = TRUE; + wcscpy(instance->dev_path, path); + } + + list_add_tail( &joystick_cache, &instance->entry ); + } + + /* Something changed, update the registry values. */ + if (cache_changed || !list_empty(&cur_joysticks)) + { + reset_joystick_instances_in_registry(); + LIST_FOR_EACH_ENTRY( cur, &joystick_cache, struct joystick_instance, entry ) + set_joystick_instance_in_registry( cur ); + } + +exit: + release_dinput_reg_mutex(); + LIST_FOR_EACH_ENTRY_SAFE( cur, cur2, &cur_joysticks, struct joystick_instance, entry ) + { + list_remove( &cur->entry ); + free(cur); + } + free( hid_list ); + 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 ) 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> Signed-off-by: Connor McAdams <cmcadams@codeweavers.com> --- dlls/dinput/dinput.c | 9 +- dlls/dinput/joystick_hid.c | 197 ++++++++++++++++------------------ dlls/dinput/tests/hotplug.c | 2 +- dlls/dinput/tests/joystick8.c | 63 ++++------- 4 files changed, 126 insertions(+), 145 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/joystick_hid.c b/dlls/dinput/joystick_hid.c index e297f393a60..ca99c1af14f 100644 --- a/dlls/dinput/joystick_hid.c +++ b/dlls/dinput/joystick_hid.c @@ -51,7 +51,6 @@ 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 ); /* @@ -228,9 +227,69 @@ static void set_joystick_instance_in_registry( struct joystick_instance *instanc RegCloseKey( dev_key ); } -/* Need to enter the mutex to access this list. */ +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); +static BOOL hid_joystick_cache_get_instance_from_guid(const GUID *guid, WCHAR *dev_path, GUID *guid_instance) +{ + struct joystick_instance *inst; + BOOL ret_val = FALSE; + + EnterCriticalSection( &joystick_cache_cs ); + LIST_FOR_EACH_ENTRY( inst, &joystick_cache, struct joystick_instance, entry ) + { + GUID guid_product = dinput_pidvid_guid; + + guid_product.Data1 = MAKELONG( inst->vid, inst->pid); + if (IsEqualGUID( &inst->guid, guid ) || IsEqualGUID( &guid_product, guid )) + { + wcscpy( dev_path, inst->dev_path ); + *guid_instance = inst->guid; + ret_val = TRUE; + break; + } + } + LeaveCriticalSection( &joystick_cache_cs ); + + return ret_val; +} + +static BOOL hid_joystick_cache_get_instance_from_index(unsigned int index, WCHAR *dev_path, GUID *guid_instance) +{ + BOOL ret_val = FALSE; + + EnterCriticalSection( &joystick_cache_cs ); + if (list_count( &joystick_cache ) > index) + { + struct joystick_instance *inst; + unsigned int i = 0; + + LIST_FOR_EACH_ENTRY( inst, &joystick_cache, struct joystick_instance, entry ) + { + if (index == i) + { + wcscpy( dev_path, inst->dev_path ); + *guid_instance = inst->guid; + ret_val = TRUE; + break; + } + i++; + } + } + LeaveCriticalSection( &joystick_cache_cs ); + + return ret_val; +} + /* * Read current joystick instances in the registry, update any instances * already in the local cache, and remove any instances in the local cache @@ -1711,18 +1770,17 @@ 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, 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; @@ -1740,14 +1798,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; @@ -1941,6 +1992,7 @@ HRESULT hid_joystick_refresh_devices( void ) hr = hid_joystick_get_class_dev_iface_list( &hid, NULL, &hid_list ); if (FAILED(hr)) return hr; + EnterCriticalSection( &joystick_cache_cs ); get_dinput_reg_mutex(); hid_joystick_cache_update_from_registry(); @@ -2029,6 +2081,7 @@ HRESULT hid_joystick_refresh_devices( void ) exit: release_dinput_reg_mutex(); + LeaveCriticalSection( &joystick_cache_cs ); LIST_FOR_EACH_ENTRY_SAFE( cur, cur2, &cur_joysticks, struct joystick_instance, entry ) { list_remove( &cur->entry ); @@ -2038,86 +2091,12 @@ 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; -} - HRESULT hid_joystick_enum_device( DWORD type, DWORD flags, DIDEVICEINSTANCEW *instance, DWORD version, int index ) { + DIDEVICEINSTANCEW dev_inst = {.dwSize = sizeof(DIDEVICEINSTANCEW)}; HIDD_ATTRIBUTES attrs = {.Size = sizeof(attrs)}; - PHIDP_PREPARSED_DATA preparsed; - WCHAR device_path[MAX_PATH]; + PHIDP_PREPARSED_DATA preparsed = NULL; + WCHAR device_path[MAX_PATH] = { 0 }; GUID guid = GUID_NULL; HIDP_CAPS caps; HANDLE device; @@ -2125,18 +2104,21 @@ HRESULT hid_joystick_enum_device( DWORD type, DWORD flags, DIDEVICEINSTANCEW *in 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 ); - if (hr != DI_OK) return hr; - - HidD_FreePreparsedData( preparsed ); - CloseHandle( device ); + if (!hid_joystick_cache_get_instance_from_index( index, device_path, &guid )) return DIERR_DEVICENOTREG; + if (SUCCEEDED(hr = hid_joystick_device_try_open( device_path, &device, &preparsed, + &attrs, &caps, &dev_inst, version ))) + { + *instance = dev_inst; + instance->guidInstance = guid; + TRACE( "found device %s, usage %04x:%04x, product %s, instance %s, name %s\n", debugstr_w(device_path), + instance->wUsagePage, instance->wUsage, debugstr_guid( &instance->guidProduct ), + debugstr_guid( &instance->guidInstance ), debugstr_w(instance->tszInstanceName) ); - TRACE( "found device %s, usage %04x:%04x, product %s, instance %s, name %s\n", debugstr_w(device_path), - instance->wUsagePage, instance->wUsage, debugstr_guid( &instance->guidProduct ), - debugstr_guid( &instance->guidInstance ), debugstr_w(instance->tszInstanceName) ); + HidD_FreePreparsedData( preparsed ); + CloseHandle( device ); + } - return DI_OK; + return hr; } static BOOL init_object_properties( struct dinput_device *device, UINT index, struct hid_value_caps *caps, @@ -2507,8 +2489,19 @@ 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 ); + { + GUID guid_instance; + + if (hid_joystick_cache_get_instance_from_guid( guid, impl->device_path, &guid_instance )) + { + hr = hid_joystick_device_try_open( impl->device_path, &impl->device, &impl->preparsed, &attrs, + &impl->caps, &impl->base.instance, dinput->dwVersion ); + if (SUCCEEDED(hr)) + impl->base.instance.guidInstance = guid_instance; + } + else + hr = DIERR_DEVICENOTREG; + } else { wcscpy( impl->device_path, *(const WCHAR **)guid ); 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 5b2ca8fe66d..ca3f5f12f6a 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)) { @@ -6191,13 +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 ); - if (SUCCEEDED(hr)) IDirectInputDevice8_Release( device ); + ok( hr == DIERR_DEVICENOTREG, "Unexpected hr %#lx.\n", hr ); expect_instances[i] = instances[i]; /* New guidInstance, same device. */ @@ -6224,8 +6223,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 ); - if (SUCCEEDED(hr)) IDirectInputDevice8_Release( device ); + ok( hr == DIERR_DEVICENOTREG, "Unexpected hr %#lx.\n", hr ); instances_end = instances; hr = dinput_enum_devices( &di, find_test_device_instances, &instances_end ); @@ -6245,8 +6243,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 ); - if (SUCCEEDED(hr)) IDirectInputDevice8_Release( device ); + ok( hr == DIERR_DEVICENOTREG, "Unexpected hr %#lx.\n", hr ); instances_end = instances; hr = dinput_enum_devices( &di, find_test_device_instances, &instances_end ); @@ -6267,8 +6264,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 ); - if (SUCCEEDED(hr)) IDirectInputDevice8_Release( device ); + ok( hr == E_FAIL, "Unexpected hr %#lx.\n", hr ); hr = dinput_create_device( &di, &instances[1], &device ); ok( hr == DI_OK, "Unexpected hr %#lx.\n", hr ); @@ -6293,11 +6289,9 @@ 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 ); - if (SUCCEEDED(hr)) IDirectInputDevice8_Release( device ); + ok( hr == E_FAIL, "Unexpected hr %#lx.\n", hr ); hr = dinput_create_device( &di, &instances[2], &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 ); /* After calling EnumDevices(), guidInstance values will be reassigned. */ instances_end = instances; @@ -6311,7 +6305,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 +6329,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 +6350,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] ) ); @@ -6397,17 +6390,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(); @@ -6434,17 +6423,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. */ @@ -6455,8 +6440,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 @@ -6468,12 +6452,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 Wed Apr 1 16:23:52 2026 +0000, Rémi Bernon wrote:
So yeah, I think it could be improved. First it would be nice to split the change in at least two parts: first part could simply update the module local cache and registry, and second part would pivot to using the cached data to enumerate / open joysticks. To that end I think moving the error cases to `dinput.c`, would be nicer. You would have one `hid_joystick_refresh_device` in two places, one when `hid_joystick_create_device` returns `DIERR_DEVICENOTREG` then call it again, and another in `EnumDevices` before enumerating HID joysticks. This would match nicely with the `hid_joystick_cleanup_devices` call in `dinput.c` as well. Then regarding the cache update I don't think it has to reuse the existing enumeration loop. It doesn't need all the complicated filtering and checks, and it's mostly just a matter of, within the global mutex, 1) reading the registry to get an up to date current device list, 2) enumerating the HID devices, 3) adding anything that's missing from the list, then 4) saving it back to the registry. I think it could also be a good opportunity to switch the device enumeration to `cfgmgr32` which is simpler to use. In the second part, `hid_joystick_device_open` could just be switched over to using the local list instead of setupapi. I've split this up into two separate parts and switched to using `cfgmgr32`. The enumeration code definitely looks cleaner, but maintaining consistency between the registry and the local cache might be a bit ugly to you. If you have any suggestions for improvements please let me know. :)
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10364#note_135552
Rémi Bernon (@rbernon) commented about dlls/dinput/joystick_hid.c:
+ for (UINT i = 0; !RegEnumKeyW( root_key, i, buf, ARRAY_SIZE(buf) ); i++) + { + HKEY dev_key; + + 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++) + RegDeleteKeyValueW( dev_key, buf, L"DevicePath" ); + + RegCloseKey( dev_key ); + } + RegCloseKey( root_key ); +} + +static HKEY get_empty_joystick_instance_in_registry(WORD vid, WORD pid) I'm not completely sure to understand why this is needed. I was imagining something simple when enumerating devices:
1) delete local cache 2) populate it again from the registry 3) enumerate cfgmgr devices and find them in the cache or add new entries (flagging them as new if needed) 4) save the local cache to the registry (possibly just the new entries as an optimisation but doesn't matter much) There may be some logic needed to keep track of the Calibration subkey names but I think that's mostly a matter of keeping the local cache sorted by vid/pid when adding entries. Regarding sorting, I'm also not sure how stable the EnumDevices order needs to be (ie: does it sort by vid/pid? First seen time? Last plug time?). We might need to sort again the cache with a different order after the refresh. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10364#note_135650
Rémi Bernon (@rbernon) commented about dlls/dinput/joystick_hid.c:
+ * devices are added, and the registry will be updated. + */ + for (tmp = hid_list; *tmp; tmp = tmp + wcslen( tmp ) + 1) + { + WCHAR instance_id[MAX_DEVICE_ID_LEN], product[MAX_PATH], path[MAX_PATH]; + struct joystick_instance *instance = NULL; + BOOL override = FALSE; + DEVPROPTYPE type; + CONFIGRET ret; + WORD vid, pid; + WCHAR *tmp2; + ULONG size; + + wcscpy( path, tmp ); + memset( product, 0, sizeof(product) ); + if (!hid_joystick_validate( path, &vid, &pid, product )) continue; I'm wondering whether we need this here, we could just enumerate every HID device and delegate until the actual calls to EnumDevices / CreateDevice to open and check if they are valid joysticks?
Similar question with the override, I think it is a per application setting, so we might not want to enumerate differently if it is set or not. There's a question to whether we want the same instance guid for both the IG device and the overridden one, possibly not. If yes then we could keep the override check only when the device is opened, fixing up its path there. If no and if we want to handle them as separate but exclusive devices maybe we could enumerate both all HID devices *and* all WINEXINPUT devices every time, then only allow to open one or the other at EnumDevices/CreateDevice time based on the override setting. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10364#note_135651
On Thu Apr 9 11:25:41 2026 +0000, Rémi Bernon wrote:
I'm not completely sure to understand why this is needed. I was imagining something simple when enumerating devices: 1) delete local cache 2) populate it again from the registry 3) enumerate cfgmgr devices and find them in the cache or add new entries (flagging them as new if needed) 4) save the local cache to the registry (possibly just the new entries as an optimisation but doesn't matter much) There may be some logic needed to keep track of the Calibration subkey names but I think that's mostly a matter of keeping the local cache sorted by vid/pid when adding entries. Regarding sorting, I'm also not sure how stable the EnumDevices order needs to be (ie: does it sort by vid/pid? First seen time? Last plug time?). We might need to sort again the cache with a different order after the refresh. We can do it the way you describe, but I guess my concern with that is why go through the second step if we're just going to end up potentially overwriting it all in step 4. Maybe that makes sense when joystick ID is taken into account, but I'm not sure I get the logic there.
`EnumDevices()` on native sorts by the first time a particular `DeviceInstance` was created, I'm guessing it uses the registry entry creation time in `HKLM\System\CurrentControlSet\Enum`. That sorting can be done on the local list, which was my plan to do later in a separate patch. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10364#note_135671
We can do it the way you describe, but I guess my concern with that is why go through the second step if we're just going to end up potentially overwriting it all in step 4. Maybe that makes sense when joystick ID is taken into account, but I'm not sure I get the logic there.
Because there might be entries in the registry that aren't yet in the local cache -because populated by another process- and also not physically present anymore but that we still want to keep for when the devices are plugged back again?
`EnumDevices()` on native sorts by the first time a particular `DeviceInstance` was created, I'm guessing it uses the registry entry creation time in `HKLM\System\CurrentControlSet\Enum`. That sorting can be done on the local list, which was my plan to do later in a separate patch.
I think we could use the instance GUID first 8 bytes as UINT64? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10364#note_135676
Because there might be entries in the registry that aren't yet in the local cache -because populated by another process- and also not physically present anymore but that we still want to keep for when the devices are plugged back again?
I'm not sure I am following this, but maybe I'm not understanding something. My understanding of the registry entries here, and I think the tests show this, is that they're essentially GUIDs that map to `VID/PID/index` for a particular device. One `guidInstance` (which comes from the registry) can represent a different device depending on which controllers are plugged in at the time `EnumDevices()` is called. There shouldn't be an issue with race conditions between processes, the registry access is locked behind a mutex. The only thing that should be done in the registry is, finding an entry for a `VID/PID/IDX` pairing. If one does not exist, it is created, and along with it a new `guidInstance` value to represent the pairing. If one already exists, the `GUID` value from the registry is used for `guidInstance`. I don't see how another process doing an enumeration would break this. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10364#note_135680
On Thu Apr 9 14:02:42 2026 +0000, Connor McAdams wrote:
Because there might be entries in the registry that aren't yet in the local cache -because populated by another process- and also not physically present anymore but that we still want to keep for when the devices are plugged back again? I'm not sure I am following this, but maybe I'm not understanding something. My understanding of the registry entries here, and I think the tests show this, is that they're essentially GUIDs that map to `VID/PID/index` for a particular device. One `guidInstance` (which comes from the registry) can represent a different device depending on which controllers are plugged in at the time `EnumDevices()` is called. There shouldn't be an issue with race conditions between processes, the registry access is locked behind a mutex. The only thing that should be done in the registry is, finding an entry for a `VID/PID/IDX` pairing. If one does not exist, it is created, and along with it a new `guidInstance` value to represent the pairing. If one already exists, the `GUID` value from the registry is used for `guidInstance`. I don't see how another process doing an enumeration would break this. I think the test here shows this behavior:
/*
* Stop devices 0 and 2. After enumeration, their guidInstance values will
* be assigned to devices 1 and 3.
*/
hid_device_stop( &descs[0], 1 );
hid_device_stop( &descs[2], 1 );
/* Stopped devices should return E_FAIL. */
hr = dinput_create_device( &di, &instances[0], &device );
ok( hr == E_FAIL, "Unexpected hr %#lx.\n", hr );
hr = dinput_create_device( &di, &instances[2], &device );
ok( hr == E_FAIL, "Unexpected hr %#lx.\n", hr );
The local cache was populated by `EnumDevices()` when devices 0 and 2 were present. At that time, the devices were: - device 0, `0x1209/0x0001/0`. - device 1, `0x1209/0x0001/1`. - device 2, `0x1209/0x0002/0`. - device 3, `0x1209/0x0002/1`. After stopping devices 0 and 2, attempting to use those `guidInstance` values will fail. However, after calling `EnumDevices()` again, the local cache changes: ``` /* After calling EnumDevices(), guidInstance values will be reassigned. */ instances_end = instances; hr = dinput_enum_devices( &di, find_test_device_instances, &instances_end ); ok( hr == DI_OK, "Unexpected hr %#lx.\n", hr ); ok( instances_end == instances + 2, "Unexpected count %Iu.\n", instances_end - instances ); for (UINT i = 0; i < instances_end - instances; i++) { const unsigned int expected_joystick = !i ? 1 : 3; winetest_push_context( "device %d", 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 ); check_device_hid_serial( device, descs[expected_joystick].serial_str ); IDirectInputDevice8_Release( device ); winetest_pop_context(); } ``` Now the local cache looks like: - device 1, `0x1209/0x0001/0`. - device 3, `0x1209/0x0002/0`. Attempting to use the `guidInstance` values representing `0x1209/0x0001/1` or `0x1209/0x0001/1` no longer works. When devices 0 and 2 are started again, they do get their old `guidInstance` value back, but that's only because the enumeration order stays consistent based on when a particular `DeviceInstance` was first created. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10364#note_135695
On Thu Apr 9 14:46:20 2026 +0000, Connor McAdams wrote:
I think the test here shows this behavior: ``` /* * Stop devices 0 and 2. After enumeration, their guidInstance values will * be assigned to devices 1 and 3. */ hid_device_stop( &descs[0], 1 ); hid_device_stop( &descs[2], 1 ); /* Stopped devices should return E_FAIL. */ hr = dinput_create_device( &di, &instances[0], &device ); ok( hr == E_FAIL, "Unexpected hr %#lx.\n", hr ); hr = dinput_create_device( &di, &instances[2], &device ); ok( hr == E_FAIL, "Unexpected hr %#lx.\n", hr ); ``` The local cache was populated by `EnumDevices()` when devices 0 and 2 were present. At that time, the devices were: - device 0, `0x1209/0x0001/0`. - device 1, `0x1209/0x0001/1`. - device 2, `0x1209/0x0002/0`. - device 3, `0x1209/0x0002/1`. After stopping devices 0 and 2, attempting to use those `guidInstance` values will fail. However, after calling `EnumDevices()` again, the local cache changes: ``` /* After calling EnumDevices(), guidInstance values will be reassigned. */ instances_end = instances; hr = dinput_enum_devices( &di, find_test_device_instances, &instances_end ); ok( hr == DI_OK, "Unexpected hr %#lx.\n", hr ); ok( instances_end == instances + 2, "Unexpected count %Iu.\n", instances_end - instances ); for (UINT i = 0; i < instances_end - instances; i++) { const unsigned int expected_joystick = !i ? 1 : 3;
winetest_push_context( "device %d", 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 );
check_device_hid_serial( device, descs[expected_joystick].serial_str ); IDirectInputDevice8_Release( device ); winetest_pop_context(); } ``` Now the local cache looks like: - device 1, `0x1209/0x0001/0`. - device 3, `0x1209/0x0002/0`. Attempting to use the `guidInstance` values representing `0x1209/0x0001/1` or `0x1209/0x0002/1` no longer works. When devices 0 and 2 are started again, they do get their old `guidInstance` value back, but that's only because the enumeration order stays consistent based on when a particular `DeviceInstance` was first created. Right okay, I got a bit confused and assumed instance meant a particular device path. Let me look at it again with that in mind.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10364#note_135701
On Thu Apr 9 15:05:00 2026 +0000, Rémi Bernon wrote:
Right okay, I got a bit confused and assumed instance meant a particular device path. Let me look at it again with that in mind. Reading it again, although I was confused about the instance/device mapping, I still think that what I suggested applies.
Right now you first read only the last known valid (ie: with DevicePath) entries from the registry to a local working copy, then enumerate physical devices to find out actual valid and invalid devices, and if that doesn't match the registry you then need to delete DevicePath properties from existing entries, and/or iterate the registry again to eventually find out entries to reuse (optionally generate GUIDs for them) or to decide whether to create new ones. This feels like it could be made simpler by working completely locally. Just read all the registry entries in the local working copy, filter valid/invalid ones there by matching / clearing their device path. Use it to find free ones or add new ones if needed, and then write it all back to the registry. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10364#note_135939
participants (3)
-
Connor McAdams -
Connor McAdams (＠cmcadams)
-
Rémi Bernon (＠rbernon)