Android support (sokol_app)

[gustavolsson]

I got sokol_app working on Android. It was surprisingly easy thanks to the excellent workflow that comes with fips! This branch is still work-in-progress though.

I haven't encountered any critical issues but it currently lacks keyboard support. Lifecycle management seems to work great (I used this blog post as a reference for that part).

1. I'm not sure what to do with android_native_app_glue.c and android_native_app_glue.h. I'm not very keen on re-implementing that functionality myself but it is also not a very elegant solution to chuck them into sokol_app.h. What about licensing, can we write "from here to the end mark the following (android) license applies" if we include them into the .h file? (Right now I add the .c file to each sample config I want to test)

2. Also, I'm not sure how to handle GLES2 properly. I currently use the #includes from the emscripten backend (if GLES3, include both gl2 and gl3 .h-files) and then request different CLIENT_VERSIONs on EGL init.

3. I changed the call to init_cb in _sapp_frame() because it relied on first_frame as opposed to init_called, is this ok? It was necessary as the app will tear down gfx and bring it up again (if the user for example returns to the home screen and then opens the app again) and it doesn't make sense to a) change first_frame to true on teardown or b) have duplicate code calling init_cb if both first_frame and init_called are false.

Todo:

• Touch events
• Set swap interval
• Handle high dpi correctly
• Check if screen actually resized in _sapp_android_update_dimensions
• ANativeWindow_setBuffersGeometry need to be called when exactly?
• Remove glue code dependency
• Screen keyboard & key events

[gustavolsson]

On second thought, it wouldn't be that difficult to base this branch on NativeActivity directly. The android_native_app_glue seems to mostly wrap the functionality of NativeActivity and execute it in another thread to avoid stalls.

Taking it further, do we really need to run sokol in another thread? sokol will mostly be used for real-time applications and one can argue that it is up to the user to not go over the frame time budget. One can argue that sokol is the UI so it makes sense to run it in the UI thread directly. Pros: the code will be simpler, there will be less overhead. Cons: might be bad practice, easier to create a badly behaved app. I've never really done Android development so I need input from more experienced people on this.

UPDATE: It might be impossible to have a main loop without threading though..

[floooh]

Nice work :) Some quick thoughts:

• sokol_app.h doesn't require a separate game loop thread (mainly because emscripten/HTML5 doesn't even allow this), I think android_app_glue does this because in many 'traditional' game code bases it's not trivial to split up the frame loop into slices. So we definitely have the option to drop the separate thread.
• if we keep android_app_glue as is, we should definitely include it directly into the implementation section of sokol_app.h, with the original license and making it clear with visible separators above and below that this is external code, there should also be a note at the start of sokol_app.h about this and the separate license, and finally there should be a define SOKOL_APP_USE_ANDROID_NATIVE_APP_GLUE (1) to allow users to compile without it (and provide their own)
• the reason why the init-callback is called on first frame is to make sure that it is definitely running in the same thread as the other callbacks, this is the only important rule
• regarding the init() changes: do we really need to teardown/recreate the entire GL context (e.g. everything that init() does) when the application is switched into the background? I'd like to avoid that if possible, init/shutdown should only be called once in the entire lifecycle. In my tests with Android this wasn't necessary so far, the application recovered automatically when it was brought back. We should support the events SAPP_EVENTTYPE_SUSPENDED and SAPP_EVENTTYPE_RESUMED, so that the user code has a chance to react to being switched to the background and brought back (e.g. freeing memory, destroying and recreating resources, but not bring down the entire GL context?)

That's all I can think of for now :)

[floooh]

PS: looking in more detail at the code, nevermind what I wrote about the "bringing the whole GL context down on SUSPEND/RESUME", as far as I understand this isn't happening in your code. Only thing I found a bit surprising is that _sapp_android_cleanup is called in so many different places ;)

[gustavolsson]

Nice work :) Some quick thoughts:

Thanks :)

sokol_app.h doesn't require a separate game loop thread (mainly because emscripten/HTML5 doesn't even allow this), I think android_app_glue does this because in many 'traditional' game code bases it's not trivial to split up the frame loop into slices. So we definitely have the option to drop the separate thread.

After looking at NativeActivity closer I don't think it's possible to have a main loop without a separate thread as the callbacks (onCreate etc) needs to be non-blocking..

if we keep android_app_glue as is, we should definitely include it directly into the implementation section of sokol_app.h, with the original license and making it clear with visible separators above and below that this is external code, there should also be a note at the start of sokol_app.h about this and the separate license, and finally there should be a define SOKOL_APP_USE_ANDROID_NATIVE_APP_GLUE (1) to allow users to compile without it (and provide their own)

Yea.. I'm leaning towards using NativeActivity directly to avoid the double-license thing and to make the code leaner.

the reason why the init-callback is called on first frame is to make sure that it is definitely running in the same thread as the other callbacks, this is the only important rule

Ok, so you think the change to _sapp_frame()is ok? See below for an argument.

regarding the init() changes: do we really need to teardown/recreate the entire GL context (e.g. everything that init() does) when the application is switched into the background? I'd like to avoid that if possible, init/shutdown should only be called once in the entire lifecycle. In my tests with Android this wasn't necessary so far, the application recovered automatically when it was brought back. We should support the events SAPP_EVENTTYPE_SUSPENDED and SAPP_EVENTTYPE_RESUMED, so that the user code has a chance to react to being switched to the background and brought back (e.g. freeing memory, destroying and recreating resources, but not bring down the entire GL context?)
PS: looking in more detail at the code, nevermind what I wrote about the "bringing the whole GL context down on SUSPEND/RESUME", as far as I understand this isn't happening in your code. ...

The current code tears down the EGL objects and calls cleanup_cb as soon as it receives a surfaceDestroyed event (APP_CMD_TERM_WINDOW) from the NativeActivity. I don't think it's allowed to use the same EGL/GL objects after the surface is created again on surfaceCreated (APP_CMD_INIT_WINDOW) but I'll do some more research.

If this teardown is necessary, would you prefer sokol to call SAPP_EVENTTYPE_SUSPENDED and SAPP_EVENTTYPE_RESUMED instead of init_cb and cleanup_cb? I see your point of only calling init/cleanup once throughout the execution. Currently, the suspend and resume sokol events are called on focus lost and received (ie. when the user for example brings down the notification bar when in the app).

Only thing I found a bit surprising is that _sapp_android_cleanup is called in so many different places ;)

Hehe ops, you mean _sapp_android_egl_cleanup in _sapp_android_egl_init? Yea that should be cleaned up :) However, apparently cleanup in general needs to be called on both surfaceDestroyed (APP_CMD_TERM_WINDOW) and onDestroy (APP_CMD_DESTROY) as the order is not guaranteed. From the Nvidia blost post:

Also note that surfaceDestroyed can occur after onPause, in rare cases even after onDestroy. As a result, applications may need to consider shutting down their EGL objects before the host surfaces are explicitly destroyed.

Thanks for the feedback!

[floooh]

The current code tears down the EGL objects and calls cleanup_cb as soon as it receives a surfaceDestroyed event (APP_CMD_TERM_WINDOW) from the NativeActivity. I don't think it's allowed to use the same EGL/GL objects after the surface is created again on surfaceCreated (APP_CMD_INIT_WINDOW) but I'll do some more research.

I don't remember the details, but AFAIK there was some sort of "easy mode" for GL applications where the surface is not teared down and the GL context "survives" when the app is switching to the background. At least (I think, don't have the tablet with me today), that in Oryol I can switch apps into the background, and and when they're brought back they continue where they left off, and Oryol doesn't have any special suspend/resume handling (not destroying the GL context etc..).

So basically it would be guaranteed that APP_CMD_INIT_WINDOW would only happen once at the start of the application, and APP_CMD_TERM_WINDOW only on shutdown of the app, not on suspend/resume. I think the trade-off was that Android may decide to shutdown the whole app and start a "new copy" on resume if it had to make room for other things or the tablet went into sleep.

...at least that's how I remember it, I may be totally wrong :D

I think for a first version this would be acceptable. If we want more advanced shutdown/resume behavior we should do this later, and coordinate with the iOS and HTML5 versions (HTML5 on mobile also has a state where the WebGL context may be "lost"). This stuff may need some API changes in sokol_api.h

[floooh]

...gosh I shouldn't write PR feedback before I had my morning coffee, 4 important edits and counting, I hope you get my meaning anyway ;)

[gustavolsson]

You're right! From what I've gathered it seems that really old devices didn't support multiple EGL contexts and required apps to tear down their context when paused to support multitasking. We can probably assume that modern devices support multiple contexts and only disconnect and destroy the surface, not the context or display, on surfaceDestroyed and then re-connect the context to the new surface on surfaceCreated.

