[PATCH 0/1] MR181: winebus: Optionally split joysticks with more than 6 axes.

[Timo Zuccarello (＠timozuccarello)]

Add registry option "Split Controllers" and optionally split joysticks
with more than 6 axes to improve compatibility with applications that
do not support more than 6 axes where on Windows a driver would be
installed that splits the device.

Signed-off-by: Timo Zuccarello <timo(a)zuccarello.eu>

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/181

[PATCH 1/1] winebus: Optionally split joysticks with more than 6 axes.

[Timo Zuccarello]

From: Timo Zuccarello <timo(a)zuccarello.eu>

Add registry option "Split Controllers" and optionally split joysticks
with more than 6 axes to improve compatibility with applications that
do not support more than 6 axes where on Windows a driver would be
installed that splits the device.

Signed-off-by: Timo Zuccarello <timo(a)zuccarello.eu>
---
 dlls/winebus.sys/bus_sdl.c | 88 ++++++++++++++++++++++++++++++++------
 dlls/winebus.sys/main.c    |  3 ++
 dlls/winebus.sys/unixlib.h |  2 +
 3 files changed, 81 insertions(+), 12 deletions(-)

diff --git a/dlls/winebus.sys/bus_sdl.c b/dlls/winebus.sys/bus_sdl.c
index a5621972493..98dfda560ec 100644
--- a/dlls/winebus.sys/bus_sdl.c
+++ b/dlls/winebus.sys/bus_sdl.c
@@ -28,6 +28,7 @@
 #include <fcntl.h>
 #include <stdarg.h>
 #include <stdlib.h>
+#include <stdio.h>
 #include <unistd.h>
 #include <sys/types.h>
 #include <dlfcn.h>
@@ -143,6 +144,7 @@ struct sdl_device
     int effect_ids[256];
     int effect_state[256];
     LONG effect_flags;
+    int axis_offset;
 };
 
 static inline struct sdl_device *impl_from_unix_device(struct unix_device *iface)
@@ -155,7 +157,17 @@ static struct sdl_device *find_device_from_id(SDL_JoystickID id)
     struct sdl_device *impl;
 
     LIST_FOR_EACH_ENTRY(impl, &device_list, struct sdl_device, unix_device.entry)
-        if (impl->id == id) return impl;
+        if (impl->id == id && impl->axis_offset == 0) return impl;
+
+    return NULL;
+}
+
+static struct sdl_device *find_device_from_id_and_offset(SDL_JoystickID id, unsigned int offset)
+{
+    struct sdl_device *impl;
+
+    LIST_FOR_EACH_ENTRY(impl, &device_list, struct sdl_device, unix_device.entry)
+        if (impl->id == id && impl->axis_offset == offset) return impl;
 
     return NULL;
 }
@@ -185,7 +197,8 @@ static BOOL descriptor_add_haptic(struct sdl_device *impl)
     USAGE usages[16];
 
     if (!pSDL_JoystickIsHaptic(impl->sdl_joystick) ||
-        !(impl->sdl_haptic = pSDL_HapticOpenFromJoystick(impl->sdl_joystick)))
+        !(impl->sdl_haptic = pSDL_HapticOpenFromJoystick(impl->sdl_joystick)) ||
+        impl->axis_offset > 0)
         impl->effect_support = 0;
     else
     {
@@ -196,7 +209,7 @@ static BOOL descriptor_add_haptic(struct sdl_device *impl)
             impl->effect_support |= WINE_SDL_HAPTIC_RUMBLE;
     }
 
-    if (pSDL_JoystickRumble && !pSDL_JoystickRumble(impl->sdl_joystick, 0, 0, 0))
+    if (pSDL_JoystickRumble && !pSDL_JoystickRumble(impl->sdl_joystick, 0, 0, 0) && impl->axis_offset == 0)
         impl->effect_support |= WINE_SDL_JOYSTICK_RUMBLE;
 
     if (impl->effect_support & EFFECT_SUPPORT_HAPTICS)
@@ -262,6 +275,17 @@ static NTSTATUS build_joystick_report_descriptor(struct unix_device *iface)
     USAGE_AND_PAGE physical_usage;
 
     axis_count = pSDL_JoystickNumAxes(impl->sdl_joystick);
