Migrate Android framework to SDL #6105

hwsmm · 2024-01-04T15:47:12Z

This PR lets Android devices use SDL for windowing/input. osuTK usage got removed in the process of doing it.
It also removes Android input handling code and replaces them with existing SDL components.

I did test using Vulkan with it and it kind of works, but it was just full of glitches (a lot of flashes and tearing). Veldrid backends (OpenGL/Vulkan) crash if they are brought up from background. I don't know well about graphics, anyway.. the only graphics backend that works well is Legacy OpenGL.

Java is needed because SDL Android heavily relies on JNI, and its binding is made with Xamarin.Android.

I didn't reuse the code, but brought an idea from https://github.com/0x0ade/SDL2Droid-CS, it helped me a lot!

Tested on my tablet with Android 14.

You can use this osu! branch if you want to test.

bdach · 2024-01-04T15:53:43Z

Interesting that you got this to work (I tried what sounds to be the exact same approach, more or less, and it didn't work - input was broken, and so was debugging). Interested to see if this actually works.

The packaging 200% needs work though. We cannot be consuming random binaries from random sources. The build toolchain for our SDL2 fork (https://github.com/ppy/SDL2-CS) must be used for SDL binaries. Maybe the extra wrapper code must live there too (or else, in a separate repository), and if possible, the changes to the SDL Android project should be upstreamed.

hwsmm · 2024-01-04T16:27:46Z

I understand that reproducibility and integrity matter a lot in packaging, so I even considered excluding my binaries from this PR, but I thought it would cause some big inconvenience for testing, so I just went with having binaries included. I should have mentioned it.

I will look into that repo and open another PR there if I come up with an okay solution.

Susko3 · 2024-01-04T16:47:56Z

SDL Android looks for the SDL main function from dynamic library symbol. Let me know if there's any better way. I'd be happy to make it better.

You could try using UnmanagedCallersOnlyAttribute for this. I think this will expose the function from the managed osu.Framework.Android.dll or SampleGame.Android.dll so hopefully SDL might be able to see it. I think that this requires compiling with NativeAOT.

With this, SDL2AndroidMainSetter should be completely unnecessary.

Example code from another project, here the native library expects a obs_module_load to be exposed on the DLL, this project is compiled with NativeAOT and the native library can see and call the function.

bdach · 2024-01-04T16:50:17Z

I think that this requires compiling with NativeAOT.

I'm not sure it will be relevant but the last time the words "AOT" and "android" were in close proximity things fizzled out very quickly due to being terminally broken. So I'm not holding my breath...

hwsmm · 2024-01-05T18:14:32Z

Above PR adds SDL2-CS-Android project to SDL2-CS. Changes in Java code are no longer needed, too.
I'll commit changes that are needed from framework-side once it gets merged, since there isn't a NuGet package to depend on yet.

You can test it by removing osu.Framework.Android.SDL2 reference from osu.Framework.Android and add a reference to SDL2-CS-Android in osu.Framework.Android.

Edit: Tried AOT, but it doesn't seem to launch on my device...

Susko3 · 2024-01-06T15:04:46Z

libsdl-org/SDL@3a482eb could be used to avoid the need for SDL2AndroidMainSetter. It's currently only available in SDL3, but we could backport it to SDL2.

…suggestion

hwsmm · 2024-01-06T18:28:18Z

I will just push my local framework as local SDL2-CS is needed to test at the moment anyway, and tidy up once things look okay.

With @Susko3's suggestion, I got it working and successfully removed SDL2AndroidMainSetter! You can test it by using SDL2-CS update-android-binaries branch.

Joehuu · 2024-02-10T22:42:36Z

On the framework apps (test and samplegame), I get a potential crash when rotating 90 degrees (osu! not affected as it can't rotate 90):

02-10 14:12:41.974 12820 12853 E AndroidRuntime: FATAL EXCEPTION: SDLThread
02-10 14:12:41.974 12820 12853 E AndroidRuntime: Process: osu.Framework.Tests.Android, PID: 12820
02-10 14:12:41.974 12820 12853 E AndroidRuntime: android.runtime.JavaProxyThrowable: [NUnit.Framework.AssertionException]: Stored Displays don't match actual displays: Stored displays:
02-10 14:12:41.974 12820 12853 E AndroidRuntime:   Name: 0, Bounds: {X=0,Y=0,Width=2400,Height=1080}, DisplayModes: 0, Index: 0
02-10 14:12:41.974 12820 12853 E AndroidRuntime: 
02-10 14:12:41.974 12820 12853 E AndroidRuntime: Actual displays:
02-10 14:12:41.974 12820 12853 E AndroidRuntime:   Name: 0, Bounds: {X=0,Y=0,Width=1080,Height=2400}, DisplayModes: 0, Index: 0
02-10 14:12:41.974 12820 12853 E AndroidRuntime: 	at osu.Framework.Logging.ThrowingTraceListener.Fail(Unknown Source:0)
02-10 14:12:41.974 12820 12853 E AndroidRuntime: 	at System.Diagnostics.TraceInternal.Fail(Unknown Source:0)
02-10 14:12:41.974 12820 12853 E AndroidRuntime: 	at System.Diagnostics.TraceInternal+TraceProvider.Fail(Unknown Source:0)
02-10 14:12:41.974 12820 12853 E AndroidRuntime: 	at System.Diagnostics.Debug.Fail(Unknown Source:0)
02-10 14:12:41.974 12820 12853 E AndroidRuntime: 	at System.Diagnostics.Debug.Assert(Unknown Source:0)
02-10 14:12:41.974 12820 12853 E AndroidRuntime: 	at osu.Framework.Platform.SDL2Window.assertDisplaysMatchSDL(Unknown Source:0)
02-10 14:12:41.974 12820 12853 E AndroidRuntime: 	at osu.Framework.Platform.SDL2Window.<handleWindowEvent>b__290_0(Unknown Source:0)
02-10 14:12:41.974 12820 12853 E AndroidRuntime: 	at osu.Framework.Threading.ScheduledDelegate.InvokeTask(Unknown Source:0)
02-10 14:12:41.974 12820 12853 E AndroidRuntime: 	at osu.Framework.Threading.ScheduledDelegate.RunTaskInternal(Unknown Source:0)
02-10 14:12:41.974 12820 12853 E AndroidRuntime: 	at osu.Framework.Threading.Scheduler.Update(Unknown Source:0)
02-10 14:12:41.974 12820 12853 E AndroidRuntime: 	at osu.Framework.Platform.SDL2Window.RunFrame(Unknown Source:0)
02-10 14:12:41.974 12820 12853 E AndroidRuntime: 	at osu.Framework.Platform.SDL2Window.RunMainLoop(Unknown Source:0)
02-10 14:12:41.974 12820 12853 E AndroidRuntime: 	at osu.Framework.Platform.SDL2Window.Run(Unknown Source:0)
02-10 14:12:41.974 12820 12853 E AndroidRuntime: 	at osu.Framework.Platform.GameHost.Run(Unknown Source:0)
02-10 14:12:41.974 12820 12853 E AndroidRuntime: 	at osu.Framework.Android.AndroidGameActivity.<CreateSDLMainRunnable>b__17_0(Unknown Source:0)
02-10 14:12:41.974 12820 12853 E AndroidRuntime: 	at Java.Lang.Runnable.Run(Unknown Source:0)
02-10 14:12:41.974 12820 12853 E AndroidRuntime: 	at Java.Lang.IRunnableInvoker.n_Run(Unknown Source:0)
02-10 14:12:41.974 12820 12853 E AndroidRuntime: 	at Android.Runtime.JNINativeWrapper.Wrap_JniMarshal_PP_V(Unknown Source:0)
02-10 14:12:41.974 12820 12853 E AndroidRuntime: 	at mono.java.lang.Runnable.n_run(Native Method)
02-10 14:12:41.974 12820 12853 E AndroidRuntime: 	at mono.java.lang.Runnable.run(Runnable.java:31)
02-10 14:12:41.974 12820 12853 E AndroidRuntime: 	at java.lang.Thread.run(Thread.java:1012)

This may also resolve ppy/osu#5236 for android, but there's a resizing bug which may be a blocker as it's really broken (on Pixel 6a with Android 14 beta if that changes anything):

screen-20240210-143706.mp4

Joehuu

As commented above.

hwsmm · 2024-02-12T17:52:53Z

I actually have run into that crash when I was writing this PR, but didn't fix it eventually since assertDisplaysMatchSDL is only present in Debug configuration. I fixed it in my last commit.

osu-framework/osu.Framework/Platform/SDL2Window_Windowing.cs

Lines 309 to 317 in 0e88708

    
           /// <summary> 
        
           /// Asserts that the current <see cref="Displays"/> match the actual displays as reported by SDL. 
        
           /// </summary> 
        
           /// <remarks> 
        
           /// This assert is not fatal, as the <see cref="Displays"/> will get updated sooner or later 
        
           /// in <see cref="handleDisplayEvent"/> or <see cref="handleWindowEvent"/>. 
        
           /// </remarks> 
        
           [Conditional("DEBUG")] 
        
           private void assertDisplaysMatchSDL()

I can't reproduce the resizing bug in the video you posted, In the latest release (osuTK), my game doesn't get moved/resized at all, so I can't see what I am typing because keyboard hides bottom half, and in my build (SDL2), the game goes up (only showing bottom half), so I can see my chat fine.

My phone is Galaxy A51 on Android 13, so something may be different with keyboard handling, but I'm not really sure..

Joehuu · 2024-02-13T01:19:13Z

I can't seem to reproduce the resizing bug anymore. Probably incidentally fixed with the new commits.

The textboxes still have an off-by-one screen offset bug though:

click/focus chat text box (no screen offset)
click chat text box again (puts it into view)
click settings text box (still offsets the screen where the chat text box used to be)

hwsmm · 2024-04-20T15:27:44Z

I applied most of the review suggestions, but there seem to be some remaining issues (keyboard screen offsets and multi-touch assert fail for now?), but I currently don't have much time to actually investigate into them.. I should be more free after a week, so I'll look into them then.

Though if anyone suggests fixes for the forementioned issues in the meantime, I'll try applying them (or just commit them yourself if you have perms?).

Susko3 · 2024-04-20T16:51:38Z

The main problem with the keyboard is that we're calling SDL_StartTextInput before SDL_SetTextInputRect. This leads to the keyboard using the stale text input rect value.

From SDL_SetTextInputRect documentation:

To start text input in a given location, this function is intended to be called before SDL_StartTextInput, although some platforms support moving the rectangle even while text input (and a composition) is active.

I'll try to figure something out.

Susko3 · 2024-04-20T18:15:38Z

I've reported #6105 (review) upstream in libsdl-org/SDL#9591. This issue is not unique to android and can fail on Windows too.

bdach · 2024-04-23T10:23:36Z

@Susko3 is this fine to move forward after all of the other PRs?

Susko3 · 2024-04-23T11:55:03Z

Yep this is good to go, just needs merge master + removal of no longer necessary workarounds:

Revert SDL3Window_Windowing.cs
AndroidGameWindow.Create() workarounds

Also the AndroidGameHost shouldn't be disposed.

bdach · 2024-04-23T13:24:01Z

Revert SDL3Window_Windowing.cs

All of it? What about this thing? I don't see it being moved anywhere?

Susko3 · 2024-04-23T17:37:54Z

What about this thing? I don't see it being moved anywhere?

You're correct, that has to stay, I must have confused it with the dropped events that were fixed with #6259.

Not sure why it's still here, I asked during review and it was supposed to be removed.

bdach · 2024-04-26T08:28:27Z

Also the AndroidGameHost shouldn't be disposed.

Have removed this in dad23c0.

2. AndroidGameWindow.Create() workarounds

I can't seem to be able to remove these. Doing so results in the window never becoming focused and as such touch input doesn't work.

I'd really like to move this along now so I think this can be fixed as a follow-up.

hwsmm · 2024-04-26T08:38:58Z

osu.Framework.Android/AndroidGameActivity.cs

-            // blocks back button
-            SDL.SDL3.SDL_SetHint(SDL.SDL3.SDL_HINT_ANDROID_TRAP_BACK_BUTTON, "1"u8);
-
-            // hints are here because they don't apply well in another location such as SDL3Window
-


The reason why I didn't remove them was because SDL docs had this comment on the hint:

* This is necessary for the right mouse button to work on some Android * devices, or to be able to trap the back button for use in your code * reliably. If this hint is true, the back button will show up as an * SDL_EVENT_KEY_DOWN / SDL_EVENT_KEY_UP pair with a keycode of * SDL_SCANCODE_AC_BACK. * * The variable can be set to the following values: * * - "0": Back button will be handled as usual for system. (default) * - "1": Back button will be trapped, allowing you to handle the key press * manually. (This will also let right mouse click work on systems where the * right mouse button functions as back.)

https://wiki.libsdl.org/SDL3/SDL_HINT_ANDROID_TRAP_BACK_BUTTON

I should have made it more clear..

Not sure how it works without the hint though, but I guess it may be fine if it works?

Seemed fine to me /shrug

Susko3

Seems to work well, just need to remove the osuTK.Android reference as it's no longer needed.

Susko3 · 2024-04-26T10:37:24Z

osu.Framework.Android/AndroidGameSurface.cs

+        }
+
+        /// <summary>
+        /// Updates the <see cref="IWindow.SafeAreaPadding"/>, taking into account screen insets that may be obstructing this <see cref="AndroidGameView"/>.


Should be updated to mention the proper class, and remove using osuTK.Android;

Suggested change

/// Updates the <see cref="IWindow.SafeAreaPadding"/>, taking into account screen insets that may be obstructing this <see cref="AndroidGameView"/>.

/// Updates the <see cref="IWindow.SafeAreaPadding"/>, taking into account screen insets that may be obstructing this <see cref="AndroidGameSurface"/>.

Susko3 · 2024-04-26T10:38:52Z

osu.Framework.Android/osu.Framework.Android.csproj

@@ -19,6 +19,7 @@
  </ItemGroup>
  <ItemGroup>
    <PackageReference Include="ppy.osuTK.Android" Version="1.0.211" />


Suggested change

<PackageReference Include="ppy.osuTK.Android" Version="1.0.211" />

Not needed if the xmldoc reference in AndroidGameSurface is removed.

Susko3 · 2024-04-26T12:29:45Z

just need to remove the osuTK.Android reference as it's no longer needed.

Or you can merge right now, and we do this later in osuTK cleanup.

This reverts commit 508d7e0, reversing changes made to 169bd07.

hwsmm added 6 commits January 5, 2024 00:23

Add Android SDL2 Native Libraries

6f2e9ed

Add a new project to make use of SDL Android

6f86f96

Convert Framework.Android to SDL2

5807910

Expose DisplayHandle to SDL2Window inheritors

90c7a3d

Expose Focused to SDL2Window inheritors

7037837

Basic support for Android in VeldridRenderer

8bd6e23

pull-request-size bot added the size/XXL label Jan 4, 2024

hwsmm mentioned this pull request Jan 5, 2024

Add an Android bindings project ppy/SDL2-CS#180

Closed

hwsmm added 3 commits January 7, 2024 03:17

Implement DefaultFocusHighlightEnabled properly

6587a9a

Use local SDL2-CS-Android

c38393c

Use backported SDL3 feature to get rid of MainSetter as per Susko3's …

a690c70

…suggestion

hwsmm added 2 commits January 7, 2024 14:10

No need to use SDL Fullscreen in AndroidGameWindow

e4e9734

Move SafeArea to AndroidGameSurface

f3dfbbb

Joehuu added the platform:android label Jan 20, 2024

Joehuu self-requested a review February 10, 2024 21:09

Joehuu suggested changes Feb 10, 2024

View reviewed changes

Merge master

cce5fee

hwsmm force-pushed the sdl-cs-android branch from b1bdead to d7dfd76 Compare February 12, 2024 17:52

hwsmm force-pushed the sdl-cs-android branch from d7dfd76 to 241d73c Compare February 12, 2024 17:54

Fix rotation crash in debug builds

241d73c

This was referenced Apr 20, 2024

Set IME rectangle before starting text input #6257

Merged

Repeated finger down events when releasing and pressing non-last finger in muli-touch scenarios libsdl-org/SDL#9591

Closed

This was referenced Apr 20, 2024

Work around broken SDL_EVENT_FINGER_DOWN events that lead to stuck touches #6258

Merged

Setting an event filter drops all current events from queue libsdl-org/SDL#9592

Closed

Set event filter early in the startup process to work around SDL issue #6259

Merged

bdach added 3 commits April 26, 2024 10:11

Do not dispose host on android

dad23c0

Remove back button disable hint

106ea66

Not sure why it's still here, I asked during review and it was supposed to be removed.

Merge branch 'master' into sdl-cs-android

f6b4c3c

bdach previously approved these changes Apr 26, 2024

View reviewed changes

hwsmm commented Apr 26, 2024

View reviewed changes

Susko3 suggested changes Apr 26, 2024

View reviewed changes

Remove osuTK reference from android project

a60499c

bdach dismissed their stale review via a60499c April 26, 2024 12:49

Susko3 approved these changes Apr 26, 2024

View reviewed changes

Joehuu approved these changes Apr 26, 2024

View reviewed changes

bdach approved these changes Apr 29, 2024

View reviewed changes

bdach merged commit 508d7e0 into ppy:master Apr 29, 2024
17 of 19 checks passed

bdach mentioned this pull request Apr 29, 2024

Apply adjustments after framework changes ppy/osu#28028

Merged

1 task

Susko3 mentioned this pull request Apr 29, 2024

Refactor DisplayHandle into seperate android and linux properties #6268

Merged

hwsmm deleted the sdl-cs-android branch May 4, 2024 05:37

peppy added a commit to peppy/osu-framework that referenced this pull request May 21, 2024

Revert "Merge pull request ppy#6105 from hwsmm/sdl-cs-android"

6dda2dd

This reverts commit 508d7e0, reversing changes made to 169bd07.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate Android framework to SDL #6105

Migrate Android framework to SDL #6105

hwsmm commented Jan 4, 2024 •

edited

Loading

bdach commented Jan 4, 2024 •

edited

Loading

hwsmm commented Jan 4, 2024

Susko3 commented Jan 4, 2024

bdach commented Jan 4, 2024

hwsmm commented Jan 5, 2024 •

edited

Loading

Susko3 commented Jan 6, 2024

hwsmm commented Jan 6, 2024

Joehuu commented Feb 10, 2024 •

edited

Loading

Joehuu left a comment

hwsmm commented Feb 12, 2024

Joehuu commented Feb 13, 2024 •

edited

Loading

hwsmm commented Apr 20, 2024 •

edited

Loading

Susko3 commented Apr 20, 2024

Susko3 commented Apr 20, 2024

bdach commented Apr 23, 2024

Susko3 commented Apr 23, 2024

bdach commented Apr 23, 2024

Susko3 commented Apr 23, 2024

bdach commented Apr 26, 2024

hwsmm Apr 26, 2024 •

edited

Loading

bdach Apr 26, 2024

Susko3 left a comment

Susko3 Apr 26, 2024

Susko3 Apr 26, 2024

Susko3 commented Apr 26, 2024

	/// Updates the <see cref="IWindow.SafeAreaPadding"/>, taking into account screen insets that may be obstructing this <see cref="AndroidGameView"/>.
	/// Updates the <see cref="IWindow.SafeAreaPadding"/>, taking into account screen insets that may be obstructing this <see cref="AndroidGameSurface"/>.

Migrate Android framework to SDL #6105

Migrate Android framework to SDL #6105

Conversation

hwsmm commented Jan 4, 2024 • edited Loading

bdach commented Jan 4, 2024 • edited Loading

hwsmm commented Jan 4, 2024

Susko3 commented Jan 4, 2024

bdach commented Jan 4, 2024

hwsmm commented Jan 5, 2024 • edited Loading

Susko3 commented Jan 6, 2024

hwsmm commented Jan 6, 2024

Joehuu commented Feb 10, 2024 • edited Loading

Joehuu left a comment

Choose a reason for hiding this comment

hwsmm commented Feb 12, 2024

Joehuu commented Feb 13, 2024 • edited Loading

hwsmm commented Apr 20, 2024 • edited Loading

Susko3 commented Apr 20, 2024

Susko3 commented Apr 20, 2024

bdach commented Apr 23, 2024

Susko3 commented Apr 23, 2024

bdach commented Apr 23, 2024

Susko3 commented Apr 23, 2024

bdach commented Apr 26, 2024

hwsmm Apr 26, 2024 • edited Loading

Choose a reason for hiding this comment

bdach Apr 26, 2024

Choose a reason for hiding this comment

Susko3 left a comment

Choose a reason for hiding this comment

Susko3 Apr 26, 2024

Choose a reason for hiding this comment

Susko3 Apr 26, 2024

Choose a reason for hiding this comment

Susko3 commented Apr 26, 2024

hwsmm commented Jan 4, 2024 •

edited

Loading

bdach commented Jan 4, 2024 •

edited

Loading

hwsmm commented Jan 5, 2024 •

edited

Loading

Joehuu commented Feb 10, 2024 •

edited

Loading

Joehuu commented Feb 13, 2024 •

edited

Loading

hwsmm commented Apr 20, 2024 •

edited

Loading

hwsmm Apr 26, 2024 •

edited

Loading