[PATCH 0/6] 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. -- 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/tests/driver_bus.c | 18 +++++++++++++++--- dlls/dinput/tests/driver_hid.c | 33 +++++++++++++++++++++++++++++++-- dlls/dinput/tests/driver_hid.h | 1 + 3 files changed, 47 insertions(+), 5 deletions(-) diff --git a/dlls/dinput/tests/driver_bus.c b/dlls/dinput/tests/driver_bus.c index e8338e49711..780db34ab62 100644 --- a/dlls/dinput/tests/driver_bus.c +++ b/dlls/dinput/tests/driver_bus.c @@ -563,7 +563,8 @@ static NTSTATUS remove_child_device( struct func_device *impl, DEVICE_OBJECT *de for (i = 0; i < impl->devices->Count; ++i) if (impl->devices->Objects[i] == device) break; if (i == impl->devices->Count) status = STATUS_NOT_FOUND; - else impl->devices->Objects[i] = impl->devices->Objects[impl->devices->Count--]; + else impl->devices->Objects[i] = impl->devices->Objects[--impl->devices->Count]; + if (status != STATUS_NOT_FOUND) impl->devices->Objects[impl->devices->Count + 1] = NULL; KeReleaseSpinLock( &impl->base.lock, irql ); return status; @@ -587,8 +588,16 @@ static NTSTATUS append_child_device( struct func_device *impl, DEVICE_OBJECT *de return status; } +static BOOL check_device_match(struct phys_device *phys, const WCHAR *device_id, const WCHAR *instance_id) +{ + if (instance_id && wcscmp( phys->instance_id, instance_id )) + return FALSE; + return !wcscmp( phys->device_id, device_id ); +} + static DEVICE_OBJECT *find_child_device( struct func_device *impl, struct hid_device_desc *desc ) { + const WCHAR *instance_id = desc->instance_id[0] ? desc->instance_id : NULL; DEVICE_OBJECT *device = NULL, **devices; WCHAR device_id[MAX_PATH]; KIRQL irql; @@ -603,7 +612,7 @@ static DEVICE_OBJECT *find_child_device( struct func_device *impl, struct hid_de for (i = 0; i < impl->devices->Count; ++i) { struct phys_device *phys = pdo_from_DEVICE_OBJECT( (device = devices[i]) ); - if (!wcscmp( phys->device_id, device_id )) break; + if (check_device_match(phys, device_id, instance_id)) break; else device = NULL; } KeReleaseSpinLock( &impl->base.lock, irql ); @@ -907,7 +916,10 @@ static NTSTATUS create_child_pdo( DEVICE_OBJECT *device, struct hid_device_desc desc->attributes.ProductID ); /* use a different device ID so that driver cache select the polled driver */ if (desc->is_polled) wcscat( impl->device_id, L"&POLL" ); - swprintf( impl->instance_id, MAX_PATH, L"0&0000&0" ); + if (desc->instance_id[0]) + swprintf( impl->instance_id, MAX_PATH, L"%s", desc->instance_id ); + else + swprintf( impl->instance_id, MAX_PATH, L"0&0000&0" ); impl->base.is_phys = TRUE; impl->fdo = fdo; diff --git a/dlls/dinput/tests/driver_hid.c b/dlls/dinput/tests/driver_hid.c index 7b20af07493..f98ec2edb7b 100644 --- a/dlls/dinput/tests/driver_hid.c +++ b/dlls/dinput/tests/driver_hid.c @@ -42,8 +42,12 @@ static DRIVER_OBJECT *expect_driver; +static KSPIN_LOCK driver_data_lock; +static struct list device_list = LIST_INIT(device_list); + struct hid_device { + struct list entry; DEVICE_OBJECT *expect_bus_pdo; DEVICE_OBJECT *expect_hid_fdo; struct hid_device *expect_hid_ext; @@ -54,11 +58,27 @@ static void check_device( DEVICE_OBJECT *device ) { HID_DEVICE_EXTENSION *ext = device->DeviceExtension; struct hid_device *impl = ext->MiniDeviceExtension; + KIRQL irql; ok( device == impl->expect_hid_fdo, "got device %p\n", device ); ok( device->DriverObject == expect_driver, "got DriverObject %p\n", device->DriverObject ); - if (!device->NextDevice) ok( device == impl->expect_hid_fdo, "got device %p\n", device ); - else ok( device->NextDevice == impl->expect_hid_fdo, "got NextDevice %p\n", device->NextDevice ); + + KeAcquireSpinLock( &driver_data_lock, &irql ); + if (!device->NextDevice) + { + struct hid_device *impl_tail = LIST_ENTRY( list_tail(&device_list), struct hid_device, entry ); + + ok( device == impl_tail->expect_hid_fdo, "got device %p\n", device ); + } + else + { + struct hid_device *next_impl = LIST_ENTRY( list_next(&device_list, &impl->entry), struct hid_device, entry ); + HID_DEVICE_EXTENSION *next_ext = device->NextDevice->DeviceExtension; + struct hid_device *next_ext_impl = next_ext->MiniDeviceExtension; + + ok( next_impl == next_ext_impl, "got NextDevice %p.\n", device->NextDevice ); + } + KeReleaseSpinLock( &driver_data_lock, irql ); ok( !device->AttachedDevice, "got AttachedDevice %p\n", device->AttachedDevice ); ok( ext->MiniDeviceExtension == impl->expect_hid_ext, "got MiniDeviceExtension %p\n", ext->MiniDeviceExtension ); @@ -113,6 +133,7 @@ static NTSTATUS WINAPI driver_pnp( DEVICE_OBJECT *device, IRP *irp ) HID_DEVICE_EXTENSION *ext = device->DeviceExtension; struct hid_device *impl = ext->MiniDeviceExtension; ULONG code = stack->MinorFunction; + KIRQL irql; if (winetest_debug > 1) trace( "%s: device %p, code %#lx %s\n", __func__, device, code, debugstr_pnp(code) ); @@ -126,6 +147,9 @@ static NTSTATUS WINAPI driver_pnp( DEVICE_OBJECT *device, IRP *irp ) IoSetDeviceInterfaceState( &impl->control_symlink, FALSE ); RtlFreeUnicodeString( &impl->control_symlink ); irp->IoStatus.Status = STATUS_SUCCESS; + KeAcquireSpinLock( &driver_data_lock, &irql ); + list_remove( &impl->entry ); + KeReleaseSpinLock( &driver_data_lock, irql ); break; case IRP_MN_STOP_DEVICE: case IRP_MN_SURPRISE_REMOVAL: @@ -229,12 +253,16 @@ static NTSTATUS WINAPI driver_add_device( DRIVER_OBJECT *driver, DEVICE_OBJECT * struct hid_device *impl = ext->MiniDeviceExtension; DEVICE_OBJECT *bus_pdo = ext->PhysicalDeviceObject; NTSTATUS status; + KIRQL irql; if (winetest_debug > 1) trace( "%s: driver %p, device %p\n", __func__, driver, device ); impl->expect_hid_fdo = device; impl->expect_bus_pdo = ext->PhysicalDeviceObject; impl->expect_hid_ext = ext->MiniDeviceExtension; + KeAcquireSpinLock( &driver_data_lock, &irql ); + list_add_head( &device_list, &impl->entry ); + KeReleaseSpinLock( &driver_data_lock, irql ); todo_wine ok( impl->expect_bus_pdo->AttachedDevice == device, "got AttachedDevice %p\n", bus_pdo->AttachedDevice ); @@ -286,6 +314,7 @@ NTSTATUS WINAPI DriverEntry( DRIVER_OBJECT *driver, UNICODE_STRING *registry ) NTSTATUS status; expect_driver = driver; + KeInitializeSpinLock( &driver_data_lock ); if ((status = winetest_init())) return status; if (winetest_debug > 1) trace( "%s: driver %p\n", __func__, driver ); diff --git a/dlls/dinput/tests/driver_hid.h b/dlls/dinput/tests/driver_hid.h index 8ba01d14b1e..e4f8f00a07a 100644 --- a/dlls/dinput/tests/driver_hid.h +++ b/dlls/dinput/tests/driver_hid.h @@ -92,6 +92,7 @@ struct hid_device_desc struct hid_expect expect[64]; ULONG context_size; char context[64]; + WCHAR instance_id[MAX_PATH]; }; /* kernel/user shared data */ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10364
From: Connor McAdams <cmcadams@codeweavers.com> Signed-off-by: Connor McAdams <cmcadams@codeweavers.com> --- include/ddk/hidport.h | 8 ++++++++ include/ddk/hidtypes.h | 3 --- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/include/ddk/hidport.h b/include/ddk/hidport.h index bd3ddb3f2a5..6574aba75bb 100644 --- a/include/ddk/hidport.h +++ b/include/ddk/hidport.h @@ -68,6 +68,14 @@ typedef struct _HID_DESCRIPTOR #define HID_HID_DESCRIPTOR_TYPE 0x21 #define HID_REPORT_DESCRIPTOR_TYPE 0x22 +/* + * String IDs used with IOCTL_HID_GET_STRING. + * These values match the string field offsets in chapter 9 of the USB spec. + */ +#define HID_STRING_ID_IMANUFACTURER 14 +#define HID_STRING_ID_IPRODUCT 15 +#define HID_STRING_ID_ISERIALNUMBER 16 + #define IOCTL_HID_GET_DEVICE_DESCRIPTOR HID_CTL_CODE(0) #define IOCTL_HID_GET_REPORT_DESCRIPTOR HID_CTL_CODE(1) #define IOCTL_HID_READ_REPORT HID_CTL_CODE(2) diff --git a/include/ddk/hidtypes.h b/include/ddk/hidtypes.h index dfd1061de18..8e6f76955e9 100644 --- a/include/ddk/hidtypes.h +++ b/include/ddk/hidtypes.h @@ -20,9 +20,6 @@ typedef enum _HID_STRING_TYPE { HID_STRING_INDEXED = 0, - HID_STRING_ID_IMANUFACTURER, - HID_STRING_ID_IPRODUCT, - HID_STRING_ID_ISERIALNUMBER, HID_STRING_MAX } HID_STRING_TYPE; -- 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/tests/joystick8.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/dlls/dinput/tests/joystick8.c b/dlls/dinput/tests/joystick8.c index eab4a7ff7e8..d267aff724f 100644 --- a/dlls/dinput/tests/joystick8.c +++ b/dlls/dinput/tests/joystick8.c @@ -5319,6 +5319,7 @@ static void test_game_input(void) HMODULE gameinput = LoadLibraryW( L"gameinput.dll" ); HRESULT (WINAPI *pGameInputCreate)( v0_IGameInput **out ); v0_IGameInput *gi0; + HRESULT hr; if (!gameinput || !(pGameInputCreate = (void *)GetProcAddress( gameinput, "GameInputCreate" ))) { @@ -5327,8 +5328,9 @@ static void test_game_input(void) } gi0 = (void *)0xdeadbeef; - todo_wine ok_hr( S_OK, pGameInputCreate( &gi0 ) ); - if (gi0 != (void *)0xdeadbeef) ok_ret( 0, v0_IGameInput_Release( gi0 ) ); + hr = pGameInputCreate( &gi0 ); + todo_wine ok( hr == S_OK || broken(hr == E_NOTIMPL), "Unexpected hr %#lx.\n", hr ); + if (gi0 && gi0 != (void *)0xdeadbeef) ok_ret( 0, v0_IGameInput_Release( gi0 ) ); } static HANDLE rawinput_device_added, rawinput_device_removed, rawinput_event; -- 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/tests/dinput_test.h | 6 + dlls/dinput/tests/driver_bus.c | 32 ++- dlls/dinput/tests/hid.c | 115 +++++++-- dlls/dinput/tests/hotplug.c | 19 +- dlls/dinput/tests/joystick8.c | 442 ++++++++++++++++++++++++++++++++ 5 files changed, 594 insertions(+), 20 deletions(-) diff --git a/dlls/dinput/tests/dinput_test.h b/dlls/dinput/tests/dinput_test.h index d19e1868d76..58030710310 100644 --- a/dlls/dinput/tests/dinput_test.h +++ b/dlls/dinput/tests/dinput_test.h @@ -64,9 +64,15 @@ void cleanup_registry_keys(void); void dinput_test_init_( const char *file, int line ); void dinput_test_exit(void); +HRESULT dinput_test_create_device_instance( DWORD version, const GUID *guid_inst, IDirectInputDevice8W **device ); HRESULT dinput_test_create_device( DWORD version, DIDEVICEINSTANCEW *devinst, IDirectInputDevice8W **device ); DWORD WINAPI dinput_test_device_thread( void *stop_event ); +#define dinput_test_get_device_hid_serial_string( a, b ) \ + dinput_test_get_device_hid_serial_string_( __FILE__, __LINE__, a, b ) +void dinput_test_get_device_hid_serial_string_( const char *file, int line, IDirectInputDevice8W *device, + WCHAR *serial_out ); + #define fill_context( a, b ) fill_context_( __FILE__, __LINE__, a, b ) void fill_context_( const char *file, int line, char *buffer, SIZE_T size ); diff --git a/dlls/dinput/tests/driver_bus.c b/dlls/dinput/tests/driver_bus.c index 780db34ab62..cf10e04e71f 100644 --- a/dlls/dinput/tests/driver_bus.c +++ b/dlls/dinput/tests/driver_bus.c @@ -1250,10 +1250,36 @@ static NTSTATUS pdo_internal_ioctl( DEVICE_OBJECT *device, IRP *irp ) } case IOCTL_HID_GET_STRING: - memcpy( irp->UserBuffer, L"Wine Test", sizeof(L"Wine Test") ); - irp->IoStatus.Information = sizeof(L"Wine Test"); - status = STATUS_SUCCESS; + { + UINT index = (UINT_PTR)stack->Parameters.DeviceIoControl.Type3InputBuffer & 0xffff; + UINT lcid = ((UINT_PTR)stack->Parameters.DeviceIoControl.Type3InputBuffer) >> 16; + + todo_wine ok( lcid, "Unexpected LCID %#x.\n", lcid ); + if (winetest_debug > 1) + trace( "%s: device %p, code %#lx %s, lcid %#x, idx %#x.\n", __func__, device, code, debugstr_ioctl(code), + lcid, index ); + switch (index) + { + case HID_STRING_ID_IMANUFACTURER: + case HID_STRING_ID_IPRODUCT: + memcpy( irp->UserBuffer, L"Wine Test", sizeof(L"Wine Test") ); + irp->IoStatus.Information = sizeof(L"Wine Test"); + status = STATUS_SUCCESS; + break; + + case HID_STRING_ID_ISERIALNUMBER: + wcscpy( irp->UserBuffer, impl->instance_id ); + irp->IoStatus.Information = (wcslen(impl->instance_id) + 1) * sizeof(WCHAR); + status = STATUS_SUCCESS; + break; + + default: + ok(0, "Unknown string type %#x.\n", index); + status = STATUS_NOT_IMPLEMENTED; + break; + } break; + } case IOCTL_GET_PHYSICAL_DESCRIPTOR: irp->IoStatus.Information = 0; diff --git a/dlls/dinput/tests/hid.c b/dlls/dinput/tests/hid.c index c9cdb649f26..966fa29c8fd 100644 --- a/dlls/dinput/tests/hid.c +++ b/dlls/dinput/tests/hid.c @@ -3587,13 +3587,36 @@ done: hid_device_stop( &desc, 1 ); } +static BOOL cleanup_test_device_keys(HKEY key, const WCHAR *path) +{ + WCHAR buf[MAX_PATH]; + BOOL ret = FALSE; + unsigned int i; + HKEY root_key; + DWORD size; + + RegCreateKeyExW( key, path, 0, NULL, 0, KEY_ALL_ACCESS, NULL, &root_key, NULL ); + size = ARRAY_SIZE(buf); + for (i = 0; !RegEnumKeyExW( root_key, i, buf, &size, NULL, NULL, NULL, NULL ); i++) + { + if (!wcsnicmp( buf, L"VID_1209", 8 )) + { + RegDeleteTreeW( root_key, buf ); + ret = TRUE; + break; + } + size = ARRAY_SIZE(buf); + } + RegCloseKey( root_key ); + return ret; +} + void cleanup_registry_keys(void) { static const WCHAR joystick_oem_path[] = L"System\\CurrentControlSet\\Control\\MediaProperties\\" "PrivateProperties\\Joystick\\OEM"; static const WCHAR dinput_path[] = L"System\\CurrentControlSet\\Control\\MediaProperties\\" "PrivateProperties\\DirectInput"; - HKEY root_key; /* These keys are automatically created by DInput and they store the list of supported force-feedback effects. OEM drivers are supposed @@ -3603,21 +3626,10 @@ void cleanup_registry_keys(void) We need to clean them up, or DInput will not refresh the list of effects from the PID report changes. */ - RegCreateKeyExW( HKEY_CURRENT_USER, joystick_oem_path, 0, NULL, 0, KEY_ALL_ACCESS, NULL, &root_key, NULL ); - RegDeleteTreeW( root_key, expect_vidpid_str ); - RegCloseKey( root_key ); - - RegCreateKeyExW( HKEY_CURRENT_USER, dinput_path, 0, NULL, 0, KEY_ALL_ACCESS, NULL, &root_key, NULL ); - RegDeleteTreeW( root_key, expect_vidpid_str ); - RegCloseKey( root_key ); - - RegCreateKeyExW( HKEY_LOCAL_MACHINE, joystick_oem_path, 0, NULL, 0, KEY_ALL_ACCESS, NULL, &root_key, NULL ); - RegDeleteTreeW( root_key, expect_vidpid_str ); - RegCloseKey( root_key ); - - RegCreateKeyExW( HKEY_LOCAL_MACHINE, dinput_path, 0, NULL, 0, KEY_ALL_ACCESS, NULL, &root_key, NULL ); - RegDeleteTreeW( root_key, expect_vidpid_str ); - RegCloseKey( root_key ); + while (cleanup_test_device_keys( HKEY_CURRENT_USER, joystick_oem_path )) {} + while (cleanup_test_device_keys( HKEY_CURRENT_USER, dinput_path )) {} + while (cleanup_test_device_keys( HKEY_LOCAL_MACHINE, joystick_oem_path )) {} + while (cleanup_test_device_keys( HKEY_LOCAL_MACHINE, dinput_path )) {} } static LRESULT CALLBACK monitor_wndproc( HWND hwnd, UINT msg, WPARAM wparam, LPARAM lparam ) @@ -3750,6 +3762,44 @@ void dinput_test_exit(void) CoUninitialize(); } +HRESULT dinput_test_create_device_instance( DWORD version, const GUID *guid_inst, IDirectInputDevice8W **device ) +{ + IDirectInput8W *di8; + IDirectInputW *di; + HRESULT hr; + ULONG ref; + + *device = NULL; + if (version >= 0x800) + { + hr = DirectInput8Create( instance, version, &IID_IDirectInput8W, (void **)&di8, NULL ); + if (FAILED(hr)) + { + win_skip( "DirectInput8Create returned %#lx\n", hr ); + return hr; + } + + hr = IDirectInput8_CreateDevice( di8, guid_inst, device, NULL ); + ref = IDirectInput8_Release( di8 ); + ok( ref == 0, "Release returned %ld\n", ref ); + } + else + { + hr = DirectInputCreateEx( instance, version, &IID_IDirectInput2W, (void **)&di, NULL ); + if (FAILED(hr)) + { + win_skip( "DirectInputCreateEx returned %#lx\n", hr ); + return hr; + } + + hr = IDirectInput_CreateDevice( di, guid_inst, (IDirectInputDeviceW **)device, NULL ); + ref = IDirectInput_Release( di ); + ok( ref == 0, "Release returned %ld\n", ref ); + } + + return hr; +} + BOOL CALLBACK find_test_device( const DIDEVICEINSTANCEW *devinst, void *context ) { if (IsEqualGUID( &devinst->guidProduct, &expect_guid_product )) @@ -3819,6 +3869,39 @@ HRESULT dinput_test_create_device( DWORD version, DIDEVICEINSTANCEW *devinst, ID return DI_OK; } +void dinput_test_get_device_hid_serial_string_( const char *file, int line, IDirectInputDevice8W *device, + WCHAR *serial_out) +{ + DIPROPGUIDANDPATH prop_guid_path = + { + .diph = + { + .dwSize = sizeof(DIPROPGUIDANDPATH), + .dwHeaderSize = sizeof(DIPROPHEADER), + .dwHow = DIPH_DEVICE, + }, + }; + WCHAR device_id[MAX_PATH] = { 0 }; + HANDLE file_handle; + HRESULT hr; + BOOL ret; + + serial_out[0] = 0; + hr = IDirectInputDevice8_GetProperty( device, DIPROP_GUIDANDPATH, &prop_guid_path.diph ); + ok_(file, line)( hr == S_OK, "Unexpected hr %#lx.\n", hr ); + if (FAILED(hr)) return; + + file_handle = CreateFileW( prop_guid_path.wszPath, FILE_READ_ACCESS | FILE_WRITE_ACCESS, + FILE_SHARE_READ | FILE_SHARE_WRITE, NULL, OPEN_EXISTING, 0, NULL ); + ok_(file, line)( file_handle != INVALID_HANDLE_VALUE, "Got error %lu.\n", GetLastError() ); + if (file_handle == INVALID_HANDLE_VALUE) return; + + ret = HidD_GetSerialNumberString( file_handle, device_id, ARRAY_SIZE(device_id) ); + ok_(file, line)( ret, "Failed to get HID serial number string.\n" ); + if (ret) wcscpy( serial_out, device_id ); + CloseHandle( file_handle ); +} + DWORD WINAPI dinput_test_device_thread( void *stop_event ) { #include "psh_hid_macros.h" diff --git a/dlls/dinput/tests/hotplug.c b/dlls/dinput/tests/hotplug.c index ce44bae754c..350050c0a42 100644 --- a/dlls/dinput/tests/hotplug.c +++ b/dlls/dinput/tests/hotplug.c @@ -156,8 +156,9 @@ static BOOL test_input_lost( DWORD version ) DIDEVICEINSTANCEW devinst = {.dwSize = sizeof(DIDEVICEINSTANCEW)}; DIDEVICEOBJECTDATA objdata[32] = {{0}}; - IDirectInputDevice8W *device = NULL; + IDirectInputDevice8W *device, *device2; ULONG ref, count, size; + GUID guid_instance; DIJOYSTATE2 state; HRESULT hr; @@ -169,6 +170,7 @@ static BOOL test_input_lost( DWORD version ) memcpy( desc.report_descriptor_buf, report_desc, sizeof(report_desc) ); fill_context( desc.context, ARRAY_SIZE(desc.context) ); + device = device2 = NULL; if (!hid_device_start( &desc, 1 )) goto done; if (FAILED(hr = dinput_test_create_device( version, &devinst, &device ))) goto done; @@ -220,6 +222,21 @@ static BOOL test_input_lost( DWORD version ) ref = IDirectInputDevice8_Release( device ); ok( ref == 0, "Release returned %ld\n", ref ); + /* Test guidInstance across hotplugs. It should remain the same. */ + guid_instance = devinst.guidInstance; + memset( &devinst, 0, sizeof(devinst) ); + devinst.dwSize = sizeof(DIDEVICEINSTANCEW); + hr = dinput_test_create_device( version, &devinst, &device2 ); + ok( hr == DI_OK, "Unexpected hr %#lx.\n", hr ); + ok( !!device2, "device2 is NULL.\n" ); + if (device2) + { + todo_wine ok( !memcmp( &guid_instance, &devinst.guidInstance, sizeof(guid_instance) ), + "Unexpected guidInstance.\n" ); + ref = IDirectInputDevice8_Release( device2 ); + ok( ref == 0, "Release returned %ld\n", ref ); + } + done: hid_device_stop( &desc, 1 ); cleanup_registry_keys(); diff --git a/dlls/dinput/tests/joystick8.c b/dlls/dinput/tests/joystick8.c index d267aff724f..c5dc5c066bf 100644 --- a/dlls/dinput/tests/joystick8.c +++ b/dlls/dinput/tests/joystick8.c @@ -5981,6 +5981,444 @@ static void test_rawinput_desktop( const char *path, BOOL input ) DestroyWindow( hwnd ); } +static void init_hid_device_desc(struct hid_device_desc *desc, const WCHAR *instance_id, WORD pid) +{ +#include "psh_hid_macros.h" + static const unsigned char report_desc[] = + { + USAGE_PAGE(1, HID_USAGE_PAGE_GENERIC), + USAGE(1, HID_USAGE_GENERIC_JOYSTICK), + COLLECTION(1, Application), + USAGE(1, HID_USAGE_GENERIC_JOYSTICK), + COLLECTION(1, Physical), + USAGE_PAGE(1, HID_USAGE_PAGE_BUTTON), + USAGE_MINIMUM(1, 1), + USAGE_MAXIMUM(1, 6), + LOGICAL_MINIMUM(1, 0), + LOGICAL_MAXIMUM(1, 1), + PHYSICAL_MINIMUM(1, 0), + PHYSICAL_MAXIMUM(1, 1), + REPORT_SIZE(1, 1), + REPORT_COUNT(1, 8), + INPUT(1, Data|Var|Abs), + END_COLLECTION, + END_COLLECTION, + }; + C_ASSERT(sizeof(report_desc) < MAX_HID_DESCRIPTOR_LEN); +#include "pop_hid_macros.h" + struct hid_device_desc init_desc = + { + .use_report_id = TRUE, + .caps = { .InputReportByteLength = 1 }, + .attributes = default_attributes, + }; + + *desc = init_desc; + desc->report_descriptor_len = sizeof(report_desc); + memcpy( desc->report_descriptor_buf, report_desc, sizeof(report_desc) ); + fill_context( desc->context, ARRAY_SIZE(desc->context) ); + if (instance_id) wcscpy(desc->instance_id, instance_id); + desc->attributes.ProductID = pid; +} + +struct joystick_test_device +{ + struct hid_device_desc desc; + BOOL started; +}; + +static BOOL start_joystick_test_device(struct joystick_test_device *device) +{ + device->started = hid_device_start( &device->desc, 1 ); + return device->started; +} + +static void stop_joystick_test_device(struct joystick_test_device *device) +{ + if (device->started) hid_device_stop( &device->desc, 1 ); + device->started = FALSE; +} + +struct dinput_test_devices +{ + DIDEVICEINSTANCEW *instances; + unsigned int instances_size; + unsigned int instances_count; +}; + +static void dinput_test_devices_struct_init(struct dinput_test_devices *devices, DIDEVICEINSTANCEW *insts, + unsigned int insts_size) +{ + const DIDEVICEINSTANCEW devinst_init = { .dwSize = sizeof(DIDEVICEINSTANCEW) }; + unsigned int i; + + for (i = 0; i < insts_size; i++) insts[i] = devinst_init; + devices->instances = insts; + devices->instances_size = insts_size; + devices->instances_count = 0; +} + +BOOL CALLBACK find_test_devices( const DIDEVICEINSTANCEW *devinst, void *context ) +{ + struct dinput_test_devices *devices = (struct dinput_test_devices *)context; + + if (!memcmp(devinst->guidProduct.Data4, expect_guid_product.Data4, sizeof(expect_guid_product.Data4)) + && (LOWORD(devinst->guidProduct.Data1) == LOWORD(EXPECT_VIDPID))) + { + devices->instances[devices->instances_count++] = *devinst; + if (devices->instances_count >= devices->instances_size) return DIENUM_STOP; + } + return DIENUM_CONTINUE; +} + +static HRESULT dinput_test_enum_devices( DWORD version, struct dinput_test_devices *context ) +{ + IDirectInput8W *di8; + IDirectInputW *di; + HRESULT hr; + ULONG ref; + + context->instances_count = 0; + if (version >= 0x800) + { + hr = DirectInput8Create( instance, version, &IID_IDirectInput8W, (void **)&di8, NULL ); + if (FAILED(hr)) + { + win_skip( "DirectInput8Create returned %#lx\n", hr ); + return hr; + } + + hr = IDirectInput8_EnumDevices( di8, DI8DEVCLASS_ALL, find_test_devices, context, DIEDFL_ALLDEVICES ); + ok( hr == DI_OK, "EnumDevices returned: %#lx\n", hr ); + + ref = IDirectInput8_Release( di8 ); + ok( ref == 0, "Release returned %ld\n", ref ); + } + else + { + hr = DirectInputCreateEx( instance, version, &IID_IDirectInput2W, (void **)&di, NULL ); + if (FAILED(hr)) + { + win_skip( "DirectInputCreateEx returned %#lx\n", hr ); + return hr; + } + + hr = IDirectInput_EnumDevices( di, 0, find_test_devices, context, DIEDFL_ALLDEVICES ); + ok( hr == DI_OK, "EnumDevices returned: %#lx\n", hr ); + + ref = IDirectInput_Release( di ); + ok( ref == 0, "Release returned %ld\n", ref ); + } + + return DI_OK; +} + +static void test_joystick_instance_guid( DWORD version ) +{ + struct dinput_test_devices dinput_test_devs; + struct joystick_test_device test_devs[4]; + WCHAR device_serial[MAX_PATH]; + DIDEVICEINSTANCEW devinsts[4]; + IDirectInputDevice8W *device; + GUID guid_instances[4]; + unsigned int i; + HRESULT hr; + + winetest_push_context( "%#lx", version ); + cleanup_registry_keys(); + + memset( test_devs, 0, sizeof(test_devs) ); + init_hid_device_desc( &test_devs[0].desc, L"1&2345&6", 0x0001 ); + init_hid_device_desc( &test_devs[1].desc, L"2&7812&1", 0x0001 ); + init_hid_device_desc( &test_devs[2].desc, L"3&4580&7", 0x0002 ); + init_hid_device_desc( &test_devs[3].desc, L"4&5880&3", 0x0002 ); + + for (i = 0; i < ARRAY_SIZE(test_devs); i++) + { + if (!start_joystick_test_device( &test_devs[i] )) + goto done; + } + + /* Enumerate all devices, each with a unique guidInstance. */ + dinput_test_devices_struct_init( &dinput_test_devs, devinsts, ARRAY_SIZE(devinsts) ); + hr = dinput_test_enum_devices( version, &dinput_test_devs ); + ok( hr == DI_OK, "Unexpected hr %#lx.\n", hr ); + ok( dinput_test_devs.instances_count == 4, "Unexpected count %u.\n", dinput_test_devs.instances_count ); + for (i = 0; i < dinput_test_devs.instances_count; i++) + { + winetest_push_context( "device %d", i ); + + hr = dinput_test_create_device_instance( version, &devinsts[i].guidInstance, &device ); + ok( hr == DI_OK, "Unexpected hr %#lx.\n", hr ); + guid_instances[i] = devinsts[i].guidInstance; + + dinput_test_get_device_hid_serial_string(device, device_serial); + ok( !wcscmp(test_devs[i].desc.instance_id, device_serial), "Unexpected device serial %s.\n", + debugstr_w(device_serial) ); + + IDirectInputDevice8_Release(device); + winetest_pop_context(); + } + + /* + * Delete the existing registry keys, this will result in a new set of + * guidInstance values being generated on the next call to EnumDevices(). + */ + cleanup_registry_keys(); + + /* + * EnumDevices() hasn't been called yet, guidInstance values are still + * valid. + */ + for (i = 0; i < dinput_test_devs.instances_count; i++) + { + winetest_push_context( "device %d", i ); + + hr = dinput_test_create_device_instance( version, &guid_instances[i], &device ); + ok( hr == DI_OK, "Unexpected hr %#lx.\n", hr ); + + dinput_test_get_device_hid_serial_string(device, device_serial); + ok( !wcscmp(test_devs[i].desc.instance_id, device_serial), "Unexpected device serial %s.\n", + debugstr_w(device_serial) ); + + IDirectInputDevice8_Release(device); + winetest_pop_context(); + } + + /* + * Call EnumDevices(), a new set of guidInstance values and registry entries + * will be created. + */ + dinput_test_devices_struct_init( &dinput_test_devs, devinsts, ARRAY_SIZE(devinsts) ); + hr = dinput_test_enum_devices( version, &dinput_test_devs ); + ok( hr == DI_OK, "Unexpected hr %#lx.\n", hr ); + ok( dinput_test_devs.instances_count == 4, "Unexpected count %u.\n", dinput_test_devs.instances_count ); + for (i = 0; i < dinput_test_devs.instances_count; i++) + { + winetest_push_context( "device %d", i ); + + todo_wine ok( !IsEqualGUID( &guid_instances[i], &devinsts[i].guidInstance ), "Unexpected guidInstance %s.\n", + debugstr_guid(&devinsts[i].guidInstance) ); + + /* Old guidInstance no longer works. */ + device = NULL; + hr = dinput_test_create_device_instance( version, &guid_instances[i], &device ); + todo_wine ok( hr == DIERR_DEVICENOTREG, "Unexpected hr %#lx.\n", hr ); + if (device) IDirectInputDevice8_Release(device); + guid_instances[i] = devinsts[i].guidInstance; + + /* New guidInstance, same device. */ + hr = dinput_test_create_device_instance( version, &devinsts[i].guidInstance, &device ); + ok( hr == DI_OK, "Unexpected hr %#lx.\n", hr ); + + dinput_test_get_device_hid_serial_string(device, device_serial); + ok( !wcscmp(test_devs[i].desc.instance_id, device_serial), "Unexpected device serial %s.\n", + debugstr_w(device_serial) ); + + IDirectInputDevice8_Release(device); + winetest_pop_context(); + } + + /* + * Stop devices 0 and 2. After enumeration, their guidInstance values will + * be assigned to devices 1 and 3. + */ + stop_joystick_test_device( &test_devs[0] ); + stop_joystick_test_device( &test_devs[2] ); + + /* Stopped devices should return E_FAIL. */ + hr = dinput_test_create_device_instance( version, &devinsts[0].guidInstance, &device ); + todo_wine ok( hr == E_FAIL, "Unexpected hr %#lx.\n", hr ); + hr = dinput_test_create_device_instance( version, &devinsts[2].guidInstance, &device ); + todo_wine ok( hr == E_FAIL, "Unexpected hr %#lx.\n", hr ); + + /* After calling EnumDevices(), guidInstance values will be reassigned. */ + dinput_test_devices_struct_init( &dinput_test_devs, devinsts, ARRAY_SIZE(devinsts) ); + hr = dinput_test_enum_devices( version, &dinput_test_devs ); + ok( hr == DI_OK, "Unexpected hr %#lx.\n", hr ); + ok( dinput_test_devs.instances_count == 2, "Unexpected count %u.\n", dinput_test_devs.instances_count ); + for (i = 0; i < dinput_test_devs.instances_count; i++) + { + const unsigned int expected_joystick = !i ? 1 : 3; + + winetest_push_context( "device %d", i ); + + todo_wine ok( IsEqualGUID( &guid_instances[expected_joystick - 1], &devinsts[i].guidInstance ), + "Unexpected guidInstance %s.\n", debugstr_guid(&devinsts[i].guidInstance) ); + hr = dinput_test_create_device_instance( version, &devinsts[i].guidInstance, &device ); + ok( hr == DI_OK, "Unexpected hr %#lx.\n", hr ); + + dinput_test_get_device_hid_serial_string(device, device_serial); + ok( !wcscmp(test_devs[expected_joystick].desc.instance_id, device_serial), "Unexpected device serial %s.\n", + debugstr_w(device_serial) ); + + IDirectInputDevice8_Release(device); + winetest_pop_context(); + } + + /* + * Restart devices 0 and 2, they should get their old guidInstance values + * back after another call to EnumDevices(). + */ + if (!start_joystick_test_device( &test_devs[0] )) goto done; + if (!start_joystick_test_device( &test_devs[2] )) goto done; + + for (i = 0; i < dinput_test_devs.instances_count; i++) + { + const unsigned int expected_joystick = !i ? 1 : 3; + + winetest_push_context( "device %d", i ); + + todo_wine ok( IsEqualGUID( &guid_instances[expected_joystick - 1], &devinsts[i].guidInstance ), + "Unexpected guidInstance %s.\n", debugstr_guid(&devinsts[i].guidInstance) ); + hr = dinput_test_create_device_instance( version, &devinsts[i].guidInstance, &device ); + ok( hr == DI_OK, "Unexpected hr %#lx.\n", hr ); + + dinput_test_get_device_hid_serial_string(device, device_serial); + ok( !wcscmp(test_devs[expected_joystick].desc.instance_id, device_serial), "Unexpected device serial %s.\n", + debugstr_w(device_serial) ); + + IDirectInputDevice8_Release(device); + winetest_pop_context(); + } + + /* guidInstance values are all back to what they were. */ + dinput_test_devices_struct_init( &dinput_test_devs, devinsts, ARRAY_SIZE(devinsts) ); + hr = dinput_test_enum_devices( version, &dinput_test_devs ); + ok( hr == DI_OK, "Unexpected hr %#lx.\n", hr ); + ok( dinput_test_devs.instances_count == 4, "Unexpected count %u.\n", dinput_test_devs.instances_count ); + for (i = 0; i < dinput_test_devs.instances_count; i++) + { + winetest_push_context( "device %d", i ); + + todo_wine_if(!(i & 0x1)) ok( IsEqualGUID( &guid_instances[i], &devinsts[i].guidInstance ), "Unexpected guidInstance %s.\n", + debugstr_guid(&devinsts[i].guidInstance) ); + + hr = dinput_test_create_device_instance( version, &devinsts[i].guidInstance, &device ); + ok( hr == DI_OK, "Unexpected hr %#lx.\n", hr ); + + dinput_test_get_device_hid_serial_string(device, device_serial); + ok( !wcscmp(test_devs[i].desc.instance_id, device_serial), "Unexpected device serial %s.\n", + debugstr_w(device_serial) ); + + IDirectInputDevice8_Release(device); + winetest_pop_context(); + } + + /* Stop all devices. */ + for (i = 0; i < ARRAY_SIZE(test_devs); i++) stop_joystick_test_device( &test_devs[i] ); + + /* + * Call EnumDevices() with all devices stopped. Test behavior of device + * creation without calling EnumDevices() with the devices present + * beforehand. + */ + dinput_test_devices_struct_init( &dinput_test_devs, devinsts, ARRAY_SIZE(devinsts) ); + hr = dinput_test_enum_devices( version, &dinput_test_devs ); + ok( hr == DI_OK, "Unexpected hr %#lx.\n", hr ); + ok( !dinput_test_devs.instances_count, "Unexpected count %u.\n", dinput_test_devs.instances_count ); + for (i = 0; i < ARRAY_SIZE(test_devs); i++) + { + winetest_push_context( "device %d", i ); + + hr = dinput_test_create_device_instance( version, &guid_instances[i], &device ); + ok( hr == DIERR_DEVICENOTREG, "Unexpected hr %#lx.\n", hr ); + + /* + * Start the joystick device. This will succeed, even without having + * called EnumDevices() beforehand. + */ + if (!start_joystick_test_device( &test_devs[i] )) goto done; + hr = dinput_test_create_device_instance( version, &guid_instances[i], &device ); + todo_wine ok( hr == DI_OK, "Unexpected hr %#lx.\n", hr ); + if (device) + { + dinput_test_get_device_hid_serial_string(device, device_serial); + ok( !wcscmp(test_devs[i].desc.instance_id, device_serial), "Unexpected device serial %s.\n", + debugstr_w(device_serial) ); + + IDirectInputDevice8_Release(device); + } + stop_joystick_test_device( &test_devs[i] ); + + hr = dinput_test_create_device_instance( version, &guid_instances[i], &device ); + todo_wine ok( hr == E_FAIL, "Unexpected hr %#lx.\n", hr ); + + if (!start_joystick_test_device( &test_devs[i] )) goto done; + winetest_pop_context(); + } + + for (i = 0; i < ARRAY_SIZE(test_devs); i++) stop_joystick_test_device( &test_devs[i] ); + + /* Reset device list. */ + dinput_test_devices_struct_init( &dinput_test_devs, devinsts, ARRAY_SIZE(devinsts) ); + hr = dinput_test_enum_devices( version, &dinput_test_devs ); + ok( hr == DI_OK, "Unexpected hr %#lx.\n", hr ); + ok( !dinput_test_devs.instances_count, "Unexpected count %u.\n", dinput_test_devs.instances_count ); + + /* + * Start device 0, create a device with guidInstance 0. + * Stop device 0. + * Start device 1, attempt to create a device with guidInstance 1. + * Fails. + * Try to create a device with guidInstance 0, which is now associated + * with device 1. + */ + if (!start_joystick_test_device( &test_devs[0] )) goto done; + + hr = dinput_test_create_device_instance( version, &guid_instances[0], &device ); + todo_wine ok( hr == DI_OK, "Unexpected hr %#lx.\n", hr ); + if (device) + { + dinput_test_get_device_hid_serial_string(device, device_serial); + ok( !wcscmp(test_devs[0].desc.instance_id, device_serial), "Unexpected device serial %s.\n", + debugstr_w(device_serial) ); + + IDirectInputDevice8_Release(device); + } + stop_joystick_test_device( &test_devs[0] ); + + hr = dinput_test_create_device_instance( version, &guid_instances[0], &device ); + todo_wine ok( hr == E_FAIL, "Unexpected hr %#lx.\n", hr ); + + /* Start device 1. */ + if (!start_joystick_test_device( &test_devs[1] )) goto done; + + /* + * This still fails with E_FAIL, as guid_instances[0] is still associated + * with device 0. + */ + hr = dinput_test_create_device_instance( version, &guid_instances[0], &device ); + todo_wine ok( hr == E_FAIL, "Unexpected hr %#lx.\n", hr ); + + /* + * guid_instances[1] is not currently associated with a device, which + * means attempting to use this GUID results in what seems to be an + * internal call to EnumDevices(). With device 0 currently stopped, + * guid_instances[0] gets reassigned to device 1. + */ + hr = dinput_test_create_device_instance( version, &guid_instances[1], &device ); + ok( hr == DIERR_DEVICENOTREG, "Unexpected hr %#lx.\n", hr ); + + hr = dinput_test_create_device_instance( version, &guid_instances[0], &device ); + todo_wine ok( hr == DI_OK, "Unexpected hr %#lx.\n", hr ); + if (device) + { + dinput_test_get_device_hid_serial_string(device, device_serial); + ok( !wcscmp(test_devs[1].desc.instance_id, device_serial), "Unexpected device serial %s.\n", + debugstr_w(device_serial) ); + + IDirectInputDevice8_Release(device); + } + stop_joystick_test_device( &test_devs[1] ); + +done: + for (i = 0; i < ARRAY_SIZE(test_devs); i++) + stop_joystick_test_device( &test_devs[i] ); + cleanup_registry_keys(); + winetest_pop_context(); +} + START_TEST( joystick8 ) { char **argv; @@ -6008,6 +6446,10 @@ START_TEST( joystick8 ) test_simple_joystick( 0x700 ); test_simple_joystick( 0x800 ); + test_joystick_instance_guid( 0x500 ); + test_joystick_instance_guid( 0x700 ); + test_joystick_instance_guid( 0x800 ); + test_many_axes_joystick(); test_driving_wheel_axes(); test_rawinput( argv ); -- 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_main.c | 1 + dlls/dinput/dinput_private.h | 1 + dlls/dinput/joystick_hid.c | 376 +++++++++++++++++++++++++++++----- dlls/dinput/tests/hotplug.c | 11 +- dlls/dinput/tests/joystick8.c | 40 ++-- 5 files changed, 346 insertions(+), 83 deletions(-) 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..7e7b1e490f5 100644 --- a/dlls/dinput/dinput_private.h +++ b/dlls/dinput/dinput_private.h @@ -56,6 +56,7 @@ 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 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 22709c4bfd9..e195d8ac43b 100644 --- a/dlls/dinput/joystick_hid.c +++ b/dlls/dinput/joystick_hid.c @@ -49,9 +49,216 @@ 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 ); +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 ); +} + +/* Need to enter the mutex prior to accessing these registry entries. */ +static const WCHAR *dinput_path = L"System\\CurrentControlSet\\Control\\MediaProperties\\" + "PrivateProperties\\DirectInput"; +static BOOL get_device_instance_id_from_registry(WORD vid, WORD pid, unsigned int idx, GUID *guid_instance) +{ + BOOL ret_val = FALSE; + HKEY dev_key = NULL; + WCHAR buf[MAX_PATH]; + GUID guid = { 0 }; + DWORD len; + + memset( guid_instance, 0, sizeof(*guid_instance) ); + swprintf( buf, ARRAY_SIZE(buf), L"%s\\VID_%04X&PID_%04X\\Calibration\\%d", dinput_path, vid, pid, idx ); + if (RegOpenKeyExW( HKEY_CURRENT_USER, buf, 0, KEY_ALL_ACCESS, &dev_key )) goto exit; + + /* No preexisting guidInstance or failure to get value, invalid key. */ + len = sizeof(guid); + if (RegGetValueW( dev_key, NULL, L"GUID", RRF_RT_REG_BINARY, NULL, &guid, &len )) + { + WARN( "Failed to get guidInstance from key.\n" ); + goto exit; + } + + ret_val = TRUE; + *guid_instance = guid; + +exit: + RegCloseKey( dev_key ); + /* Got a non-existent or malformed key, delete it. */ + if (dev_key && !ret_val) RegDeleteKeyW( HKEY_CURRENT_USER, buf ); + return ret_val; +} + +static void set_device_instance_id_in_registry(WORD vid, WORD pid, unsigned int idx, GUID *guid_instance) +{ + HKEY dev_key = NULL; + WCHAR buf[MAX_PATH]; + + swprintf( buf, ARRAY_SIZE(buf), L"%s\\VID_%04X&PID_%04X\\Calibration\\%d", dinput_path, vid, pid, idx ); + if (RegCreateKeyExW( HKEY_CURRENT_USER, buf, 0, NULL, 0, KEY_ALL_ACCESS, NULL, &dev_key, NULL )) + { + WARN( "Failed to create HKEY for device.\n" ); + goto exit; + } + + if (RegSetValueExW( dev_key, L"GUID", 0, REG_BINARY, (const BYTE *)guid_instance, + sizeof(*guid_instance) )) + WARN( "Failed to set instance GUID value in registry.\n" ); +exit: + RegCloseKey( dev_key ); +} + +/* + * 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 BOOL dinput_array_reserve(void **elements, SIZE_T *capacity, SIZE_T count, SIZE_T size) +{ + SIZE_T max_capacity, new_capacity; + void *new_elements; + + if (count <= *capacity) + return TRUE; + + max_capacity = ~(SIZE_T)0 / size; + if (count > max_capacity) + return FALSE; + + new_capacity = max(1, *capacity); + while (new_capacity < count && new_capacity <= max_capacity / 2) + new_capacity *= 2; + if (new_capacity < count) + new_capacity = count; + + new_elements = _recalloc( *elements, new_capacity, size ); + if (!new_elements) + return FALSE; + + *elements = new_elements; + *capacity = new_capacity; + return TRUE; +} + +struct dinput_joystick_instance +{ + DIDEVICEINSTANCEW di_device_instance; + WCHAR device_path[MAX_PATH]; + DWORD dev_idx; + + BOOL dev_idx_assigned; + WORD vid, pid; +}; + +static CRITICAL_SECTION joystick_crit; +static CRITICAL_SECTION_DEBUG joystick_critsect_debug = +{ + 0, 0, &joystick_crit, + { &joystick_critsect_debug.ProcessLocksList, &joystick_critsect_debug.ProcessLocksList }, + 0, 0, { (DWORD_PTR)(__FILE__ ": joystick_crit") } +}; +static CRITICAL_SECTION joystick_crit = { &joystick_critsect_debug, -1, 0, 0, 0, 0 }; + +/* Need to enter the CS to access this array. */ +static struct +{ + struct dinput_joystick_instance *joysticks; + unsigned int count; + SIZE_T size; +} dinput_joysticks; + +static struct dinput_joystick_instance *dinput_get_joystick_instance(unsigned int idx) +{ + if (dinput_joysticks.count && (idx < dinput_joysticks.count)) + return &dinput_joysticks.joysticks[idx]; + return NULL; +} + +static BOOL dinput_add_joystick_instance(const struct dinput_joystick_instance *inst) +{ + if (!dinput_array_reserve( (void **)&dinput_joysticks.joysticks, + &dinput_joysticks.size, dinput_joysticks.count + 1, sizeof(*inst) )) + { + ERR("Failed to allocate memory for a new device instance.\n"); + return FALSE; + } + + dinput_joysticks.joysticks[dinput_joysticks.count++] = *inst; + return TRUE; +} + +static struct dinput_joystick_instance *dinput_get_joystick_instance_from_guid(const GUID *guid) +{ + struct dinput_joystick_instance *ret = NULL; + unsigned int i; + + for (i = 0; i < dinput_joysticks.count; i++) + { + struct dinput_joystick_instance *instance = &dinput_joysticks.joysticks[i]; + + if (IsEqualGUID( &instance->di_device_instance.guidInstance, guid ) + || IsEqualGUID( &instance->di_device_instance.guidProduct, guid )) + { + ret = instance; + break; + } + } + return ret; +} + +void hid_joystick_cleanup( void ) +{ + if (dinput_reg_mutex) CloseHandle( dinput_reg_mutex ); + if (dinput_joysticks.joysticks) free(dinput_joysticks.joysticks); +} + struct pid_control_report { BYTE id; @@ -1421,13 +1628,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, @@ -1450,14 +1656,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; @@ -1572,21 +1771,24 @@ failed: return DIERR_DEVICENOTREG; } -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 ) +static HRESULT hid_joystick_refresh_devices( void ) { 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)}; + struct dinput_joystick_instance cur_instance = { 0 }; WCHAR device_id[MAX_PATH], *tmp; + PHIDP_PREPARSED_DATA preparsed; + HIDD_ATTRIBUTES attrs = { 0 }; HDEVINFO set, xi_set; + HIDP_CAPS caps; + HANDLE device; BOOL override; UINT32 i = 0; GUID hid; - TRACE( "index %d, guid %s\n", index, debugstr_guid( guid ) ); + TRACE( "\n" ); HidD_GetHidGuid( &hid ); @@ -1594,18 +1796,25 @@ static HRESULT hid_joystick_device_open( int index, const GUID *guid, DIDEVICEIN if (set == INVALID_HANDLE_VALUE) return DIERR_DEVICENOTREG; xi_set = SetupDiGetClassDevsW( &GUID_DEVINTERFACE_WINEXINPUT, NULL, NULL, DIGCF_DEVICEINTERFACE | DIGCF_PRESENT ); - *device = NULL; - *preparsed = NULL; + device = NULL; + preparsed = NULL; + + EnterCriticalSection( &joystick_crit ); + memset( dinput_joysticks.joysticks, 0, sizeof(*dinput_joysticks.joysticks) * dinput_joysticks.size ); + dinput_joysticks.count = 0; while (SetupDiEnumDeviceInterfaces( set, NULL, &hid, i++, &iface )) { + memset( &cur_instance, 0, sizeof(cur_instance) ); + cur_instance.di_device_instance.dwSize = sizeof(cur_instance.di_device_instance); + 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 ))) + if (FAILED(hid_joystick_device_try_open( detail->DevicePath, &device, &preparsed, + &attrs, &caps, &cur_instance.di_device_instance, 0x0800 ))) continue; - if (device_instance_is_disabled( instance, &override )) + if (device_instance_is_disabled( &cur_instance.di_device_instance, &override )) goto next; if (override && SetupDiGetDeviceInstanceIdW( set, &devinfo, device_id, MAX_PATH, NULL ) && @@ -1619,58 +1828,102 @@ static HRESULT hid_joystick_device_open( int index, const GUID *guid, DIDEVICEIN 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 ))) + CloseHandle( device ); + HidD_FreePreparsedData( preparsed ); + if (FAILED(hid_joystick_device_try_open( detail->DevicePath, &device, &preparsed, + &attrs, &caps, &cur_instance.di_device_instance, 0x0800 ))) continue; } - /* enumerate device by GUID */ - if (IsEqualGUID( guid, &instance->guidProduct ) || IsEqualGUID( guid, &instance->guidInstance )) break; + cur_instance.vid = attrs.VendorID; + cur_instance.pid = attrs.ProductID; + wcsncpy( cur_instance.device_path, detail->DevicePath, MAX_PATH ); + dinput_add_joystick_instance(&cur_instance); + next: + CloseHandle( device ); + HidD_FreePreparsedData( preparsed ); + device = NULL; + preparsed = NULL; + } - /* enumerate all devices */ - if (index >= 0 && !index--) break; + /* Assign device index for a particular VID/PID pair. */ + for (i = 0; i < dinput_joysticks.count; i++) + { + struct dinput_joystick_instance *inst = &dinput_joysticks.joysticks[i]; + unsigned int j, dev_idx; - next: - CloseHandle( *device ); - HidD_FreePreparsedData( *preparsed ); - *device = NULL; - *preparsed = NULL; + if (inst->dev_idx_assigned) continue; + + inst->dev_idx = dev_idx = 0; + inst->dev_idx_assigned = TRUE; + for (j = (i + 1); j < dinput_joysticks.count; j++) + { + struct dinput_joystick_instance *inst2 = &dinput_joysticks.joysticks[j]; + + if (inst2->vid == inst->vid && inst2->pid == inst->pid) + { + inst2->dev_idx = ++dev_idx; + inst2->dev_idx_assigned = TRUE; + } + } + } + + /* + * Get guidInstance values for each joystick device. If there is one in + * the registry already use that, and if not, create one. + */ + get_dinput_reg_mutex(); + for (i = 0; i < dinput_joysticks.count; i++) + { + struct dinput_joystick_instance *inst = &dinput_joysticks.joysticks[i]; + + if (!get_device_instance_id_from_registry( inst->vid, inst->pid, inst->dev_idx, + &inst->di_device_instance.guidInstance )) + { + create_v1_uuid( &inst->di_device_instance.guidInstance ); + set_device_instance_id_in_registry( inst->vid, inst->pid, inst->dev_idx, + &inst->di_device_instance.guidInstance ); + } } + release_dinput_reg_mutex(); + + LeaveCriticalSection( &joystick_crit ); 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 ) { - HIDD_ATTRIBUTES attrs = {.Size = sizeof(attrs)}; - PHIDP_PREPARSED_DATA preparsed; + struct dinput_joystick_instance *inst = NULL; WCHAR device_path[MAX_PATH]; - GUID guid = GUID_NULL; - HIDP_CAPS caps; - HANDLE device; - HRESULT hr; + HRESULT hr = DI_OK; 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 (!index) hr = hid_joystick_refresh_devices(); if (hr != DI_OK) return hr; - HidD_FreePreparsedData( preparsed ); - CloseHandle( device ); + EnterCriticalSection( &joystick_crit ); + if ((inst = dinput_get_joystick_instance( index ))) + { + DWORD type = inst->di_device_instance.dwDevType & ~DIDEVTYPE_HID; + + *instance = inst->di_device_instance; + instance->dwDevType = device_type_for_version( type, version ) | DIDEVTYPE_HID; + wcsncpy( device_path, inst->device_path, MAX_PATH ); + } - 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) ); + if (!inst) hr = DIERR_DEVICENOTREG; + LeaveCriticalSection( &joystick_crit ); - return DI_OK; + if (hr == DI_OK) + 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) ); + + return hr; } static BOOL init_object_properties( struct dinput_device *device, UINT index, struct hid_value_caps *caps, @@ -2040,15 +2293,30 @@ HRESULT hid_joystick_create_device( struct dinput *dinput, const GUID *guid, IDi impl->base.dwCoopLevel = DISCL_NONEXCLUSIVE | DISCL_BACKGROUND; 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 ); - else + if (!memcmp( device_path_guid.Data4, guid->Data4, sizeof(device_path_guid.Data4) )) { wcscpy( impl->device_path, *(const WCHAR **)guid ); hr = hid_joystick_device_try_open( impl->device_path, &impl->device, &impl->preparsed, &attrs, &impl->caps, &impl->base.instance, dinput->dwVersion ); } + else + { + struct dinput_joystick_instance *inst = NULL; + + EnterCriticalSection( &joystick_crit ); + /* If we don't get a device for this GUID initially, try enumerating. */ + if (!(inst = dinput_get_joystick_instance_from_guid( guid ))) hid_joystick_refresh_devices(); + if ((inst = dinput_get_joystick_instance_from_guid( guid ))) + { + wcscpy( impl->device_path, inst->device_path ); + hr = hid_joystick_device_try_open( impl->device_path, &impl->device, &impl->preparsed, &attrs, + &impl->caps, &impl->base.instance, dinput->dwVersion ); + if (hr == DI_OK) impl->base.instance.guidInstance = inst->di_device_instance.guidInstance; + } + else + hr = DIERR_DEVICENOTREG; + LeaveCriticalSection( &joystick_crit ); + } if (hr != DI_OK) goto failed; impl->base.caps.dwDevType = impl->base.instance.dwDevType; diff --git a/dlls/dinput/tests/hotplug.c b/dlls/dinput/tests/hotplug.c index 350050c0a42..e47fd768c18 100644 --- a/dlls/dinput/tests/hotplug.c +++ b/dlls/dinput/tests/hotplug.c @@ -229,13 +229,10 @@ static BOOL test_input_lost( DWORD version ) hr = dinput_test_create_device( version, &devinst, &device2 ); ok( hr == DI_OK, "Unexpected hr %#lx.\n", hr ); ok( !!device2, "device2 is NULL.\n" ); - if (device2) - { - todo_wine ok( !memcmp( &guid_instance, &devinst.guidInstance, sizeof(guid_instance) ), - "Unexpected guidInstance.\n" ); - ref = IDirectInputDevice8_Release( device2 ); - ok( ref == 0, "Release returned %ld\n", ref ); - } + ok( !memcmp( &guid_instance, &devinst.guidInstance, sizeof(guid_instance) ), + "Unexpected guidInstance.\n" ); + ref = IDirectInputDevice8_Release( device2 ); + ok( ref == 0, "Release returned %ld\n", ref ); done: hid_device_stop( &desc, 1 ); diff --git a/dlls/dinput/tests/joystick8.c b/dlls/dinput/tests/joystick8.c index c5dc5c066bf..ee2e6357344 100644 --- a/dlls/dinput/tests/joystick8.c +++ b/dlls/dinput/tests/joystick8.c @@ -6197,13 +6197,13 @@ static void test_joystick_instance_guid( DWORD version ) { winetest_push_context( "device %d", i ); - todo_wine ok( !IsEqualGUID( &guid_instances[i], &devinsts[i].guidInstance ), "Unexpected guidInstance %s.\n", + ok( !IsEqualGUID( &guid_instances[i], &devinsts[i].guidInstance ), "Unexpected guidInstance %s.\n", debugstr_guid(&devinsts[i].guidInstance) ); /* Old guidInstance no longer works. */ device = NULL; hr = dinput_test_create_device_instance( version, &guid_instances[i], &device ); - todo_wine ok( hr == DIERR_DEVICENOTREG, "Unexpected hr %#lx.\n", hr ); + ok( hr == DIERR_DEVICENOTREG, "Unexpected hr %#lx.\n", hr ); if (device) IDirectInputDevice8_Release(device); guid_instances[i] = devinsts[i].guidInstance; @@ -6243,7 +6243,7 @@ static void test_joystick_instance_guid( DWORD version ) winetest_push_context( "device %d", i ); - todo_wine ok( IsEqualGUID( &guid_instances[expected_joystick - 1], &devinsts[i].guidInstance ), + ok( IsEqualGUID( &guid_instances[expected_joystick - 1], &devinsts[i].guidInstance ), "Unexpected guidInstance %s.\n", debugstr_guid(&devinsts[i].guidInstance) ); hr = dinput_test_create_device_instance( version, &devinsts[i].guidInstance, &device ); ok( hr == DI_OK, "Unexpected hr %#lx.\n", hr ); @@ -6269,7 +6269,7 @@ static void test_joystick_instance_guid( DWORD version ) winetest_push_context( "device %d", i ); - todo_wine ok( IsEqualGUID( &guid_instances[expected_joystick - 1], &devinsts[i].guidInstance ), + ok( IsEqualGUID( &guid_instances[expected_joystick - 1], &devinsts[i].guidInstance ), "Unexpected guidInstance %s.\n", debugstr_guid(&devinsts[i].guidInstance) ); hr = dinput_test_create_device_instance( version, &devinsts[i].guidInstance, &device ); ok( hr == DI_OK, "Unexpected hr %#lx.\n", hr ); @@ -6291,7 +6291,7 @@ static void test_joystick_instance_guid( DWORD version ) { winetest_push_context( "device %d", i ); - todo_wine_if(!(i & 0x1)) ok( IsEqualGUID( &guid_instances[i], &devinsts[i].guidInstance ), "Unexpected guidInstance %s.\n", + ok( IsEqualGUID( &guid_instances[i], &devinsts[i].guidInstance ), "Unexpected guidInstance %s.\n", debugstr_guid(&devinsts[i].guidInstance) ); hr = dinput_test_create_device_instance( version, &devinsts[i].guidInstance, &device ); @@ -6330,7 +6330,7 @@ static void test_joystick_instance_guid( DWORD version ) */ if (!start_joystick_test_device( &test_devs[i] )) goto done; hr = dinput_test_create_device_instance( version, &guid_instances[i], &device ); - todo_wine ok( hr == DI_OK, "Unexpected hr %#lx.\n", hr ); + ok( hr == DI_OK, "Unexpected hr %#lx.\n", hr ); if (device) { dinput_test_get_device_hid_serial_string(device, device_serial); @@ -6367,15 +6367,13 @@ static void test_joystick_instance_guid( DWORD version ) if (!start_joystick_test_device( &test_devs[0] )) goto done; hr = dinput_test_create_device_instance( version, &guid_instances[0], &device ); - todo_wine ok( hr == DI_OK, "Unexpected hr %#lx.\n", hr ); - if (device) - { - dinput_test_get_device_hid_serial_string(device, device_serial); - ok( !wcscmp(test_devs[0].desc.instance_id, device_serial), "Unexpected device serial %s.\n", - debugstr_w(device_serial) ); + ok( hr == DI_OK, "Unexpected hr %#lx.\n", hr ); + + dinput_test_get_device_hid_serial_string(device, device_serial); + ok( !wcscmp(test_devs[0].desc.instance_id, device_serial), "Unexpected device serial %s.\n", + debugstr_w(device_serial) ); + IDirectInputDevice8_Release(device); - IDirectInputDevice8_Release(device); - } stop_joystick_test_device( &test_devs[0] ); hr = dinput_test_create_device_instance( version, &guid_instances[0], &device ); @@ -6401,15 +6399,13 @@ static void test_joystick_instance_guid( DWORD version ) ok( hr == DIERR_DEVICENOTREG, "Unexpected hr %#lx.\n", hr ); hr = dinput_test_create_device_instance( version, &guid_instances[0], &device ); - todo_wine ok( hr == DI_OK, "Unexpected hr %#lx.\n", hr ); - if (device) - { - dinput_test_get_device_hid_serial_string(device, device_serial); - ok( !wcscmp(test_devs[1].desc.instance_id, device_serial), "Unexpected device serial %s.\n", - debugstr_w(device_serial) ); + ok( hr == DI_OK, "Unexpected hr %#lx.\n", hr ); + + dinput_test_get_device_hid_serial_string(device, device_serial); + ok( !wcscmp(test_devs[1].desc.instance_id, device_serial), "Unexpected device serial %s.\n", + debugstr_w(device_serial) ); + IDirectInputDevice8_Release(device); - IDirectInputDevice8_Release(device); - } stop_joystick_test_device( &test_devs[1] ); done: -- 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/joystick_hid.c | 2 +- dlls/dinput/tests/joystick8.c | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/dlls/dinput/joystick_hid.c b/dlls/dinput/joystick_hid.c index e195d8ac43b..b620603abd3 100644 --- a/dlls/dinput/joystick_hid.c +++ b/dlls/dinput/joystick_hid.c @@ -1638,7 +1638,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 ee2e6357344..7dfd2b61527 100644 --- a/dlls/dinput/tests/joystick8.c +++ b/dlls/dinput/tests/joystick8.c @@ -6228,9 +6228,9 @@ static void test_joystick_instance_guid( DWORD version ) /* Stopped devices should return E_FAIL. */ hr = dinput_test_create_device_instance( version, &devinsts[0].guidInstance, &device ); - todo_wine ok( hr == E_FAIL, "Unexpected hr %#lx.\n", hr ); + ok( hr == E_FAIL, "Unexpected hr %#lx.\n", hr ); hr = dinput_test_create_device_instance( version, &devinsts[2].guidInstance, &device ); - todo_wine ok( hr == E_FAIL, "Unexpected hr %#lx.\n", hr ); + ok( hr == E_FAIL, "Unexpected hr %#lx.\n", hr ); /* After calling EnumDevices(), guidInstance values will be reassigned. */ dinput_test_devices_struct_init( &dinput_test_devs, devinsts, ARRAY_SIZE(devinsts) ); @@ -6342,7 +6342,7 @@ static void test_joystick_instance_guid( DWORD version ) stop_joystick_test_device( &test_devs[i] ); hr = dinput_test_create_device_instance( version, &guid_instances[i], &device ); - todo_wine ok( hr == E_FAIL, "Unexpected hr %#lx.\n", hr ); + ok( hr == E_FAIL, "Unexpected hr %#lx.\n", hr ); if (!start_joystick_test_device( &test_devs[i] )) goto done; winetest_pop_context(); @@ -6377,7 +6377,7 @@ static void test_joystick_instance_guid( DWORD version ) stop_joystick_test_device( &test_devs[0] ); hr = dinput_test_create_device_instance( version, &guid_instances[0], &device ); - todo_wine ok( hr == E_FAIL, "Unexpected hr %#lx.\n", hr ); + ok( hr == E_FAIL, "Unexpected hr %#lx.\n", hr ); /* Start device 1. */ if (!start_joystick_test_device( &test_devs[1] )) goto done; @@ -6387,7 +6387,7 @@ static void test_joystick_instance_guid( DWORD version ) * with device 0. */ hr = dinput_test_create_device_instance( version, &guid_instances[0], &device ); - todo_wine ok( hr == E_FAIL, "Unexpected hr %#lx.\n", hr ); + ok( hr == E_FAIL, "Unexpected hr %#lx.\n", hr ); /* * guid_instances[1] is not currently associated with a device, which -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10364
Rémi Bernon (@rbernon) commented about dlls/dinput/tests/driver_bus.c:
for (i = 0; i < impl->devices->Count; ++i) if (impl->devices->Objects[i] == device) break; if (i == impl->devices->Count) status = STATUS_NOT_FOUND; - else impl->devices->Objects[i] = impl->devices->Objects[impl->devices->Count--]; + else impl->devices->Objects[i] = impl->devices->Objects[--impl->devices->Count]; + if (status != STATUS_NOT_FOUND) impl->devices->Objects[impl->devices->Count + 1] = NULL;
Do we really need to set it to NULL? If that's not the case already I think we should use device count everywhere and not rely on NULL pointers. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10364#note_132758
Rémi Bernon (@rbernon) commented about dlls/dinput/joystick_hid.c:
+{ + 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; +} This should probably use `UuidCreateSequential`? Also note how its implementation differs from here (and idk which one is correct):
``` Uuid->Data4[0] = sequence & 0xff; Uuid->Data4[1] = (sequence & 0x3f00) >> 8; Uuid->Data4[1] |= 0x80; ``` -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10364#note_132760
Rémi Bernon (@rbernon) commented about dlls/dinput/tests/joystick8.c:
}
gi0 = (void *)0xdeadbeef; - todo_wine ok_hr( S_OK, pGameInputCreate( &gi0 ) ); - if (gi0 != (void *)0xdeadbeef) ok_ret( 0, v0_IGameInput_Release( gi0 ) ); + hr = pGameInputCreate( &gi0 ); + todo_wine ok( hr == S_OK || broken(hr == E_NOTIMPL), "Unexpected hr %#lx.\n", hr ); + if (gi0 && gi0 != (void *)0xdeadbeef) ok_ret( 0, v0_IGameInput_Release( gi0 ) );
Commit title says W.G.I but that's actually a GameInput test. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10364#note_132759
Rémi Bernon (@rbernon) commented about dlls/dinput/joystick_hid.c:
+static CRITICAL_SECTION joystick_crit; +static CRITICAL_SECTION_DEBUG joystick_critsect_debug = +{ + 0, 0, &joystick_crit, + { &joystick_critsect_debug.ProcessLocksList, &joystick_critsect_debug.ProcessLocksList }, + 0, 0, { (DWORD_PTR)(__FILE__ ": joystick_crit") } +}; +static CRITICAL_SECTION joystick_crit = { &joystick_critsect_debug, -1, 0, 0, 0, 0 }; + +/* Need to enter the CS to access this array. */ +static struct +{ + struct dinput_joystick_instance *joysticks; + unsigned int count; + SIZE_T size; +} dinput_joysticks; A struct list would be simpler than an array here IMO. Also I think the local cache as it is implemented might be prone to race conditions, when multiple processes are refreshing the device lists at the same time. The registry should be the primary source and updated from the enumerated devices, and then the local cache updated from the registry when necessary, not the other way around. But maybe we don't even need a cache and we could store the device path in the registry as well?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10364#note_132761
On Thu Mar 19 11:18:57 2026 +0000, Rémi Bernon wrote:
This should probably use `UuidCreateSequential`? Also note how its implementation differs from here (and idk which one is correct): ``` Uuid->Data4[0] = sequence & 0xff; Uuid->Data4[1] = (sequence & 0x3f00) >> 8; Uuid->Data4[1] |= 0x80; ``` From [rfc4122](https://www.ietf.org/rfc/rfc4122.txt):
0 1 2 3
0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
| time_low |
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
| time_mid | time_hi_and_version |
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
|clk_seq_hi_res | clk_seq_low | node (0-1) |
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
| node (2-5) |
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
For UUID version 1: ``` o Set the clock_seq_low field to the eight least significant bits (bits zero through 7) of the clock sequence in the same order of significance. o Set the 6 least significant bits (bits zero through 5) of the clock_seq_hi_and_reserved field to the 6 most significant bits (bits 8 through 13) of the clock sequence in the same order of significance. ``` So I think that part of `UuidCreateSequential()`'s implementation is incorrect. I didn't know that `UuidCreateSequential` created version 1 UUIDs, however it has some differences with how dinput generates its `guidInstance` values. It sets the `node` fields to the MAC address of the computer, whereas dinput sets the node field to all 0s and an ASCII `DEST` as the final bytes. It also does not increment the clock sequence value each time a new UUID is created. I could imagine a scenario where the node portion of the ID being different might trip a game up, but I have no concrete evidence for it. If all that matters is that a unique ID is created, we can use `UuidCreateSequential()` if that's what you'd prefer. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10364#note_132772
On Thu Mar 19 12:49:07 2026 +0000, Connor McAdams wrote:
From [rfc4122](https://www.ietf.org/rfc/rfc4122.txt): ``` 0 1 2 3 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ | time_low | +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ | time_mid | time_hi_and_version | +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ |clk_seq_hi_res | clk_seq_low | node (0-1) | +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ | node (2-5) | +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ ``` For UUID version 1: ``` o Set the clock_seq_low field to the eight least significant bits (bits zero through 7) of the clock sequence in the same order of significance. o Set the 6 least significant bits (bits zero through 5) of the clock_seq_hi_and_reserved field to the 6 most significant bits (bits 8 through 13) of the clock sequence in the same order of significance. ``` So I think that part of `UuidCreateSequential()`'s implementation is incorrect. I didn't know that `UuidCreateSequential` created version 1 UUIDs, however it has some differences with how dinput generates its `guidInstance` values. It sets the `node` fields to the MAC address of the computer, whereas dinput sets the node field to all 0s and an ASCII `DEST` as the final bytes. It also does not increment the clock sequence value each time a new UUID is created. I could imagine a scenario where the node portion of the ID being different might trip a game up, but I have no concrete evidence for it. If all that matters is that a unique ID is created, we can use `UuidCreateSequential()` if that's what you'd prefer. Okay, I was thinking that the tail bytes could be set later but other differences make it sound reasonable to duplicate the implementation.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10364#note_132777
On Thu Mar 19 11:19:31 2026 +0000, Rémi Bernon wrote:
A struct list would be simpler than an array here IMO. Also I think the local cache as it is implemented might be prone to race conditions, when multiple processes are refreshing the device lists at the same time. The registry should be the primary source and updated from the enumerated devices, and then the local cache updated from the registry when necessary, not the other way around. But maybe we don't even need a cache and we could store the device path in the registry as well? This is a bit tricky, and IMO comes down to how closely we want to match native's behavior.
A `guidInstance` value can represent a different device path in separate processes, it has to do with device enumeration order. Enumeration order is determined by the time of when a process first sees a joystick's device instance ID. I have tests for this in my branch [here](https://gitlab.winehq.org/cmcadams/wine/-/commits/WIP/dinput-instance-guid-v...), under `dinput/tests: Add tests for joystick enumeration order.` This is also why I chose to use an array instead of a list, to make it easier to reorder things after populating the array. Our enumeration code uses setupapi, which enumerates by `VID/PID` pairs, and within `VID/PID` pairs it enumerates based on the device instance ID string in ascending order. I have code in that branch to sort the array based on connection order after populating it. So, the registry only stores `guidInstance` and `DIPROP_JOYSTICKID` values for a particular VID/PID/device index pair, and that VID/PID/device index pair can represent a different device path per-process. Beyond that the device path for a pairing can change depending on if a device is connected/disconnected, which is why I think a local cache is necessary. Does that make sense? The whole thing is kind of a mess, and I may have misunderstood what you're proposing :) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10364#note_132796
On Thu Mar 19 13:34:52 2026 +0000, Connor McAdams wrote:
This is a bit tricky, and IMO comes down to how closely we want to match native's behavior. A `guidInstance` value can represent a different device path in separate processes, it has to do with device enumeration order. Enumeration order is determined by the time of when a process first sees a joystick's device instance ID. I have tests for this in my branch [here](https://gitlab.winehq.org/cmcadams/wine/-/commits/WIP/dinput-instance-guid-v...), under `dinput/tests: Add tests for joystick enumeration order.` This is also why I chose to use an array instead of a list, to make it easier to reorder things after populating the array. Our enumeration code uses setupapi, which enumerates by `VID/PID` pairs, and within `VID/PID` pairs it enumerates based on the device instance ID string in ascending order. I have code in that branch to sort the array based on connection order after populating it. So, the registry only stores `guidInstance` and `DIPROP_JOYSTICKID` values for a particular VID/PID/device index pair, and that VID/PID/device index pair can represent a different device path per-process. Beyond that the device path for a pairing can change depending on if a device is connected/disconnected, which is why I think a local cache is necessary. Does that make sense? The whole thing is kind of a mess, and I may have misunderstood what you're proposing :) I'm not convinced that it is like that. From your branch, I believe this patch: [dinput-processes.patch](/uploads/6daf552644835699012fbf57f7dcdf4b/dinput-processes.patch) shows that every process has the same view of enumerated devices as soon as they have been enumerated once.
The `test_joystick_order_process` function (executed from `run_in_process( "test_joystick_order" )`) clears the system-wide registry, plugs devices in some order, enumerates them, then unplug the devices and plugs them back in reverse order, then enumerates them again. It shows that from a given process, the device enumeration is stable wrt plugging order as soon as devices have been enumerated. The patch adds extra checks, in `test_joystick_order_check_process` (executed from `run_in_process( "test_joystick_order_check" )`) with dinput device enumeration only (no registry reset or device changes) done in a separate process, each time the process enumerates its devices, and it shows that from a separate process, the order is the same as from the parent process, indicating that the device enumeration is also stable system-wide and not only per-process. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10364#note_133528
On Tue Mar 24 11:55:21 2026 +0000, Rémi Bernon wrote:
I'm not convinced that it is like that. From your branch, I believe this patch: [dinput-processes.patch](/uploads/6daf552644835699012fbf57f7dcdf4b/dinput-processes.patch) shows that every process has the same view of enumerated devices as soon as they have been enumerated once. The `test_joystick_order_process` function (executed from `run_in_process( "test_joystick_order" )`) clears the system-wide registry, plugs devices in some order, enumerates them, then unplug the devices and plugs them back in reverse order, then enumerates them again. It shows that from a given process, the device enumeration is stable wrt plugging order as soon as devices have been enumerated. The patch adds extra checks, in `test_joystick_order_check_process` (executed from `run_in_process( "test_joystick_order_check" )`) with dinput device enumeration only (no registry reset or device changes) done in a separate process, each time the process enumerates its devices, and it shows that from a separate process, the order is the same as from the parent process, indicating that the device enumeration is also stable system-wide and not only per-process. Thanks for taking the time to look into this :)
From testing/modifying your tests, it looks like the device instance strings change between calls to `bus_device_stop()` and `bus_device_start()`. So, in my previous tests I was seeing the behavior for new device instances being created each time. I'll work on implementing this in the way you describe and find or add a way to determine the time that a device was first encountered. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10364#note_133541
participants (3)
-
Connor McAdams -
Connor McAdams (＠cmcadams)
-
Rémi Bernon