+    if (options.split_controllers && axis_count > 6)
+    {
+        if (impl->axis_offset == axis_count / 6)
+        {
+            axis_count = axis_count % 6;
+        }
+        else
+        {
+            axis_count = 6;
+        }
+    }
     if (axis_count > ARRAY_SIZE(absolute_usages))
     {
         FIXME("More than %zu absolute axes found, ignoring.\n", ARRAY_SIZE(absolute_usages));
@@ -275,8 +299,16 @@ static NTSTATUS build_joystick_report_descriptor(struct unix_device *iface)
         ball_count = ARRAY_SIZE(relative_usages) / 2;
     }
 
-    hat_count = pSDL_JoystickNumHats(impl->sdl_joystick);
-    button_count = pSDL_JoystickNumButtons(impl->sdl_joystick);
+    if (impl->axis_offset == 0)
+    {
+        hat_count = pSDL_JoystickNumHats(impl->sdl_joystick);
+        button_count = pSDL_JoystickNumButtons(impl->sdl_joystick);
+    }
+    else
+    {
+        hat_count = 0;
+        button_count = 0;
+    }
 
     if (!pSDL_JoystickGetType) physical_usage = device_usage;
     else switch (pSDL_JoystickGetType(impl->sdl_joystick))
@@ -904,6 +936,8 @@ static void sdl_add_device(unsigned int index)
     SDL_GameController *controller = NULL;
     const char *str;
     char guid_str[33];
+    int i;
+    int axis_count = 0;
 
     if ((joystick = pSDL_JoystickOpen(index)) == NULL)
     {
@@ -939,7 +973,7 @@ static void sdl_add_device(unsigned int index)
     if (controller) desc.is_gamepad = TRUE;
     else
     {
-        int button_count, axis_count;
+        int button_count;
 
         axis_count = pSDL_JoystickNumAxes(joystick);
         button_count = pSDL_JoystickNumButtons(joystick);
@@ -948,19 +982,41 @@ static void sdl_add_device(unsigned int index)
 
     TRACE("%s id %d, desc %s.\n", controller ? "controller" : "joystick", id, debugstr_device_desc(&desc));
 
-    if (!(impl = hid_device_create(&sdl_device_vtbl, sizeof(struct sdl_device)))) return;
-    list_add_tail(&device_list, &impl->unix_device.entry);
-    impl->sdl_joystick = joystick;
-    impl->sdl_controller = controller;
-    impl->id = id;
+    i = 0;
+    do {
+        if (!(impl = hid_device_create(&sdl_device_vtbl, sizeof(struct sdl_device)))) return;
+        list_add_tail(&device_list, &impl->unix_device.entry);
+        impl->sdl_joystick = joystick;
+        impl->sdl_controller = controller;
+        impl->id = id;
+        impl->axis_offset = i;
 
-    bus_event_queue_device_created(&event_queue, &impl->unix_device, &desc);
+        if (i != 0)
+        {
+            char offset_name[12];
+            int offset_name_length = snprintf(NULL, 0, " %d", i+1);
+            /* If there is not enough place to append " %d" replace some of the last characters. */
+            int offset_name_start = min(strlen(str), ARRAY_SIZE(desc.product) - 1 - offset_name_length);
+            /* Ensure we only write in desc.product and not past it.
+             * Otherwise just keep the name as is. */
+            if (offset_name_length < 12 && offset_name_start >= 0 && offset_name_start + offset_name_length < ARRAY_SIZE(desc.product))
+            {
+                snprintf(offset_name, 12, " %d", i+1);
+                ntdll_umbstowcs(offset_name, 12, desc.product + offset_name_start, 12);
+            }
+            TRACE("%s id %d, split for axis %d-%d.\n", controller ? "controller" : "joystick", id, 6*i, min(axis_count, 6*(i+1)-1));
+        }
+
+        bus_event_queue_device_created(&event_queue, &impl->unix_device, &desc);
+        ++i;
+    } while (options.split_controllers && !desc.is_gamepad && 6*i < axis_count);
 }
 
 static void process_device_event(SDL_Event *event)
 {
     struct sdl_device *impl;
     SDL_JoystickID id;
+    int axis_offset = 0;
 
     TRACE("Received action %x\n", event->type);
 
@@ -979,6 +1035,14 @@ static void process_device_event(SDL_Event *event)
     {
         id = ((SDL_JoyButtonEvent *)event)->which;
         impl = find_device_from_id(id);
+        if (impl && options.split_controllers && event->type == SDL_JOYAXISMOTION) {
+          axis_offset = ((SDL_JoyAxisEvent *)event)->axis / 6;
+          if (axis_offset > 0)
+          {
+            ((SDL_JoyAxisEvent*)event)->axis = ((SDL_JoyAxisEvent*)event)->axis % 6;
+            impl = find_device_from_id_and_offset(id, axis_offset);
+          }
+        }
         if (impl) set_report_from_joystick_event(impl, event);
         else WARN("failed to find device with id %d\n", id);
     }
diff --git a/dlls/winebus.sys/main.c b/dlls/winebus.sys/main.c
index 4588fee1b02..80a6da22496 100644
--- a/dlls/winebus.sys/main.c
+++ b/dlls/winebus.sys/main.c
@@ -720,6 +720,9 @@ static NTSTATUS sdl_driver_init(void)
     };
     NTSTATUS status;
 
+    bus_options.split_controllers = check_bus_option(L"Split Controllers", 0);
+    if (bus_options.split_controllers) TRACE("SDL controller splitting enabled\n");
+
     bus_options.map_controllers = check_bus_option(L"Map Controllers", 1);
     if (!bus_options.map_controllers) TRACE("SDL controller to XInput HID gamepad mapping disabled\n");
     sdl_bus_load_mappings(&bus_options);
diff --git a/dlls/winebus.sys/unixlib.h b/dlls/winebus.sys/unixlib.h
index a84a3331a25..c20d234c9ec 100644
--- a/dlls/winebus.sys/unixlib.h
+++ b/dlls/winebus.sys/unixlib.h
@@ -38,6 +38,7 @@ struct device_desc
     UINT input;
     UINT uid;
     BOOL is_gamepad;
+    UINT axis_offset;
 
     WCHAR manufacturer[MAX_PATH];
     WCHAR product[MAX_PATH];
@@ -47,6 +48,7 @@ struct device_desc
 struct sdl_bus_options
 {
     BOOL map_controllers;
+    BOOL split_controllers;
     /* freed after bus_init */
     UINT mappings_count;
     char **mappings;
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/181

[Rémi Bernon]

Rémi Bernon (@rbernon) commented about dlls/winebus.sys/bus_sdl.c:
> USAGE usages[16];
>
> if (!pSDL_JoystickIsHaptic(impl->sdl_joystick) ||
> -        !(impl->sdl_haptic = pSDL_HapticOpenFromJoystick(impl->sdl_joystick)))
> +        !(impl->sdl_haptic = pSDL_HapticOpenFromJoystick(impl->sdl_joystick)) ||
> +        impl->axis_offset > 0)

I thknk it would be better with the `!impl->axis_offset ||` condition first.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/181#note_1643

[Rémi Bernon]

Rémi Bernon (@rbernon) commented about dlls/winebus.sys/bus_sdl.c:
> impl->effect_support |= WINE_SDL_HAPTIC_RUMBLE;
>      }
>
> -    if (pSDL_JoystickRumble && !pSDL_JoystickRumble(impl->sdl_joystick, 0, 0, 0))
> +    if (pSDL_JoystickRumble && !pSDL_JoystickRumble(impl->sdl_joystick, 0, 0, 0) && impl->axis_offset == 0)

