[PATCH v2 1/5] dinput: Introduce new HID joystick backend.

[Arkadiusz Hiler]

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 :-)

-- 
Cheers,
Arek