More info: https://groups.google.com/d/msg/android-ndk/jwVMF6zINus/wwYSBAeilTgJ and here https://source.android.com/devices/graphics/arch-egl-opengl.html

I think for a first version this would be acceptable. If we want more advanced shutdown/resume behavior we should do this later, and coordinate with the iOS and HTML5 versions (HTML5 on mobile also has a state where the WebGL context may be "lost"). This stuff may need some API changes in sokol_api.h

Sounds good!

I guess the only downside is GPU-memory use when paused and the only problem a potential threshold for max number of contexts (the thread above suggests checking that setting the context works out and if not recreate everything, which we could do at a later stage). The app could quit on on(System)LowMemory and/or if it has been in the background for too long to preserve memory.

Also, setting "android:configChanges=orientation|screenSize|keyboard|keyboardHidden" should trigger onConfigurationChanged instead of onSurfaceDestroyed/onSurfaceCreated for those events, further reducing the teardown/bringup delay.

I'm currently working on removing the glue code (in a separate branch for now), I think that is the most elegant solution (even if it is heavily inspired by the glue implementation).

[gustavolsson]

FYI: I've now removed the app glue dependency!

No changes have been made to the egl code yet though, so it still tears down everything onSurfaceDestroyed. That's what I'm going to work on next. After that touch events and general cleanup :)

[gustavolsson]

Getting pretty solid now.. only touch events and keyboard experiments left!

[gustavolsson]

Good article on lifecycle management: http://developer.download.nvidia.com/assets/mobile/docs/android_lifecycle_app_note.pdf (note to self)

Sigh, static variables will be a problem. Not for sokol_sapp.h, but for user code and 3rd party libraries (I get crashes related to asserts in sokol_time.h, for example). https://groups.google.com/forum/#!topic/android-ndk/PgZhN5h1x8o and https://stackoverflow.com/questions/6375495/initializing-global-variables-to-zero-on-android-ndk

[floooh]

That said, I would like to be able to work on my old Mac from 2010 that doesn't support Metal, is there any chance that we can bring back OpenGL support for OSX?

Yes, but I was wondering if there is demand. AFAIK SDL2 has fixed the GL problems in Mojave, and GLFW is going to implement the same fixes. We need to check what those fixes are (and how ugly they are).

Another (somewhat unconventional but much simpler) solution I had in mind was to create a GLFW and/or SDL2 backend for sokol_app.h, this would make sense when GLFW or SDL2 is installed as a system-wide DLL and headers (which is quite usual on Linux, or on Mac when installed GLFW or SDL2 are installed with homebrew).

[gustavolsson]

I was thinking of bringing back the old OpenGL code, as it was, for builds targeting pre-Mojave versions of macOS.

The way I see it, OpenGL support is only necessary for old Macs that can't upgrade to Mojave, if a build targets Mojave and up, we can assume that the Mac running that version supports Metal and then there is no point in getting the OpenGL build running. What is your opinion?

[floooh]

Yes makes sense for the short term at least. In the meantime I'll be keeping an eye on GLFW to see what solution they come up with.

[gustavolsson]

Do you have a hint for me how I would go about to bring back OpenGL for macOS? I can't seem to find the commit that removes it..

[floooh]

This is the commit that removed GL support for OSX:

ef97196#diff-79cd62acec3f18414ce0d06e64aa07c4

However that commit includes a failed attempt to fix the GL code for Mojave:

0b9478a#diff-79cd62acec3f18414ce0d06e64aa07c4

I guess it's best to go with the code from right before that "fix" commit.

[floooh]

Ok, re the Android PR: I got the sokol-samples working on my Android tablet, and the code looks good to me.

On my Nexus 7 tablet I can switch the samples into the background and foreground without a crash.

What do you think, should I hit the merge button?

I found a few problems in fips when building the complete sokol-samples project (somehow the wrong DLLs slip into the APKs when building a project with several exe targets, probably some sort of parallel build problem), and running the samples via "./fips run ..." doesn't work anymore, only the install, but that's all entirely unrelated, and I'll have a look at that soon-ish.

[floooh]

Ah ok, found one small thing: when I started testing the release configs, the sokol-samples didn't work.

It's just a small oversight: there's critical code in SOKOL_ASSERT() macros, but the asserts are removed in release mode. I found 5 places:

https://github.com/gustavolsson/sokol/blob/09397b173da4ecc085e9efd84ba38aa3ddd9cc32/sokol_app.h#L4131-L4132

