Fix Xbox One gamepad axis assignment on SDL_JOYSTICK_ANDROID API #7405
Conversation
For gamepads with three axis pairs (like most standard layout ones), SDL currently groups them in X,Y Z,RX RY,RZ order. It seems like the grouping X,Y RX,RY Z,RZ makes a lot more sense.
|
I should note: I named the issue specific to my case, but I would suspect that right now and for quite a while, SDL_JOYSTICK_ANDROID handled all gamepads with two sticks and two triggers wrong unless they have a SDL_GameControllerDB entry. |
|
Well, not ALL, but all that use AXIS_Z and AXIS_RZ for the trigger, which seems to be Xbox, Xbox One, Xbox 360 standard. If they use AXIS_LTRIGGER and AXIS_RTRIGGER, they'd be previously handled right and become wrong with this patch. Thus, we may need a better solution than this - I would suggest as an alternate option changing the sorting order of axes to sort AXIS_Z and AXIS_RZ after AXIS_RY, which seems cleanest, however that would break SDL_GameControllerDB mappings for existing gamepads. Thus, a decision needs to be made here. |
|
That fixing of the sort order would be: |
|
Hm... as for the re-sorting approach... I assume that any gamepads that currently have rightx or lefttrigger on a3 will be affected. gamecontrollerdb has 40 Android gamepads with more than four axes: Of those, only eight are likely broken if we change the sort oder: Those are: That looks impactful. Thus it may be required to first fix their mapping before doing the sort order change. The change from this PR itself won't need this, but may break automatic gamepad mappings for gamepads that use AXIS_LEFTTRIGGER and AXIS_RIGHTTRIGGER instead of AXIS_Z and AXIS_RZ. If only we could change the sort order for the automatic mapping only but keep gamecontrollerdb mapping as is... like, if we could export an extra bit of info from Java (basically, it'd suffice to know whether AXIS_Z exists), so the automatic mapping can be correct. The catch is, we can't put this info into the mask, as that ends up in the SDL ID. |
|
We could set another bit of info down the line, but it'd be hard to plumb this into SDL_PrivateGetGamepadMappingForGUID. |
|
If we were to do the alternate fix (sorting order), mappings would break as follows:
So... the RX/RY vs Z/RZ swap is annoying (Android docs generally say the swap is correct, so SDL's axes correspond to X, Y, Z, RZ, RX, RY in this order). Let's thus do some "surveying" of 05 entries for Android:
If axes are actually X, Y, Z, RX, RY, RZ, then two sensible possibilities exist:
If axes are actually X, Y, RX, RY, LEFTTRIGGER, RIGHTTRIGGER, then assignment is unambigous and we don't want to change it: leftx:a0,lefty:a1,rightx:a2,righty:a3,lefttrigger:a4,righttrigger:a5 - this seems to have 26 entries total. Changing the sorting order will ONLY break the mappings for the three gamepads listed above as not fixed: GPD XD Plus, NVIDIA Controller, PS5 Controller. I'd thus vote for the Xbox solution - let's change sorting order so it is X, Y, RX, RY, Z, RZ. Can anyone confirm this - I'll then change this PR to rather change the sorting function on the Java side. As for the broken mappings, I would suggest I could add code into SDL to simply ignore these eight lines until someone makes new ones (and could also send a PR to update them)? |
|
Thanks for the research! |
This reverts commit 8757687.
|
Re-pushed it as the sorting order change then. Would ask to squash when merging, of course. If you can cover PS5 and NVIDIA, that'd be great, as they're some of the more complicated cases. |
|
(BTW, I noticed the 05 prefix is defined to mean bluetooth, but the gamepad I have is a USB one - well, by default SDL doesn't even use this code path for USB gamepads anymore, but e.g. Ebitengine has derived code from this code, and I would like to update it in sync with SDL whichever decision we make) |
|
Sure. Can you add the SDL_gamepad_db.h changes to this PR so we have working mappings before and after this is merged? |
|
We don't know whether the gamepad is USB or Bluetooth. I assumed Bluetooth because at the time it was harder to connect USB than pair wirelessly with most Android devices. |
|
I will update these mappings in a few hours, sure! |
(Xbox ones still TBD)
|
I updated the NVIDIA and PS5 ones right away; also, I did my previous analysis on gamecontrollerdb.txt, not the include file, so I'll also analyze that. |
|
All mappings audited and updated. Now checking if I can delete some of the Xbox ones, i.e. if they fully match the default. |
|
Hm... I found 33 mappings that MAY be redundant with the default mappings after this change, but am not 100% sure as there may be extra axes or buttons that are intentionally unmapped. |
The SDLIDs of these seem wrong. Note how the latter two controllers have a button mask of just 0x7800, which only has four bits set (the dpad ones?), but assigns way more buttons. The first one here has a button mask of 0x07ff, which assigns buttons 0 to 10, and will be perfectly mapped automatically - but I am not going to do that, as the code line for it has a comment about some dpad issue that is worth keeping. |
|
So I conclude that I will delete none of these mappings. Some restrict available buttons or axes to a subset, and some have comments, and both may be worth keeping. |
|
So I confirmed the DualSense (PS5) controller mapping works correctly, but I connected an Xbox Elite 2 Core controller, which isn't in the mapping database, and the automatic mapping looked identical to the existing Xbox Elite 2 entry, but it didn't work correctly. Pulling the left trigger activated the right trigger mapping and pressing up and down on the d-pad activated the left trigger. Here's a log with the controller connected, pressing the d-pad in a clockwise circle, and then pulling the left trigger: |
|
I then connected an Xbox Series X controller, which was in the mapping database, and it worked correctly: |
|
This might just be an oddity of this controller on Android. Android reports that it has 14 axes. |
(cherry picked from commit 6f1f586)
|
I went ahead and added a mapping for the Xbox Elite controller in d08338d. |
|
Thanks for the research and the PR! |
|
Sure!
As for the Xbox Elite 2 Core, can you test that one in the app
https://play.google.com/store/apps/details?id=com.catalyst06.gamecontrollerverifier
- it can tell you the native Android axis and button IDs?
The way I use it is that I go to try/assign controller, ignore the pretty
graphics and just use the bottom raw view which corresponds to Android's
API. Essentially play with the axes and see what moves.
(Alternatively you could of course add debug code into the
SDLControllerManager.java file of SDL to log what happens)
…On Mon, Mar 6, 2023, 18:39 Sam Lantinga ***@***.***> wrote:
Thanks for the research and the PR!
—
Reply to this email directly, view it on GitHub
<#7405 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAB5NMCYLHTFR6GAAX5VY5DW2ZYU3ANCNFSM6AAAAAAVPTGCGE>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
|
I just added a binding using gamepadmap, now it works fine. :) |
|
Yeah, just would like to know what the Android-native axis IDs are, to
ensure the gamepad is really weird and rule out that my code has a bug :)
Am Di., 7. März 2023 um 08:14 Uhr schrieb Sam Lantinga <
***@***.***>:
… I just added a binding using gamepadmap, now it works fine. :)
—
Reply to this email directly, view it on GitHub
<#7405 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAB5NMD42FZU3LOGWSNVWC3W25MBNANCNFSM6AAAAAAVPTGCGE>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
|
It has AXIS_X, AXIS_Y, AXIS_Z, AXIS_RZ, AXIS_BRAKE, AXIS_RX, AXIS_RY, LTRIGGER, RTRIGGER, and AXIS_GAS. |
|
Great, thanks. So the old sorted order is: (X, Y), (Z, RX), (RY, RZ), HAT_X, HAT_Y, LTRIGGER, RTRIGGER, BRAKE, GAS The new sorted order is: (X, Y), (RX, RY), (Z, RZ), HAT_X, HAT_Y, LTRIGGER, RTRIGGER, BRAKE, GAS
To be fair I find that very odd - as the right stick is Z/RZ, I'd expect the right stick to control the triggers and the triggers to do nothing as unused LTRIGGER/RTRIGGER and RX/RY are unnecessarily taking up axis slots and SDL only ever auto assigns the three lowest axis pairs. Either way, it doesn't seem possible that this pad worked with the previous mapping. Your axis mapping is:
So the odd thing here is having an unused a4 that comes after righty (must be RZ) and before lefttrigger (which is BRAKE). AXIS_BRAKE is 23, but sorted as 22 due to brake/gas swap. AXIS_RZ is 14, unchanged by this patch. The automatic mapping would be correct if there were nothing between those. We have in between:
Based on your description, it seems like AXIS_HAT_Y isn't skipped for some reason... |
|
Also, without the sort order change, we should still get a wrong mapping, as all that'd happen is that Z (rightx) is sorted before RX. There's no space in the mapping for that. Even if I am totally mistaken, this would instead make the right stick misassigned. |
|
Hm... skipping hat axes is done only if it has SOURCE_CLASS_JOYSTICK. Could it be that it has SOURCE_DPAD or SOURCE_GAMEPAD instead? |
|
Here's the output of axes as they're added: The sorting isn't relevant because RX and RY aren't present. |
|
I assume this is post-order, as I do see the GAS/BRAKE swap here: Your order is, converted to Android's enums:
As AXIS_RX and AXIS_RY don't even exist, the change from this PR shouldn't even affect this gamepad at all. The confusion you observe probably comes from AXIS_RUDDER. The automatic pairing would be (X, Y), (Z, RZ), (RUDDER, BRAKE), which is sure wrong for the last one. AXIS_RUDDER is your unused a4 that was messing things up - so your pad should have needed an explicit mapping both before and after this change (and the mapping is the same, even). |
|
Yes, I had never connected this controller to Android before, so I'm sure it would have needed a mapping. From your description above, this looks correct to me. |
|
Hm... should THROTTLE/RUDDER even be paired ever? Pairing BRAKE/GAS is also questionable, but seems right for your pad (and the pedals in a car are also kinda laid out like the triggers on a gamepad). I wonder if the proper fix for gamepads like yours would be sorting AXIS_THROTTLE/AXIS_RUDDER behind the others. But this seems even more tricky to evaluate - anyway, it's established that this gamepad does require special handling for now and that it's not a bug. |
|
Hm... if I understand it right, throttle and rudder are simply the Y and X axes of a flight joystick (with some fun reversal of direction that the flight simulators themselves usually take care of). In a real airplane, according to FAA, rudder is controlled with a pair of pedals, but as there's only one axis in the MotionEvent, this info doesn't really help here. https://www.amazon.com/Thrustmaster-T-Flight-Hotas-One-xbox/dp/B07643TW2V/ref=sr_1_4?crid=2BL6LFTM230TY&keywords=joystick%2Bwith%2Bthrottle%2Band%2Brudder&qid=1678384004&sprefix=joystick%2Bwith%2Bthrottle%2Band%2Brudde%2Caps%2C110&sr=8-4&ufe=app_do%3Aamzn1.fos.006c50ae-5d4c-4777-9bc0-4513d670b6bc&th=1 seems to have a "progressive tilting lever" for rudder on the throttle stick. I guess that makes them behave a bit like a regular pair of axes of a joystick, except that at least the throttle doesn't snap back to center when you release it. It still kinda makes sense to pair them. Of course... AXIS_THROTTLE has index 19 and AXIS_RUDDER has index 20, makes throttle appear as an X axis or left trigger, and rudder as an Y axis or a right trigger. Maybe these two should be flipped around just like gas/brake, then they kinda behave right when treated as a right stick X and Y axis - but OTOH, shouldn't only flight sticks even export these? And flight sticks are too specific to support by standard layout anyway... so, meh. |
|
Agreed, I think we shouldn't try to automatically map anything that has a throttle or rudder. |
This commit mirrors libsdl-org/SDL#7405 (libsdl-org/SDL@6f1f586). Note that for SDL, this code is used a lot less than for Ebitengine, as SDL mostly migrated to HIDAPI and direct USB device access rather than using Android's APIs. For Bluetooth devices, however, the Java APIs are used the same way. This was the remaining problem to be solved to automatically support standard layout on most standard gamepads (this should cover most Xbox-ish and PS-ish gamepads on the market). In particular this covers gamepads with the following assignment: - Left stick = X/Y, right stick = Z/RZ, triggers = LEFTTRIGGER/RIGHTTRIGGER (which basically is what Android docs say and some PS gamepads do) - Left stick = X/Y, right stick = RX/RY, triggers = Z/RZ (Xbox gamepad style, apparently) - Left stick = X/Y, right stick = RX/RY, triggers = LEFTTRIGGER/RIGHTTRIGGER (Not sure if this exists, but it's conceivable) As we found on the SDL pull request discussion, gamepads that offer flight controls (e.g. THROTTLE and RUDDER) will likely not work well, before and after this change. Fixes hajimehoshi#2557
|
Seems like I can't get this change into SDL_GameControllerDB - see here: mdqinc/SDL_GameControllerDB#671 (comment) I could offer setting an unused GUID bit (e.g. given axis mask can currently be at most 0x003f, could use 0x803f to represent pads where the sorting order change affected anything, i.e. where both AXIS_Z and any of AXIS_RX or AXIS_RY exist. Would you be OK with me sending a followup PR for that? |
|
Yes, that would be fine. Maybe a sort order version of 2 bits or something like that? |
This commit mirrors libsdl-org/SDL#7405 (libsdl-org/SDL@6f1f586). Note that for SDL, this code is used a lot less than for Ebitengine, as SDL mostly migrated to HIDAPI and direct USB device access rather than using Android's APIs. For Bluetooth devices, however, the Java APIs are used the same way. This was the remaining problem to be solved to automatically support standard layout on most standard gamepads (this should cover most Xbox-ish and PS-ish gamepads on the market). In particular this covers gamepads with the following assignment: - Left stick = X/Y, right stick = Z/RZ, triggers = LEFTTRIGGER/RIGHTTRIGGER (which basically is what Android docs say and some PS gamepads do) - Left stick = X/Y, right stick = RX/RY, triggers = Z/RZ (Xbox gamepad style, apparently) - Left stick = X/Y, right stick = RX/RY, triggers = LEFTTRIGGER/RIGHTTRIGGER (Not sure if this exists, but it's conceivable) As we found on the SDL pull request discussion, gamepads that offer flight controls (e.g. THROTTLE and RUDDER) will likely not work well, before and after this change. Closes #2557
Description
For gamepads with three axis pairs (like most standard layout ones), SDL currently groups them in X,Y Z,RX RY,RZ order.
It seems like the grouping X,Y RX,RY Z,RZ makes a lot more sense.
More details in the attached issue.
Tested: no degradation of with SDL_JOYSTICK_HIDAPI, and fixes axis assignment with SDL_JOYSTICK_ANDROID.
Existing Issue(s)
#7404