[RFC PATCH] dinput: Introduce new HID joystick backend.
Sending this as RFC because I'm not completely sure how to proceed (and also because ideally it would depend on 206872 which is not merged yet). The problem is that this new backend will fail most dinput(8) tests until it's more properly implemented, although the testbot doesn't have HID devices to test with. It could also cause issues if the new backend is enumerated and somehow is selected by default for some reason. Either we don't really care, or the new backend could be conditionally enabled with a configure flag? dlls/dinput/Makefile.in | 3 +- dlls/dinput/dinput_main.c | 3 +- dlls/dinput/dinput_private.h | 1 + dlls/dinput/joystick_hid.c | 431 +++++++++++++++++++++++++++++++++++ dlls/dinput8/Makefile.in | 3 +- 5 files changed, 438 insertions(+), 3 deletions(-) create mode 100644 dlls/dinput/joystick_hid.c diff --git a/dlls/dinput/Makefile.in b/dlls/dinput/Makefile.in index a22c8a72180..f2347927ed6 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 +IMPORTS = dinput dxguid uuid comctl32 ole32 user32 advapi32 hid EXTRADEFS = -DDIRECTINPUT_VERSION=0x0700 EXTRALIBS = $(IOKIT_LIBS) $(FORCEFEEDBACK_LIBS) @@ -12,6 +12,7 @@ C_SRCS = \ dinput_main.c \ effect_linuxinput.c \ joystick.c \ + joystick_hid.c \ joystick_linux.c \ joystick_linuxinput.c \ joystick_osx.c \ diff --git a/dlls/dinput/dinput_main.c b/dlls/dinput/dinput_main.c index c7b932aca44..88ee1855675 100644 --- a/dlls/dinput/dinput_main.c +++ b/dlls/dinput/dinput_main.c @@ -80,7 +80,8 @@ static const struct dinput_device *dinput_devices[] = &keyboard_device, &joystick_linuxinput_device, &joystick_linux_device, - &joystick_osx_device + &joystick_osx_device, + &joystick_hid_device, }; HINSTANCE DINPUT_instance; diff --git a/dlls/dinput/dinput_private.h b/dlls/dinput/dinput_private.h index 7e0f56c68df..c11b64585d9 100644 --- a/dlls/dinput/dinput_private.h +++ b/dlls/dinput/dinput_private.h @@ -67,6 +67,7 @@ struct DevicePlayer { extern const struct dinput_device mouse_device DECLSPEC_HIDDEN; extern const struct dinput_device keyboard_device DECLSPEC_HIDDEN; +extern const struct dinput_device joystick_hid_device DECLSPEC_HIDDEN; extern const struct dinput_device joystick_linux_device DECLSPEC_HIDDEN; extern const struct dinput_device joystick_linuxinput_device DECLSPEC_HIDDEN; extern const struct dinput_device joystick_osx_device DECLSPEC_HIDDEN; diff --git a/dlls/dinput/joystick_hid.c b/dlls/dinput/joystick_hid.c new file mode 100644 index 00000000000..3e9d1dc69b9 --- /dev/null +++ b/dlls/dinput/joystick_hid.c @@ -0,0 +1,431 @@ +/* DirectInput HID Joystick device + * + * Copyright 2021 Rémi Bernon for CodeWeavers + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA + */ + +#include <assert.h> +#include <stdarg.h> +#include <string.h> + +#include "windef.h" +#include "winbase.h" +#include "winternl.h" +#include "winuser.h" +#include "winerror.h" +#include "winreg.h" + +#include "ddk/hidsdi.h" +#include "devguid.h" +#include "dinput.h" +#include "setupapi.h" + +#include "wine/debug.h" + +#include "dinput_private.h" +#include "device_private.h" +#include "joystick_private.h" + +#include "initguid.h" + +WINE_DEFAULT_DEBUG_CHANNEL(hidjoy); + +DEFINE_GUID( hid_joystick_guid, 0x9e573edb, 0x7734, 0x11d2, 0x8d, 0x4a, 0x23, 0x90, 0x3f, 0xb6, 0xbd, 0xf7 ); + +struct hid_joystick +{ + IDirectInputDeviceImpl base; + + HANDLE device; + PHIDP_PREPARSED_DATA preparsed; +}; + +static inline struct hid_joystick *impl_from_IDirectInputDevice8W( IDirectInputDevice8W *iface ) +{ + return CONTAINING_RECORD( CONTAINING_RECORD( iface, IDirectInputDeviceImpl, IDirectInputDevice8W_iface ), + struct hid_joystick, base ); +} + +static ULONG WINAPI hid_joystick_Release( IDirectInputDevice8W *iface ) +{ + struct hid_joystick *impl = impl_from_IDirectInputDevice8W( iface ); + struct hid_joystick tmp = *impl; + ULONG res; + + if (!(res = IDirectInputDevice2WImpl_Release( iface ))) + { + HidD_FreePreparsedData( tmp.preparsed ); + CloseHandle( tmp.device ); + } + + return res; +} + +static HRESULT WINAPI hid_joystick_GetCapabilities( IDirectInputDevice8W *iface, DIDEVCAPS *caps ) +{ + FIXME( "iface %p, caps %p stub!\n", iface, caps ); + + if (!caps) return E_POINTER; + + return E_NOTIMPL; +} + +static HRESULT WINAPI hid_joystick_EnumObjects( IDirectInputDevice8W *iface, + LPDIENUMDEVICEOBJECTSCALLBACKW callback, void *ref, DWORD flags ) +{ + FIXME( "iface %p, callback %p, ref %p, flags %#x stub!\n", iface, callback, ref, flags ); + + if (!callback) return DIERR_INVALIDPARAM; + + return E_NOTIMPL; +} + +static HRESULT WINAPI hid_joystick_GetProperty( IDirectInputDevice8W *iface, REFGUID guid, DIPROPHEADER *header ) +{ + FIXME( "iface %p, guid %s, header %p stub!\n", iface, debugstr_guid( guid ), header ); + + if (!header) return DIERR_INVALIDPARAM; + if (!IS_DIPROP( guid )) return DI_OK; + + switch (LOWORD( guid )) + { + default: + return IDirectInputDevice2WImpl_GetProperty( iface, guid, header ); + } +} + +static HRESULT WINAPI hid_joystick_SetProperty( IDirectInputDevice8W *iface, REFGUID guid, const DIPROPHEADER *header ) +{ + FIXME( "iface %p, guid %s, header %p stub!\n", iface, debugstr_guid( guid ), header ); + + if (!header) return DIERR_INVALIDPARAM; + if (!IS_DIPROP( guid )) return DI_OK; + + switch (LOWORD( guid )) + { + default: + return IDirectInputDevice2WImpl_SetProperty( iface, guid, header ); + } +} + +static HRESULT WINAPI hid_joystick_GetDeviceState( IDirectInputDevice8W *iface, DWORD len, void *ptr ) +{ + FIXME( "iface %p, len %u, ptr %p stub!\n", iface, len, ptr ); + + if (!ptr) return DIERR_INVALIDPARAM; + + return E_NOTIMPL; +} + +static HRESULT WINAPI hid_joystick_GetObjectInfo( IDirectInputDevice8W *iface, + DIDEVICEOBJECTINSTANCEW *instance, DWORD obj, DWORD how ) +{ + FIXME( "iface %p, instance %p, obj %#x, how %#x stub!\n", iface, instance, obj, how ); + + if (!instance) return E_POINTER; + if (instance->dwSize != sizeof(DIDEVICEOBJECTINSTANCE_DX3W) && instance->dwSize != sizeof(DIDEVICEOBJECTINSTANCEW)) + return DIERR_INVALIDPARAM; + + return E_NOTIMPL; +} + +static HRESULT WINAPI hid_joystick_GetDeviceInfo( IDirectInputDevice8W *iface, DIDEVICEINSTANCEW *instance ) +{ + FIXME( "iface %p, instance %p stub!\n", iface, instance ); + + if (!instance) return E_POINTER; + if (instance->dwSize != sizeof(DIDEVICEINSTANCE_DX3W) && instance->dwSize != sizeof(DIDEVICEINSTANCEW)) + return DIERR_INVALIDPARAM; + + return E_NOTIMPL; +} + +static HRESULT WINAPI hid_joystick_CreateEffect( IDirectInputDevice8W *iface, REFGUID rguid, + const DIEFFECT *effect, IDirectInputEffect **out, + IUnknown *outer ) +{ + FIXME( "iface %p, rguid %s, effect %p, out %p, outer %p stub!\n", iface, debugstr_guid( rguid ), + effect, out, outer ); + + if (!out) return E_POINTER; + if (!rguid || !effect) return DI_NOEFFECT; + + return E_NOTIMPL; +} + +static HRESULT WINAPI hid_joystick_EnumEffects( IDirectInputDevice8W *iface, LPDIENUMEFFECTSCALLBACKW callback, + void *ref, DWORD type ) +{ + FIXME( "iface %p, callback %p, ref %p, type %#x stub!\n", iface, callback, ref, type ); + + if (!callback) return DIERR_INVALIDPARAM; + + return E_NOTIMPL; +} + +static HRESULT WINAPI hid_joystick_GetEffectInfo( IDirectInputDevice8W *iface, DIEFFECTINFOW *info, REFGUID guid ) +{ + FIXME( "iface %p, info %p, guid %s stub!\n", iface, info, debugstr_guid( guid ) ); + + if (!info) return E_POINTER; + + return E_NOTIMPL; +} + +static HRESULT WINAPI hid_joystick_GetForceFeedbackState( IDirectInputDevice8W *iface, DWORD *out ) +{ + FIXME( "iface %p, out %p stub!\n", iface, out ); + + if (!out) return E_POINTER; + + return E_NOTIMPL; +} + +static HRESULT WINAPI hid_joystick_SendForceFeedbackCommand( IDirectInputDevice8W *iface, DWORD flags ) +{ + FIXME( "iface %p, flags %x stub!\n", iface, flags ); + return E_NOTIMPL; +} + +static HRESULT WINAPI hid_joystick_EnumCreatedEffectObjects( IDirectInputDevice8W *iface, + LPDIENUMCREATEDEFFECTOBJECTSCALLBACK callback, + void *ref, DWORD flags ) +{ + FIXME( "iface %p, callback %p, ref %p, flags %#x stub!\n", iface, callback, ref, flags ); + + if (!callback) return DIERR_INVALIDPARAM; + + return E_NOTIMPL; +} + +static HRESULT WINAPI hid_joystick_Poll( IDirectInputDevice8W *iface ) +{ + FIXME( "iface %p stub!\n", iface ); + return E_NOTIMPL; +} + +static HRESULT WINAPI hid_joystick_BuildActionMap( IDirectInputDevice8W *iface, DIACTIONFORMATW *format, + const WCHAR *username, DWORD flags ) +{ + FIXME( "iface %p, format %p, username %s, flags %#x stub!\n", iface, format, debugstr_w(username), flags ); + + if (!format) return DIERR_INVALIDPARAM; + + return E_NOTIMPL; +} + +static HRESULT WINAPI hid_joystick_SetActionMap( IDirectInputDevice8W *iface, DIACTIONFORMATW *format, + const WCHAR *username, DWORD flags ) +{ + struct hid_joystick *impl = impl_from_IDirectInputDevice8W( iface ); + + TRACE( "iface %p, format %p, username %s, flags %#x.\n", iface, format, debugstr_w(username), flags ); + + if (!format) return DIERR_INVALIDPARAM; + + return _set_action_map( iface, format, username, flags, impl->base.data_format.wine_df ); +} + +static const IDirectInputDevice8WVtbl hid_joystick_vtbl = +{ + /*** IUnknown methods ***/ + IDirectInputDevice2WImpl_QueryInterface, + IDirectInputDevice2WImpl_AddRef, + hid_joystick_Release, + /*** IDirectInputDevice methods ***/ + hid_joystick_GetCapabilities, + hid_joystick_EnumObjects, + hid_joystick_GetProperty, + hid_joystick_SetProperty, + IDirectInputDevice2WImpl_Acquire, + IDirectInputDevice2WImpl_Unacquire, + hid_joystick_GetDeviceState, + IDirectInputDevice2WImpl_GetDeviceData, + IDirectInputDevice2WImpl_SetDataFormat, + IDirectInputDevice2WImpl_SetEventNotification, + IDirectInputDevice2WImpl_SetCooperativeLevel, + hid_joystick_GetObjectInfo, + hid_joystick_GetDeviceInfo, + IDirectInputDevice2WImpl_RunControlPanel, + IDirectInputDevice2WImpl_Initialize, + /*** IDirectInputDevice2 methods ***/ + hid_joystick_CreateEffect, + hid_joystick_EnumEffects, + hid_joystick_GetEffectInfo, + hid_joystick_GetForceFeedbackState, + hid_joystick_SendForceFeedbackCommand, + hid_joystick_EnumCreatedEffectObjects, + IDirectInputDevice2WImpl_Escape, + hid_joystick_Poll, + IDirectInputDevice2WImpl_SendDeviceData, + /*** IDirectInputDevice7 methods ***/ + IDirectInputDevice7WImpl_EnumEffectsInFile, + IDirectInputDevice7WImpl_WriteEffectToFile, + /*** IDirectInputDevice8 methods ***/ + hid_joystick_BuildActionMap, + hid_joystick_SetActionMap, + IDirectInputDevice8WImpl_GetImageInfo, +}; + +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; + } + + 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 ) +{ + PHIDP_PREPARSED_DATA preparsed; + HIDD_ATTRIBUTES attrs = {sizeof(attrs)}; + HIDP_CAPS caps; + HRESULT hr; + HANDLE device; + WCHAR device_path[MAX_PATH]; + + TRACE( "type %x, flags %#x, instance %p, version %04x, index %d\n", type, flags, instance, version, index ); + + if (instance->dwSize != sizeof(DIDEVICEINSTANCEW)) return DIERR_INVALIDPARAM; + if (version < 0x0800 && type != DIDEVTYPE_JOYSTICK) return DIERR_INVALIDPARAM; + if (version >= 0x0800 && type != DI8DEVCLASS_ALL && type != DI8DEVCLASS_GAMECTRL) + return DIERR_INVALIDPARAM; + + if (FAILED(hr = hid_joystick_device_open( index, device_path, &device, version, + &preparsed, &attrs, &caps, instance ))) + return hr; + if (hr != DI_OK) return hr; + + HidD_FreePreparsedData( preparsed ); + CloseHandle( device ); + + 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 ) +{ + struct hid_joystick *impl = NULL; + PHIDP_PREPARSED_DATA preparsed; + DIDEVICEINSTANCEW instance = {sizeof(instance)}; + HIDD_ATTRIBUTES attrs = {sizeof(attrs)}; + DIDATAFORMAT *format = NULL; + HIDP_CAPS caps; + HRESULT hr; + HANDLE device; + WCHAR device_path[MAX_PATH]; + DWORD index, size = sizeof(*impl); + GUID tmp_guid = *guid; + + 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; + + if (FAILED(hr = hid_joystick_device_open( index, device_path, &device, dinput->dwVersion, + &preparsed, &attrs, &caps, &instance ))) + return hr; + if (hr != DI_OK) return hr; + + if (FAILED(hr = direct_input_device_alloc( size, &hid_joystick_vtbl, guid, dinput, (void **)&impl ))) + goto failed; + + impl->base.crit.DebugInfo->Spare[0] = (DWORD_PTR)(__FILE__ ": hid_joystick.base.crit"); + impl->base.dwCoopLevel = DISCL_NONEXCLUSIVE | DISCL_BACKGROUND; + + impl->device = device; + impl->preparsed = preparsed; + + if (!(format = HeapAlloc( GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(*format) ))) goto failed; + impl->base.data_format.wine_df = format; + + TRACE( "Created %p\n", impl ); + + *out = &impl->base.IDirectInputDevice8W_iface; + return DI_OK; + +failed: + HeapFree( GetProcessHeap(), 0, format ); + HeapFree( GetProcessHeap(), 0, impl ); + HidD_FreePreparsedData( preparsed ); + CloseHandle( device ); + return hr; +} + +const struct dinput_device joystick_hid_device = +{ + "Wine HID joystick driver", + hid_joystick_enum_device, + hid_joystick_create_device, +}; diff --git a/dlls/dinput8/Makefile.in b/dlls/dinput8/Makefile.in index 35b3bfb75f5..71eecd770b3 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 +IMPORTS = dinput8 dxguid uuid comctl32 ole32 user32 advapi32 hid EXTRADEFS = -DDIRECTINPUT_VERSION=0x0800 EXTRALIBS = $(IOKIT_LIBS) $(FORCEFEEDBACK_LIBS) PARENTSRC = ../dinput @@ -13,6 +13,7 @@ C_SRCS = \ dinput_main.c \ effect_linuxinput.c \ joystick.c \ + joystick_hid.c \ joystick_linux.c \ joystick_linuxinput.c \ joystick_osx.c \ -- 2.31.0
On 6/1/21 4:09 AM, Rémi Bernon wrote:
Sending this as RFC because I'm not completely sure how to proceed (and also because ideally it would depend on 206872 which is not merged yet).
The problem is that this new backend will fail most dinput(8) tests until it's more properly implemented, although the testbot doesn't have HID devices to test with.
It could also cause issues if the new backend is enumerated and somehow is selected by default for some reason.
Either we don't really care, or the new backend could be conditionally enabled with a configure flag?
For whatever it's worth, this strikes me as a good idea. Probably what should be done is to deprioritize or even explicitly disable this backend in the registry until it's ready to work, at which point we can make it the default, keeping the old backends around until all the bugs get ironed out. I don't know whether it's worth completely disabling it just to make the tests pass for the TestBot and Alexandre; I guess I'll leave that one to them. Of course, introducing a switch like that is a good way to ensure that concerned users always turn it off, and that the new backend never really gets tested, but it would be unfair to offer no way to revert to the old behaviour... (Side note: looking at this patch shows me just how ugly the IDirectInputDevice8WVtbl implementation is. If it's really worth sharing all that code, I'd advocate for using a struct dinput_device_ops instead...)
On 6/17/21 6:40 PM, Zebediah Figura (she/her) wrote:
On 6/1/21 4:09 AM, Rémi Bernon wrote:
Sending this as RFC because I'm not completely sure how to proceed (and also because ideally it would depend on 206872 which is not merged yet).
The problem is that this new backend will fail most dinput(8) tests until it's more properly implemented, although the testbot doesn't have HID devices to test with.
It could also cause issues if the new backend is enumerated and somehow is selected by default for some reason.
Either we don't really care, or the new backend could be conditionally enabled with a configure flag?
For whatever it's worth, this strikes me as a good idea.
Probably what should be done is to deprioritize or even explicitly disable this backend in the registry until it's ready to work, at which point we can make it the default, keeping the old backends around until all the bugs get ironed out. I don't know whether it's worth completely disabling it just to make the tests pass for the TestBot and Alexandre; I guess I'll leave that one to them.
I don't think there's a registry entry for that? The backends are just enumerated in order. It could be added I guess but I'm not even sure it's worth it. I think the way it usually works is that each backend lists their device, creating physically duplicated devices but with different features (like the linux and linuxinput joysticks).
Of course, introducing a switch like that is a good way to ensure that concerned users always turn it off, and that the new backend never really gets tested, but it would be unfair to offer no way to revert to the old behaviour...
I don't even think the TestBot will notice, as it doesn't have any joysticks.
(Side note: looking at this patch shows me just how ugly the IDirectInputDevice8WVtbl implementation is. If it's really worth sharing all that code, I'd advocate for using a struct dinput_device_ops instead...)
Yeah although that would require cleaning up all the other joystick backend code, which I understand will ultimately be removed so it's a bit of a waste of time. Finishing the cleanup after the HID backend is complete is probably simpler. -- Rémi Bernon <rbernon(a)codeweavers.com>
On 6/17/21 11:47 AM, Rémi Bernon wrote:
On 6/17/21 6:40 PM, Zebediah Figura (she/her) wrote:
On 6/1/21 4:09 AM, Rémi Bernon wrote:
Sending this as RFC because I'm not completely sure how to proceed (and also because ideally it would depend on 206872 which is not merged yet).
The problem is that this new backend will fail most dinput(8) tests until it's more properly implemented, although the testbot doesn't have HID devices to test with.
It could also cause issues if the new backend is enumerated and somehow is selected by default for some reason.
Either we don't really care, or the new backend could be conditionally enabled with a configure flag?
For whatever it's worth, this strikes me as a good idea.
Probably what should be done is to deprioritize or even explicitly disable this backend in the registry until it's ready to work, at which point we can make it the default, keeping the old backends around until all the bugs get ironed out. I don't know whether it's worth completely disabling it just to make the tests pass for the TestBot and Alexandre; I guess I'll leave that one to them.
I don't think there's a registry entry for that? The backends are just enumerated in order. It could be added I guess but I'm not even sure it's worth it. I think the way it usually works is that each backend lists their device, creating physically duplicated devices but with different features (like the linux and linuxinput joysticks).
Right, we'd have to add one.
Of course, introducing a switch like that is a good way to ensure that concerned users always turn it off, and that the new backend never really gets tested, but it would be unfair to offer no way to revert to the old behaviour...
I don't even think the TestBot will notice, as it doesn't have any joysticks.
Well, I'm thinking of users here. I think it's inevitable that the new backend will expose a bunch of bugs, in various parts of the HID stack, and we'll want a way to confirm that a bug is due to the new HID backend, and maybe also a way to let users work around such bugs. Ideally once the HID dinput backend is fully implemented it should be made the default (for applications which e.g. default to the first joystick available, or for those which don't even offer the user a choice). Otherwise it won't get tested at all.
(Side note: looking at this patch shows me just how ugly the IDirectInputDevice8WVtbl implementation is. If it's really worth sharing all that code, I'd advocate for using a struct dinput_device_ops instead...)
Yeah although that would require cleaning up all the other joystick backend code, which I understand will ultimately be removed so it's a bit of a waste of time. Finishing the cleanup after the HID backend is complete is probably simpler.
Yeah, agreed.
participants (2)
-
Rémi Bernon -
Zebediah Figura (she/her)