Same here.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/181#note_1644

[Rémi Bernon]

Rémi Bernon (@rbernon) commented about dlls/winebus.sys/bus_sdl.c:
> {
>          id = ((SDL_JoyButtonEvent *)event)->which;
>          impl = find_device_from_id(id);
> +        if (impl && options.split_controllers && event->type == SDL_JOYAXISMOTION) {
> +          axis_offset = ((SDL_JoyAxisEvent *)event)->axis / 6;
> +          if (axis_offset > 0)
> +          {
> +            ((SDL_JoyAxisEvent*)event)->axis = ((SDL_JoyAxisEvent*)event)->axis % 6;
> +            impl = find_device_from_id_and_offset(id, axis_offset);
> +          }
> +        }

I think this would be cleaner in a dedicated earlier `else if (event->type == SDL_JOYAXISMOTION && options.split_controllers)` block. Please also keep the original indentation and bracing styles.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/181#note_1649

[Rémi Bernon]

Rémi Bernon (@rbernon) commented about dlls/winebus.sys/unixlib.h:
> UINT input;
>      UINT uid;
>      BOOL is_gamepad;
> +    UINT axis_offset;

This isn't used anywhere.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/181#note_1645

[Rémi Bernon]

Rémi Bernon (@rbernon) commented about dlls/winebus.sys/bus_sdl.c:
> struct sdl_device *impl;
>
> LIST_FOR_EACH_ENTRY(impl, &device_list, struct sdl_device, unix_device.entry)
> -        if (impl->id == id) return impl;
> +        if (impl->id == id && impl->axis_offset == 0) return impl;
> +
> +    return NULL;
> +}
> +
> +static struct sdl_device *find_device_from_id_and_offset(SDL_JoystickID id, unsigned int offset)
> +{
> +    struct sdl_device *impl;
> +
> +    LIST_FOR_EACH_ENTRY(impl, &device_list, struct sdl_device, unix_device.entry)
> +        if (impl->id == id && impl->axis_offset == offset) return impl;

With the `axis_offset` change described below I think this should take an axis number as parameter, and look for a device which matches `impl->axis_offset <= axis && impl->axis_offset + impl->unix_device.hid_device_state.abs_axis_count > axis`.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/181#note_1648

[Rémi Bernon]

Rémi Bernon (@rbernon) commented about dlls/winebus.sys/main.c:
> };
>      NTSTATUS status;
>
> +    bus_options.split_controllers = check_bus_option(L"Split Controllers", 0);
> +    if (bus_options.split_controllers) TRACE("SDL controller splitting enabled\n");
> +

