[PATCH v2 0/2] MR9789: winebus: Use smallest report size for joystick axis
Some games don't handle 32-bit sized joystick axes well. Assetto Corsa Rally, which was released a month ago as an early access and is still v0.1, is a recent example. The proof of concept 16-bit patch resolves the problem for many users and hardware on Linux, (https://github.com/ValveSoftware/Proton/issues/9220#issuecomment-3647591465) and users with open-source hardware confirmed the behavior of the game on Windows. Since it's the case that the game is non-compliant while the wine is perfectly compliant, we might be able to push and wait for the developer to release the fix, but I believe that the fix will also help to improve compatibility for some old, non-maintained, non-compliant games. -- v2: winebus: Use 16-bit relative axis range for bus_sdl https://gitlab.winehq.org/wine/wine/-/merge_requests/9789
From: SeongChan Lee <foriequal@gmail.com> --- dlls/winebus.sys/hid.c | 66 ++++++++++++++++++++++++++------- dlls/winebus.sys/unix_private.h | 4 +- 2 files changed, 54 insertions(+), 16 deletions(-) diff --git a/dlls/winebus.sys/hid.c b/dlls/winebus.sys/hid.c index 46ad0c69065..7973981f23f 100644 --- a/dlls/winebus.sys/hid.c +++ b/dlls/winebus.sys/hid.c @@ -256,7 +256,14 @@ BOOL hid_device_add_hatswitch(struct unix_device *iface, INT count) return TRUE; } -static BOOL hid_device_add_axis_count(struct unix_device *iface, BOOL rel, BYTE count, +static BYTE hid_device_determine_axis_size(LONG min, LONG max) +{ + if (min >= -128 && max <= 127) return 8; + if (min >= -32768 && max <= 32767) return 16; + return 32; +} + +static BOOL hid_device_add_axis_count(struct unix_device *iface, BOOL rel, BYTE size, BYTE count, USAGE usage_page, const USAGE *usages) { struct hid_device_state *state = &iface->hid_device_state; @@ -268,13 +275,21 @@ static BOOL hid_device_add_axis_count(struct unix_device *iface, BOOL rel, BYTE ERR("axes should be added before buttons or hatswitches!\n"); else if ((state->bit_size % 8)) ERR("axes should be byte aligned, missing padding!\n"); - else if (state->bit_size + 32 * count > 0x80000) + else if (size != 8 && size != 16 && size != 32) + ERR("unsupported axis size, not one of 8, 16, 32!\n"); + else if (state->bit_size + size * count > 0x80000) ERR("report size overflow, too many elements!\n"); + else if (state->abs_axis_count + state->rel_axis_count + count > ARRAY_SIZE(state->axis_byte_offsets)) + ERR("axis usage overflow, too many elements!\n"); else if (rel) { - if (!state->rel_axis_count) state->rel_axis_start = offset; + for (i = 0; i < count; ++i) { + int axis = state->abs_axis_count + state->rel_axis_count + i; + state->axis_byte_offsets[axis] = offset + i * size / 8; + state->axis_sizes[axis] = size; + } state->rel_axis_count += count; - state->bit_size += 32 * count; + state->bit_size += size * count; return TRUE; } else @@ -286,13 +301,15 @@ static BOOL hid_device_add_axis_count(struct unix_device *iface, BOOL rel, BYTE } for (i = 0; i < count; ++i) { + state->axis_byte_offsets[state->abs_axis_count + i] = offset + i * size / 8; + state->axis_sizes[state->abs_axis_count + i] = size; + state->abs_axis_usages[state->abs_axis_count + i].UsagePage = usage_page; state->abs_axis_usages[state->abs_axis_count + i].Usage = usages[i]; } - if (!state->abs_axis_count) state->abs_axis_start = offset; state->abs_axis_count += count; - state->bit_size += 32 * count; + state->bit_size += size * count; return TRUE; } @@ -312,17 +329,18 @@ BOOL hid_device_add_axes(struct unix_device *iface, BYTE count, USAGE usage_page { END_COLLECTION, }; + BYTE size = hid_device_determine_axis_size(min, max); const BYTE template[] = { LOGICAL_MINIMUM(4, min), LOGICAL_MAXIMUM(4, max), - REPORT_SIZE(1, 32), + REPORT_SIZE(1, size), REPORT_COUNT(1, count), INPUT(1, Data|Var|(rel ? Rel : Abs)), }; int i; - if (!hid_device_add_axis_count(iface, rel, count, usage_page, usages)) + if (!hid_device_add_axis_count(iface, rel, size, count, usage_page, usages)) return FALSE; if (!hid_report_descriptor_append(desc, template_begin, sizeof(template_begin))) @@ -1457,26 +1475,46 @@ void *hid_device_create(const struct hid_device_vtbl *vtbl, SIZE_T size) #ifdef WORDS_BIGENDIAN # define LE_ULONG(x) RtlUlongByteSwap((ULONG)(x)) +# define LE_USHORT(x) RtlUshortByteSwap((USHORT)(x)) #else # define LE_ULONG(x) ((ULONG)(x)) +# define LE_USHORT(x) ((USHORT)(x)) #endif +static BOOL hid_device_set_axis(struct hid_device_state* state, ULONG axis, LONG value) +{ + USHORT offset = state->axis_byte_offsets[axis]; + BYTE size = state->axis_sizes[axis]; + switch (size) { + case 8: + *(state->report_buf + offset) = (BYTE)value; + break; + case 16: + *(USHORT *)(state->report_buf + offset) = LE_USHORT(value); + break; + case 32: + *(ULONG *)(state->report_buf + offset) = LE_ULONG(value); + break; + default: + return FALSE; + } + + return TRUE; +} + BOOL hid_device_set_abs_axis(struct unix_device *iface, ULONG index, LONG value) { struct hid_device_state *state = &iface->hid_device_state; - ULONG offset = state->abs_axis_start + index * 4; if (index >= state->abs_axis_count) return FALSE; - *(ULONG *)(state->report_buf + offset) = LE_ULONG(value); - return TRUE; + return hid_device_set_axis(state, index, value); } BOOL hid_device_set_rel_axis(struct unix_device *iface, ULONG index, LONG value) { struct hid_device_state *state = &iface->hid_device_state; - ULONG offset = state->rel_axis_start + index * 4; + ULONG axis = state->abs_axis_count + index; if (index >= state->rel_axis_count) return FALSE; - *(ULONG *)(state->report_buf + offset) = LE_ULONG(value); - return TRUE; + return hid_device_set_axis(state, axis, value); } BOOL hid_device_set_button(struct unix_device *iface, ULONG index, BOOL is_set) diff --git a/dlls/winebus.sys/unix_private.h b/dlls/winebus.sys/unix_private.h index 11328caee08..c39c8787247 100644 --- a/dlls/winebus.sys/unix_private.h +++ b/dlls/winebus.sys/unix_private.h @@ -190,10 +190,10 @@ struct hid_physical struct hid_device_state { ULONG bit_size; + USHORT axis_byte_offsets[64]; + BYTE axis_sizes[64]; USAGE_AND_PAGE abs_axis_usages[32]; - USHORT abs_axis_start; USHORT abs_axis_count; - USHORT rel_axis_start; USHORT rel_axis_count; USHORT hatswitch_start; USHORT hatswitch_count; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9789
From: SeongChan Lee <foriequal@gmail.com> --- dlls/winebus.sys/bus_sdl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dlls/winebus.sys/bus_sdl.c b/dlls/winebus.sys/bus_sdl.c index bc7904059ed..0048dea8712 100644 --- a/dlls/winebus.sys/bus_sdl.c +++ b/dlls/winebus.sys/bus_sdl.c @@ -373,7 +373,7 @@ static NTSTATUS build_joystick_report_descriptor(struct unix_device *iface, cons for (i = 0; i < ball_count; i++) { if (!hid_device_add_axes(iface, 2, relative_axis_usages[2 * i].UsagePage, - &relative_axis_usages[2 * i].Usage, TRUE, INT32_MIN, INT32_MAX)) + &relative_axis_usages[2 * i].Usage, TRUE, INT16_MIN, INT16_MAX)) return STATUS_NO_MEMORY; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9789
SDL provides 16-bit normalized values for all axes, so I changed the min/max range of relative axes to INT16_MIN, INT16_MAX, too. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9789#note_125774
Tomasz Pakuła (@Lawstorant) commented about dlls/winebus.sys/hid.c:
return TRUE; }
-static BOOL hid_device_add_axis_count(struct unix_device *iface, BOOL rel, BYTE count, +static BYTE hid_device_determine_axis_size(LONG min, LONG max) +{ + if (min >= -128 && max <= 127) return 8; + if (min >= -32768 && max <= 32767) return 16; + return 32; +}
Axes can have unsigned values and this doesn't take this into consideration. ```suggestion:-5+0 static BYTE hid_device_determine_axis_size(LONG min, ULONG max) { if (min >= -128 && max <= 255) return 8; if (min >= -32768 && max <= 65535) return 16; return 32; } ``` -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9789#note_125775
On Thu Dec 18 08:39:06 2025 +0000, Tomasz Pakuła wrote:
Axes can have unsigned values and this doesn't take this into consideration. ```suggestion:-5+0 static BYTE hid_device_determine_axis_size(LONG min, ULONG max) { if (min >= -128 && max <= 255) return 8; if (min >= -32768 && max <= 65535) return 16; return 32; } ``` I originally wrote the predicate as `(min >= -128 && max <= 127) || (min >= 0 && max <= 255)` .
But I wasn't sure about how the signed and unsigned integer conversions would be handled between the C language, Wine, SDL, evdev, and games, so I've let the next biggest size cover them. It would use a smaller size if the range is -32768-32767, and it would behave the same as before if the range is 0-65535. It's not perfect, but it's better than before in some cases without potentially breaking something:stuck_out_tongue_winking_eye:. SDL always provides signed 16-bit range (-32768, 32767) axes, `struct input_value` in `evdev` has a `__s32 value` field, and the HID spec says, "Otherwise, all integer values are signed values represented in 2’s complement format," so I assumed they're normally signed values. But I'm not sure how to interpret "If Logical Minimum and Logical Maximum are both positive values then a sign bit is unnecessary in the report field and the contents of a field can be assumed to be an unsigned value." in the HID spec. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9789#note_125781
On Thu Dec 18 09:50:30 2025 +0000, SeongChan Lee wrote:
I originally wrote the predicate as `(min >= -128 && max <= 127) || (min
= 0 && max <= 255)` . But I wasn't sure about how the signed and unsigned integer conversions would be handled between the C language, Wine, SDL, evdev, and games, so I've let the next biggest size cover them. It would use a smaller size if the range is -32768-32767, and it would behave the same as before if the range is 0-65535. It's not perfect, but it's better than before in some cases without potentially breaking something:stuck_out_tongue_winking_eye:. SDL always provides signed 16-bit range (-32768, 32767) axes, `struct input_value` in `evdev` has a `__s32 value` field, and the HID spec says, "Otherwise, all integer values are signed values represented in 2’s complement format," so I assumed they're normally signed values. But I'm not sure how to interpret "If Logical Minimum and Logical Maximum are both positive values then a sign bit is unnecessary in the report field and the contents of a field can be assumed to be an unsigned value." in the HID spec. Simple. As much as it doesn't make much difference, signed values should be used for axes that have a defined center (like steering wheel_ and unsigned values should be used for axes without a center (like pedals). God only knows why Moza for example has this flipped and steering range is 0-65535 but all other axes (including pedals) are defined as -32768:32767. This actually creates a a bit of a deadzone in the middle of the pedal stroke on linux if fuzz/flat values aren't corrected.
As much as it doesn't matter for the SDL bus since it always normalizes values, it would still matter with udev bus. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9789#note_125782
On Thu Dec 18 10:04:54 2025 +0000, Tomasz Pakuła wrote:
Simple. As much as it doesn't make much difference, signed values should be used for axes that have a defined center (like steering wheel_ and unsigned values should be used for axes without a center (like pedals). God only knows why Moza for example has this flipped and steering range is 0-65535 but all other axes (including pedals) are defined as -32768:32767. This actually creates a a bit of a deadzone in the middle of the pedal stroke on linux if fuzz/flat values aren't corrected. As much as it doesn't matter for the SDL bus since it always normalizes values, it would still matter with udev bus. Currently, an axis with a range of 0-65536 created from a device on the udev bus will be classified as 32 bit instead of 16 bit.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9789#note_125804
Tomasz Pakuła (@Lawstorant) commented about dlls/winebus.sys/hid.c:
return TRUE; }
-static BOOL hid_device_add_axis_count(struct unix_device *iface, BOOL rel, BYTE count, +static BYTE hid_device_determine_axis_size(LONG min, LONG max) +{ + if (min >= -128 && max <= 127) return 8; + if (min >= -32768 && max <= 32767) return 16;
Instead of raw values, I'd go for INT8_MIN, UINT8_MAX, INT16_MIN, UINT16_MAX -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9789#note_125805
On Thu Dec 18 12:25:24 2025 +0000, Tomasz Pakuła wrote:
Currently, an axis with a range of 0-65536 created from a device on the udev bus will be classified as 32 bit instead of 16 bit. Hmm, now that I think of it, it's possible, though unlikely, to have an axis that would define it's range as -128:255 and it would not fit into 8 bits.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9789#note_125807
participants (3)
-
SeongChan Lee -
SeongChan Lee (＠foriequal0)
-
Tomasz Pakuła (＠Lawstorant)