Android asynchronous rendering

[ivovandongen]

Builds on #8820 to implement asynchronous rendering on Android

Pre-requisites:

• Make thread.hpp public
• Make Thread::pause/resume thread safe. So we can pause resume from arbitrary threads (main/opengl).
• Allow safe direct access to Actor's managed object through Thread. We need to be able to pause the scheduler and invoke the object (Renderer) from another thread (OpenGl renderer thread).
• Add ask pattern to Actor/ActorRef to make blocking calls from the main thread to the orchestration thread (query*Features) Actor enhancements #9591
• Make self reference on Actors optional so we can make the Renderer an Actor without modifying it. Actor enhancements #9591

Steps:

• Remove texture view support for now
• Separate RenderBackend and View from NativeMapView and make it owned by the RendererFrontend to make locking and dispatching more contained.
• Introduce orchestration thread + synchronization
• Add GLSurfaceview
• Render from OpenGl thread
• Handle GLSurfaceView life-cycle
• Move EGL / OpenGL management code to Java
• Handle snapshots
• Handle fps logging
• (optionally) Implement Asynchronous rendering with TextureView

Fixes #9956

[ivovandongen]

@jfirebaugh Making some headway, but ran into an interesting crash:

Crash dump

********* Crash dump: **********
Build fingerprint: 'google/sailfish/sailfish:7.1.2/NJH47D/4045516:user/release-keys'
pid: 21745, tid: 22301, name: GLThread 5777  >>> com.mapbox.mapboxsdk.testapp <<<
signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr 0x24
Stack frame #00 pc 0025d97a  /data/app/com.mapbox.mapboxsdk.testapp-2/lib/arm/libmapbox-gl.so: Routine std::__ndk1::unique_ptr >::get() const at /Users/ivo/Library/Android/sdk/ndk-bundle/sources/cxx-stl/llvm-libc++/include/memory:2725
Stack frame #01 pc 002675db  /data/app/com.mapbox.mapboxsdk.testapp-2/lib/arm/libmapbox-gl.so: Routine Impl at /Users/ivo/git/mapbox-gl-native/platform/android/src/timer.cpp:57
Stack frame #02 pc 002671df  /data/app/com.mapbox.mapboxsdk.testapp-2/lib/arm/libmapbox-gl.so: Routine std::__ndk1::__unique_if::__unique_single std::__ndk1::make_unique() at /Users/ivo/Library/Android/sdk/ndk-bundle/sources/cxx-stl/llvm-libc++/include/memory:3149
Stack frame #03 pc 003f9bb7  /data/app/com.mapbox.mapboxsdk.testapp-2/lib/arm/libmapbox-gl.so: Routine Throttler at /Users/ivo/git/mapbox-gl-native/src/mbgl/util/throttler.cpp:6
Stack frame #04 pc 0037f797  /data/app/com.mapbox.mapboxsdk.testapp-2/lib/arm/libmapbox-gl.so: Routine GeometryTile at /Users/ivo/git/mapbox-gl-native/src/mbgl/tile/geometry_tile.cpp:45
Stack frame #05 pc 002e28e5  /data/app/com.mapbox.mapboxsdk.testapp-2/lib/arm/libmapbox-gl.so: Routine AnnotationTile at /Users/ivo/git/mapbox-gl-native/src/mbgl/annotation/annotation_tile.cpp:13
Stack frame #06 pc 005974ef  /data/app/com.mapbox.mapboxsdk.testapp-2/lib/arm/libmapbox-gl.so: Routine std::__ndk1::__unique_if::__unique_single std::__ndk1::make_unique(mbgl::OverscaledTileID const&, mbgl::TileParameters const&) at /Users/ivo/Library/Android/sdk/ndk-bundle/sources/cxx-stl/llvm-libc++/include/memory:3149
Stack frame #07 pc 00597355  /data/app/com.mapbox.mapboxsdk.testapp-2/lib/arm/libmapbox-gl.so: Routine _ZNSt6__ndk18__invokeIRZN4mbgl22RenderAnnotationSource6updateENS1_9ImmutableINS1_5style6Source4ImplEEERKNS_6vectorINS3_INS4_5Layer4ImplEEENS_9allocatorISB_EEEEbbRKNS1_14TileParametersEE3$_0JRKNS1_16OverscaledTileIDEEEEDTclclsr3std6__ndk1E7forwardIT_Efp_Espclsr3std6__ndk1E7forwardIT0_Efp0_EEEOSP_DpOSQ_ at /Users/ivo/Library/Android/sdk/ndk-bundle/sources/cxx-stl/llvm-libc++/include/type_traits:4287
Stack frame #08 pc 005972a1  /data/app/com.mapbox.mapboxsdk.testapp-2/lib/arm/libmapbox-gl.so: Routine std::__ndk1::__function::__func, std::__ndk1::vector, std::__ndk1::allocator > > const&, bool, bool, mbgl::TileParameters const&)::$_0, std::__ndk1::allocator, std::__ndk1::vector, std::__ndk1::allocator > > const&, bool, bool, mbgl::TileParameters const&)::$_0>, std::__ndk1::unique_ptr > (mbgl::OverscaledTileID const&)>::operator()(mbgl::OverscaledTileID const&) at /Users/ivo/Library/Android/sdk/ndk-bundle/sources/cxx-stl/llvm-libc++/include/functional:1533
Stack frame #09 pc 0059c321  /data/app/com.mapbox.mapboxsdk.testapp-2/lib/arm/libmapbox-gl.so: Routine std::__ndk1::function > (mbgl::OverscaledTileID const&)>::operator()(mbgl::OverscaledTileID const&) const at /Users/ivo/Library/Android/sdk/ndk-bundle/sources/cxx-stl/llvm-libc++/include/functional:1896
Stack frame #10 pc 0059bd4d  /data/app/com.mapbox.mapboxsdk.testapp-2/lib/arm/libmapbox-gl.so: Routine operator() at /Users/ivo/git/mapbox-gl-native/src/mbgl/renderer/tile_pyramid.cpp:140
Stack frame #11 pc 00599a9d  /data/app/com.mapbox.mapboxsdk.testapp-2/lib/arm/libmapbox-gl.so: Routine void mbgl::algorithm::updateRenderables, std::__ndk1::allocator > > const&, bool, bool, mbgl::TileParameters const&, mbgl::SourceType, unsigned short, mbgl::Range, std::__ndk1::function > (mbgl::OverscaledTileID const&)>)::$_0, mbgl::TilePyramid::update(std::__ndk1::vector, std::__ndk1::allocator > > const&, bool, bool, mbgl::TileParameters const&, mbgl::SourceType, unsigned short, mbgl::Range, std::__ndk1::function > (mbgl::OverscaledTileID const&)>)::$_1, mbgl::TilePyramid::update(std::__ndk1::vector, std::__ndk1::allocator > > const&, bool, bool, mbgl::TileParameters const&, mbgl::SourceType, unsigned short, mbgl::Range, std::__ndk1::function > (mbgl::OverscaledTileID const&)>)::$_2, mbgl::TilePyramid::update(std::__ndk1::vector, std::__ndk1::allocator > > const&, bool, bool, mbgl::TileParameters const&, mbgl::SourceType, unsigned short, mbgl::Range, std::__ndk1::function > (mbgl::OverscaledTileID const&)>)::$_4, std::__ndk1::vector > >(mbgl::TilePyramid::update(std::__ndk1::vector, std::__ndk1::allocator > > const&, bool, bool, mbgl::TileParameters const&, mbgl::SourceType, unsigned short, mbgl::Range, std::__ndk1::function > (mbgl::OverscaledTileID const&)>)::$_0, mbgl::TilePyramid::update(std::__ndk1::vector, std::__ndk1::allocator > > const&, bool, bool, mbgl::TileParameters const&, mbgl::SourceType, unsigned short, mbgl::Range, std::__ndk1::function > (mbgl::OverscaledTileID const&)>)::$_1, mbgl::TilePyramid::update(std::__ndk1::vector, std::__ndk1::allocator > > const&, bool, bool, mbgl::TileParameters const&, mbgl::SourceType, unsigned short, mbgl::Range, std::__ndk1::function > (mbgl::OverscaledTileID const&)>)::$_2, mbgl::TilePyramid::update(std::__ndk1::vector, std::__ndk1::allocator > > const&, bool, bool, mbgl::TileParameters const&, mbgl::SourceType, unsigned short, mbgl::Range, std::__ndk1::function > (mbgl::OverscaledTileID const&)>)::$_4, std::__ndk1::vector > const&, mbgl::Range const&, unsigned char) at /Users/ivo/git/mapbox-gl-native/platform/android/MapboxGLAndroidSDK/.externalNativeBuild/cmake/debug/armeabi-v7a/../../../../../../../src/mbgl/algorithm/update_renderables.hpp:37
Stack frame #12 pc 00598491  /data/app/com.mapbox.mapboxsdk.testapp-2/lib/arm/libmapbox-gl.so: Routine mbgl::TilePyramid::update(std::__ndk1::vector, std::__ndk1::allocator > > const&, bool, bool, mbgl::TileParameters const&, mbgl::SourceType, unsigned short, mbgl::Range, std::__ndk1::function > (mbgl::OverscaledTileID const&)>) at /Users/ivo/git/mapbox-gl-native/src/mbgl/renderer/tile_pyramid.cpp:162
Stack frame #13 pc 00596c49  /data/app/com.mapbox.mapboxsdk.testapp-2/lib/arm/libmapbox-gl.so: Routine mbgl::RenderAnnotationSource::update(mbgl::Immutable, std::__ndk1::vector, std::__ndk1::allocator > > const&, bool, bool, mbgl::TileParameters const&) at /Users/ivo/git/mapbox-gl-native/src/mbgl/annotation/render_annotation_source.cpp:35
Stack frame #14 pc 00482afd  /data/app/com.mapbox.mapboxsdk.testapp-2/lib/arm/libmapbox-gl.so: Routine mbgl::RenderStyle::update(mbgl::UpdateParameters const&) at /Users/ivo/git/mapbox-gl-native/src/mbgl/renderer/render_style.cpp:226
Stack frame #15 pc 002f507b  /data/app/com.mapbox.mapboxsdk.testapp-2/lib/arm/libmapbox-gl.so: Routine mbgl::Renderer::Impl::render(mbgl::View&, mbgl::UpdateParameters const&) at /Users/ivo/git/mapbox-gl-native/src/mbgl/renderer/renderer_impl.cpp:53
Stack frame #16 pc 002f21cd  /data/app/com.mapbox.mapboxsdk.testapp-2/lib/arm/libmapbox-gl.so: Routine mbgl::Renderer::render(mbgl::View&, mbgl::UpdateParameters const&) at /Users/ivo/git/mapbox-gl-native/src/mbgl/renderer/renderer.cpp:25
Stack frame #17 pc 002ac0c1  /data/app/com.mapbox.mapboxsdk.testapp-2/lib/arm/libmapbox-gl.so: Routine mbgl::android::AndroidRendererFrontend::render() at /Users/ivo/git/mapbox-gl-native/platform/android/src/android_renderer_frontend.cpp:119
Stack frame #18 pc 000b991d  /data/app/com.mapbox.mapboxsdk.testapp-2/lib/arm/libmapbox-gl.so: Routine mbgl::android::NativeMapView::render(_JNIEnv&) at /Users/ivo/git/mapbox-gl-native/platform/android/src/native_map_view.cpp:189
Stack frame #19 pc 000cd4cb  /data/app/com.mapbox.mapboxsdk.testapp-2/lib/arm/libmapbox-gl.so: Routine operator() at /Users/ivo/git/mapbox-gl-native/platform/android/MapboxGLAndroidSDK/.externalNativeBuild/cmake/debug/armeabi-v7a/../../../../../../../mason_packages/headers/jni.hpp/3.0.0/include/jni/native_method.hpp:245
Stack frame #20 pc 000cd461  /data/app/com.mapbox.mapboxsdk.testapp-2/lib/arm/libmapbox-gl.so: Routine operator() at /Users/ivo/git/mapbox-gl-native/platform/android/MapboxGLAndroidSDK/.externalNativeBuild/cmake/debug/armeabi-v7a/../../../../../../../mason_packages/headers/jni.hpp/3.0.0/include/jni/native_method.hpp:112
Stack frame #21 pc 000cd429  /data/app/com.mapbox.mapboxsdk.testapp-2/lib/arm/libmapbox-gl.so: Routine __invoke at /Users/ivo/git/mapbox-gl-native/platform/android/MapboxGLAndroidSDK/.externalNativeBuild/cmake/debug/armeabi-v7a/../../../../../../../mason_packages/headers/jni.hpp/3.0.0/include/jni/native_method.hpp:110
Stack frame #22 pc 000cd555  /data/app/com.mapbox.mapboxsdk.testapp-2/lib/arm/libmapbox-gl.so: Routine operator() at /Users/ivo/git/mapbox-gl-native/platform/android/MapboxGLAndroidSDK/.externalNativeBuild/cmake/debug/armeabi-v7a/../../../../../../../mason_packages/headers/jni.hpp/3.0.0/include/jni/native_method.hpp:59
Stack frame #23 pc 000cd515  /data/app/com.mapbox.mapboxsdk.testapp-2/lib/arm/libmapbox-gl.so: Routine __invoke at /Users/ivo/git/mapbox-gl-native/platform/android/MapboxGLAndroidSDK/.externalNativeBuild/cmake/debug/armeabi-v7a/../../../../../../../mason_packages/headers/jni.hpp/3.0.0/include/jni/native_method.hpp:55
Stack frame #24 pc 0054ede7  /data/app/com.mapbox.mapboxsdk.testapp-2/oat/arm/base.odex (offset 0x524000)