NP: Please remove this empty line.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/181#note_1646

[Rémi Bernon]

Rémi Bernon (@rbernon) commented about dlls/winebus.sys/bus_sdl.c:
> USAGE_AND_PAGE physical_usage;
>
> axis_count = pSDL_JoystickNumAxes(impl->sdl_joystick);
> +    if (options.split_controllers && axis_count > 6)
> +    {
> +        if (impl->axis_offset == axis_count / 6)
> +        {
> +            axis_count = axis_count % 6;
> +        }
> +        else
> +        {
> +            axis_count = 6;
> +        }
> +    }

Overall I'd say that the `axis_offset` field is a bit misleading, as it's not an offset but an index of 6-axis groups. I think it'd be better to have it truly an offset, so that devices may be split differently than in groups of 6 axes in the future without requiring too many changes.

For now it's okay to keep a hardcoded 6 axes limit here and in the device creation loop below.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/181#note_1647

[Rémi Bernon]

Rémi Bernon (@rbernon) commented about dlls/winebus.sys/bus_sdl.c:
> +            int offset_name_length = snprintf(NULL, 0, " %d", i+1);
> +            /* If there is not enough place to append " %d" replace some of the last characters. */
> +            int offset_name_start = min(strlen(str), ARRAY_SIZE(desc.product) - 1 - offset_name_length);
> +            /* Ensure we only write in desc.product and not past it.
> +             * Otherwise just keep the name as is. */
> +            if (offset_name_length < 12 && offset_name_start >= 0 && offset_name_start + offset_name_length < ARRAY_SIZE(desc.product))
> +            {
> +                snprintf(offset_name, 12, " %d", i+1);
> +                ntdll_umbstowcs(offset_name, 12, desc.product + offset_name_start, 12);
> +            }
> +            TRACE("%s id %d, split for axis %d-%d.\n", controller ? "controller" : "joystick", id, 6*i, min(axis_count, 6*(i+1)-1));
> +        }
> +
> +        bus_event_queue_device_created(&event_queue, &impl->unix_device, &desc);
> +        ++i;
> +    } while (options.split_controllers && !desc.is_gamepad && 6*i < axis_count);
> This could be simpler, without needing a special i != 0 case. You can fill the product buffer with something like that in the loop, before the `hid_device_create` and original `TRACE` call:

```C++
char buffer[ARRAY_SIZE(desc.product)];

if (axis_offset) snprintf(buffer, ARRAY_SIZE(buffer), "%s %u", product, axis_offset / 6);
else snprintf(buffer, ARRAY_SIZE(buffer), "%s", product);
ntdll_umbstowcs(buffer, strlen(buffer) + 1, desc.product, ARRAY_SIZE(desc.product));
```

With `product` the string retrieved from pSDL_JoystickName, replaced with a default value if it was `NULL`.

Imho you could use a for loop there, and replace i with an axis_offset variable. The increment would depend on `options.split_controllers`. The `desc.is_gamepad` case above should set `axis_count = 6`, so you don't have to bother about afterwards.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/181#note_1650

[Rémi Bernon]

Rémi Bernon (@rbernon) commented about dlls/winebus.sys/bus_sdl.c:
> SDL_GameController *controller = NULL;
>      const char *str;
>      char guid_str[33];
> +    int i;
> +    int axis_count = 0;

NP: Please use grouped declarations.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/181#note_1651