godot icon indicating copy to clipboard operation
godot copied to clipboard

Use SDL for joypad input on Linux

Open EIREXE opened this issue 1 year ago • 23 comments

SDL is loaded in the same way other Linux system libraries are loaded by using a wrapped and dlopen.

Optionally, SDL can be dynamically linked into the binary.

Currently for Linux only since that platform direly needs it, but should be easy to make work on Windows once stable.

Proposal at: godotengine/godot-proposals#9000

P.S: I've made sure to strip the files as much as possible, but unless you want to be pulling your hair out by stripping files individually this is the best we can do IMO.

  • Bugsquad edit, closes godotengine/godot-proposals#9000 and closes https://github.com/godotengine/godot/issues/59250

EIREXE avatar Feb 04 '24 04:02 EIREXE

Does this PR support using a more recent SDL library if it's statically linked using an environment variable? See https://github.com/godotengine/godot/pull/86180#issuecomment-1919004562.

Calinou avatar Feb 05 '24 18:02 Calinou

Does this PR support using a more recent SDL library if it's statically linked using an environment variable? See #86180 (comment).

This PR doesn't do static linking at all, it instead either does normal linux shared linking (without use_sowrap) or dynamic loading (if use_sowrap is enabled).

As far as I understand, SDL_DYNAMIC_API is something SDL itself takes care of, not ourselves. (assuming the SDL we are linking or loading is built with dynamic api support that is).

EIREXE avatar Feb 05 '24 21:02 EIREXE

After some testing in production for Project Heartbeat I realised I made a mistake and was polling constantly, which is no bueno for CPU usage, so I switched to waiting with a timeout (so we can do the exit condition properly when the input thread is disposed of).

EIREXE avatar Feb 21 '24 11:02 EIREXE

SDL by default adds handlers for sigint/sigquit. You can use SDL_SetHint to disable that behavior before SDL_Init.

For example: https://github.com/mpv-player/mpv-examples/blob/master/libmpv/sdl/main.c#L53

tdaven avatar Mar 11 '24 17:03 tdaven

SDL by default adds handlers for sigint/sigquit. You can use SDL_SetHint to disable that behavior before SDL_Init.

For example: https://github.com/mpv-player/mpv-examples/blob/master/libmpv/sdl/main.c#L53

I'll implement these changes today, ty

EIREXE avatar Mar 12 '24 08:03 EIREXE

Changes should be done now

EIREXE avatar Mar 15 '24 14:03 EIREXE

I Had a problem wear Gamepads on Linux were being reported twice but not on Windows [90795] however this Pull Request [87925] from my testing has fixed all issues

Output from 4.2.1 - Gentoo Linux Gamepad 0 = PS5 Controller Gamepad 1 = PS5 Controller Gamepad 2 = PS4 Controller Gamepad 3 = PS4 Controller Gamepad 4 = Gamepad 5 =

Output from 4.3 PR [52eff38] - Gentoo Linux Gamepad 0 = DualSense Wireless Controller Gamepad 1 = PS4 Controller Gamepad 2 = Gamepad 3 = Gamepad 4 = Gamepad 5 =

jlogostini avatar Apr 18 '24 14:04 jlogostini

I am uncertain if this is intended behavior or a bug ether way if I connected my Left Joycon via Bluetooth it's eating 2 device slots causing issues with split-screen input

Gamepad 0 = Joy-Con (L) Gamepad 1 = Joy-Con (L) (IMU) Gamepad 2 = Gamepad 3 = Gamepad 4 = Gamepad 5 =

jlogostini avatar Apr 19 '24 17:04 jlogostini

I am uncertain if this is intended behavior or a bug ether way if I connected my Left Joycon via Bluetooth it's eating 2 device slots causing issues with split-screen input

Gamepad 0 = Joy-Con (L) Gamepad 1 = Joy-Con (L) (IMU) Gamepad 2 = Gamepad 3 = Gamepad 4 = Gamepad 5 =

What version of SDL are you running in your OS? from what I can tell that issue was fixed in SDL two years ago.

EIREXE avatar Apr 20 '24 08:04 EIREXE

I am on Gentoo Linux everything is up to date and I am running SDL 2.30.2

jlogostini avatar Apr 23 '24 06:04 jlogostini

I just ran some tests. As expected this makes my WiiU Gamecube adapter to work, as it is supported correctly in SDL. It won't work without this PR.

I agree with the discussion and think this offers huge benefits in term of controller support and code re-usability. This feature is important for me. I will follow this PR closely.

Thank you for your work.

bemug avatar May 02 '24 10:05 bemug

I just ran some tests. As expected this makes my WiiU Gamecube adapter to work, as it is supported correctly in SDL. It won't work without this PR.

I agree with the discussion and think this offers huge benefits in term of controller support and code re-usability. This feature is important for me. I will follow this PR closely.

Thank you for your work.

I've also been shipping it in Project Heartbeat Phoenix for a while without any major hiccup.

EIREXE avatar May 03 '24 15:05 EIREXE

