Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve detection of gamepads on Linux #59412

Closed
wants to merge 1 commit into from

Conversation

madmiraal
Copy link
Contributor

Requires gamepads to have a right x and y axis.

#57612 reduced the restriction on gamepads from having buttons and both a left and right x and y axis (joystick) to having buttons and either a left or right x and y axis (joystick). This was required to support gamepads that only have a right joystick.

However there are probably a lot of devices that have both buttons and a left x and y axis that are not gamepads and will be incorrectly detected as such. Therefore this PR increases the restriction on gamepads to have buttons and a right x and y axis. This will prevent devises that have buttons and only a left x and y axis from being detected as a gamepad, but allow devices that have buttons and a right x and y axis (joystick) from being detected as a gamepad.

Should fix #59250.

Note: I'm unable to reproduce #59250 so I cannot confirm that it fixes #59250. I also don't have a gamepad with only a right joystick e.g. Nintendo Switch Right Joy-Con; so I also cannot confirm that it doesn't revert the issue fixed by #57612. Therefore both @thomaslenzi and @maiself should test with their respective setups and confirm that this PR is good.

Requires gamepads to have a right x and y axis.
@thomaslenzi
Copy link

I can confirm that this PR solves my issue (#59250).
My mouse and trackpad are not detected as gamepads anymore and my two Xbox 360 controllers are detected correctly.
Unfortunately I don't have Joycons or other gamepads to perform further tests on my side.
Thank you for your work!

@maiself
Copy link
Contributor

maiself commented Mar 26, 2022

Here's some output from a quick test project on my system with various patches applied...

master (including #57612):

0 Wacom Intuos BT S Pen 050000005761636f6d20496e74756f00 connected: true is_known: false
1 Wacom Intuos BT S Pad 050000005761636f6d20496e74756f00 connected: true is_known: false
2 Joy-Con (L) 050000007e0500000620000001800000 connected: true is_known: true
3 Joy-Con (R) 050000007e0500000720000001800000 connected: true is_known: true

We see non-gamepad devices along with gamepads.

This pull request (#59412):

0 Joy-Con (R) 050000007e0500000720000001800000 connected: true is_known: true

We lose the Wacom tablet, but also the left Joy-Con!

But also note that the Wacom devices have Input.is_joy_known() returning false. While they were incorrectly detected as joystick devices, they were not detected as gamepads. This is because they lack a gamepad mapping.

#59250 doesn't make any mention of this distinction, but I doubt the touchpad was actually detected as a gamepad. The Godot documentation could probably be a bit more clear in this area. While it is weird to have touchpads and drawing tablets alongside joysticks, it might be better to have false positives, than to have false negatives. SDL2 will also sometimes report such devices as joysticks...

A real, proper solution might be problematic. Taken from https://www.kernel.org/doc/html/v4.15/input/gamepad.html (emphasis added):

All gamepads that follow the protocol described here map BTN_GAMEPAD. This is an alias for BTN_SOUTH/BTN_A. It can be used to identify a gamepad as such. However, not all gamepads provide all features, so you need to test for all features that you need, first. How each feature is mapped is described below.

Legacy drivers often don’t comply to these rules. As we cannot change them for backwards-compatibility reasons, you need to provide fixup mappings in user-space yourself. Some of them might also provide module-options that change the mappings so you can advise users to set these.

The left analog-stick is reported as ABS_X, ABS_Y. The right analog stick is reported as ABS_RX, ABS_RY. Zero, one or two sticks may be present.

With patch in postscript:

0 Joy-Con (L) 050000007e0500000620000001800000 connected: true is_known: true
1 Joy-Con (R) 050000007e0500000720000001800000 connected: true is_known: true

No Wacom, both gamepads. This is what the spec describes, however I don't have many gamepads to test with, only some Switch and PS3 controllers, and don't know how older / less common devices might report themselves.

PS:

diff --git a/platform/linuxbsd/joypad_linux.cpp b/platform/linuxbsd/joypad_linux.cpp
index 65d53b266f..81adf973b2 100644
--- a/platform/linuxbsd/joypad_linux.cpp
+++ b/platform/linuxbsd/joypad_linux.cpp
@@ -333,9 +333,7 @@ void JoypadLinux::open_joypad(const char *p_path) {
        }
 
        // Check if the device supports basic gamepad events
-       bool has_abs_left = (test_bit(ABS_X, absbit) && test_bit(ABS_Y, absbit));
-       bool has_abs_right = (test_bit(ABS_RX, absbit) && test_bit(ABS_RY, absbit));
-       if (!(test_bit(EV_KEY, evbit) && test_bit(EV_ABS, evbit) && (has_abs_left || has_abs_right))) {
+       if (!(test_bit(EV_KEY, evbit) && test_bit(BTN_GAMEPAD, keybit))) {
            close(fd);
            return;
        }

@madmiraal
Copy link
Contributor Author

@maiself

diff --git a/platform/linuxbsd/joypad_linux.cpp b/platform/linuxbsd/joypad_linux.cpp
index 65d53b266f..81adf973b2 100644
--- a/platform/linuxbsd/joypad_linux.cpp
+++ b/platform/linuxbsd/joypad_linux.cpp
@@ -333,9 +333,7 @@ void JoypadLinux::open_joypad(const char *p_path) {
        }
 
        // Check if the device supports basic gamepad events
-       bool has_abs_left = (test_bit(ABS_X, absbit) && test_bit(ABS_Y, absbit));
-       bool has_abs_right = (test_bit(ABS_RX, absbit) && test_bit(ABS_RY, absbit));
-       if (!(test_bit(EV_KEY, evbit) && test_bit(EV_ABS, evbit) && (has_abs_left || has_abs_right))) {
+       if (!(test_bit(EV_KEY, evbit) && test_bit(BTN_GAMEPAD, keybit))) {
            close(fd);
            return;
        }

I think limiting the test to any device that had a single, specific button would both potentially include other non-gamepad devices being identified as gamepads (à la #59250) and not include devices that are gamepad devices such as the Nintendo Switch Left Joy-Con (have you tested it?).

Perhaps testing whether a device has a right joystick and a BTN_GAMEPAD or a D-Pad and a left joystick would be a suitable test.

Accurately identifying devices that are gamepads is difficult as your referenced documentation suggests, and it's not limited to Linux (see #47656).

it might be better to have false positives, than to have false negatives

#47656 and #59250 highlight why including non-gamepads as gamepads is a problem.

@Zireael07
Copy link
Contributor

Both false positives and false negatives suck. I'd go with false negatives (it's easier to have a list of joysticks that aren't detected by code X than to filter out all non-joystick clutter that gets picked up, like the touchpads and fingerprint readers that often have weird drivers)

Copy link
Member

@bruvzg bruvzg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like an arbitrary restriction. I'm not sure why having these devices in the gamepad list is an issue. If device input is triggering unwanted actions, maybe we should ignore device input until mapping is set.

@akien-mga akien-mga modified the milestones: 4.0, 4.1 Feb 17, 2023
Copy link
Contributor

@Eoin-ONeill-Yokai Eoin-ONeill-Yokai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like a mistake to require both ABS L and R in order to determine whether a device is a gamepad or not. This definition will leave a lot of devices unsupported that people would expect (previously mentioned joycons, for example.)

My 2 cents on the matter here is that we should probably take a page out of SDLs book and be more pragmatic about classifying the feature-set of a given device before filtering the device. We then create a few different masks that we deem to be the "bare minimum gamepad" based on some of the gamepads on the market that have limited feature sets. Additionally, we should consider making a "excludes from gamepad" mask that filters out problematic devices that often have some input overlap with game controllers (drawing tablets, etc.)

So the breakdown of what should be done here on a design level

  • Create an enum listing all possible device features that forms a mask. This includes buttons, axis, touch screen, touch pad, stylus, etc.
  • Create a method that returns the feature set of a given device with a breakdown of all possible features. It is worth noting in the function name that this is a guess, not a concrete definition and all future modification to guess work would need to be done in this given method.
  • Define a constant that defines a (few) minimum viable controllers. For example, we currently (in master) would have a set of the following minimum requirements HAS_KEYS | HAS_AXIS | HAS_AXIS_LEFT and HAS_KEYS | HAS_AXIS | HAS_AXIS_RIGHT. This creates a easy to parse minimum set of required features for contributors to modify later as more edge cases crop up.
  • Define a constant that defines a feature blacklist. Any device that has these features are automatically removed from being considered valid. For example, filtering out tablets would involve filtering devices with HAS_STYLUS. This gives us a concrete list of features that we decide as a group as being "exclusive" from being considered a game controller.
  • Algorithm of open_joypad would roughly follow the logic of
    • Guess feature-set by calling previously mentioned method; Base the logic of guesses off SDL procedure if necessary.
    • Match against any minimum viable controller flag combo, return and close if we don't match any of the minimum requirements.
    • Return and Close device if we match against any gamepad feature on the blacklist. This will help remove laptop touch screens and graphics tablets from listings.

So, in short, I think that we should design from the ground up with regards to improving the input situation and start by trying to get a better understanding of the USB device's entire feature-set before attempting to enforce rules.

@smcv
Copy link

smcv commented May 10, 2023

For users who have access to input devices that the project maintainers don't have (and especially unusual ones), it might be useful to provide a dump of the device's evdev capability flags, so that any future changes to this heuristic can be tested against the input device metadata without having to have the device physically available.

The evemu-describe tool from evemu is a good way to get this information. test/testevdev.c in SDL contains data structures derived from evemu-describe output, which are used to test SDL's heuristic, and pull requests like libsdl-org/SDL#7597 add more entries to it.

Steam's steam-runtime-input-monitor (available via ~/.steam/root/ubuntu12_32/steam-runtime/run.sh steam-runtime-input-monitor) is also a useful tool, although it doesn't display the full data dump by default.

@clayjohn clayjohn modified the milestones: 4.1, 4.x Jun 20, 2023
@clayjohn
Copy link
Member

Bumping to 4.x as it is clear there is no consensus yet on the correct solution.

@pennyloafers
Copy link

Hello, I'm wondering if there is a Linux way to block Godot from looking at the Touchpad as a device, or disable the device so Godot does not consider it? (while also allowing it to still function for the OS)

To chime in, I like @maiself proposal, I think rare or exotic controllers (like drawing tablets and stylus) should be lower priority in this case. Also wouldn't these type of controllers be able to provide relative mouse input? In the least we could append BTN_STYLUS to the bit test for a joypad.

In my case where I'm trying to support mouse-and-keys and controller, it makes it mighty difficult to develop on a laptop with a touchpad in Linux.

I think there should also be a way for the user to ignore or accept specific devices sensed by Godot. maybe even an API to build a controller selection to the end user.

@pennyloafers
Copy link

Update: Thinking about my own words, I was able to work up a solution in my case since all joypad action events have a device associated with them. you could disable joystick inputs by change the "all" devices (-1) to a value of your actual joypad, or to a device number that is not utilized. (e.g. device 7). Then with a small script and an OptionBox update every joypad action event to an acceptable device or a uninitialized device number.

@smcv
Copy link

smcv commented Aug 21, 2023

we should probably take a page out of SDLs book

SDL is the closest thing the Linux world has to a standard application-facing API for game-related interfaces (and in particular the closest equivalent of DirectX on Windows), so either using SDL or following what it does reasonably closely would seem like a good idea. Conversely, if Godot has tricks for doing better than SDL currently can, please share them with the SDL developers so that everyone can benefit.

As I'm sure you've noticed, the Linux kernel and libudev interfaces are really low level, and in some cases the Linux gamepad interface spec doesn't match the behaviour of real gamepads (the Linux gamepad interface spec documents one convention, but Android documentation recommends something incompatible, and older Linux drivers for Xbox and Playstation controllers use a third incompatible convention; see SDL source code for details).

Steam's steam-runtime-input-monitor (available via ~/.steam/root/ubuntu12_32/steam-runtime/run.sh steam-runtime-input-monitor) is also a useful tool, although it doesn't display the full data dump by default.

For users of the Steam beta client, steam-runtime-input-monitor now includes a hexdump of the evdev properties and the HID report descriptor in its output, making it a superset of evemu-describe.

@RandomShaper
Copy link
Member

From the PR meeting:

We'd rather have false positives than false negatives.

For the time being, we'd suggest addressing the root issue in a simpler, yet more focused, way: blacklist devices that contain "touchpad" in their name (case-insentive, just in case).

In the future we may have a more exhaustive white and/or blacklisting logic, involving other substrings and vendor-device ids.

@geekley
Copy link

geekley commented Oct 30, 2023

blacklist devices that contain "touchpad" in their name (case-insentive, just in case).

Yes, please match it case-insensitive as my device name is "SynPS/2 Synaptics TouchPad".
In the script I made to workaround this bug (while this isn't merged yet), I'm also matching "trackpad" and "clickpad" just in case, using the regex /\b(?:touch|track|click)pad\b/i. I don't know if there are problematic devices with those names, but I assume it should at least not cause any issues.

@geekley
Copy link

geekley commented Nov 11, 2023

I updated the script (see #59250 (comment)). Commenting here to notify whoever might be interested.

Reasons:

  • Even a touchpad device may not appear immediately at startup.
  • My Wacom drawing tablet being recognized as a joypad doesn't seem right to me (I get joypad input when I merely approach the pen, even though it's meant as a pointer device, like a mouse). Though I admit ignoring this one is arguable. Should we? Are there other ways to get its input?

@Calinou
Copy link
Member

Calinou commented Nov 12, 2023

I suggest not using a regex and using a list of matches against the lowercase controller name string, so that it keeps working when Godot is compiled with the regex module is disabled (which people tend to do when targeting mobile/web platforms for binary size reasons).

@geekley
Copy link

geekley commented Nov 14, 2023

I suggest not using a regex

I updated it again, and put this version in a gist.

@geekley
Copy link

geekley commented Nov 15, 2023

I'm having another gamepad issue and thought I should mention it here: flathub/org.godotengine.Godot#144
Sorry if this is the wrong place to link it (it happens in the flatpak version, so I opened the issue there, but it could also be due to some upstream bug).

Does anyone else have this issue on flatpak (or with no udev) when using a single-USB multi-joypad device?

@geekley
Copy link

geekley commented Dec 12, 2023

I updated my gist to send input events once to reset to zero in all axes and buttons when a device is disabled - otherwise axes would be stuck if you were moving the touchpad at the game's startup.
Dunno if this info is relevant for this PR at all, but I'm leaving this comment here too just in case.

@akien-mga
Copy link
Member

Superseded by #93352.

@akien-mga akien-mga closed this Jul 9, 2024
@akien-mga akien-mga removed this from the 4.x milestone Jul 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Laptop touchpads are seen as gamepads