It seems like updating the render style from the render thread is not working as the placement throttler is trying to access the Runloop which doesn't exist on the Render thread.

We could install a runloop on the render thread and call runOnce() on every render iteration, but I doubt that is going to work as we want. Alternative is to split Renderer::update of from Renderer::render and make sure the former is only executed on the orchestration thread and the latter on the render thread.

[jfirebaugh]

Could we move the throttling into the worker itself? Or is the important thing to throttle actually the worker invocation itself? cc @ChrisLoer

[ivovandongen]

Could we move the throttling into the worker itself

If throttling is the only blocker here, we could probably find a solution. I'm wondering a bit if there are no catches when updating from the render thread. But I'll hack it up a bit for now to see if anything else comes up.

[ChrisLoer]

Sending a bunch of unused invocations to the worker mailbox shouldn't be a big deal if the handling is throttled by the worker. But you still have to somehow self-send a "do placement once my throttling duration is finished" message, which needs something like Timer/RunLoop.

[jfirebaugh]

If throttling is the only blocker here, we could probably find a solution. I'm wondering a bit if there are no catches when updating from the render thread.

You're right, it's not going to be the only blocker -- GeometryTile also assumes there is a RunLoop when creating the mailbox for messages coming back from the worker.

Will think about this more.

[jfirebaugh]

