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.

Sign up for GitHub

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

Jump to bottom

[3.x] Improve validation of gamepads on Linux #67414

Open
wants to merge 1 commit into
base: 3.x
Choose a base branch
from
Open

[3.x] Improve validation of gamepads on Linux #67414

wants to merge 1 commit into from
+10 −10

Conversation

JyKin
Copy link

@JyKin JyKin commented Oct 14, 2022 •
edited by akien-mga

EDIT: Changed idea and provide new fix after discussion

Bugsquad edit:

old message:
cherry pick those commits from master :
8fe2ecf
b966ca6

@JyKin JyKin requested a review from a team as a code owner October 14, 2022 22:00
@JyKin JyKin mentioned this pull request Oct 14, 2022
Open
@Calinou Calinou added bug platform:linuxbsd topic:porting topic:input cherrypick:3.5 Considered for cherry-picking into a future 3.5.x release labels Oct 15, 2022
@Calinou Calinou added this to the 3.6 milestone Oct 15, 2022
@madmiraal
Copy link
Contributor

Note: #57612 (b966ca6) caused #59250. So, this probably shouldn't be merged without #59412 (bc17abc) which has not been approved, or some other better solution that more accurately identifies what is and what is not a gamepad.

@JyKin
Improve Linux Gamepad detection
456f128 
Check if device has key: should filter out motion-control input from
controller like recent playstation or nintendo.

Check if device has direction input: joystick or dpad.

Check if device has no touch-like input: should filter out touchscreen
and fingerprint reader.
@JyKin
Copy link
Author

JyKin commented Oct 18, 2022 •
edited

Ok, I went back and made a entirely new commit, without cherry-picking anything, with my solution.
I tested what I could (controller with motion controls, individual joycons) but I don't have wacom like tablet to verify they're not picked-up

@JyKin JyKin changed the title import fix for linux gamepad support provide fix for linux gamepad support Oct 18, 2022
@akien-mga akien-mga changed the title provide fix for linux gamepad support Improve validation of gamepads on Linux Dec 12, 2022
@akien-mga
Copy link
Member

WDYT about this solution @madmiraal?

I'm a bit unsure about trying yet another approach which would differ from the one now used in 4.0 - but the code has diverged so it's possible that the 4.0 code isn't applicable here as is. The added booleans here are also nice for readability of what this is checking exactly.

madmiraal
madmiraal suggested changes Dec 12, 2022
View reviewed changes
Copy link
Contributor

@madmiraal madmiraal left a comment

Choose a reason for hiding this comment

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

This solution is problematic. It requires a device to have keys to be treated as a gamepad, which is the opposite of the current behaviour, which requires a device not to have keys to be treated as a gamepad.

More importantly, this PR should be made against master not 3.x. As identified in the OP, the code on master has diverged from 3.x, because #40591 and #57612 haven't been backported to 3.x. This is not a reason to start making changes to 3.x without first making them on master.

@madmiraal
Copy link
Contributor

Accurately detecting gamepads is hard. #40591 made our detection of gamepads the same as SDL's detection of gamepads. However, SDL's detection of gamepads (and devices in general) has since moved on:
https://github.com/libsdl-org/SDL/blob/9eb4d1f020bdc43e32375942120214eee5d70fee/src/core/linux/SDL_evdev_capabilities.c#L78-L105
It's also trying to copy the logic used by systemd:
https://github.com/systemd/systemd/blob/30338b8b66174a87727c8a8493ebf3a043f65b9d/src/udev/udev-builtin-input_id.c#L208-L295

Clearly, getting this right is going to take a more effort than just #40591, #57612, #59412 and #67414.

@akien-mga akien-mga changed the title Improve validation of gamepads on Linux [3.x] Improve validation of gamepads on Linux Aug 18, 2023
AThousandShips
AThousandShips reviewed Aug 18, 2023
View reviewed changes
platform/x11/joypad_linux.cpp
!(test_bit(EV_ABS, evbit) &&
test_bit(ABS_X, absbit) && test_bit(ABS_Y, absbit) &&
test_bit(ABS_RX, absbit) && test_bit(ABS_RY, absbit))) {
// Check if the device supports basic gamepad events and not non-joystick events
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// Check if the device supports basic gamepad events and not non-joystick events
// Check if the device supports basic gamepad events and not non-joystick events.

Per the comment style.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AThousandShips AThousandShips AThousandShips left review comments

@madmiraal madmiraal madmiraal requested changes

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
bug cherrypick:3.5 Considered for cherry-picking into a future 3.5.x release platform:linuxbsd topic:input topic:porting
Projects
3.x release management
Status: PRs for 3.6 beta 1
Milestone
3.6
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@JyKin @madmiraal @akien-mga @AThousandShips @Calinou