Add support for SDL3 joystick input driver for Android, iOS, visionOS and Web #109645
Conversation
Nintorch
force-pushed
the
more-sdl-joypad-platforms
branch
from
a4474f6 to
4f21671
Compare
Nintorch
force-pushed
the
more-sdl-joypad-platforms
branch
from
4f21671 to
4e9de1a
Compare
|
Web seems to be extremely buggy:
Android is crashing instantly, in Will check iOS later. |
Thank you for testing, bruvzg! That's weird, for me on Windows on web I didn't have problems with dpad, face buttons, vibration and input lag with Dualshock 4 (the other problems did happen, I will try to fix them), and I didn't have crashes on Android. May I ask if there's any other information about the Android crash you're experiencing? |
|
Some extra info:
|
|
On iOS, linking fails due to following missing symbols: |
Oh, that's also strange, because the check in this PR for iOS export template passed. |
CI only build a static library, linking is done when project is exported. |
It's crashing on start, tested with "Joypads" demo on Pixel 8 Pro (Android 16) and POCO F3 (LineageOS, Android 15). |
Hm, is there any exception thrown or a JNI error shown? |
Here's full logcat output - https://gist.github.com/bruvzg/9822262842e885170289d40572fc4002 |
|
I see, thank you! I will try to fix it |
|
A fix for iOS: diff --git a/drivers/sdl/SCsub b/drivers/sdl/SCsub
index 3cd3761c4b..8bed438fe6 100644
--- a/drivers/sdl/SCsub
+++ b/drivers/sdl/SCsub
@@ -217,6 +217,7 @@ if env["builtin_sdl"]:
thirdparty_sources += [
"core/unix/SDL_appid.c",
"core/unix/SDL_poll.c",
+ "haptic/dummy/SDL_syshaptic.c",
"joystick/apple/SDL_mfijoystick.m",
"thread/pthread/SDL_syscond.c",
"thread/pthread/SDL_sysmutex.c",
diff --git a/drivers/sdl/SDL_build_config_private.h b/drivers/sdl/SDL_build_config_private.h
index 54c60f32a5..512894146e 100644
--- a/drivers/sdl/SDL_build_config_private.h
+++ b/drivers/sdl/SDL_build_config_private.h
@@ -155,6 +155,7 @@
#define HAVE_STDIO_H 1
#define HAVE_LIBC 1
#define SDL_JOYSTICK_MFI 1
+#define SDL_HAPTIC_DUMMY 1
#define SDL_TIMER_UNIX 1
#define SDL_THREAD_PTHREAD 1With this fix, everything seems to work. |
|
Thank you! Also, may I ask if the Android crash happened while running the editor build of Godot on your devices or the exported games? I just realized that I only initialize SDL in |
| // Web (Emscripten) defines | ||
| #elif defined(SDL_PLATFORM_EMSCRIPTEN) | ||
|
|
||
| #define SDL_PLATFORM_PRIVATE_NAME "Android" |
There was a problem hiding this comment.
| #define SDL_PLATFORM_PRIVATE_NAME "Android" | |
| #define SDL_PLATFORM_PRIVATE_NAME "Emscripten" |
I will need to include this fix later
|
|
||
| #ifdef ANDROID_ENABLED | ||
| // Ignore fingerprint sensors | ||
| if (joy_name_lower.begins_with("uinput-")) { |
There was a problem hiding this comment.
This has been fixed in upstream SDL, so I will need to remove this fix here
Nintorch
force-pushed
the
more-sdl-joypad-platforms
branch
8 times, most recently
from
47afe93 to
13ca900
Compare
This commit adds support for SDL joystick input for the rest of the platforms Godot Engine supports. Previously it was only available on desktop platforms (Windows, Linux and macOS). This commit also adds joystick vibration feature to the web platform in browsers that support this feature.
Nintorch
force-pushed
the
more-sdl-joypad-platforms
branch
from
13ca900 to
3f07147
Compare
There was a problem hiding this comment.
Are those changes intended?
There was a problem hiding this comment.
No, I wanted to test the web joysticks more easily. I will revert the changes later!
There was a problem hiding this comment.
Are those changes intended?
There was a problem hiding this comment.
No, I wanted to test the web joysticks more easily. I will revert the changes later!
| final int godotJoyId = mJoystickIds.get(deviceId); | ||
| handleJoystickButtonEvent(godotJoyId, button, false); | ||
| } | ||
| SDLControllerManager.onNativePadUp(deviceId, keyCode); |
There was a problem hiding this comment.
Does SDL specify which thread its native methods should be invoked on?
Depending on the answer, we may want to move invocations of the SDLControllerManager methods to the InputEventRunnable class as is done with the current joystick logic.
| protected static SDLGenericMotionListener_API14 mMotionListener; | ||
| protected static SDLGenericMotionListener_API14 getMotionListener() { | ||
| if (mMotionListener == null) { | ||
| if (Build.VERSION.SDK_INT >= 26 /* Android 8.0 (O) */) { | ||
| mMotionListener = new SDLGenericMotionListener_API26(); | ||
| } else if (Build.VERSION.SDK_INT >= 24 /* Android 7.0 (N) */) { | ||
| mMotionListener = new SDLGenericMotionListener_API24(); | ||
| } else { | ||
| mMotionListener = new SDLGenericMotionListener_API14(); | ||
| } | ||
| } | ||
|
|
||
| return mMotionListener; | ||
| } |
There was a problem hiding this comment.
mMotionListener should be initialized in the constructor for GodotInputHandler, and not be set as a static field.
| // TODO: Should this library be loaded here? | ||
| System.loadLibrary("godot_android") |
There was a problem hiding this comment.
You don't need to load godot_android, it's already handled for you.
| // Set up JNI | ||
| SDL.setupJNI() | ||
|
|
||
| // Initialize state | ||
| SDL.initialize() | ||
|
|
||
| mHIDDeviceManager = HIDDeviceManager.acquire(this) |
There was a problem hiding this comment.
This should be in Godot.kt, in the same section where we initialize GodotInputHandler.
| main { | ||
| manifest.srcFile 'AndroidManifest.xml' | ||
| java.srcDirs = ['src'] | ||
| java.srcDirs = ['src', '../../../../thirdparty/sdl/android-files'] |
There was a problem hiding this comment.
Where does the sdl android files originates from and what is their default package?
Are they available as a java or Android library we can just depend on?
There was a problem hiding this comment.
They originate from SDL's repository, "android-project" folder, their original package is org.libsdl.app. I couldn't get it to work with their original package, for some reason JNI code inside SDL's Android C code couldn't find the classes, but it does with Godot's package. (I'm also not experienced with Android programming, so that's most likely why it didn't work)
I just checked and SDL provides a compiled binary for Android. But is it possible to use it considering they have their own Activity class? (See https://github.com/libsdl-org/SDL/blob/main/android-project/app/src/main/java/org/libsdl/app/SDLActivity.java )
| if env["builtin_sdl"]: | ||
| env.Append(CPPDEFINES=["SDL_ENABLED"]) | ||
| else: | ||
| print_warning("`builtin_sdl` was explicitly disabled. Disabling SDL input driver support.") |
There was a problem hiding this comment.
| print_warning("`builtin_sdl` was explicitly disabled. Disabling SDL input driver support.") | |
| print_warning("`builtin_sdl` was explicitly disabled. Disabling SDL joystick input driver support.") |
Disabling SDL should only affect joystick input, right?
There was a problem hiding this comment.
Yes, thanks for the change!
| class GodotIOJavaWrapper; | ||
|
|
||
| struct ANativeWindow; | ||
| class JoypadSDL; |
There was a problem hiding this comment.
Should this be under an ifdef?
There was a problem hiding this comment.
Since it's only a class declaration, I think it's okay since we only need a pointer to this class inside the OS_Android class, that's also how it's done in other platforms.
|
Thank you for your review, m4gr3d! I'm not sure I will be able to work on this PR for a while, but when I do, I will make sure I include your requested changes! |
|
Just as a note, I think HIDAPI driver for Android won't mostly be needed since it's now possible to implement the motion sensors and LED light by using just the Android API, but I haven't found anything for the Dualsense adaptive triggers yet :( |
|
I have split this PR into 2 PRs for iOS, visionOS, and Web. Android still needs some more work done, particularly I think some of the Java input code has to be separated from the code that deals with Activity and View classes (I don't think Godot can use SDL's Activity and View classes), I might make a PR for that later if it's okay with SDL's developers. |
|
Although now that I think about it, is it worth it to integrate SDL's Android joystick driver into Godot? Would that fix any current issues? Or should we do it because that might allow the users to improve the SDL code? |
|
I decided it's not really worth it to integrate SDL's Android and web joystick drivers since that wouldn't fix a lot of issues and it might introduce regressions, and the only benefit might be that we'll be able to port small fixes from/to SDL more easily, unless I'm misunderstanding something, so I'll close this PR for now. |
4 participants
Follow-up to #106218
Closes #47656
Closes #96985
This PR adds support for SDL joystick input for the rest of the platforms Godot Engine supports. Previously it was only available on desktop platforms (Windows, Linux and macOS).
I also decided to refactor
joypad_sdl.cppa little bit and add joystick vibration feature to the web platform in this PR :DI have tested this PR on Android and web, I haven't tested it on iOS and visionOS, but judging by how SDL input was integrated on macOS I hope this approach works for iOS and visionOS too.
I haven't made Android use HIDAPI yet (which was used for new joypad features in my other PR: #107967 ), I'm not sure I can, because I'm not an Android developer and it was tiring to make SDL's Android Java code work properly in Godot's build system 😅
(EDIT: It can't be done properly: libsdl-org/SDL@e3cf2e4 https://github.com/libsdl-org/SDL/blob/main/src/hidapi/android/hid.cpp#L1052 )
This PR also fixes 3 issues on Android (tested with my DualShock 4 on the master branch using this script):
1. Trigger axes values were not reset after the user stops pressing on them;
I'm not pressing any buttons on the screenshot:
Relevant issues #79263 #108982
The same situation after my PR:
2. Pressing the left trigger caused the Share button (or JoyButton::BACK) to be pressed as well;
I'm only pressing the left and right triggers on the screenshot:
Pressing Share on the controller didn't trigger anything, but after releasing the left trigger the BACK button stopped being red.
After my PR Godot can properly detect the Share button being pressed:
3. Fingerpring scanners were recognized as joypads.
Relevant issue: #47656
Before my PR:
After my PR:
There's also another Android issue ( #56181 ), but I can't reproduce it on master with my DualShock 4, so I can't test if it was fixed by my PR.
There's another problem, I haven't tested it on master yet but after this PR the joypad triggers are now digital (just 0.0 or 1.0) on web(EDIT: I hope it's now fixed), related SDL issue: libsdl-org/SDL#13051Old TODO:
SDL_ShouldIgnoreJoystickto ignore unnecessary gamepads ( Allow Android to ignore unnecessary joysticks (like fingerprint sensors) libsdl-org/SDL#13759 )TODO for separate PRs:
Input.get_joy_info()regression after the SDL input driver PR #108214 is merged:Input.get_joy_info()fields for new platformsTODO for Android:
TODO for splitting: