[PATCH 0/4] MR5352: winebus: Prefer hidraw for everything that is not a game controller.
Supersedes !5331 -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5352
From: Rémi Bernon <rbernon(a)codeweavers.com> As a hint for the frontend to decide whether to use hidraw. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=56450 --- dlls/winebus.sys/bus_iohid.c | 9 ++++----- dlls/winebus.sys/bus_sdl.c | 4 ++++ dlls/winebus.sys/bus_udev.c | 3 +++ dlls/winebus.sys/unixlib.h | 7 +++++-- 4 files changed, 16 insertions(+), 7 deletions(-) diff --git a/dlls/winebus.sys/bus_iohid.c b/dlls/winebus.sys/bus_iohid.c index a44944576e9..e7960e185e9 100644 --- a/dlls/winebus.sys/bus_iohid.c +++ b/dlls/winebus.sys/bus_iohid.c @@ -276,18 +276,17 @@ static void handle_DeviceMatchingCallback(void *context, IOReturn result, void * }; struct iohid_device *impl; CFStringRef str; - UINT usage_page, usage; - usage_page = CFNumberToDWORD(IOHIDDeviceGetProperty(IOHIDDevice, CFSTR(kIOHIDPrimaryUsagePageKey))); - usage = CFNumberToDWORD(IOHIDDeviceGetProperty(IOHIDDevice, CFSTR(kIOHIDPrimaryUsageKey))); + desc.usages.UsagePage = CFNumberToDWORD(IOHIDDeviceGetProperty(IOHIDDevice, CFSTR(kIOHIDPrimaryUsagePageKey))); + desc.usages.Usage = CFNumberToDWORD(IOHIDDeviceGetProperty(IOHIDDevice, CFSTR(kIOHIDPrimaryUsageKey))); desc.vid = CFNumberToDWORD(IOHIDDeviceGetProperty(IOHIDDevice, CFSTR(kIOHIDVendorIDKey))); desc.pid = CFNumberToDWORD(IOHIDDeviceGetProperty(IOHIDDevice, CFSTR(kIOHIDProductIDKey))); desc.version = CFNumberToDWORD(IOHIDDeviceGetProperty(IOHIDDevice, CFSTR(kIOHIDVersionNumberKey))); desc.uid = CFNumberToDWORD(IOHIDDeviceGetProperty(IOHIDDevice, CFSTR(kIOHIDLocationIDKey))); - if (usage_page != HID_USAGE_PAGE_GENERIC || - !(usage == HID_USAGE_GENERIC_JOYSTICK || usage == HID_USAGE_GENERIC_GAMEPAD)) + if (desc.usages.UsagePage != HID_USAGE_PAGE_GENERIC || + !(desc.usages.Usage == HID_USAGE_GENERIC_JOYSTICK || desc.usages.Usage == HID_USAGE_GENERIC_GAMEPAD)) { /* winebus isn't currently meant to handle anything but these, and * opening keyboards, mice, or the Touch Bar on older MacBooks triggers diff --git a/dlls/winebus.sys/bus_sdl.c b/dlls/winebus.sys/bus_sdl.c index 7b43df82f83..2a60f112a83 100644 --- a/dlls/winebus.sys/bus_sdl.c +++ b/dlls/winebus.sys/bus_sdl.c @@ -990,6 +990,8 @@ static void sdl_add_device(unsigned int index) if (controller) { desc.is_gamepad = TRUE; + desc.usages.UsagePage = HID_USAGE_PAGE_GENERIC; + desc.usages.Usage = HID_USAGE_GENERIC_GAMEPAD; axis_count = 6; } else @@ -997,6 +999,8 @@ static void sdl_add_device(unsigned int index) int button_count = pSDL_JoystickNumButtons(joystick); axis_count = pSDL_JoystickNumAxes(joystick); desc.is_gamepad = (axis_count == 6 && button_count >= 14); + desc.usages.UsagePage = HID_USAGE_PAGE_GENERIC; + desc.usages.Usage = HID_USAGE_GENERIC_JOYSTICK; } for (axis_offset = 0; axis_offset < axis_count; axis_offset += (options.split_controllers ? 6 : axis_count)) diff --git a/dlls/winebus.sys/bus_udev.c b/dlls/winebus.sys/bus_udev.c index f8c6ccb9060..419de2e7af3 100644 --- a/dlls/winebus.sys/bus_udev.c +++ b/dlls/winebus.sys/bus_udev.c @@ -1332,6 +1332,7 @@ static void udev_add_device(struct udev_device *dev, int fd) #ifdef HAS_PROPER_INPUT_HEADER else if (!strcmp(subsystem, "input")) { + const USAGE_AND_PAGE device_usage = *what_am_I(dev, fd); static const WCHAR evdev[] = {'e','v','d','e','v',0}; struct input_id device_id = {0}; char buffer[MAX_PATH]; @@ -1352,6 +1353,8 @@ static void udev_add_device(struct udev_device *dev, int fd) if (!desc.serialnumber[0] && ioctl(fd, EVIOCGUNIQ(sizeof(buffer)), buffer) >= 0) ntdll_umbstowcs(buffer, strlen(buffer) + 1, desc.serialnumber, ARRAY_SIZE(desc.serialnumber)); + + desc.usages = device_usage; } #endif diff --git a/dlls/winebus.sys/unixlib.h b/dlls/winebus.sys/unixlib.h index 80852696dd1..753886304db 100644 --- a/dlls/winebus.sys/unixlib.h +++ b/dlls/winebus.sys/unixlib.h @@ -25,6 +25,7 @@ #include <winbase.h> #include <winternl.h> #include <ddk/hidclass.h> +#include <ddk/hidpi.h> #include <hidusage.h> #include "wine/debug.h" @@ -37,6 +38,7 @@ struct device_desc UINT version; UINT input; UINT uid; + USAGE_AND_PAGE usages; BOOL is_gamepad; BOOL is_hidraw; @@ -148,8 +150,9 @@ 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}", - desc->vid, desc->pid, desc->version, desc->input, desc->uid, desc->is_gamepad, desc->is_hidraw); + return wine_dbg_sprintf("{vid %04x, pid %04x, version %04x, input %d, uid %08x, usage %04x:%04x, is_gamepad %u, is_hidraw %u}", + desc->vid, desc->pid, desc->version, desc->input, desc->uid, desc->usages.UsagePage, desc->usages.Usage, + 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/5352
From: Rémi Bernon <rbernon(a)codeweavers.com> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=56450 --- dlls/winebus.sys/main.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/dlls/winebus.sys/main.c b/dlls/winebus.sys/main.c index 044b5e0141c..5244d7dda54 100644 --- a/dlls/winebus.sys/main.c +++ b/dlls/winebus.sys/main.c @@ -388,7 +388,7 @@ static DWORD check_bus_option(const WCHAR *option, DWORD default_value) return default_value; } -static BOOL is_hidraw_enabled(WORD vid, WORD pid) +static BOOL is_hidraw_enabled(WORD vid, WORD pid, const USAGE_AND_PAGE *usages) { char buffer[FIELD_OFFSET(KEY_VALUE_PARTIAL_INFORMATION, Data[1024])]; KEY_VALUE_PARTIAL_INFORMATION *info = (KEY_VALUE_PARTIAL_INFORMATION *)buffer; @@ -398,6 +398,8 @@ static BOOL is_hidraw_enabled(WORD vid, WORD pid) DWORD size; if (check_bus_option(L"DisableHidraw", FALSE)) return FALSE; + if (usages->UsagePage != HID_USAGE_PAGE_GENERIC) return TRUE; + if (usages->Usage != HID_USAGE_GENERIC_GAMEPAD && usages->Usage != HID_USAGE_GENERIC_JOYSTICK) return TRUE; if (!check_bus_option(L"Enable SDL", 1) && check_bus_option(L"DisableInput", 0)) prefer_hidraw = TRUE; @@ -666,12 +668,16 @@ static DWORD CALLBACK bus_main_thread(void *args) { struct device_desc desc = event->device_created.desc; if (desc.is_hidraw && !desc.usages.UsagePage) desc.usages = get_hidraw_device_usages(event->device); - if (!desc.is_hidraw != !is_hidraw_enabled(desc.vid, desc.pid)) + if (!desc.is_hidraw != !is_hidraw_enabled(desc.vid, desc.pid, &desc.usages)) { - WARN("ignoring %shidraw device %04x:%04x\n", desc.is_hidraw ? "" : "non-", desc.vid, desc.pid); + WARN("ignoring %shidraw device %04x:%04x with usages %04x:%04x\n", desc.is_hidraw ? "" : "non-", + desc.vid, desc.pid, desc.usages.UsagePage, desc.usages.Usage); break; } + TRACE("creating %shidraw device %04x:%04x with usages %04x:%04x\n", desc.is_hidraw ? "" : "non-", + desc.vid, desc.pid, desc.usages.UsagePage, desc.usages.Usage); + device = bus_create_hid_device(&event->device_created.desc, event->device); if (device) IoInvalidateDeviceRelations(bus_pdo, BusRelations); else -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5352
From: Rémi Bernon <rbernon(a)codeweavers.com> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=56450 --- dlls/winebus.sys/main.c | 119 ++++++++++++++++++++++++++-------------- 1 file changed, 77 insertions(+), 42 deletions(-) diff --git a/dlls/winebus.sys/main.c b/dlls/winebus.sys/main.c index dceba4747b8..044b5e0141c 100644 --- a/dlls/winebus.sys/main.c +++ b/dlls/winebus.sys/main.c @@ -119,19 +119,6 @@ static NTSTATUS unix_device_start(DEVICE_OBJECT *device) return winebus_call(device_start, ¶ms); } -static NTSTATUS unix_device_get_report_descriptor(DEVICE_OBJECT *device, BYTE *buffer, UINT length, UINT *out_length) -{ - struct device_extension *ext = (struct device_extension *)device->DeviceExtension; - struct device_descriptor_params params = - { - .device = ext->unix_device, - .buffer = buffer, - .length = length, - .out_length = out_length - }; - return winebus_call(device_get_report_descriptor, ¶ms); -} - static void unix_device_set_output_report(DEVICE_OBJECT *device, HID_XFER_PACKET *packet, IO_STATUS_BLOCK *io) { struct device_extension *ext = (struct device_extension *)device->DeviceExtension; @@ -572,6 +559,66 @@ static void keyboard_device_create(void) IoInvalidateDeviceRelations(bus_pdo, BusRelations); } +static NTSTATUS get_device_descriptors(UINT64 unix_device, BYTE **report_desc, UINT *report_desc_length, + HIDP_DEVICE_DESC *device_desc) +{ + struct device_descriptor_params params = + { + .device = unix_device, + .out_length = report_desc_length, + }; + NTSTATUS status; + + status = winebus_call(device_get_report_descriptor, ¶ms); + if (status != STATUS_SUCCESS && status != STATUS_BUFFER_TOO_SMALL) + { + ERR("Failed to get device %#I64x report descriptor, status %#lx\n", unix_device, status); + return status; + } + + if (!(params.buffer = RtlAllocateHeap(GetProcessHeap(), 0, *report_desc_length))) + return STATUS_NO_MEMORY; + params.length = *report_desc_length; + + if ((status = winebus_call(device_get_report_descriptor, ¶ms))) + { + ERR("Failed to get device %#I64x report descriptor, status %#lx\n", unix_device, status); + RtlFreeHeap(GetProcessHeap(), 0, params.buffer); + return status; + } + + params.length = *report_desc_length; + status = HidP_GetCollectionDescription(params.buffer, params.length, PagedPool, device_desc); + if (status != HIDP_STATUS_SUCCESS) + { + ERR("Failed to get device %#I64x report descriptor, status %#lx\n", unix_device, status); + RtlFreeHeap(GetProcessHeap(), 0, params.buffer); + return status; + } + + *report_desc = params.buffer; + return STATUS_SUCCESS; +} + +static USAGE_AND_PAGE get_hidraw_device_usages(UINT64 unix_device) +{ + HIDP_DEVICE_DESC device_desc; + USAGE_AND_PAGE usages = {0}; + UINT report_desc_length; + BYTE *report_desc; + NTSTATUS status; + + if (!(status = get_device_descriptors(unix_device, &report_desc, &report_desc_length, &device_desc))) + { + usages.UsagePage = device_desc.CollectionDesc[0].UsagePage; + usages.Usage = device_desc.CollectionDesc[0].Usage; + HidP_FreeCollectionDescription(&device_desc); + RtlFreeHeap(GetProcessHeap(), 0, report_desc); + } + + return usages; +} + static DWORD bus_count; static HANDLE bus_thread[16]; @@ -617,10 +664,11 @@ static DWORD CALLBACK bus_main_thread(void *args) break; case BUS_EVENT_TYPE_DEVICE_CREATED: { - const struct device_desc *desc = &event->device_created.desc; - if (!desc->is_hidraw != !is_hidraw_enabled(desc->vid, desc->pid)) + struct device_desc desc = event->device_created.desc; + if (desc.is_hidraw && !desc.usages.UsagePage) desc.usages = get_hidraw_device_usages(event->device); + if (!desc.is_hidraw != !is_hidraw_enabled(desc.vid, desc.pid)) { - WARN("ignoring %shidraw device %04x:%04x\n", desc->is_hidraw ? "" : "non-", desc->vid, desc->pid); + WARN("ignoring %shidraw device %04x:%04x\n", desc.is_hidraw ? "" : "non-", desc.vid, desc.pid); break; } @@ -899,37 +947,24 @@ static NTSTATUS pdo_pnp_dispatch(DEVICE_OBJECT *device, IRP *irp) else if (ext->state == DEVICE_STATE_REMOVED) status = STATUS_DELETE_PENDING; else if ((status = unix_device_start(device))) ERR("Failed to start device %p, status %#lx\n", device, status); - else + else if (!(status = get_device_descriptors(ext->unix_device, &ext->report_desc, &ext->report_desc_length, + &ext->collection_desc))) { - status = unix_device_get_report_descriptor(device, NULL, 0, &ext->report_desc_length); - if (status != STATUS_SUCCESS && status != STATUS_BUFFER_TOO_SMALL) - ERR("Failed to get device %p report descriptor, status %#lx\n", device, status); - else if (!(ext->report_desc = RtlAllocateHeap(GetProcessHeap(), 0, ext->report_desc_length))) - status = STATUS_NO_MEMORY; - else if ((status = unix_device_get_report_descriptor(device, ext->report_desc, ext->report_desc_length, - &ext->report_desc_length))) - ERR("Failed to get device %p report descriptor, status %#lx\n", device, status); - else if ((status = HidP_GetCollectionDescription(ext->report_desc, ext->report_desc_length, - PagedPool, &ext->collection_desc)) != HIDP_STATUS_SUCCESS) - ERR("Failed to parse device %p report descriptor, status %#lx\n", device, status); - else + status = STATUS_SUCCESS; + reports = ext->collection_desc.ReportIDs; + for (i = 0; i < ext->collection_desc.ReportIDsLength; ++i) { - status = STATUS_SUCCESS; - reports = ext->collection_desc.ReportIDs; - for (i = 0; i < ext->collection_desc.ReportIDsLength; ++i) + if (!(size = reports[i].InputLength)) continue; + size = offsetof( struct hid_report, buffer[size] ); + if (!(report = RtlAllocateHeap(GetProcessHeap(), HEAP_ZERO_MEMORY, size))) status = STATUS_NO_MEMORY; + else { - if (!(size = reports[i].InputLength)) continue; - size = offsetof( struct hid_report, buffer[size] ); - if (!(report = RtlAllocateHeap(GetProcessHeap(), HEAP_ZERO_MEMORY, size))) status = STATUS_NO_MEMORY; - else - { - report->length = reports[i].InputLength; - report->buffer[0] = reports[i].ReportID; - ext->last_reports[reports[i].ReportID] = report; - } + report->length = reports[i].InputLength; + report->buffer[0] = reports[i].ReportID; + ext->last_reports[reports[i].ReportID] = report; } - if (!status) ext->state = DEVICE_STATE_STARTED; } + if (!status) ext->state = DEVICE_STATE_STARTED; } RtlLeaveCriticalSection(&ext->cs); break; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5352
From: Rémi Bernon <rbernon(a)codeweavers.com> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=56450 --- dlls/winebus.sys/main.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/dlls/winebus.sys/main.c b/dlls/winebus.sys/main.c index 5244d7dda54..8d3b72d93f1 100644 --- a/dlls/winebus.sys/main.c +++ b/dlls/winebus.sys/main.c @@ -670,8 +670,10 @@ static DWORD CALLBACK bus_main_thread(void *args) if (desc.is_hidraw && !desc.usages.UsagePage) desc.usages = get_hidraw_device_usages(event->device); if (!desc.is_hidraw != !is_hidraw_enabled(desc.vid, desc.pid, &desc.usages)) { + struct device_remove_params params = {.device = event->device}; WARN("ignoring %shidraw device %04x:%04x with usages %04x:%04x\n", desc.is_hidraw ? "" : "non-", desc.vid, desc.pid, desc.usages.UsagePage, desc.usages.Usage); + winebus_call(device_remove, ¶ms); break; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5352
I tested this and it seems to work and fix [the bug](https://bugs.winehq.org/show_bug.cgi?id=56450). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5352#note_65354
Awesome, thanks! -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5352#note_65356
Tuomas Räsänen (@tuomasjjrasanen) commented about dlls/winebus.sys/main.c:
DWORD size;
if (check_bus_option(L"DisableHidraw", FALSE)) return FALSE; + if (usages->UsagePage != HID_USAGE_PAGE_GENERIC) return TRUE; + if (usages->Usage != HID_USAGE_GENERIC_GAMEPAD && usages->Usage != HID_USAGE_GENERIC_JOYSTICK) return TRUE;
if (!check_bus_option(L"Enable SDL", 1) && check_bus_option(L"DisableInput", 0))
Actually, does this mean that hidraw is now preferred for keyboard and mice regardless of DisableInput bus option? My understanding is that DisableInput=0 means that evdev is used which needs event devices, so doesn't this cause event devices to be removed for all other hardware devices than gamepads and joysticks? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5352#note_65376
On Tue Mar 19 16:41:16 2024 +0000, Tuomas Räsänen wrote:
Actually, does this mean that hidraw is now preferred for keyboard and mice regardless of DisableInput bus option? My understanding is that DisableInput=0 means that evdev is used which needs event devices, so doesn't this cause event devices to be removed for all other hardware devices than gamepads and joysticks? You're right, but we don't really support mouse and keyboard input through this anyway. Instead we use X11 / Wayland APIs for that.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5352#note_65381
participants (2)
-
Rémi Bernon -
Tuomas Räsänen