This adds a new joystick backend, implemented on top of HID and without any host dependencies. This will be progressively implementated, and it's not going to be usable until at least a few more patches.
Because of that, and because it may also introduce regressions compared to the existing backends, it is disabled by default and is optionally enabled using the following global registry key:
[HKCU\Software\Wine\DirectInput\Joysticks] "HID"="enabled"
Or using the corresponding AppDefaults registry key:
[HKCU\Software\Wine\AppDefaults\<app.exe>\DirectInput\Joysticks] "HID"="enabled"
This setting will be removed later, when it becomes usable enough, to use the individual device disable mechanism available in joy.cpl.
Signed-off-by: Rémi Bernon rbernon@codeweavers.com ---
v3: Use setupapi directly, as user32 api usage was confusing. This also now matches device on either their index, HID handle or vid_pid, depending on what's been provided.
The instance GUID now includes the HID handle, which will be more stable than the device index in setupapi device list.
Matching the vid_pid assumes that setupapi device path is upper case, which I think is the case. We could make sure otherwise but I don't think we have to and it's not very convenient without wcsupr.
dlls/dinput/Makefile.in | 3 +- dlls/dinput/dinput_main.c | 3 +- dlls/dinput/dinput_private.h | 1 + dlls/dinput/joystick.c | 17 +- dlls/dinput/joystick_hid.c | 491 ++++++++++++++++++++++++++++++ dlls/dinput/joystick_linux.c | 2 +- dlls/dinput/joystick_linuxinput.c | 2 +- dlls/dinput/joystick_private.h | 2 +- dlls/dinput8/Makefile.in | 3 +- 9 files changed, 513 insertions(+), 11 deletions(-) create mode 100644 dlls/dinput/joystick_hid.c
diff --git a/dlls/dinput/Makefile.in b/dlls/dinput/Makefile.in index a22c8a72180..29d3e8ece65 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 setupapi 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.c b/dlls/dinput/joystick.c index 8ea7850621c..60153d0d0f3 100644 --- a/dlls/dinput/joystick.c +++ b/dlls/dinput/joystick.c @@ -271,13 +271,13 @@ void dump_DIEFFECT(LPCDIEFFECT eff, REFGUID guid, DWORD dwFlags) } }
-BOOL device_disabled_registry(const char* name) +BOOL device_disabled_registry(const char* name, BOOL disable) { static const char disabled_str[] = "disabled"; + static const char enabled_str[] = "enabled"; static const char joystick_key[] = "Joysticks"; char buffer[MAX_PATH]; HKEY hkey, appkey, temp; - BOOL do_disable = FALSE;
get_app_key(&hkey, &appkey);
@@ -297,16 +297,23 @@ BOOL device_disabled_registry(const char* name)
/* Look for the "controllername"="disabled" key */ if (!get_config_key(hkey, appkey, name, buffer, sizeof(buffer))) - if (!strcmp(disabled_str, buffer)) + { + if (!disable && !strcmp(disabled_str, buffer)) { TRACE("Disabling joystick '%s' based on registry key.\n", name); - do_disable = TRUE; + disable = TRUE; + } + else if (disable && !strcmp(enabled_str, buffer)) + { + TRACE("Enabling joystick '%s' based on registry key.\n", name); + disable = FALSE; } + }
if (appkey) RegCloseKey(appkey); if (hkey) RegCloseKey(hkey);
- return do_disable; + return disable; }
BOOL is_xinput_device(const DIDEVCAPS *devcaps, WORD vid, WORD pid) diff --git a/dlls/dinput/joystick_hid.c b/dlls/dinput/joystick_hid.c new file mode 100644 index 00000000000..f29c806a51d --- /dev/null +++ b/dlls/dinput/joystick_hid.c @@ -0,0 +1,491 @@ +/* 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 "setupapi.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" +#include "devpkey.h" + +WINE_DEFAULT_DEBUG_CHANNEL(dinput); + +DEFINE_GUID( hid_joystick_guid, 0x9e573edb, 0x7734, 0x11d2, 0x8d, 0x4a, 0x23, 0x90, 0x3f, 0xb6, 0xbd, 0xf7 ); +DEFINE_DEVPROPKEY( DEVPROPKEY_HID_HANDLE, 0xbc62e415, 0xf4fe, 0x405c, 0x8e, 0xda, 0x63, 0x6f, 0xb5, 0x9f, 0x08, 0x98, 2 ); + +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_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; +} + +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_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, + hid_joystick_Acquire, + hid_joystick_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, + IDirectInputDevice2WImpl_Poll, + IDirectInputDevice2WImpl_SendDeviceData, + /*** IDirectInputDevice7 methods ***/ + IDirectInputDevice7WImpl_EnumEffectsInFile, + IDirectInputDevice7WImpl_WriteEffectToFile, + /*** IDirectInputDevice8 methods ***/ + hid_joystick_BuildActionMap, + hid_joystick_SetActionMap, + IDirectInputDevice8WImpl_GetImageInfo, +}; + +static BOOL hid_joystick_device_try_open( const WCHAR *path, HANDLE *device, PHIDP_PREPARSED_DATA *preparsed, + HIDD_ATTRIBUTES *attrs, HIDP_CAPS *caps, DIDEVICEINSTANCEW *instance ) +{ + PHIDP_PREPARSED_DATA preparsed_data = NULL; + HANDLE device_file; + + device_file = CreateFileW( path, GENERIC_READ | GENERIC_WRITE, FILE_SHARE_READ | FILE_SHARE_WRITE, + NULL, OPEN_EXISTING, 0, 0 ); + if (device_file == INVALID_HANDLE_VALUE) return FALSE; + + if (!HidD_GetPreparsedData( device_file, &preparsed_data )) goto failed; + if (!HidD_GetAttributes( device_file, attrs )) goto failed; + if (HidP_GetCaps( preparsed_data, 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; + + if (!HidD_GetProductString( device_file, instance->tszInstanceName, MAX_PATH )) goto failed; + if (!HidD_GetProductString( device_file, instance->tszProductName, MAX_PATH )) goto failed; + + *device = device_file; + *preparsed = preparsed_data; + return TRUE; + +failed: + CloseHandle( device_file ); + HidD_FreePreparsedData( preparsed_data ); + return FALSE; +} + +static HRESULT hid_joystick_device_open( DWORD index, UINT32 handle, UINT32 vid_pid, WCHAR *device_path, + HANDLE *device, DWORD version, PHIDP_PREPARSED_DATA *preparsed, + HIDD_ATTRIBUTES *attrs, HIDP_CAPS *caps, DIDEVICEINSTANCEW *instance ) +{ + static const WCHAR vid_pid_fmt_w[] = {'V','I','D','_','%','0','4','X','&','P','I','D','_','%','0','4','X',0}; + 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 = {sizeof(iface)}; + SP_DEVINFO_DATA devinfo = {sizeof(devinfo)}; + WCHAR vid_pid_w[18] = {0}; + UINT32 i = 0, hid_handle; + HDEVINFO set; + DWORD type; + GUID hid; + + TRACE( "index %u, handle %u, vid_pid %d\n", index, handle, vid_pid ); + + HidD_GetHidGuid( &hid ); + snprintfW( vid_pid_w, ARRAY_SIZE(vid_pid_w), vid_pid_fmt_w, vid_pid & 0xffff, vid_pid >> 16 ); + + set = SetupDiGetClassDevsW( &hid, NULL, NULL, DIGCF_DEVICEINTERFACE | DIGCF_PRESENT ); + if (set == INVALID_HANDLE_VALUE) return DIERR_DEVICENOTREG; + + *device = NULL; + *preparsed = NULL; + while (SetupDiEnumDeviceInterfaces( set, NULL, &hid, i++, &iface )) + { + detail->cbSize = sizeof(SP_DEVICE_INTERFACE_DETAIL_DATA_W); + if (!SetupDiGetDeviceInterfaceDetailW( set, &iface, detail, sizeof(buffer), NULL, &devinfo )) + continue; + if (!SetupDiGetDevicePropertyW( set, &devinfo, &DEVPROPKEY_HID_HANDLE, &type, + (BYTE *)&hid_handle, sizeof(hid_handle), NULL, 0 ) || + type != DEVPROP_TYPE_UINT32) + continue; + if (!hid_joystick_device_try_open( detail->DevicePath, device, preparsed, attrs, caps, instance )) + continue; + + if (!handle && !vid_pid && !index--) break; + if (handle && handle == hid_handle) break; + if (vid_pid && strstrW( detail->DevicePath, vid_pid_w )) break; + + CloseHandle( *device ); + HidD_FreePreparsedData( *preparsed ); + *device = NULL; + *preparsed = NULL; + } + + SetupDiDestroyDeviceInfoList( set ); + if (!*device || !*preparsed) return DIERR_DEVICENOTREG; + + lstrcpynW( device_path, detail->DevicePath, MAX_PATH ); + instance->guidInstance = hid_joystick_guid; + instance->guidInstance.Data3 = hid_handle; + 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; + return DI_OK; +} + +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, 0, 0, device_path, &device, version, + &preparsed, &attrs, &caps, instance ); + 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 %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 DI_OK; +} + +static HRESULT hid_joystick_create_device( IDirectInputImpl *dinput, REFGUID guid, IDirectInputDevice8W **out ) +{ + DIDEVICEINSTANCEW instance = {sizeof(instance)}; + GUID inst_guid = *guid, prod_guid = *guid; + DWORD size = sizeof(struct hid_joystick); + HIDD_ATTRIBUTES attrs = {sizeof(attrs)}; + struct hid_joystick *impl = NULL; + PHIDP_PREPARSED_DATA preparsed; + UINT32 handle = 0, vid_pid = 0; + DIDATAFORMAT *format = NULL; + WCHAR device_path[MAX_PATH]; + HIDP_CAPS caps; + HANDLE device; + HRESULT hr; + + TRACE( "dinput %p, guid %s, out %p\n", dinput, debugstr_guid( guid ), out ); + + *out = NULL; + + inst_guid.Data3 = hid_joystick_guid.Data3; + prod_guid.Data1 = DInput_PIDVID_Product_GUID.Data1; + if (IsEqualGUID( &hid_joystick_guid, &inst_guid )) handle = guid->Data3; + else if (IsEqualGUID( &DInput_PIDVID_Product_GUID, &prod_guid )) vid_pid = guid->Data1; + else return DIERR_DEVICENOTREG; + + hr = hid_joystick_device_open( 0, handle, vid_pid, device_path, &device, dinput->dwVersion, + &preparsed, &attrs, &caps, &instance ); + if (hr != DI_OK) return hr; + + hr = direct_input_device_alloc( size, &hid_joystick_vtbl, guid, dinput, (void **)&impl ); + if (FAILED(hr)) 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/dinput/joystick_linux.c b/dlls/dinput/joystick_linux.c index 3215978c995..312ef3d9c42 100644 --- a/dlls/dinput/joystick_linux.c +++ b/dlls/dinput/joystick_linux.c @@ -175,7 +175,7 @@ static INT find_joystick_devices(void) /* Append driver name */ strcat(joydev.name, JOYDEVDRIVER);
- if (device_disabled_registry(joydev.name)) { + if (device_disabled_registry(joydev.name, FALSE)) { close(fd); continue; } diff --git a/dlls/dinput/joystick_linuxinput.c b/dlls/dinput/joystick_linuxinput.c index 2b970271ec3..558c8cc19e5 100644 --- a/dlls/dinput/joystick_linuxinput.c +++ b/dlls/dinput/joystick_linuxinput.c @@ -264,7 +264,7 @@ static void find_joydevs(void) else joydev.name = joydev.device;
- if (device_disabled_registry(joydev.name)) { + if (device_disabled_registry(joydev.name, FALSE)) { close(fd); HeapFree(GetProcessHeap(), 0, joydev.name); if (joydev.name != joydev.device) diff --git a/dlls/dinput/joystick_private.h b/dlls/dinput/joystick_private.h index 874bf3e69a7..9cc30605234 100644 --- a/dlls/dinput/joystick_private.h +++ b/dlls/dinput/joystick_private.h @@ -57,7 +57,7 @@ HRESULT setup_dinput_options(JoystickGenericImpl *This, const int *default_axis_
DWORD joystick_map_pov(const POINTL *p) DECLSPEC_HIDDEN;
-BOOL device_disabled_registry(const char* name) DECLSPEC_HIDDEN; +BOOL device_disabled_registry(const char* name, BOOL disable) DECLSPEC_HIDDEN;
ULONG WINAPI JoystickWGenericImpl_Release(LPDIRECTINPUTDEVICE8W iface);
diff --git a/dlls/dinput8/Makefile.in b/dlls/dinput8/Makefile.in index 35b3bfb75f5..5032c7689bc 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 setupapi 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 \
On Wed, Jun 30, 2021 at 10:54:19AM +0200, Rémi Bernon wrote:
This adds a new joystick backend, implemented on top of HID and without any host dependencies. This will be progressively implementated, and it's not going to be usable until at least a few more patches.
Because of that, and because it may also introduce regressions compared to the existing backends, it is disabled by default and is optionally enabled using the following global registry key:
[HKCU\Software\Wine\DirectInput\Joysticks] "HID"="enabled"
Or using the corresponding AppDefaults registry key:
[HKCU\Software\Wine\AppDefaults\<app.exe>\DirectInput\Joysticks] "HID"="enabled"
This setting will be removed later, when it becomes usable enough, to use the individual device disable mechanism available in joy.cpl.
Signed-off-by: Rémi Bernon rbernon@codeweavers.com
v3: Use setupapi directly, as user32 api usage was confusing. This also now matches device on either their index, HID handle or vid_pid, depending on what's been provided.
The instance GUID now includes the HID handle, which will be more stable than the device index in setupapi device list. Matching the vid_pid assumes that setupapi device path is upper case, which I think is the case. We could make sure otherwise but I don't think we have to and it's not very convenient without wcsupr.
Yes, setupapi seems to be always uppercasing thing.
+static HRESULT hid_joystick_device_open( DWORD index, UINT32 handle, UINT32 vid_pid, WCHAR *device_path,
HANDLE *device, DWORD version, PHIDP_PREPARSED_DATA *preparsed,HIDD_ATTRIBUTES *attrs, HIDP_CAPS *caps, DIDEVICEINSTANCEW *instance )+{
- static const WCHAR vid_pid_fmt_w[] = {'V','I','D','_','%','0','4','X','&','P','I','D','_','%','0','4','X',0};
- 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 = {sizeof(iface)};
- SP_DEVINFO_DATA devinfo = {sizeof(devinfo)};
- WCHAR vid_pid_w[18] = {0};
- UINT32 i = 0, hid_handle;
- HDEVINFO set;
- DWORD type;
- GUID hid;
- TRACE( "index %u, handle %u, vid_pid %d\n", index, handle, vid_pid );
- HidD_GetHidGuid( &hid );
- snprintfW( vid_pid_w, ARRAY_SIZE(vid_pid_w), vid_pid_fmt_w, vid_pid & 0xffff, vid_pid >> 16 );
- set = SetupDiGetClassDevsW( &hid, NULL, NULL, DIGCF_DEVICEINTERFACE | DIGCF_PRESENT );
- if (set == INVALID_HANDLE_VALUE) return DIERR_DEVICENOTREG;
- *device = NULL;
- *preparsed = NULL;
- while (SetupDiEnumDeviceInterfaces( set, NULL, &hid, i++, &iface ))
- {
detail->cbSize = sizeof(SP_DEVICE_INTERFACE_DETAIL_DATA_W);if (!SetupDiGetDeviceInterfaceDetailW( set, &iface, detail, sizeof(buffer), NULL, &devinfo ))continue;
This should never fail because MAX_PATH, so how about a FIXME / WARN in addition to that continue?
if (!SetupDiGetDevicePropertyW( set, &devinfo, &DEVPROPKEY_HID_HANDLE, &type,(BYTE *)&hid_handle, sizeof(hid_handle), NULL, 0 ) ||type != DEVPROP_TYPE_UINT32)continue;
I was thinking about sticking to RawInput enumeration and using the handles that you get from there instead of the indexes. This way you would need to do GetRawInputDeviceInfo() only in hid_joystick_enum_device() or maybe another helper that would try to map vidpid from product GUID to a handle.
With that you would be able to make hid_joystick_device_open() take the RawInput handle as the only parameter identifying the device.
This would also work on Windows (after the PE conversion), but I don't think it's our goal, so I believe that setupapi may be a better choice here.
if (!hid_joystick_device_try_open( detail->DevicePath, device, preparsed, attrs, caps, instance ))continue;if (!handle && !vid_pid && !index--) break;if (handle && handle == hid_handle) break;if (vid_pid && strstrW( detail->DevicePath, vid_pid_w )) break;
You have the device handle here, so why not just use HidD_GetAttibutes()?
Another option would be to use GetRawInputDeviceInfo() with the hid_handle.
CloseHandle( *device );HidD_FreePreparsedData( *preparsed );*device = NULL;*preparsed = NULL;- }
- SetupDiDestroyDeviceInfoList( set );
- if (!*device || !*preparsed) return DIERR_DEVICENOTREG;
- lstrcpynW( device_path, detail->DevicePath, MAX_PATH );
- instance->guidInstance = hid_joystick_guid;
- instance->guidInstance.Data3 = hid_handle;
- 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;
- return DI_OK;
+}
+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, 0, 0, device_path, &device, version,
&preparsed, &attrs, &caps, instance );
hid_joystick_device_open() is getting a bit unwieldy with all the possible parameter combinations. I don't think it needs the enumeration / vidpid lookup logic.
IMO it would read better if hid_joystick_device_open() would take the path or RI handle and enum logic would be moved either here or to a helper.
- 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 %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 DI_OK;
+}
+static HRESULT hid_joystick_create_device( IDirectInputImpl *dinput, REFGUID guid, IDirectInputDevice8W **out ) +{
- DIDEVICEINSTANCEW instance = {sizeof(instance)};
- GUID inst_guid = *guid, prod_guid = *guid;
- DWORD size = sizeof(struct hid_joystick);
- HIDD_ATTRIBUTES attrs = {sizeof(attrs)};
- struct hid_joystick *impl = NULL;
- PHIDP_PREPARSED_DATA preparsed;
- UINT32 handle = 0, vid_pid = 0;
- DIDATAFORMAT *format = NULL;
- WCHAR device_path[MAX_PATH];
- HIDP_CAPS caps;
- HANDLE device;
- HRESULT hr;
- TRACE( "dinput %p, guid %s, out %p\n", dinput, debugstr_guid( guid ), out );
- *out = NULL;
- inst_guid.Data3 = hid_joystick_guid.Data3;
- prod_guid.Data1 = DInput_PIDVID_Product_GUID.Data1;
- if (IsEqualGUID( &hid_joystick_guid, &inst_guid )) handle = guid->Data3;
Data3 is only a short. I think we should use 64 bits for our magic and another 64 bits for the RawInput handle.
Or 32 if we go with our internal knowledge about how those handles are implemented in Wine.
On 6/30/21 1:08 PM, Arkadiusz Hiler wrote:
On Wed, Jun 30, 2021 at 10:54:19AM +0200, Rémi Bernon wrote:
This adds a new joystick backend, implemented on top of HID and without any host dependencies. This will be progressively implementated, and it's not going to be usable until at least a few more patches.
Because of that, and because it may also introduce regressions compared to the existing backends, it is disabled by default and is optionally enabled using the following global registry key:
[HKCU\Software\Wine\DirectInput\Joysticks] "HID"="enabled"
Or using the corresponding AppDefaults registry key:
[HKCU\Software\Wine\AppDefaults\<app.exe>\DirectInput\Joysticks] "HID"="enabled"
This setting will be removed later, when it becomes usable enough, to use the individual device disable mechanism available in joy.cpl.
Signed-off-by: Rémi Bernon rbernon@codeweavers.com
v3: Use setupapi directly, as user32 api usage was confusing. This also now matches device on either their index, HID handle or vid_pid, depending on what's been provided.
The instance GUID now includes the HID handle, which will be more stable than the device index in setupapi device list. Matching the vid_pid assumes that setupapi device path is upper case, which I think is the case. We could make sure otherwise but I don't think we have to and it's not very convenient without wcsupr.Yes, setupapi seems to be always uppercasing thing.
+static HRESULT hid_joystick_device_open( DWORD index, UINT32 handle, UINT32 vid_pid, WCHAR *device_path,
HANDLE *device, DWORD version, PHIDP_PREPARSED_DATA *preparsed,HIDD_ATTRIBUTES *attrs, HIDP_CAPS *caps, DIDEVICEINSTANCEW *instance )+{
- static const WCHAR vid_pid_fmt_w[] = {'V','I','D','_','%','0','4','X','&','P','I','D','_','%','0','4','X',0};
- 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 = {sizeof(iface)};
- SP_DEVINFO_DATA devinfo = {sizeof(devinfo)};
- WCHAR vid_pid_w[18] = {0};
- UINT32 i = 0, hid_handle;
- HDEVINFO set;
- DWORD type;
- GUID hid;
- TRACE( "index %u, handle %u, vid_pid %d\n", index, handle, vid_pid );
- HidD_GetHidGuid( &hid );
- snprintfW( vid_pid_w, ARRAY_SIZE(vid_pid_w), vid_pid_fmt_w, vid_pid & 0xffff, vid_pid >> 16 );
- set = SetupDiGetClassDevsW( &hid, NULL, NULL, DIGCF_DEVICEINTERFACE | DIGCF_PRESENT );
- if (set == INVALID_HANDLE_VALUE) return DIERR_DEVICENOTREG;
- *device = NULL;
- *preparsed = NULL;
- while (SetupDiEnumDeviceInterfaces( set, NULL, &hid, i++, &iface ))
- {
detail->cbSize = sizeof(SP_DEVICE_INTERFACE_DETAIL_DATA_W);if (!SetupDiGetDeviceInterfaceDetailW( set, &iface, detail, sizeof(buffer), NULL, &devinfo ))continue;This should never fail because MAX_PATH, so how about a FIXME / WARN in addition to that continue?
Well I'm not sure if it's even worth it if it's not even supposed to fail. I also considered adding TRACEs to the enumeration, to be able to more easily see which devices were skipped, but this quickly becomes verbose.
if (!SetupDiGetDevicePropertyW( set, &devinfo, &DEVPROPKEY_HID_HANDLE, &type,(BYTE *)&hid_handle, sizeof(hid_handle), NULL, 0 ) ||type != DEVPROP_TYPE_UINT32)continue;I was thinking about sticking to RawInput enumeration and using the handles that you get from there instead of the indexes. This way you would need to do GetRawInputDeviceInfo() only in hid_joystick_enum_device() or maybe another helper that would try to map vidpid from product GUID to a handle.
With that you would be able to make hid_joystick_device_open() take the RawInput handle as the only parameter identifying the device.
This would also work on Windows (after the PE conversion), but I don't think it's our goal, so I believe that setupapi may be a better choice here.
Yeah it's actually inconvenient that it doesn't work anymore on Windows (compared to the previous version), I was trying the PE-converted dinput on Windows too and only realized late that it doesn't work anymore.
It's probably not too much of an issue, but if we're interested in making it work there we could perhaps use "i" as a hid_handle fallback? Assuming that all HID devices will have a handle on Wine it should not have any impact there.
I don't have any better idea right now, and using setupapi is actually an improvement IMHO. In any case it can probably be done as a separate later change, after the conversion, as there's the critical section debug field usage which is also incompatible.
if (!hid_joystick_device_try_open( detail->DevicePath, device, preparsed, attrs, caps, instance ))continue;if (!handle && !vid_pid && !index--) break;if (handle && handle == hid_handle) break;if (vid_pid && strstrW( detail->DevicePath, vid_pid_w )) break;You have the device handle here, so why not just use HidD_GetAttibutes()?
Another option would be to use GetRawInputDeviceInfo() with the hid_handle.
Indeed, much better idea to use the attrs, I moved the code around a few times after adding the vid_pid match and didn't reconsider that logic.
CloseHandle( *device );HidD_FreePreparsedData( *preparsed );*device = NULL;*preparsed = NULL;- }
- SetupDiDestroyDeviceInfoList( set );
- if (!*device || !*preparsed) return DIERR_DEVICENOTREG;
- lstrcpynW( device_path, detail->DevicePath, MAX_PATH );
- instance->guidInstance = hid_joystick_guid;
- instance->guidInstance.Data3 = hid_handle;
- 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;
- return DI_OK;
+}
+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, 0, 0, device_path, &device, version,
&preparsed, &attrs, &caps, instance );hid_joystick_device_open() is getting a bit unwieldy with all the possible parameter combinations. I don't think it needs the enumeration / vidpid lookup logic.
IMO it would read better if hid_joystick_device_open() would take the path or RI handle and enum logic would be moved either here or to a helper.
I don't think we need yet another device list cache, and I would rather avoid it if possible. It's really just two call sites.
Instead of the two additional params we could use the DIDEVICEINSTANCEW parameter as an in/out filter, providing the desired GUIDs to match (or zeroed when enumerating all devices).
- 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 %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 DI_OK;
+}
+static HRESULT hid_joystick_create_device( IDirectInputImpl *dinput, REFGUID guid, IDirectInputDevice8W **out ) +{
- DIDEVICEINSTANCEW instance = {sizeof(instance)};
- GUID inst_guid = *guid, prod_guid = *guid;
- DWORD size = sizeof(struct hid_joystick);
- HIDD_ATTRIBUTES attrs = {sizeof(attrs)};
- struct hid_joystick *impl = NULL;
- PHIDP_PREPARSED_DATA preparsed;
- UINT32 handle = 0, vid_pid = 0;
- DIDATAFORMAT *format = NULL;
- WCHAR device_path[MAX_PATH];
- HIDP_CAPS caps;
- HANDLE device;
- HRESULT hr;
- TRACE( "dinput %p, guid %s, out %p\n", dinput, debugstr_guid( guid ), out );
- *out = NULL;
- inst_guid.Data3 = hid_joystick_guid.Data3;
- prod_guid.Data1 = DInput_PIDVID_Product_GUID.Data1;
- if (IsEqualGUID( &hid_joystick_guid, &inst_guid )) handle = guid->Data3;
Data3 is only a short. I think we should use 64 bits for our magic and another 64 bits for the RawInput handle.
Or 32 if we go with our internal knowledge about how those handles are implemented in Wine.
Should we really worry about overflowing the short capacity for the HID handle? If we do we could combine Data2 and Data3, or XOR it in Data1 (just to keep a bit of randomness), but as the property is also checked to be 32bit I don't think we need to use more than that.
On 6/30/21 2:30 PM, Rémi Bernon wrote:
On 6/30/21 1:08 PM, Arkadiusz Hiler wrote:
On Wed, Jun 30, 2021 at 10:54:19AM +0200, Rémi Bernon wrote:
+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, 0, 0, device_path, &device, version, + &preparsed, &attrs, &caps, instance );
hid_joystick_device_open() is getting a bit unwieldy with all the possible parameter combinations. I don't think it needs the enumeration / vidpid lookup logic.
IMO it would read better if hid_joystick_device_open() would take the path or RI handle and enum logic would be moved either here or to a helper.
I don't think we need yet another device list cache, and I would rather avoid it if possible. It's really just two call sites.
Instead of the two additional params we could use the DIDEVICEINSTANCEW parameter as an in/out filter, providing the desired GUIDs to match (or zeroed when enumerating all devices).
+ 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 %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 DI_OK; +}
+static HRESULT hid_joystick_create_device( IDirectInputImpl *dinput, REFGUID guid, IDirectInputDevice8W **out ) +{ + DIDEVICEINSTANCEW instance = {sizeof(instance)}; + GUID inst_guid = *guid, prod_guid = *guid; + DWORD size = sizeof(struct hid_joystick); + HIDD_ATTRIBUTES attrs = {sizeof(attrs)}; + struct hid_joystick *impl = NULL; + PHIDP_PREPARSED_DATA preparsed; + UINT32 handle = 0, vid_pid = 0; + DIDATAFORMAT *format = NULL; + WCHAR device_path[MAX_PATH]; + HIDP_CAPS caps; + HANDLE device; + HRESULT hr;
+ TRACE( "dinput %p, guid %s, out %p\n", dinput, debugstr_guid( guid ), out );
+ *out = NULL;
+ inst_guid.Data3 = hid_joystick_guid.Data3; + prod_guid.Data1 = DInput_PIDVID_Product_GUID.Data1; + if (IsEqualGUID( &hid_joystick_guid, &inst_guid )) handle = guid->Data3;
Data3 is only a short. I think we should use 64 bits for our magic and another 64 bits for the RawInput handle.
Or 32 if we go with our internal knowledge about how those handles are implemented in Wine.
Should we really worry about overflowing the short capacity for the HID handle? If we do we could combine Data2 and Data3, or XOR it in Data1 (just to keep a bit of randomness), but as the property is also checked to be 32bit I don't think we need to use more than that.
For instance I think something along those lines would actually be nice, with the "instance" filled in try_open, and then returned into "filter" as output when device matches:
/* enumerate device by GUID */ if (IsEqualGUID( &filter->guidProduct, &instance.guidProduct )) break; if (IsEqualGUID( &filter->guidInstance, &instance.guidInstance )) break; /* enumerate all devices */ if (IsEqualGUID( &filter->guidProduct, &DInput_PIDVID_Product_GUID ) && IsEqualGUID( &filter->guidInstance, &hid_joystick_guid ) && !index--) break;
And the two call sites would look like:
instance->guidProduct = DInput_PIDVID_Product_GUID; instance->guidInstance = hid_joystick_guid; hr = hid_joystick_device_open( index, instance, device_path, &device, &preparsed, &attrs, &caps, version ); if (hr != DI_OK) return hr;
DIDEVICEINSTANCEW instance = {.dwSize = sizeof(instance), .guidProduct = *guid, .guidInstance = *guid}; instance.guidProduct.Data1 = DInput_PIDVID_Product_GUID.Data1; instance.guidInstance.Data1 = hid_joystick_guid.Data1; if (IsEqualGUID( &DInput_PIDVID_Product_GUID, &instance.guidProduct )) { instance.guidProduct = *guid; instance.guidInstance = hid_joystick_guid; } else if (IsEqualGUID( &hid_joystick_guid, &instance.guidInstance )) { instance.guidProduct = DInput_PIDVID_Product_GUID; instance.guidInstance = *guid; } else { return DIERR_DEVICENOTREG; } hr = hid_joystick_device_open( 0, &instance, device_path, &device, &preparsed, &attrs, &caps, dinput->dwVersion ); if (hr != DI_OK) return hr;
It also fulfill the vid_pid matching nicely.
On Wed, Jun 30, 2021 at 03:16:13PM +0200, Rémi Bernon wrote:
On 6/30/21 2:30 PM, Rémi Bernon wrote:
On 6/30/21 1:08 PM, Arkadiusz Hiler wrote:
On Wed, Jun 30, 2021 at 10:54:19AM +0200, Rémi Bernon wrote:
+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, 0, 0, device_path, &device, version, + &preparsed, &attrs, &caps, instance );
hid_joystick_device_open() is getting a bit unwieldy with all the possible parameter combinations. I don't think it needs the enumeration / vidpid lookup logic.
IMO it would read better if hid_joystick_device_open() would take the path or RI handle and enum logic would be moved either here or to a helper.
I don't think we need yet another device list cache, and I would rather avoid it if possible. It's really just two call sites.
Oh sorry, this is not what I was suggesting.
I was thinking about factoring out the setupapi enumeration / device finding logic to a separate function than the one that does all the Hid?_* handling.
Something like:
HANDLE hid_joystick_find_device( int index, HANDLE ri_handle, int vid_pid, HANDLE *ri_handle_out );
and then you can either:
device = hid_joystick_find_device( index, INVALID_HANDLE, -1, &ri_handle ); device = hid_joystick_find_device( -1, ri_handle, -1, NULL ); device = hid_joystick_find_device( -1, INVALID_HANDLE, vid_pid, NULL );
open would then look more like:
hr = hid_joystick_device_read( device, version, &preparsed, &attrs, &caps );
Those two should give you all the thing you need to fill DIDEVICEINSTANCEW in _enum_ and to fill in hid_joystick in _create_.
Also I like to use obviously wrong values for parameters that are not used.
Instead of the two additional params we could use the DIDEVICEINSTANCEW parameter as an in/out filter, providing the desired GUIDs to match (or zeroed when enumerating all devices).
+ 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 %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 DI_OK; +}
+static HRESULT hid_joystick_create_device( IDirectInputImpl *dinput, REFGUID guid, IDirectInputDevice8W **out ) +{ + DIDEVICEINSTANCEW instance = {sizeof(instance)}; + GUID inst_guid = *guid, prod_guid = *guid; + DWORD size = sizeof(struct hid_joystick); + HIDD_ATTRIBUTES attrs = {sizeof(attrs)}; + struct hid_joystick *impl = NULL; + PHIDP_PREPARSED_DATA preparsed; + UINT32 handle = 0, vid_pid = 0; + DIDATAFORMAT *format = NULL; + WCHAR device_path[MAX_PATH]; + HIDP_CAPS caps; + HANDLE device; + HRESULT hr;
+ TRACE( "dinput %p, guid %s, out %p\n", dinput, debugstr_guid( guid ), out );
+ *out = NULL;
+ inst_guid.Data3 = hid_joystick_guid.Data3; + prod_guid.Data1 = DInput_PIDVID_Product_GUID.Data1; + if (IsEqualGUID( &hid_joystick_guid, &inst_guid )) handle = guid->Data3;
Data3 is only a short. I think we should use 64 bits for our magic and another 64 bits for the RawInput handle.
Or 32 if we go with our internal knowledge about how those handles are implemented in Wine.
Should we really worry about overflowing the short capacity for the HID handle? If we do we could combine Data2 and Data3, or XOR it in Data1 (just to keep a bit of randomness), but as the property is also checked to be 32bit I don't think we need to use more than that.
as long as we are using the DEVPROPKEY_HID_HANDLE 32 bits should be enough
For instance I think something along those lines would actually be nice, with the "instance" filled in try_open, and then returned into "filter" as output when device matches:
/* enumerate device by GUID */ if (IsEqualGUID( &filter->guidProduct, &instance.guidProduct )) break; if (IsEqualGUID( &filter->guidInstance, &instance.guidInstance )) break; /* enumerate all devices */ if (IsEqualGUID( &filter->guidProduct, &DInput_PIDVID_Product_GUID ) && IsEqualGUID( &filter->guidInstance, &hid_joystick_guid ) && !index--) break;And the two call sites would look like:
instance->guidProduct = DInput_PIDVID_Product_GUID; instance->guidInstance = hid_joystick_guid; hr = hid_joystick_device_open( index, instance, device_path, &device, &preparsed, &attrs, &caps, version ); if (hr != DI_OK) return hr;DIDEVICEINSTANCEW instance = {.dwSize = sizeof(instance), .guidProduct = *guid, .guidInstance = *guid}; instance.guidProduct.Data1 = DInput_PIDVID_Product_GUID.Data1; instance.guidInstance.Data1 = hid_joystick_guid.Data1; if (IsEqualGUID( &DInput_PIDVID_Product_GUID, &instance.guidProduct )) { instance.guidProduct = *guid; instance.guidInstance = hid_joystick_guid; } else if (IsEqualGUID( &hid_joystick_guid, &instance.guidInstance )) { instance.guidProduct = DInput_PIDVID_Product_GUID; instance.guidInstance = *guid; } else { return DIERR_DEVICENOTREG; } hr = hid_joystick_device_open( 0, &instance, device_path, &device, &preparsed, &attrs, &caps, dinput->dwVersion ); if (hr != DI_OK) return hr;It also fulfill the vid_pid matching nicely.
That's even more logic added to _device_open... I am still not a fan of how hard it is to parse through hid_joystick_device_open() and I would prefer it to be split.
This seems to be a theme in Wine and Win32 APIs though, so I won't NAK or nag you on this anymore :-P
On 7/1/21 12:38 PM, Arkadiusz Hiler wrote:
On Wed, Jun 30, 2021 at 03:16:13PM +0200, Rémi Bernon wrote:
On 6/30/21 2:30 PM, Rémi Bernon wrote:
On 6/30/21 1:08 PM, Arkadiusz Hiler wrote:
On Wed, Jun 30, 2021 at 10:54:19AM +0200, Rémi Bernon wrote:
+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, 0, 0, device_path, &device, version, + &preparsed, &attrs, &caps, instance );
hid_joystick_device_open() is getting a bit unwieldy with all the possible parameter combinations. I don't think it needs the enumeration / vidpid lookup logic.
IMO it would read better if hid_joystick_device_open() would take the path or RI handle and enum logic would be moved either here or to a helper.
I don't think we need yet another device list cache, and I would rather avoid it if possible. It's really just two call sites.
Oh sorry, this is not what I was suggesting.
I was thinking about factoring out the setupapi enumeration / device finding logic to a separate function than the one that does all the Hid?_* handling.
Something like:
HANDLE hid_joystick_find_device( int index, HANDLE ri_handle, int vid_pid, HANDLE *ri_handle_out );
and then you can either:
device = hid_joystick_find_device( index, INVALID_HANDLE, -1, &ri_handle ); device = hid_joystick_find_device( -1, ri_handle, -1, NULL ); device = hid_joystick_find_device( -1, INVALID_HANDLE, vid_pid, NULL );
open would then look more like:
hr = hid_joystick_device_read( device, version, &preparsed, &attrs, &caps );
Those two should give you all the thing you need to fill DIDEVICEINSTANCEW in _enum_ and to fill in hid_joystick in _create_.
Also I like to use obviously wrong values for parameters that are not used.
Well the issue is that DInput enumeration is a bit weird and it enumerates the backend with increasing indexes until it fails.
If, for some reason, we can find a device at the provided index, but the the read fails, we need to try again with another index or the enumeration will consider there's no more devices.
So I don't think we can do anything different from the current loop, where we try to open device until it succeeds, and only consider the DInput index when the device could be fully opened.
On Thu, Jul 01, 2021 at 03:11:57PM +0200, Rémi Bernon wrote:
On 7/1/21 12:38 PM, Arkadiusz Hiler wrote:
On Wed, Jun 30, 2021 at 03:16:13PM +0200, Rémi Bernon wrote:
On 6/30/21 2:30 PM, Rémi Bernon wrote:
On 6/30/21 1:08 PM, Arkadiusz Hiler wrote:
On Wed, Jun 30, 2021 at 10:54:19AM +0200, Rémi Bernon wrote:
+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, 0, 0, device_path, &device, version, + &preparsed, &attrs, &caps, instance );
hid_joystick_device_open() is getting a bit unwieldy with all the possible parameter combinations. I don't think it needs the enumeration / vidpid lookup logic.
IMO it would read better if hid_joystick_device_open() would take the path or RI handle and enum logic would be moved either here or to a helper.
I don't think we need yet another device list cache, and I would rather avoid it if possible. It's really just two call sites.
Oh sorry, this is not what I was suggesting.
I was thinking about factoring out the setupapi enumeration / device finding logic to a separate function than the one that does all the Hid?_* handling.
Something like:
HANDLE hid_joystick_find_device( int index, HANDLE ri_handle, int vid_pid, HANDLE *ri_handle_out );
and then you can either:
device = hid_joystick_find_device( index, INVALID_HANDLE, -1, &ri_handle ); device = hid_joystick_find_device( -1, ri_handle, -1, NULL ); device = hid_joystick_find_device( -1, INVALID_HANDLE, vid_pid, NULL );
open would then look more like:
hr = hid_joystick_device_read( device, version, &preparsed, &attrs, &caps );
Those two should give you all the thing you need to fill DIDEVICEINSTANCEW in _enum_ and to fill in hid_joystick in _create_.
Also I like to use obviously wrong values for parameters that are not used.
Well the issue is that DInput enumeration is a bit weird and it enumerates the backend with increasing indexes until it fails.
If, for some reason, we can find a device at the provided index, but the the read fails, we need to try again with another index or the enumeration will consider there's no more devices.
So I don't think we can do anything different from the current loop, where we try to open device until it succeeds, and only consider the DInput index when the device could be fully opened.
You are righ. That would require even more gymnastics, which is probably not worth it.
I once tried to fix the limits of axes and buttons https://www.winehq.org/pipermail/wine-devel/2020-May/166377.html for my HOTAS. I don't see any headers with the constants for maximum numbers of axes and buttons in this new subsystem, it has no limits? Will it solve the problem for people with HOTAS with too many controls?
On Wed, Jun 30, 2021 at 10:54 AM Rémi Bernon rbernon@codeweavers.com wrote:
This adds a new joystick backend, implemented on top of HID and without any host dependencies. This will be progressively implementated, and it's not going to be usable until at least a few more patches.
On 7/1/21 7:45 AM, Cláudio Sampaio wrote:
I once tried to fix the limits of axes and buttons https://www.winehq.org/pipermail/wine-devel/2020-May/166377.html for my HOTAS. I don't see any headers with the constants for maximum numbers of axes and buttons in this new subsystem, it has no limits? Will it solve the problem for people with HOTAS with too many controls?
On Wed, Jun 30, 2021 at 10:54 AM Rémi Bernon rbernon@codeweavers.com wrote:
This adds a new joystick backend, implemented on top of HID and without any host dependencies. This will be progressively implementated, and it's not going to be usable until at least a few more patches.
No idea, it depends where the limitation comes from. Your patch doesn't seem to change anything to dinput, so I understand the limitation doesn't come from here and this backend is unlikely to change anything.
-
Arkadiusz Hiler -
Cláudio Sampaio -
Rémi Bernon