On Tue, Jun 29, 2021 at 03:15:20PM +0200, Rémi Bernon wrote:
+static HRESULT WINAPI hid_joystick_Acquire( IDirectInputDevice8W *iface ) +{
- HRESULT hr;
- TRACE( "iface %p.\n", iface );
- if ((hr = IDirectInputDevice2WImpl_Acquire( iface )) != DI_OK) return hr;
- return DI_OK;
+}
+static HRESULT WINAPI hid_joystick_Unacquire( IDirectInputDevice8W *iface ) +{
- HRESULT hr;
- TRACE( "iface %p.\n", iface );
- if ((hr = IDirectInputDevice2WImpl_Unacquire( iface )) != DI_OK) return hr;
- return DI_OK;
+}
Is there a reason for not using the IDirectInputDevice2WImpl_Acquire / IDirectInputDevice2WImpl_Unacquire in the vtable directly or not returning the values without the extra steps?
+static HRESULT hid_joystick_device_open( DWORD index, WCHAR *device_path, HANDLE *device, DWORD version,
PHIDP_PREPARSED_DATA *preparsed, HIDD_ATTRIBUTES *attrs,HIDP_CAPS *caps, DIDEVICEINSTANCEW *instance )+{
- RAWINPUTDEVICELIST *list;
- DWORD count, i, j;
- GetRawInputDeviceList( NULL, &count, sizeof(*list) );
- if (!(list = HeapAlloc( GetProcessHeap(), 0, count * sizeof(*list) ))) return DIERR_OUTOFMEMORY;
- GetRawInputDeviceList( list, &count, sizeof(*list) );
- for (i = 0; i < count; ++i) if (list[i].dwType == RIM_TYPEHID) break;
- for (j = 0; j < index && i < count; ++i) if (list[i].dwType == RIM_TYPEHID) ++j;
- if (i == count)
- {
HeapFree( GetProcessHeap(), 0, list );return DIERR_DEVICENOTREG;- }
I am not sure you can depend on RIM_TYPEMOUSE and RIM_TYPEKEYBOARD being clumped at the beggining of the list, as it's not guaranteed by the API an may break in Wine in the future.
- count = MAX_PATH;
- GetRawInputDeviceInfoW( list[i].hDevice, RIDI_DEVICENAME, device_path, &count );
- HeapFree( GetProcessHeap(), 0, list );
- TRACE( "Opening %s\n", debugstr_w(device_path) );
- *device = CreateFileW( device_path, GENERIC_READ | GENERIC_WRITE,
FILE_SHARE_READ | FILE_SHARE_WRITE, NULL, OPEN_EXISTING, 0, 0 );- *preparsed = NULL;
- if (*device == INVALID_HANDLE_VALUE) return DI_NOTATTACHED;
- if (!HidD_GetPreparsedData( *device, preparsed )) goto failed;
- if (!HidD_GetAttributes( *device, attrs )) goto failed;
- if (HidP_GetCaps( *preparsed, caps ) != HIDP_STATUS_SUCCESS) goto failed;
- if (caps->UsagePage == HID_USAGE_PAGE_GAME) FIXME( "Unimplemented HID game usage page!\n" );
- if (caps->UsagePage == HID_USAGE_PAGE_SIMULATION) FIXME( "Unimplemented HID simulation usage page!\n" );
- if (caps->UsagePage != HID_USAGE_PAGE_GENERIC) goto failed;
- if (caps->Usage != HID_USAGE_GENERIC_GAMEPAD && caps->Usage != HID_USAGE_GENERIC_JOYSTICK) goto failed;
- instance->guidInstance = hid_joystick_guid;
- instance->guidInstance.Data3 = index;
- instance->guidProduct = DInput_PIDVID_Product_GUID;
- instance->guidProduct.Data1 = MAKELONG( attrs->VendorID, attrs->ProductID );
- instance->dwDevType = get_device_type( version, caps->Usage != HID_USAGE_GENERIC_GAMEPAD ) | DIDEVTYPE_HID;
- instance->guidFFDriver = GUID_NULL;
- instance->wUsagePage = caps->UsagePage;
- instance->wUsage = caps->Usage;
- if (!HidD_GetProductString( *device, instance->tszInstanceName, MAX_PATH )) goto failed;
- if (!HidD_GetProductString( *device, instance->tszProductName, MAX_PATH )) goto failed;
- return DI_OK;
+failed:
- CloseHandle( *device );
- *device = INVALID_HANDLE_VALUE;
- HidD_FreePreparsedData( *preparsed );
- *preparsed = NULL;
- return DI_NOTATTACHED;
+}
+static HRESULT hid_joystick_enum_device( DWORD type, DWORD flags, DIDEVICEINSTANCEW *instance,
DWORD version, int index )+{
- HIDD_ATTRIBUTES attrs = {sizeof(attrs)};
- PHIDP_PREPARSED_DATA preparsed;
- WCHAR device_path[MAX_PATH];
- HIDP_CAPS caps;
- HANDLE device;
- HRESULT hr;
- TRACE( "type %x, flags %#x, instance %p, version %04x, index %d\n", type, flags, instance, version, index );
- hr = hid_joystick_device_open( index, device_path, &device, version,
&preparsed, &attrs, &caps, instance );
Does GetRawInputDeviceList() give us guarantees on device ordering? I think the relative ordering should be preserved (plus/minus the hotplugs, but IMO that's not an issue), but I haven't checked.
- if (hr != DI_OK) return hr;
- HidD_FreePreparsedData( preparsed );
- CloseHandle( device );
- if (instance->dwSize != sizeof(DIDEVICEINSTANCEW))
return S_FALSE;- if (version < 0x0800 && type != DIDEVTYPE_JOYSTICK)
return S_FALSE;- if (version >= 0x0800 && type != DI8DEVCLASS_ALL && type != DI8DEVCLASS_GAMECTRL)
return S_FALSE;- if (device_disabled_registry( "HID", TRUE ))
return DIERR_DEVICENOTREG;- TRACE( "Found device with usage %04x:%04x, type %x, instance %s %s, product %s %s, ffdriver %s\n",
instance->wUsagePage, instance->wUsage, instance->dwDevType, debugstr_guid( &instance->guidProduct ),debugstr_w( instance->tszProductName ), debugstr_guid( &instance->guidInstance ),debugstr_w( instance->tszInstanceName ), debugstr_guid( &instance->guidFFDriver ) );- return DI_OK;
+}
+static HRESULT hid_joystick_create_device( IDirectInputImpl *dinput, REFGUID guid, IDirectInputDevice8W **out ) +{
- DIDEVICEINSTANCEW instance = {sizeof(instance)};
- DWORD index, size = sizeof(struct hid_joystick);
- HIDD_ATTRIBUTES attrs = {sizeof(attrs)};
- struct hid_joystick *impl = NULL;
- PHIDP_PREPARSED_DATA preparsed;
- DIDATAFORMAT *format = NULL;
- WCHAR device_path[MAX_PATH];
- GUID tmp_guid = *guid;
- HIDP_CAPS caps;
- HANDLE device;
- HRESULT hr;
- TRACE( "dinput %p, guid %s, out %p\n", dinput, debugstr_guid( guid ), out );
- *out = NULL;
- tmp_guid.Data3 = hid_joystick_guid.Data3;
- if (!IsEqualGUID( &hid_joystick_guid, &tmp_guid )) return DIERR_DEVICENOTREG;
- index = guid->Data3;
It's undocumented but you can create device using the product guid, see 0a82d891fc0b ("dinput: Implement device creation using product GUID.")
- hr = hid_joystick_device_open( index, device_path, &device, dinput->dwVersion,
&preparsed, &attrs, &caps, &instance );
I find it unlikely that hotplug is going to be an issue for enumerating, but using only the idx for device creation may be problematic, especially that Enum -> Crate can be more separated in time.
How about using VID + PID + ordinal? This should also help with making the enumeration logic a bit more robust.
Otherwise looks good to me. Thanks for working on this, I think it's much needed :-)