I think the cleanest way to resolve this is by avoiding the need for an orchestration/renderer thread distinction and have only the renderer thread. To do this we need to fix code that currently assumes that it's running on a thread with a RunLoop:

• Replace the current implicitly global "current RunLoop" with an implicitly global "current Scheduler": expose a static Scheduler::current() and use that for constructing Mailboxes instead of RunLoop::Get(). (Ideally we'd actually pass a reference to the current Scheduler down to everyone who needed it rather than have implicit thread-specific globals, but I'm not sure how feasible this is.)
• AndroidRendererFrontend should replace util::Thread<Renderer> with a custom Scheduler implementation that schedules onto the GLSurfaceView thread using GLSurfaceView::queueEvent. For the renderer thread, this will be the scheduler returned by Scheduler::current().
• That still leaves Throttler/Timer. Likely what needs to happen there is that both are rewritten in terms of actors instead of in terms of std::function and RunLoop. We should look into how this is handled in akka or other actor systems.

[ivovandongen]

@jfirebaugh I've been thinking a bit more about using only the render thread instead of a three thread setup. I like the simplification, but see one possible issue with this; it would probably make any background work impossible. The processing of the RenderStyle would be tightly coupled to the lifecycle of the visible parts of the map as the GL Thread is stopped when the containing activity is paused (not the current activity / the app is backgrounded). This would mainly be a problem for offline downloads, but would also limit continuation of resource loading while the user switches to other apps.

From the documentation:

A GLSurfaceView must be notified when to pause and resume rendering. GLSurfaceView clients are required to call onPause() when the activity stops and onResume() when the activity starts. These calls allow GLSurfaceView to pause and resume the rendering thread, and also allow GLSurfaceView to release and recreate the OpenGL display.

This is in line with what @kkaefer previously wrote up about the GLSurface view: #5766 (comment)

Another complication is continued support of TextureView. Needed for certain use cases where mapviews are layered on top of other elements. Although we need to do the threading here ourselves, it would be great not to diverge too much for maintainability.

I'll verify first what the actual behaviour is of the GL Thread when the activity is paused and if any work can be scheduled before continuing here.

[ivovandongen]

I'll verify first what the actual behaviour is of the GL Thread when the activity is paused and if any work can be scheduled before continuing here.

I've setup a basic Activity that continuously schedules on the GL Thread using GLSurfaceView#queueEvent. When paused, the work continues to execute on the test phones I have. Not sure if we can rely on that behaviour on all phones we support though.

[jfirebaugh]

Yes, good point. Even if we do keep two threads though, I think the actions to get that working are largely the same: removing reliance on a per-thread RunLoop.

[jfirebaugh]

The processing of the RenderStyle would be tightly coupled to the lifecycle of the visible parts of the map as the GL Thread is stopped when the containing activity is paused (not the current activity / the app is backgrounded).