Used SDL_HINT_JOYSTICK_THREAD flag, and changed the styling for setting SDL_HINT_NO_SIGNAL_HANDLERS

EIREXE avatar May 13 '24 20:05 EIREXE

I confirm that this fixes the multiple problems my dualsense edge controller has with godot. On the current master, multiple buttons are inverted and the triggers and joysticks are also mixed up, which makes it unusable.

Vibrations work correctly and I'm able to CTRL+C to quit the application normally. So the problems raised by @Calinou seem to have been fixed?

20kb size increase is totally acceptable to me given that controller support for me or my users would be broken without it.

Is this PR considered for 4.3?

geowarin avatar May 16 '24 23:05 geowarin

I'm generally favorable to exploring using SDL for gamepad input, as we've been playing catch up with them over the years and we're lacking in many areas.

There are a couple problems with the approach in this PR though, which will require further work before a solution is mergeable (not suggesting to do it now, this requires more discussion with the team first):

  • If we start using SDL for input, we should aim to do it for all platforms, not just Linux. We also have significant issues on Windows and macOS that it might help address.
  • This PR uses SDL as a system library, which assumes that SDL is available on all Linux systems. That's fine for Steam as it's part of the Steam Linux runtime, but it's not a reasonable assumption for general portable Linux binaries. If we want to use SDL, we will probably need to build and link it statically together with Godot.
  • If we're building SDL and linking it statically for all platforms, we should assess how much of SDL we actually need to build/include. It might be that we just need to reuse hidapi, or their input code, without pulling in the whole framework.

akien-mga avatar May 17 '24 06:05 akien-mga

I'm generally favorable to exploring using SDL for gamepad input, as we've been playing catch up with them over the years and we're lacking in many areas.

There are a couple problems with the approach in this PR though, which will require further work before a solution is mergeable (not suggesting to do it now, this requires more discussion with the team first):

* If we start using SDL for input, we should aim to do it for all platforms, not just Linux. We also have significant issues on Windows and macOS that it might help address.

* This PR uses SDL as a system library, which assumes that SDL is available on all Linux systems. That's fine for Steam as it's part of the Steam Linux runtime, but it's not a reasonable assumption for general portable Linux binaries. If we want to use SDL, we will probably need to build and link it statically together with Godot.

* If we're building SDL and linking it statically for all platforms, we should assess how much of SDL we actually need to build/include. It might be that we just need to reuse hidapi, or their input code, without pulling in the whole framework.
  • I do have it working on Windows, but I think starting with Linux where its essentially a necessity is the best way to go.
  • Generally, its not recommended to statically link SDL, since you should always try to use the system SDL install and allow the user to replace it for compatibility with future controllers.
  • I honestly don't think we should do this, we aren't pulling the whole framework here either, I personally think we should also be using SDL for windowing and for input, but I think that would make this PR too big.

I think we have to think of SDL like we do about Pulse or xlib, it's a system library that lets us do things, we wouldn't ship our own pulse library version ever, would we?

If you really want to statically link SDL even though it's not a very good idea, SDL has a dynamic api loading mechanism that users can use to replace the library.

Additionally, note that this PR falls back to the traditional Godot input driver if SDL is not present.

EIREXE avatar May 17 '24 07:05 EIREXE

Additionally, note that this PR falls back to the traditional Godot input driver if SDL is not present.

Something to note, there should be a way to communicate that the user is missing SDL in this case, otherwise they might blame the game for poor input (especially since the built-in input driver will likely become neglected after this). Also telling people to install dependencies on things like itch pages is often a major turnaway for people.

So I'd suggest just adding a checkbox in the export settings for bundling SDL with the game.

ettiSurreal avatar May 17 '24 09:05 ettiSurreal

Makes sense tbh, i'll work on static linking in a bit

EIREXE avatar May 17 '24 19:05 EIREXE

What's the status for this PR? Now that 4.4 has started, I'd appreciate it if this was merged early on to catch as many errors as possible before 4.4 reaches stable.

Is it just static linking that needs to be worked on? Personally, I think it should be statically linked, just for those who don't have SDL installed on their system, since SDL comes with a dynamic loading system anyway.

mubinulhaque avatar Sep 13 '24 16:09 mubinulhaque

Is it just static linking that needs to be worked on?

See https://github.com/godotengine/godot/pull/87925#issuecomment-2116845337, which is still relevant. If we merge SDL gamepad support on Linux for 4.4, I feel this also needs to be done for Windows and macOS in the same release.

Calinou avatar Sep 16 '24 22:09 Calinou

Rebased against master, still haven't had time to work on statically linked SDL, but will do so ASAP.

EIREXE avatar Oct 01 '24 00:10 EIREXE

What do you want me to do with the SDL library? Should it be in thirdparty like all other builtin libraries?

EIREXE avatar Oct 01 '24 17:10 EIREXE

Nevermind, it's going to be big trouble to get it to compile inside godot, so we should just make static builds like we do with nir.

EIREXE avatar Oct 01 '24 18:10 EIREXE