[PATCH v2 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. -- v2: dinput: Make guidInstance consistent through Wine launches. winebus.sys: Make container_id consistent through Wine launches. winebus.sys: When no serial number is available, use bus_num and port_path when generating instance_id. winebus.sys: Add bus_num and port_path to struct device_desc. winebus.sys: Generate serial numbers only if there is no other way to get the real ones (SDL). winebus.sys: Change device index only on devices of the same model that have the same SN. winebus.sys: Don't use predefined (zeroed) serial numbers for devices without SN. winebus.sys: Return an error on IOCTL_HID_GET_STRING when no serial number is available. 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> --- 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 4e2227bb636..f8449db9b4a 100644 --- a/dlls/winebus.sys/main.c +++ b/dlls/winebus.sys/main.c @@ -1451,6 +1451,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> 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 f8449db9b4a..1ac9964e64a 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 && + !wcsicmp(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 1ac9964e64a..b1f4c1e99b8 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/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 | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/dlls/winebus.sys/main.c b/dlls/winebus.sys/main.c index b1f4c1e99b8..6f8b0b42fc8 100644 --- a/dlls/winebus.sys/main.c +++ b/dlls/winebus.sys/main.c @@ -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
On Fri Dec 5 15:10:22 2025 +0000, Rémi Bernon wrote:
This should probably be done before removing the default zero serial numbers. Moved it before that commit.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9687#note_124930
On Fri Dec 5 11:34:58 2025 +0000, Rémi Bernon wrote:
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. According to the [docs](https://learn.microsoft.com/en-us/windows-hardware/drivers/install/avoiding-...), which lines up with what I've seen:
- The operating system concatenates the USB device serial number, vendor ID, product ID, and revision number to generate a string. - The string that results is hashed into a GUID by using the UUID Version 5 (SHA-1) hash algorithm under a USB-specific namespace. The generated container ID will be unique, provided the independent hardware vendor (IHV) provides a unique serial number on each device. We probably don't need to find the exact same string layout, but I think the general idea sounds easy to implement. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9687#note_124931
On Fri Dec 5 15:28:09 2025 +0000, Rémi Bernon wrote:
Should this be merged with the change that introduced the comparison? Sorry, my mistake :smile:. It was initially wcscmp, but since SN is being used in instance_id and then the registry, decided to make it case insensitive. There are various parts in registry where this id will be stored with upper case, others (as device path) with lower case. So decided to use wcsicmp to guard from such issues. We use device path to generate an unique guidInstance later on.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9687#note_124933
On Fri Dec 5 11:34:58 2025 +0000, Rémi Bernon wrote:
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. Yes, whatever we come up with will need to be something that uses information accessible from other places, i.e in `winepulse.drv`. I haven't looked too deeply into these patches, but the information acquired through udev here seems to be fine for that use case.
The MacOS case I did not track down, but I _think_ there might be a way to get the underlying USB device from a USB audio device there as well. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9687#note_124934
On Fri Dec 5 15:21:29 2025 +0000, Connor McAdams wrote:
According to the [docs](https://learn.microsoft.com/en-us/windows-hardware/drivers/install/avoiding-...), which lines up with what I've seen: - The operating system concatenates the USB device serial number, vendor ID, product ID, and revision number to generate a string. - The string that results is hashed into a GUID by using the UUID Version 5 (SHA-1) hash algorithm under a USB-specific namespace. The generated container ID will be unique, provided the independent hardware vendor (IHV) provides a unique serial number on each device. We probably don't need to find the exact same string layout, but I think the general idea sounds easy to implement. Oh, I also didn't realize this comment was unrelated to container ID and on the dinput code.
dinput creates instance GUIDs and stores them in the registry under `HKCU\\System\\CurrentControlSet\\Control\\MediaProperties\\PrivateProperties\\DirectInput`, they then get reused from there, at least for devices that lack a serial number. This seems unrelated to whatever bus or port they get plugged into, it just reuses the same instance GUID for a particular VID/PID pair, and generates a new one if there are multiple controllers of the same kind plugged in. I have tests for this and am currently working on an implementation, but they're not ready for an MR yet. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9687#note_124935
On Fri Dec 5 11:34:57 2025 +0000, Rémi Bernon wrote:
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? The change in get_device_index() takes care of duplicate serial numbers - it'll increase the index, which is used later to generate an instance_id. The logic in get_instance_id() will use the serial number only if the index is 0, ensuring that there is no duplicate serial numbers with other devices of the same model. Otherwise it'll use bus_num/port_path or index to make the instance_id unique for this device interface.
I'm not very confident that only SDL lacks serial numbers and this seems like a good fallback?
I think only old SDL versions lack SDL_JoystickGetSerial(). Newer Linuxes/Androids should be using more recent versions in my opinion. Maybe it'll be a problem for older Android versions. Bus_iohid retrieves it, as well as bus_udev (once via hid and twice via usb_device), so IMO we cover most cases. I tested Windows hid drivers specifically for SN retrieval, and they only return real serial numbers. Otherwise return an error. Since most joystick devices don't have serial numbers, I think the apps are prepared for this situation. Since we always had a serial number in Wine (mostly default zeroes), IMO this caused some of the problems that forced us to generate unique serial numbers. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9687#note_124944
On Fri Dec 5 11:34:57 2025 +0000, Rémi Bernon wrote:
Could we just make port_path a UINT64? Using an array only to parse it, or does the format matter for something? I thought about that, but it would generate huge numbers depending on the endianness. In most cases only the first level is used, filling the others with zeroes. I chose 8 byte array, since the USB3 spec defines max 7 levels of chained ports, and memcmp, memcpy, etc. are well optimized to deal with 64bit buffers.
I have an implementation of IOCTL_HID_GET_INDEXED_STRING (still not ready for upstream), which relies on matching bus_num/port_path on the device instances of both winebus.sys and wineusb.sys. Since wineusb.sys uses libusb, that needs a byte array to retrieve the port path, decided to follow the same logic in winebus.sys as well. Still no problem implementing it as UINT64 if we decide to do so. Although, the byte array gives us the flexibility to easily expand it further, if the USB spec requires that in the future. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9687#note_124975
On Fri Dec 5 15:28:21 2025 +0000, Connor McAdams wrote:
Yes, whatever we come up with will need to be something that uses information accessible from other places, i.e in `winepulse.drv`. I haven't looked too deeply into these patches, but the information acquired through udev here seems to be fine for that use case. The MacOS case I did not track down, but I _think_ there might be a way to get the underlying USB device from a USB audio device there as well. I didn't know about the mmdevapi side. My main concern about the current implementation of container_id is that it is not consistent. It changes with each Wine launch, so apps that use it as an unique device id, wouldn't recognize it. Which would lead to various other issues, such as not loading previously saved device configurations (button mappings, axis settings, FFB settings), etc.
I think that until the mmdevapi part is ready, something like these suggested changes would be sufficient. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9687#note_124978
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?
Yeah, I was going to use another non-crypt hashing algorithm, but decided to see some comparison results, and surprisingly for 32bit hashes CRC32 was doing very well - one of the fastest with comparable collision rate to the others. I think using two 32bit hashes significantly reduces the chance of collisions here. I could use SHA-1 for a 64bit+ hash if needed. @cmcadams Yes, implementing MediaProperties registry would be ideal. I think some games still rely on it to recognize the capabilities of FFB devices for example. I used the device_id and instance_id from the device_path to generate two crc32 hashes. Since these include vid, pid, version, SN, bus_num, port_num, etc., I think this is sufficient to generate unique guid that serves the purpose. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9687#note_124985
Yeah I know. I couldn't finish it earlier as I am not working full time on this. Still was hoping to make it before the code freeze :smile:, mostly because of the current temporary implementation of container_id. Which is not ideal as I described above. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9687#note_124986
Sure, no worry and thank you for the contribution, it looks promising regardless of the concerns discussed above. Fwiw I think we can try getting them in Proton even if they don't make it in Wine 11. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9687#note_124993
participants (4)
-
Connor McAdams (＠cmcadams) -
Ivo Ivanov
-
Ivo Ivanov (＠logos128)
-
Rémi Bernon