This actually seems like the desired behavior to me. When the activity is paused, rendering work should also pause, no?

This would mainly be a problem for offline downloads, but would also limit continuation of resource loading while the user switches to other apps.

Offline downloads happen on a different thread though, so they shouldn't be affected.

[ivovandongen]

@jfirebaugh In a3afcb5 I've implemented the async rendering with 2 threads (dropping the Orchestration thread).

Since the GLThread has no runloop, and cannot have a runloop as it blocks on a mutex when used in RENDERMODE_WHEN_DIRTY until GLSurfaceView#requestRender is called, I've added a scheduler implementation that calls back into the main thread to request a render whenever Scheduler::schedule is called and processes the mailbox when a render is actually performed.

This doesn't, however, work when we need to make a blocking call into the renderer (query*Features for example). On the main thread the request is made through ActorRef::ask and then blocks on a future to await the result. The GLThread requests a render to process the request, but since the main thread is blocked, this will never happen and the main thread remains block while the GLThread is halted at it's mutex.

82f03a9 addresses that by skipping the actor for callbacks and the async task in between. This does mean that the invalidation is now done by two threads and the calls from the gl thread are not coalesced anymore. Need to investigate if this hampers performance.

117b899 adds a mutex to guard the update parameters. This solves a lot of the crashes I've been seeing (should have seen this before), but also fixes the flickering we saw on pinch gestures.

There are still a couple of crashes left, mostly around annotations (bitmaps) it seems. I'm going to dig into that now.

Do you still prefer this direction, with two threads over the 3 thread approach? It seems to work well for the most part now, I'm not sure if it is much cleaner as it is less obvious which parts are running on which thread(s).

[jfirebaugh]

Did you try the approach we discussed of using GLSurfaceView.queueEvent to implement schedule?

[kkaefer]

Instead of implementing our custom EGLConfigChooser object, we could also use the other overload, which—from a cursory glance—does the same thing:

    glSurfaceView.setEGLConfigChooser(5, // red
                                      6, // green
                                      5, // blue
                                      0, // alpha
                                      16, // depth
                                      8); // stencil

[ivovandongen]

Actually, this is how I started out (with a couple of variations). However, this didn't result in working context's on the two devices I have. This didn't give me confidence that it would work on the whole range of devices we support, that's why I ported the original to Java.

[jfirebaugh]

🎉 💯