https://github.com/gustavolsson/sokol/blob/09397b173da4ecc085e9efd84ba38aa3ddd9cc32/sokol_app.h#L4138-L4139

And:

https://github.com/gustavolsson/sokol/blob/09397b173da4ecc085e9efd84ba38aa3ddd9cc32/sokol_app.h#L4298

Moving those calls out of the SOKOL_ASSERT()'s made it work in release mode. Maybe you could for other places which I might have missed.

PS: the fips problems I mentioned in the comment above have been fixed, the sokol-samples can now be tested by using the build-configs:

  sapp-android-make-debug
  sapp-android-make-release
  sapp-android-vscode-debug
  sapp-android-vscode-release

[gustavolsson]

Ok, re the Android PR: I got the sokol-samples working on my Android tablet, and the code looks good to me.

Oh, I forgot to mention my changes to that repo in order to test it out. You seem to have made the same changes, with the exception of missing to link with the "log" library in addition to "GLESv3", "EGL" and "android" (for default SOKOL_LOG() behavior).

I also made some changes to the default android manifest in fips but that was mostly to get "app-with-dashes" names working, enabling orientation changes and telling the os to send "onConfigChanged" events instead of recreating the surface for a few events. I can create a PR for those as well.

On my Nexus 7 tablet I can switch the samples into the background and foreground without a crash.

Great :)

What do you think, should I hit the merge button?

I think it's ready as soon as I've fixed the assert problems. I would like to add soft keyboard as well but that can be a separate PR.

I found a few problems in fips when building the complete sokol-samples project (somehow the wrong DLLs slip into the APKs when building a project with several exe targets, probably some sort of parallel build problem), and running the samples via "./fips run ..." doesn't work anymore, only the install, but that's all entirely unrelated, and I'll have a look at that soon-ish.

The problem I've been having the last few days must have been related to this: The "linux" define was not present when trying to build sokol for android using nearly the exact same CMakeLists.txt file as sokol-samples in a completely new project. Using the latest fips tonight fixed it!

... Moving those calls out of the SOKOL_ASSERT()'s made it work in release mode. Maybe you could for other places which I might have missed.

Sure!

Did you get a change to look at the high_dpi code?

[floooh]

I also made some changes to the default android manifest in fips but that was mostly to get "app-with-dashes" names working, enabling orientation changes and telling the os to send "onConfigChanged" events instead of recreating the surface for a few events. I can create a PR for those as well.

Yes please :)

Did you get a change to look at the high_dpi code?

No, where should I look? I noticed that my emulators got stretched over the entire display, instead of preserving the aspect ratio through a smaller scissor rect, maybe there's something wrong with the returned width/height. I wanted to debug this later, I think it's better to first get the big merge behind us :)

[gustavolsson]

I also made some changes to the default android manifest in fips ...

Yes please :)

Cool, I will try to get everything done this week. However, since my smartphone is in pieces at the moment you will have to see if it worked :)

Did you get a change to look at the high_dpi code?

No, where should I look? ...

The high_dpi logic is in _sapp_android_update_dimensions().

[gustavolsson]

I think I've fixed everything that we talked about now but I can't test it myself as I mentioned. If it works for you in both debug/release mode I would say it is ready for merging :)

I added floooh/fips#214 so that you can test rotating the screen.

[floooh]

...this must actually be #if TARGET_OS_IPHONE, because on OSX this is defined as (0) (so it's defined, but not true). But I'll just go ahead with the merge and fix this in master afterwards.

[floooh]

Regarding _sapp_android_update_dimensions(): I'm seeing a strange effect in the sokol-samples ImGui demo when rotating the screen on my Nexus 7 tablet. In vertical orientation, the ImGui windows looks "squished" (and I have a feeling that in horizontal orientation the windows look wider than they should), as if there's a non-uniform scaling happening between the framebuffer and window. The Nexus 7 has a fairly extreme 16:10 aspect ratio, maybe that's why any aspect ratio problems are more apparent there.

But nervertheless, I'll go ahead with the merge and do a bit of debugging afterwards on the master branch (as well as updating the READMEs).

[floooh]

Alright merged! Thanks a lot for doing all the hard work :)

As mentioned above I'll do a few small fixes in the master branch now, update sokol-samples and some READMEs, and do some debugging on the aspect-ratio thing.

[floooh]

FYI...there are also a couple of problems in the new cpp-compile-test in sokol-samples (so basically compiling as C++ instead of C, going to fix those now...)

[floooh]

