[PATCH 0/10] MR9687: winebus, hidclass, dinput: Add bus_num and port_path to device_desc and use them in instance_id.
Use only real serial numbers, otherwise return error on IOCTL_HID_GET_STRING, similar to Windows. Generate consistent container_id (winebus) based on either serial number, uid (macos LocationID), bus_num/port_path (Linux bus_udev) or input/index. Generate consistent guidInstance (dinput) based on device_id/instance_id extracted from the device_path. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9687
From: Ivo Ivanov <logos128(a)gmail.com> --- dlls/hidclass.sys/hid.h | 1 - dlls/hidclass.sys/pnp.c | 13 ++----------- 2 files changed, 2 insertions(+), 12 deletions(-) diff --git a/dlls/hidclass.sys/hid.h b/dlls/hidclass.sys/hid.h index f47402efbd9..076f02e33f4 100644 --- a/dlls/hidclass.sys/hid.h +++ b/dlls/hidclass.sys/hid.h @@ -56,7 +56,6 @@ struct func_device struct device base; HID_DEVICE_ATTRIBUTES attrs; HIDP_DEVICE_DESC device_desc; - WCHAR serial[256]; ULONG poll_interval; KEVENT halt_event; diff --git a/dlls/hidclass.sys/pnp.c b/dlls/hidclass.sys/pnp.c index 4774ba2b9fc..f80a1341096 100644 --- a/dlls/hidclass.sys/pnp.c +++ b/dlls/hidclass.sys/pnp.c @@ -218,7 +218,6 @@ static NTSTATUS get_hid_device_desc( minidriver *minidriver, DEVICE_OBJECT *devi static NTSTATUS initialize_device( minidriver *minidriver, DEVICE_OBJECT *device ) { struct func_device *fdo = fdo_from_DEVICE_OBJECT( device ); - ULONG index = HID_STRING_ID_ISERIALNUMBER; IO_STATUS_BLOCK io; NTSTATUS status; @@ -230,14 +229,6 @@ static NTSTATUS initialize_device( minidriver *minidriver, DEVICE_OBJECT *device return io.Status; } - call_minidriver( IOCTL_HID_GET_STRING, device, ULongToPtr(index), sizeof(index), - &fdo->serial, sizeof(fdo->serial), &io ); - if (io.Status != STATUS_SUCCESS) - { - ERR( "Minidriver failed to get serial number, status %#lx.\n", io.Status ); - return io.Status; - } - if ((status = get_hid_device_desc( minidriver, device, &fdo->device_desc ))) { ERR( "Failed to get HID device description, status %#lx\n", status ); @@ -297,8 +288,8 @@ static NTSTATUS create_child_pdos( minidriver *minidriver, DEVICE_OBJECT *device { swprintf( pdo->base.device_id, ARRAY_SIZE(pdo->base.device_id), L"%s&Col%02d", fdo->base.device_id, pdo->collection_desc->CollectionNumber ); - swprintf( pdo->base.instance_id, ARRAY_SIZE(pdo->base.instance_id), L"%u&%s&%x&%u&%04u", - fdo->attrs.VersionNumber, fdo->serial, 0, 0, i ); + swprintf( pdo->base.instance_id, ARRAY_SIZE(pdo->base.instance_id), L"%s&%04u", + fdo->base.instance_id, i ); } else { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9687
From: Ivo Ivanov <logos128(a)gmail.com> --- dlls/winebus.sys/unixlib.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dlls/winebus.sys/unixlib.c b/dlls/winebus.sys/unixlib.c index bd4351f059c..94263ec4d0a 100644 --- a/dlls/winebus.sys/unixlib.c +++ b/dlls/winebus.sys/unixlib.c @@ -110,7 +110,7 @@ static const struct device_desc mouse_device_desc = .input = -1, .manufacturer = {'T','h','e',' ','W','i','n','e',' ','P','r','o','j','e','c','t',0}, .product = {'W','i','n','e',' ','H','I','D',' ','m','o','u','s','e',0}, - .serialnumber = {'0','0','0','0',0}, + .serialnumber = {'8','4','5','E','0','0','0','1',0}, }; static NTSTATUS mouse_device_create(void *args) @@ -205,7 +205,7 @@ static const struct device_desc keyboard_device_desc = .input = -1, .manufacturer = {'T','h','e',' ','W','i','n','e',' ','P','r','o','j','e','c','t',0}, .product = {'W','i','n','e',' ','H','I','D',' ','k','e','y','b','o','a','r','d',0}, - .serialnumber = {'0','0','0','0',0}, + .serialnumber = {'8','4','5','E','0','0','0','2',0}, }; static NTSTATUS keyboard_device_create(void *args) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9687
From: Ivo Ivanov <logos128(a)gmail.com> Many apps rely on SN to uniquely identify a device, so adding the same predefined number to every device without SN could potentially confuse such apps. --- dlls/winebus.sys/bus_iohid.c | 1 - dlls/winebus.sys/bus_sdl.c | 1 - dlls/winebus.sys/bus_udev.c | 4 ---- 3 files changed, 6 deletions(-) diff --git a/dlls/winebus.sys/bus_iohid.c b/dlls/winebus.sys/bus_iohid.c index 0b9a0c52c46..99d9ce44574 100644 --- a/dlls/winebus.sys/bus_iohid.c +++ b/dlls/winebus.sys/bus_iohid.c @@ -272,7 +272,6 @@ static void handle_DeviceMatchingCallback(void *context, IOReturn result, void * struct device_desc desc = { .input = -1, .is_hidraw = TRUE, - .serialnumber = {'0','0','0','0',0}, }; struct iohid_device *impl; USAGE_AND_PAGE usages; diff --git a/dlls/winebus.sys/bus_sdl.c b/dlls/winebus.sys/bus_sdl.c index bc7904059ed..ea09c02b708 100644 --- a/dlls/winebus.sys/bus_sdl.c +++ b/dlls/winebus.sys/bus_sdl.c @@ -932,7 +932,6 @@ static void sdl_add_device(unsigned int index) { .input = -1, .manufacturer = {'S','D','L',0}, - .serialnumber = {'0','0','0','0',0}, }; struct sdl_device *impl; diff --git a/dlls/winebus.sys/bus_udev.c b/dlls/winebus.sys/bus_udev.c index 1ad4653fd8b..b4009ba02ee 100644 --- a/dlls/winebus.sys/bus_udev.c +++ b/dlls/winebus.sys/bus_udev.c @@ -1175,7 +1175,6 @@ static NTSTATUS hidraw_device_create(struct udev_device *dev, int fd, const char { #ifdef HAVE_LINUX_HIDRAW_H static const WCHAR hidraw[] = {'h','i','d','r','a','w',0}; - static const WCHAR zeros[] = {'0','0','0','0',0}; struct base_device *impl; char buffer[MAX_PATH]; @@ -1184,7 +1183,6 @@ static NTSTATUS hidraw_device_create(struct udev_device *dev, int fd, const char ntdll_umbstowcs(buffer, strlen(buffer) + 1, desc.product, ARRAY_SIZE(desc.product)); if (!desc.manufacturer[0]) memcpy(desc.manufacturer, hidraw, sizeof(hidraw)); - if (!desc.serialnumber[0]) memcpy(desc.serialnumber, zeros, sizeof(zeros)); if (!(impl = raw_device_create(&hidraw_device_vtbl, sizeof(struct hidraw_device)))) return STATUS_NO_MEMORY; @@ -1206,7 +1204,6 @@ static NTSTATUS lnxev_device_create(struct udev_device *dev, int fd, const char { #ifdef HAS_PROPER_INPUT_HEADER static const WCHAR evdev[] = {'e','v','d','e','v',0}; - static const WCHAR zeros[] = {'0','0','0','0',0}; int axis_count = 0, button_count = 0; struct lnxev_info info = {0}; struct lnxev_device *impl; @@ -1225,7 +1222,6 @@ static NTSTATUS lnxev_device_create(struct udev_device *dev, int fd, const char if (!desc.manufacturer[0]) memcpy(desc.manufacturer, evdev, sizeof(evdev)); if (!desc.product[0]) ntdll_umbstowcs(info.name, strlen(info.name) + 1, desc.product, ARRAY_SIZE(desc.product)); if (!desc.serialnumber[0]) ntdll_umbstowcs(info.uniq, strlen(info.uniq) + 1, desc.serialnumber, ARRAY_SIZE(desc.serialnumber)); - if (!desc.serialnumber[0]) memcpy(desc.serialnumber, zeros, sizeof(zeros)); if (!(impl = hid_device_create(&lnxev_device_vtbl, sizeof(struct lnxev_device)))) return STATUS_NO_MEMORY; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9687
From: Ivo Ivanov <logos128(a)gmail.com> Also make sure that devices of the same model with different serial numbers always have zero indexes. This ensures that the device instance_id wont change if the device is connected to different ports, in presence of other devices of the same model. --- dlls/winebus.sys/main.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dlls/winebus.sys/main.c b/dlls/winebus.sys/main.c index 4e2227bb636..82de2193c3e 100644 --- a/dlls/winebus.sys/main.c +++ b/dlls/winebus.sys/main.c @@ -175,7 +175,8 @@ static DWORD get_device_index(struct device_desc *desc, struct list **before) /* The device list is sorted, so just increment the index until it doesn't match an index already in the list */ LIST_FOR_EACH_ENTRY(ext, &device_list, struct device_extension, entry) { - if (ext->desc.vid == desc->vid && ext->desc.pid == desc->pid && ext->desc.input == desc->input) + if (ext->desc.vid == desc->vid && ext->desc.pid == desc->pid && ext->desc.input == desc->input && + !wcscmp(ext->desc.serialnumber, desc->serialnumber)) { if (ext->index != index) { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9687
From: Ivo Ivanov <logos128(a)gmail.com> Partially reverts a19c9e2. --- dlls/winebus.sys/bus_sdl.c | 18 +++++++++++++++--- dlls/winebus.sys/main.c | 16 ---------------- 2 files changed, 15 insertions(+), 19 deletions(-) diff --git a/dlls/winebus.sys/bus_sdl.c b/dlls/winebus.sys/bus_sdl.c index ea09c02b708..63003af6d26 100644 --- a/dlls/winebus.sys/bus_sdl.c +++ b/dlls/winebus.sys/bus_sdl.c @@ -940,7 +940,7 @@ static void sdl_add_device(unsigned int index) SDL_JoystickType joystick_type; SDL_GameController *controller = NULL; const char *product, *sdl_serial; - char buffer[ARRAY_SIZE(desc.product)]; + char guid_str[33], buffer[ARRAY_SIZE(desc.product)]; int axis_count, axis_offset; if ((joystick = pSDL_JoystickOpen(index)) == NULL) @@ -973,8 +973,20 @@ static void sdl_add_device(unsigned int index) desc.version = 0; } - if (pSDL_JoystickGetSerial && (sdl_serial = pSDL_JoystickGetSerial(joystick))) - ntdll_umbstowcs(sdl_serial, strlen(sdl_serial) + 1, desc.serialnumber, ARRAY_SIZE(desc.serialnumber)); + if (pSDL_JoystickGetSerial) + { + if ((sdl_serial = pSDL_JoystickGetSerial(joystick))) + ntdll_umbstowcs(sdl_serial, strlen(sdl_serial) + 1, desc.serialnumber, ARRAY_SIZE(desc.serialnumber)); + } + else + { + /* Overcooked! All You Can Eat only adds controllers with unique serial numbers + * Prefer keeping serial numbers unique over keeping them consistent across runs */ + pSDL_JoystickGetGUIDString(pSDL_JoystickGetGUID(joystick), guid_str, sizeof(guid_str)); + snprintf(buffer, sizeof(buffer), "%s.%d", guid_str, index); + TRACE("Making up serial number for %s: %s\n", product, buffer); + ntdll_umbstowcs(buffer, strlen(buffer) + 1, desc.serialnumber, ARRAY_SIZE(desc.serialnumber)); + } if (controller) { diff --git a/dlls/winebus.sys/main.c b/dlls/winebus.sys/main.c index 82de2193c3e..91171b63e59 100644 --- a/dlls/winebus.sys/main.c +++ b/dlls/winebus.sys/main.c @@ -337,18 +337,6 @@ static void remove_pending_irps(DEVICE_OBJECT *device) } } -static void make_unique_serial(struct device_extension *device) -{ - struct device_extension *ext; - - LIST_FOR_EACH_ENTRY(ext, &device_list, struct device_extension, entry) - if (!wcscmp(device->desc.serialnumber, ext->desc.serialnumber)) break; - if (&ext->entry == &device_list && *device->desc.serialnumber) return; - - swprintf(device->desc.serialnumber, ARRAY_SIZE(device->desc.serialnumber), L"%04x%08x%04x%04x", - device->index, device->desc.input, device->desc.pid, device->desc.vid); -} - static void make_unique_container_id(struct device_extension *device) { struct device_extension *ext; @@ -409,10 +397,6 @@ static DEVICE_OBJECT *bus_create_hid_device(struct device_desc *desc, UINT64 uni InitializeCriticalSectionEx(&ext->cs, 0, RTL_CRITICAL_SECTION_FLAG_FORCE_DEBUG_INFO); ext->cs.DebugInfo->Spare[0] = (DWORD_PTR)(__FILE__ ": cs"); - /* Overcooked! All You Can Eat only adds controllers with unique serial numbers - * Prefer keeping serial numbers unique over keeping them consistent across runs */ - make_unique_serial(ext); - /* * Some games use container ID to match the bus device to the HID * device in order to get things like DEVPKEY_Device_BusReportedDeviceDesc. -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9687
From: Ivo Ivanov <logos128(a)gmail.com> --- dlls/winebus.sys/main.c | 1 + 1 file changed, 1 insertion(+) diff --git a/dlls/winebus.sys/main.c b/dlls/winebus.sys/main.c index 91171b63e59..32fd0ca06f7 100644 --- a/dlls/winebus.sys/main.c +++ b/dlls/winebus.sys/main.c @@ -1436,6 +1436,7 @@ static NTSTATUS hid_get_device_string(DEVICE_OBJECT *device, DWORD index, WCHAR case HID_STRING_ID_ISERIALNUMBER: len = (wcslen(ext->desc.serialnumber) + 1) * sizeof(WCHAR); if (len > buffer_len) return STATUS_BUFFER_TOO_SMALL; + else if (len == sizeof(WCHAR)) return STATUS_INVALID_PARAMETER; else memcpy(buffer, ext->desc.serialnumber, len); return STATUS_SUCCESS; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9687
From: Ivo Ivanov <logos128(a)gmail.com> --- dlls/winebus.sys/bus_udev.c | 11 +++++++++++ dlls/winebus.sys/unixlib.h | 11 ++++++++--- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/dlls/winebus.sys/bus_udev.c b/dlls/winebus.sys/bus_udev.c index b4009ba02ee..f4fac511a85 100644 --- a/dlls/winebus.sys/bus_udev.c +++ b/dlls/winebus.sys/bus_udev.c @@ -1168,6 +1168,17 @@ static void get_device_subsystem_info(struct udev_device *dev, const char *subsy if ((tmp = udev_device_get_sysattr_value(parent, "serial"))) ntdll_umbstowcs(tmp, strlen(tmp) + 1, desc->serialnumber, ARRAY_SIZE(desc->serialnumber)); + + if ((tmp = udev_device_get_sysattr_value(parent, "busnum"))) + desc->bus_num = atoi(tmp); + + if ((tmp = udev_device_get_sysattr_value(parent, "devpath"))) + { + *(UINT64 *)desc->port_path = 0; + sscanf(tmp, "%hhu.%hhu.%hhu.%hhu.%hhu.%hhu.%hhu.%hhu", &desc->port_path[0], &desc->port_path[1], + &desc->port_path[2], &desc->port_path[3], &desc->port_path[4], &desc->port_path[5], + &desc->port_path[6], &desc->port_path[7]); + } } } diff --git a/dlls/winebus.sys/unixlib.h b/dlls/winebus.sys/unixlib.h index d2b17f4c6d3..05bf28d8e2d 100644 --- a/dlls/winebus.sys/unixlib.h +++ b/dlls/winebus.sys/unixlib.h @@ -39,6 +39,8 @@ struct device_desc UINT input; UINT uid; UINT bus_type; + UINT bus_num; + UINT8 port_path[8]; BOOL is_gamepad; BOOL is_hidraw; @@ -159,9 +161,12 @@ enum unix_funcs static inline const char *debugstr_device_desc(struct device_desc *desc) { if (!desc) return "(null)"; - return wine_dbg_sprintf("{vid %04x, pid %04x, version %04x, input %d, uid %08x, is_gamepad %u, is_hidraw %u, bus_type %u}", - desc->vid, desc->pid, desc->version, desc->input, desc->uid, - desc->is_gamepad, desc->is_hidraw, desc->bus_type); + return wine_dbg_sprintf("{vid %04x, pid %04x, version %04x, input %d, uid %08x, bus_type %u, bus_num %u, port_path %u.%u.%u.%u.%u.%u.%u.%u, is_gamepad %u, is_hidraw %u}", + desc->vid, desc->pid, desc->version, desc->input, desc->uid, desc->bus_type, + desc->bus_num, desc->port_path[0], desc->port_path[1], + desc->port_path[2], desc->port_path[3], desc->port_path[4], + desc->port_path[5], desc->port_path[6], desc->port_path[7], + desc->is_gamepad, desc->is_hidraw); } static inline BOOL is_xbox_gamepad(WORD vid, WORD pid) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9687
From: Ivo Ivanov <logos128(a)gmail.com> If there are identical devices with the same serial numbers (unlikely and goes against the specification), use SN only for the first device with index 0. Otherwise use bus_num and port_path for the rest. To make sure the instance_id is unique, use index if there is no bus_num (and probably no port_path) available. --- dlls/winebus.sys/main.c | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/dlls/winebus.sys/main.c b/dlls/winebus.sys/main.c index 32fd0ca06f7..6f8b0b42fc8 100644 --- a/dlls/winebus.sys/main.c +++ b/dlls/winebus.sys/main.c @@ -176,7 +176,7 @@ static DWORD get_device_index(struct device_desc *desc, struct list **before) LIST_FOR_EACH_ENTRY(ext, &device_list, struct device_extension, entry) { if (ext->desc.vid == desc->vid && ext->desc.pid == desc->pid && ext->desc.input == desc->input && - !wcscmp(ext->desc.serialnumber, desc->serialnumber)) + !wcsicmp(ext->desc.serialnumber, desc->serialnumber)) { if (ext->index != index) { @@ -193,13 +193,30 @@ static DWORD get_device_index(struct device_desc *desc, struct list **before) static WCHAR *get_instance_id(DEVICE_OBJECT *device) { struct device_extension *ext = (struct device_extension *)device->DeviceExtension; - DWORD len = wcslen(ext->desc.serialnumber) + 33; + struct device_desc *desc = &ext->desc; + const DWORD sn_len = wcslen(desc->serialnumber); + DWORD len = (sn_len ? sn_len : ARRAY_SIZE(desc->port_path)*2) + 33; + WCHAR tmp[ARRAY_SIZE(desc->port_path)*2+1]; WCHAR *dst; + int i; if ((dst = ExAllocatePool(PagedPool, len * sizeof(WCHAR)))) { - swprintf(dst, len, L"%u&%s&%x&%u&%u", ext->desc.version, ext->desc.serialnumber, - ext->desc.uid, ext->index, ext->desc.is_gamepad); + if (sn_len && !ext->index) + { + swprintf(dst, len, L"%u&%x&%u&%s&%u", desc->version, desc->uid, ext->index, + desc->serialnumber, desc->is_gamepad); + } + else + { + i = 0; + do + swprintf(&tmp[i*2], ARRAY_SIZE(tmp) - (i * 2), L"%02X", desc->port_path[i]); + while (++i < ARRAY_SIZE(desc->port_path) && desc->port_path[i]); + + swprintf(dst, len, L"%u&%x&%u&%s&%u", desc->version, desc->uid, + desc->bus_num ? desc->bus_num : ext->index, tmp, desc->is_gamepad); + } } return dst; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9687
From: Ivo Ivanov <logos128(a)gmail.com> container_id uniquely identifies a device on Windows, and some apps store it for further reference. Changing this id on every Wine run, may cause such app to decide that it is a new device and not use its previously stored settings (button assignments, FFB settings, etc.). Additionally on Windows container_id identifies the entire device incl. its interfaces and collections, i.e. it is the same for all these instances (in USB and HID). When generating container_id, use any of the following device specific parameters, when available: serial number, or uid (macos LocationID), or bus_num and port_path (Linux bus_udev backend). If none of these are available use input and index, which are interface specific, but will still generate an unique id. --- dlls/winebus.sys/main.c | 32 ++++++++++++++++++++++++++------ 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/dlls/winebus.sys/main.c b/dlls/winebus.sys/main.c index 6f8b0b42fc8..b7115b6883c 100644 --- a/dlls/winebus.sys/main.c +++ b/dlls/winebus.sys/main.c @@ -357,17 +357,37 @@ static void remove_pending_irps(DEVICE_OBJECT *device) static void make_unique_container_id(struct device_extension *device) { struct device_extension *ext; - LARGE_INTEGER ticks; + struct device_desc *desc = &device->desc; + const DWORD sn_len = wcslen(desc->serialnumber); + DWORD crc32; LIST_FOR_EACH_ENTRY(ext, &device_list, struct device_extension, entry) if (IsEqualGUID(&device->container_id, &ext->container_id)) break; if (&ext->entry == &device_list && !IsEqualGUID(&device->container_id, &GUID_NULL)) return; - device->container_id.Data1 = MAKELONG(device->desc.vid, device->desc.pid); - device->container_id.Data2 = device->index; - device->container_id.Data3 = device->desc.input; - QueryPerformanceCounter(&ticks); - memcpy(device->container_id.Data4, &ticks.QuadPart, sizeof(device->container_id.Data4)); + device->container_id.Data1 = MAKELONG(desc->vid, desc->pid); + device->container_id.Data2 = desc->version; + if (sn_len && !device->index) + { + device->container_id.Data3 = 0; + crc32 = RtlComputeCrc32(0, (const BYTE *)desc->serialnumber, sn_len * sizeof(WCHAR)); + *(UINT64 *)device->container_id.Data4 = crc32; + } + else if (desc->uid) + { + device->container_id.Data3 = 0; + *(UINT64 *)device->container_id.Data4 = desc->uid; + } + else if (desc->bus_num || desc->port_path[0]) + { + device->container_id.Data3 = desc->bus_num; + memcpy(device->container_id.Data4, desc->port_path, sizeof(desc->port_path)); + } + else + { + device->container_id.Data3 = desc->input; + *(UINT64 *)device->container_id.Data4 = device->index; + } } static DEVICE_OBJECT *bus_create_hid_device(struct device_desc *desc, UINT64 unix_device) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9687
From: Ivo Ivanov <logos128(a)gmail.com> Many apps use this guid to uniquely identify a dinput device. Making this guid solely dependent on the device enumeration order (through the rawinput handle), may result in the same guid being reused for another device on the next Wine launch. To avoid this, when generating the device guidInstance use the device_id and instance_id from DevicePath. These ids are consistent through runs (depending on the parameters available to winebus), and uniquely identify a HID device on the system. Also change hid_joystick_guid.Data4 to a value similar to the one used in Windows. --- dlls/dinput/joystick_hid.c | 43 ++++++++++++++++++++++++++++++++------ 1 file changed, 37 insertions(+), 6 deletions(-) diff --git a/dlls/dinput/joystick_hid.c b/dlls/dinput/joystick_hid.c index e492fa51bbf..d7800e43298 100644 --- a/dlls/dinput/joystick_hid.c +++ b/dlls/dinput/joystick_hid.c @@ -34,6 +34,7 @@ #include "ddk/hidclass.h" #include "ddk/hidsdi.h" +#include "cfgmgr32.h" #include "setupapi.h" #include "devguid.h" #include "dinput.h" @@ -50,7 +51,7 @@ WINE_DEFAULT_DEBUG_CHANNEL(dinput); DEFINE_GUID( GUID_DEVINTERFACE_WINEXINPUT,0x6c53d5fd,0x6480,0x440f,0xb6,0x18,0x47,0x67,0x50,0xc5,0xe1,0xa6 ); -DEFINE_GUID( hid_joystick_guid, 0x9e573edb, 0x7734, 0x11d2, 0x8d, 0x4a, 0x23, 0x90, 0x3f, 0xb6, 0xbd, 0xf7 ); +DEFINE_GUID( hid_joystick_guid, 0x9e573edb, 0x0000, 0x0000, 0x00, 0x00, 'D', 'E', 'S', 'T', 0x00, 0x00 ); DEFINE_GUID( device_path_guid, 0x00000000, 0x0000, 0x0000, 0x8d, 0x4a, 0x23, 0x90, 0x3f, 0xb6, 0xbd, 0xf8 ); struct pid_control_report @@ -1422,9 +1423,11 @@ static HRESULT hid_joystick_device_try_open( const WCHAR *path, HANDLE *device, BOOL has_accelerator, has_brake, has_clutch, has_z, has_pov; PHIDP_PREPARSED_DATA preparsed_data = NULL; HIDP_LINK_COLLECTION_NODE nodes[256]; - DWORD type, size, button_count = 0; + DWORD type, size, crc32, button_count = 0; HIDP_BUTTON_CAPS buttons[10]; HIDP_VALUE_CAPS value; + const WCHAR sep = L'#'; + WCHAR *tmp, *device_path = NULL, *device_id = NULL, *instance_id = NULL; HANDLE device_file; ULONG node_count; NTSTATUS status; @@ -1451,14 +1454,40 @@ static HRESULT hid_joystick_device_try_open( const WCHAR *path, HANDLE *device, if (!HidD_GetProductString( device_file, instance->tszInstanceName, MAX_PATH * sizeof(WCHAR) )) goto failed; if (!HidD_GetProductString( device_file, instance->tszProductName, MAX_PATH * sizeof(WCHAR) )) goto failed; - if (!DeviceIoControl( device_file, IOCTL_HID_GET_WINE_RAWINPUT_HANDLE, NULL, 0, &handle, sizeof(handle), &size, NULL )) + if ((device_path = wcsdup( path )) && *device_path == L'\\' && (tmp = wcschr( device_path, sep ))) { - ERR( "failed to get raw input handle, error %lu\n", GetLastError() ); - goto failed; + device_id = ++tmp; + if ((tmp = wcschr( device_id, sep ))) + { + *tmp = 0; + instance_id = ++tmp; + if ((tmp = wcsrchr( instance_id, sep )) && *(tmp + 1) == L'{') + *tmp = 0; + else instance_id = NULL; + } } instance->guidInstance = hid_joystick_guid; - instance->guidInstance.Data1 ^= handle; + if (device_id && *device_id && instance_id && *instance_id) + { + crc32 = RtlComputeCrc32( 0, (const BYTE *)CharUpperW( device_id ), wcslen( device_id ) * sizeof(WCHAR) ); + instance->guidInstance.Data1 = crc32; + crc32 = RtlComputeCrc32( 0, (const BYTE *)CharUpperW( instance_id ), wcslen( instance_id ) * sizeof(WCHAR) ); + instance->guidInstance.Data2 = HIWORD( crc32 ); + instance->guidInstance.Data3 = LOWORD( crc32 ); + } + else + { + if (!DeviceIoControl( device_file, IOCTL_HID_GET_WINE_RAWINPUT_HANDLE, NULL, 0, &handle, sizeof(handle), &size, NULL )) + { + ERR( "failed to get raw input handle, error %lu\n", GetLastError() ); + goto failed; + } + + instance->guidInstance.Data1 ^= handle; + instance->guidInstance.Data2 = attrs->VendorID; + instance->guidInstance.Data3 = attrs->ProductID; + } instance->guidProduct = dinput_pidvid_guid; instance->guidProduct.Data1 = MAKELONG( attrs->VendorID, attrs->ProductID ); instance->guidFFDriver = GUID_NULL; @@ -1563,11 +1592,13 @@ static HRESULT hid_joystick_device_try_open( const WCHAR *path, HANDLE *device, instance->dwDevType = device_type_for_version( type, version ) | DIDEVTYPE_HID; TRACE("detected device type %#lx\n", instance->dwDevType); + free( device_path ); *device = device_file; *preparsed = preparsed_data; return DI_OK; failed: + free( device_path ); CloseHandle( device_file ); HidD_FreePreparsedData( preparsed_data ); return DIERR_DEVICENOTREG; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9687
Rémi Bernon (@rbernon) commented about dlls/winebus.sys/main.c:
LIST_FOR_EACH_ENTRY(ext, &device_list, struct device_extension, entry) { if (ext->desc.vid == desc->vid && ext->desc.pid == desc->pid && ext->desc.input == desc->input && - !wcscmp(ext->desc.serialnumber, desc->serialnumber)) + !wcsicmp(ext->desc.serialnumber, desc->serialnumber))
Should this be merged with the change that introduced the comparison? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9687#note_124900
Rémi Bernon (@rbernon) commented about dlls/winebus.sys/main.c:
case HID_STRING_ID_ISERIALNUMBER: len = (wcslen(ext->desc.serialnumber) + 1) * sizeof(WCHAR); if (len > buffer_len) return STATUS_BUFFER_TOO_SMALL; + else if (len == sizeof(WCHAR)) return STATUS_INVALID_PARAMETER; else memcpy(buffer, ext->desc.serialnumber, len);
This should probably be done before removing the default zero serial numbers. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9687#note_124899
Rémi Bernon (@rbernon) commented about dlls/winebus.sys/main.c:
} }
-static void make_unique_serial(struct device_extension *device) -{ - struct device_extension *ext; - - LIST_FOR_EACH_ENTRY(ext, &device_list, struct device_extension, entry) - if (!wcscmp(device->desc.serialnumber, ext->desc.serialnumber)) break; - if (&ext->entry == &device_list && *device->desc.serialnumber) return; - - swprintf(device->desc.serialnumber, ARRAY_SIZE(device->desc.serialnumber), L"%04x%08x%04x%04x", - device->index, device->desc.input, device->desc.pid, device->desc.vid); -}
As I understand this should only generate a unique serial number if we found another device with the same serial, so it should not make much difference for cases where the device has already a proper serial? I'm not very confident that only SDL lacks serial numbers and this seems like a good fallback? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9687#note_124901
Rémi Bernon (@rbernon) commented about dlls/winebus.sys/main.c:
if ((dst = ExAllocatePool(PagedPool, len * sizeof(WCHAR)))) { - swprintf(dst, len, L"%u&%s&%x&%u&%u", ext->desc.version, ext->desc.serialnumber, - ext->desc.uid, ext->index, ext->desc.is_gamepad); + if (sn_len && !ext->index) + { + swprintf(dst, len, L"%u&%x&%u&%s&%u", desc->version, desc->uid, ext->index, + desc->serialnumber, desc->is_gamepad); + } + else + { + i = 0; + do + swprintf(&tmp[i*2], ARRAY_SIZE(tmp) - (i * 2), L"%02X", desc->port_path[i]); + while (++i < ARRAY_SIZE(desc->port_path) && desc->port_path[i]);
Could we just make port_path a UINT64? Using an array only to parse it, or does the format matter for something? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9687#note_124902
Rémi Bernon (@rbernon) commented about dlls/winebus.sys/main.c:
+ } + else if (desc->uid) + { + device->container_id.Data3 = 0; + *(UINT64 *)device->container_id.Data4 = desc->uid; + } + else if (desc->bus_num || desc->port_path[0]) + { + device->container_id.Data3 = desc->bus_num; + memcpy(device->container_id.Data4, desc->port_path, sizeof(desc->port_path)); + } + else + { + device->container_id.Data3 = desc->input; + *(UINT64 *)device->container_id.Data4 = device->index; + } I think this was meant as a temporary solution but the main issue is that we will need to be able to do the same kind of thing on the mmdevapi side, because the ContainerID is used to match the haptics audio device with its controller.
I don't think it's implemented in Wine yet but I know @cmcadams was looking into it, and I'm not very up to date to what we'll need or be able to do there. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9687#note_124903
Rémi Bernon (@rbernon) commented about dlls/dinput/joystick_hid.c:
+ *tmp = 0; + instance_id = ++tmp; + if ((tmp = wcsrchr( instance_id, sep )) && *(tmp + 1) == L'{') + *tmp = 0; + else instance_id = NULL; + } + } + + instance->guidInstance = hid_joystick_guid; + if (device_id && *device_id && instance_id && *instance_id) + { + crc32 = RtlComputeCrc32( 0, (const BYTE *)CharUpperW( device_id ), wcslen( device_id ) * sizeof(WCHAR) ); + instance->guidInstance.Data1 = crc32; + crc32 = RtlComputeCrc32( 0, (const BYTE *)CharUpperW( instance_id ), wcslen( instance_id ) * sizeof(WCHAR) ); + instance->guidInstance.Data2 = HIWORD( crc32 ); + instance->guidInstance.Data3 = LOWORD( crc32 ); The same question can apply to the winebus side, but CRC32 doesn't seem like a very good hashing method? Could we use something a bit more robust or does it not matter for some reason? Ideally of course we would do the same hashing as Windows but it's probably tricky to figure it out by trial and error.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9687#note_124904
Fwiw the code freeze today, so I'm a bit nervous about making all those changes. There's some concerns that will probably need to be addressed anyway and it may have to wait until later. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9687#note_124905
participants (3)
-
Ivo Ivanov -
Ivo Ivanov (＠logos128)
-
Rémi Bernon