linux: Improve gamepad mapping heuristic to accept Android conventions

[smcv]

Please review and test carefully: I do not have the affected hardware, so I am unable to test this.

Description

• linux: Reduce magic numbers when mapping gamepad axes

  The bitfield mapped has two different sets of meanings, depending
  whether we're setting up the triggers or the d-pad. Represent them
  as symbolic constants rather than opaque integers.

• linux: Improve gamepad mapping heuristic to accept Android conventions

  This heuristic for gamepads without a more specific mapping already
  tried two incompatible conventions for handling triggers: the Linux
  Gamepad Specification uses hat switch 2 for the triggers (for whatever
  reason), but the de facto standard set by the drivers for older Xbox
  and Playstation controllers represents each trigger as the Z-axis of
  the nearest analog stick.

  Android documentation encourages Bluetooth gamepad manufacturers to use
  a third incompatible convention where the left and right triggers are
  represented as the brake and gas pedals of a driving simulator
  controller. The Android convention also changes the representation of
  the right stick: instead of using X and Y rotation as a second pair
  of axes, Android uses Z position as a second horizontal axis, and
  Z rotation as a second vertical axis.

  Try to cope gracefully with all of these. This will hopefully resolve
  the issue described in Add Xbox Series X controller bluetooth profile #5406 (when using unpatched kernels).

Existing Issue(s)

There is no clear issue report as far as I know, but #5406 is trying to solve the same issue differently.

[smcv]

@jelly, please could you try reverting your local SDL changes and using an unpatched kernel, then applying this?

If you also apply

--- a/src/joystick/linux/SDL_sysjoystick.c
+++ b/src/joystick/linux/SDL_sysjoystick.c
@@ -80,7 +80,7 @@
 #define DEBUG_INPUT_EVENTS 1
 #endif
 
-#if 0
+#if 1
 #define DEBUG_GAMEPAD_MAPPING 1
 #endif

then testgamepad will show some useful information from the affected function.

[smcv]

I can at least confirm that if I comment out all the entries in s_GamepadMappings, this heuristic still gives me the correct mapping for a wired Xbox 360 gamepad, so that at least has not regressed.

I don't think I have any hardware that follows one of the other two conventions for how triggers and the right stick are represented. #7791 suggests that the 8BitDo SNES30 via Bluetooth would be another suitable test device for the Android convention.

[slouken]

This looks good to me, FYI.

[slouken]

The only downside to this approach is that we might start interpreting wheels as gamepads and break them in racing games under Proton. Since there are many more gamepads than wheels, I think this could be okay, we just have to make sure that the controller isn't a known wheel or other type of controller before trying to apply an automatic gamepad mapping.

[smcv]

The only downside to this approach is that we might start interpreting wheels as gamepads and break them in racing games under Proton.

I believe this should be reasonably regression-proof, because the code I've changed in this PR doesn't alter whether a device is considered to be a gamepad or whether we return a mapping: it only alters the level of completeness of the mapping it gets.

We early-return if !joystick->hwdata->has_key[BTN_GAMEPAD], which should rule out most PC-specific steering wheels. For example G27 Racing Wheel, 0003:046d:c29b v0111 and Logitech Driving Force, 0003:046d:c294 v0100 in test/testevdev.c don't get a mapping.

Steering wheels designed for games consoles already identify as an oddly-shaped gamepad (the same behaviour as Guitar Hero controllers and other games console accessories), for example Thrustmaster Racing Wheel FFB in the recorded test data.

(I don't have any of these devices myself, so the only way I can help you to identify them is by curating other people's sample data.)

The only way I can see that this change could cause a regression is that if we have both BRAKE and Z, or both GAS and RZ, it changes the interpretation to prefer the driving-control-style axes. I think we have to do this, because gamepads that follow the Android convention will have all four of those axes, and for those gamepads we definitely want to interpret Z and RZ as being the right stick.

If gamepads exist that have RX and RY as the right stick, Z and RZ as analog triggers, and vestigial GAS and BRAKE inputs that aren't physically connected, then this change would make them regress (their triggers would stop working). Or if gamepads exist that have RX and RY as the right stick, Z and RZ as analog triggers, and GAS and BRAKE inputs somewhere else (foot pedals? back buttons? shoulder buttons?), then this will make them regress, because the analog triggers will no longer do anything, and you'll now need to press GAS and BRAKE to get trigger inputs.

If you're worried about such devices maybe existing, we could tighten the conditions, for example only using GAS for the left trigger if the controller has all of GAS, BRAKE, Z, RZ but lacks RX and RY? But I think the more complicated we make this heuristic, the harder it will be to explain or understand.

If we refactor this heuristic so it can act on mock data rather than literally having to open a joystick device (like the way I tested SDL_EVDEV_GuessDeviceClass in testevdev.c), then we could unit-test it to ensure there are no regressions for any of the devices for which we have recorded test data?

[smcv]

racing games under Proton

For Proton specifically, Proton specifically excludes anything that SDL identifies as a SDL_JOYSTICK_TYPE_WHEEL or SDL_JOYSTICK_TYPE_FLIGHT_STICK from using the game controller code path.

[slouken]

Yeah, good point about BTN_GAMEPAD, and your other points make me comfortable with this change.

I like the idea of refactoring it to work on test data. It would be interesting to see the mappings that are generated for the existing set of device data.

[smcv]

I've marked this as not a draft, but I'd really appreciate some testing from @jelly, since this is a fix for hardware that I don't have.

[slouken]

I was able to test and confirm this patch fixes the issue in #5406

Thanks!