...ok, I've been debugging a bit with Android Studio (had to do another fix in fips first for that to work again), and I think I understand now why the aspect ratio looks odd.

In the _sapp_android_update_dimensions() function, the backbuffer size is taken from _sapp.desc.width and _sapp.desc.height (in the cube sample this is 800x600), on my device the window size (ANativeWindow_getWidth/Height) is 1920x1200. This causes an "odd" upscaling from 800x600 to 1920x1200.

What I'm doing in the iOS and HTML5 backends is to simply ignore the "preferred size" in sapp_desc, and instead use the devices display resolution. Using this idea for the Android backend means:

The backbuffer should be either equal to "NativeWindowSize" (with high_dpi=true), or "NativeWindowSize * 0.5" (with high_dpi=false). This way either no upscaling will take place, or the upscaling will always be "even" (x2).

I'll see if I can implement this tomorrow or so...

[gustavolsson]

... Using this idea for the Android backend means:

The backbuffer should be either equal to "NativeWindowSize" (with high_dpi=true), or "NativeWindowSize * 0.5" (with high_dpi=false). This way either no upscaling will take place, or the upscaling will always be "even" (x2).

I'll see if I can implement this tomorrow or so...

Ah, now the code for the other platforms make sense :) That simplifies things considerably. If you don't have the time I can take a stab at it later next week (problem is that I still can't test it though).

[floooh]

Ah, now the code for the other platforms make sense :) That simplifies things considerably. If you don't have the time I can take a stab at it later next week (problem is that I still can't test it though).

I think I can do this tomorrow or early next week, don't worry :)

[floooh]

Ah... now I'm getting the sokol_time.h assertion you mentioned when switching the app into the background... hmm how annoying...

...after a bit of trial-and-error I did something naughty... at the end of _sapp_android_on_destroy() I'm calling exit(0). I think this is the next best solution to forcing all application code to dealing with static data not being cleared on startup (which is a guaranteed behaviour in C, and it would be even more messy with global C++ objects where the destructor wouldn't be called).

I also added a comment that when handling the Android Back button, we should give the application a chance to bring up a "do you really want to quit?" dialog. This is currently generally missing in sokol_app.h for all platforms.

PS: if we can figure out the proper "_entry()" and "_exit()" functions in the NDK "C runtime" which are called before and after main(), this would be an option too. I think emscripten is doing a similar type of hackery to make sure the C/C++ environment is properly set up and teared down (because in the beginning, emscripten had exactly the same type of problems).

[floooh]

...oh and I also rewrote the framebuffer sizing stuff, this was pretty straightforward and seems to work well. I'm kinda assuming that all Android devices have HighDPI displays though, when high_dpi is false I'm generally setting the framebuffer size to half the window size, and when HighDPI is set, I keep it at the native resolution (I was running into the same weird problems you mentioned in a comment that setting the resolution to the same as the window didn't work, I was getting a very weird broken display, but just leaving the framebuffer size alone when HighDPI is enabled seems to work..).

[gustavolsson]

...oh and I also rewrote the framebuffer sizing stuff, this was pretty straightforward and seems to work well. I'm kinda assuming that all Android devices have HighDPI displays though, when high_dpi is false I'm generally setting the framebuffer size to half the window size, and when HighDPI is set, I keep it at the native resolution

Great!

(I was running into the same weird problems you mentioned in a comment that setting the resolution to the same as the window didn't work, I was getting a very weird broken display, but just leaving the framebuffer size alone when HighDPI is enabled seems to work..).

Yea, I got similar artifacts. Maybe it's a good idea to keep a comment regarding the issue near still:

sokol/sokol_app.h

Line 4114 in 79b28b8

int32_t result = ANativeWindow_setBuffersGeometry(window, buf_w, buf_h, format);

These gotchas are not documented anywhere it seems..

[gustavolsson]

Oh, forgot to mention that I meant to clean up the SOKOL_LOG calls in _sapp_android_touch_even() as well. I started creating a touch-sapp sample for sokol-samples that would visualize the touch points (it was difficult to tell by only looking at the logs), but got sidetracked when it wouldn't build on my mac. I'll see if I can finish it after the old GL code is back..

[floooh]

Maybe it's a good idea to keep a comment regarding the issue near still:

Yes, good idea, I'll add a comment back.

Oh, forgot to mention that I meant to clean up the SOKOL_LOG calls in _sapp_android_touch_event()

I've set the log level to INFO and noticed the "touch event spam", I simply removed the message for the "moved event", I think that's ok for now :)