Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Implement GLSurfaceView #5766

Closed
bleege opened this issue Jul 22, 2016 · 10 comments
Closed

Implement GLSurfaceView #5766

bleege opened this issue Jul 22, 2016 · 10 comments
Assignees
Labels
Android Mapbox Maps SDK for Android performance Speed, stability, CPU usage, memory usage, or power usage

Comments

@bleege
Copy link
Contributor

bleege commented Jul 22, 2016

This ticket is where work on refactoring Mapbox Android to use GLSurfaceView will take place.

In #5000 it was originally proposed to bring back SurfaceView and GLSurfaceView in favor of TextureView to increase performance of Map rendering and interactivity. While working on #5000 it was suggested to the team to implement SurfaceView first as it's a more straightforward implementation and doesn't require as much Core GL / C++ work while still providing a performance improvement so that's the direction the ticket took. To keep things more clear, compartmentalized, and easier to track we decided to break the two implementations up into separate tickets.

/cc @mapbox/mobile

@bleege bleege added performance Speed, stability, CPU usage, memory usage, or power usage Android Mapbox Maps SDK for Android labels Jul 22, 2016
@tobrun
Copy link
Member

tobrun commented Nov 16, 2016

SurfaceView vs GLSurfaceView

GLSurfaceView is based on SurfaceView and provides the following features:

  • Manages a surface, which is a special piece of memory that can be composited into the Android view system.
  • Manages an EGL display, which enables OpenGL to render into a surface.
  • Accepts a user-provided Renderer object that does the actual rendering.
  • Renders on a dedicated thread to decouple rendering performance from the UI thread.
  • Supports both on-demand and continuous rendering.
  • Optionally wraps, traces, and/or error-checks the renderer's OpenGL calls.

The biggest difference will be that GLSurfaceView sets up the EGL configuration and provides a dedicated render (thread). These two differences will resolve some issues we have been seeing on eglConfig/eglContext switching and should make our map more performant.

Migration

  • Strip out eglconfig/eglcontext from current implementation and let GLSurfaceView set this up
  • Post all events going into NativeMapView to the render thread (enqueue events on thread)
  • Post all events coming from NativeMapView to the ui thread (offline/callback setup)
  • Make it work

cc @mapbox/android

@tobrun tobrun added this to the android-v5.0.0 milestone Nov 16, 2016
@tobrun tobrun self-assigned this Nov 16, 2016
@tobrun
Copy link
Member

tobrun commented Nov 16, 2016

I was able to get the map to render:

device-2016-11-16-180117

Note that this is not a full working map, it only renders.
Next step is to start integrating the GLSurfaceView into the current MapView.
Biggest issue I currently foresee is getting messages from the Render thread to UI-thread.

update: I wasn't able to make above implementation work without crashing.

@tobrun
Copy link
Member

tobrun commented Jan 4, 2017

Capturing from chat that map object needs created and live on the render thread. Creating the map object calls into the platform specific AsyncTask::Impl. I'm currently hitting a crash when RunLoop::Impl is created as part of it. After reading some of the comments there:

// This is needed only for the RunLoop living on the main thread because of
// how we implement timers. We sleep on `ALooper_pollAll` until the next
// timeout, but on the main thread `ALooper_pollAll` is called by the activity
// automatically, thus we cannot set the timeout. Instead we wake the loop
// with an external file descriptor event coming from this thread.

I'm thinking that current implementation is optimized to work with the main thread and we need some changes to make it work from the render thread.

@tobrun
Copy link
Member

tobrun commented Jan 16, 2017

WIP PR in #7736

@tobrun
Copy link
Member

tobrun commented Jan 18, 2017

In #7736, I have setup where I'm creating the Map object on the main thread and using the GLSurfaceView created egl components (context, display). This is working, as-is, but we aren't rendering on the render thread yet. When attempting to move invocation to the render thread I'm seeing GeometryTileWorker crash when it calls parent.invoke.

eg.

parent.invoke(&GeometryTile::onLayout, GeometryTile::LayoutResult {
        std::move(buckets),
        std::move(featureIndex),
        *data ? (*data)->clone() : nullptr,
        correlationID
    });

Related stacktrace:

********** Crash dump: **********
Build fingerprint: 'motorola/surnia_reteu/surnia_umts:5.0.2/LXI22.50-53.8/17:user/release-keys'
pid: 25585, tid: 26026, name: pboxsdk.testapp  >>> com.mapbox.mapboxsdk.testapp <<<
signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr 0x0
Stack frame #00 pc 000ef90e  /data/app/com.mapbox.mapboxsdk.testapp-2/lib/arm/libmapbox-gl.so: Routine mbgl::Mailbox::push(std::__ndk1::unique_ptr<mbgl::Message, std::__ndk1::default_delete<mbgl::Message> >) at /Users/Nurbot/mapbox-gl-native/build/android-arm-v7/Debug/../../../src/mbgl/actor/mailbox.cpp:20
Stack frame #01 pc 00158a81  /data/app/com.mapbox.mapboxsdk.testapp-2/lib/arm/libmapbox-gl.so: Routine void mbgl::ActorRef<mbgl::GeometryTile>::invoke<void (mbgl::GeometryTile::*)(mbgl::GeometryTile::LayoutResult), mbgl::GeometryTile::LayoutResult>(void (mbgl::GeometryTile::*)(mbgl::GeometryTile::LayoutResult), mbgl::GeometryTile::LayoutResult&&) at /Users/Nurbot/mapbox-gl-native/build/android-arm-v7/Debug/../../../src/mbgl/actor/actor_ref.hpp:34 (discriminator 8)
Stack frame #02 pc 00157ef1  /data/app/com.mapbox.mapboxsdk.testapp-2/lib/arm/libmapbox-gl.so: Routine mbgl::GeometryTileWorker::redoLayout() at /Users/Nurbot/mapbox-gl-native/build/android-arm-v7/Debug/../../../src/mbgl/tile/geometry_tile_worker.cpp:263 (discriminator 1)
Stack frame #03 pc 001588f7  /data/app/com.mapbox.mapboxsdk.testapp-2/lib/arm/libmapbox-gl.so: Routine mbgl::GeometryTileWorker::coalesced() at /Users/Nurbot/mapbox-gl-native/build/android-arm-v7/Debug/../../../src/mbgl/tile/geometry_tile_worker.cpp:181
Stack frame #04 pc 000efa31  /data/app/com.mapbox.mapboxsdk.testapp-2/lib/arm/libmapbox-gl.so: Routine mbgl::Mailbox::receive() at /Users/Nurbot/mapbox-gl-native/build/android-arm-v7/Debug/../../../src/mbgl/actor/mailbox.cpp:48 (discriminator 1)
Stack frame #05 pc 000efb03  /data/app/com.mapbox.mapboxsdk.testapp-2/lib/arm/libmapbox-gl.so: Routine mbgl::Mailbox::maybeReceive(std::__ndk1::weak_ptr<mbgl::Mailbox>) at /Users/Nurbot/mapbox-gl-native/build/android-arm-v7/Debug/../../../src/mbgl/actor/mailbox.cpp:57
Stack frame #06 pc 00164f01  /data/app/com.mapbox.mapboxsdk.testapp-2/lib/arm/libmapbox-gl.so: Routine operator() at /Users/Nurbot/mapbox-gl-native/build/android-arm-v7/Debug/../../../platform/default/mbgl/util/default_thread_pool.cpp:25
Stack frame #07 pc 00015d9b  /system/lib/libc.so (__pthread_start(void*)+30)
Stack frame #08 pc 00013d5b  /system/lib/libc.so (__start_thread+6)

Since this is a crash found in core code and not the android binding specific c++ code. I'm going to revisit creating the map object on the render thread. This requires resolving the RunLoop crash at startup.

Related stacktrace:

********** Crash dump: **********
Build fingerprint: 'google/angler/angler:7.1.1/NPF10C/3347772:user/release-keys'
pid: 7131, tid: 7341, name: GLThread 781  >>> com.mapbox.mapboxsdk.testapp <<<
signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr 0x24
Stack frame #00 pc 0009e384  /data/app/com.mapbox.mapboxsdk.testapp-1/lib/arm/libmapbox-gl.so: Routine std::__ndk1::unique_ptr<mbgl::util::RunLoop::Impl, std::__ndk1::default_delete<mbgl::util::RunLoop::Impl> >::get() const at /home/tvn/Mapbox/mapbox-gl-native/mason_packages/linux-x86_64/android-ndk/arm-9-r13b/bin/../lib/gcc/arm-linux-androideabi/4.9.x/../../../../include/c++/4.9.x/memory:2725
Stack frame #01 pc 0009d997  /data/app/com.mapbox.mapboxsdk.testapp-1/lib/arm/libmapbox-gl.so: Routine Impl at /home/tvn/Mapbox/mapbox-gl-native/build/android-arm-v7/Debug/../../../platform/android/src/async_task.cpp:44
Stack frame #02 pc 0009d93b  /data/app/com.mapbox.mapboxsdk.testapp-1/lib/arm/libmapbox-gl.so: Routine std::__ndk1::__unique_if<mbgl::util::AsyncTask::Impl>::__unique_single std::__ndk1::make_unique<mbgl::util::AsyncTask::Impl, std::__ndk1::function<void ()> >(std::__ndk1::function<void ()>&&) at /home/tvn/Mapbox/mapbox-gl-native/mason_packages/linux-x86_64/android-ndk/arm-9-r13b/bin/../lib/gcc/arm-linux-androideabi/4.9.x/../../../../include/c++/4.9.x/memory:3149
Stack frame #03 pc 0005cf77  /data/app/com.mapbox.mapboxsdk.testapp-1/lib/arm/libmapbox-gl.so: Routine Impl at /home/tvn/Mapbox/mapbox-gl-native/build/android-arm-v7/Debug/../../../src/mbgl/map/map.cpp:145 (discriminator 1)
Stack frame #04 pc 0005ce93  /data/app/com.mapbox.mapboxsdk.testapp-1/lib/arm/libmapbox-gl.so: Routine std::__ndk1::__unique_if<mbgl::Map::Impl>::__unique_single std::__ndk1::make_unique<mbgl::Map::Impl, mbgl::Map&, mbgl::Backend&, float const&, mbgl::FileSource&, mbgl::Scheduler&, mbgl::MapMode&, mbgl::GLContextMode&, mbgl::ConstrainMode&, mbgl::ViewportMode&>(mbgl::Map&, mbgl::Backend&, float const&, mbgl::FileSource&, mbgl::Scheduler&, mbgl::MapMode&, mbgl::GLContextMode&, mbgl::ConstrainMode&, mbgl::ViewportMode&) at /home/tvn/Mapbox/mapbox-gl-native/mason_packages/linux-x86_64/android-ndk/arm-9-r13b/bin/../lib/gcc/arm-linux-androideabi/4.9.x/../../../../include/c++/4.9.x/memory:3149
Stack frame #05 pc 000e9b6d  /data/app/com.mapbox.mapboxsdk.testapp-1/lib/arm/libmapbox-gl.so: Routine std::__ndk1::__unique_if<mbgl::Map>::__unique_single std::__ndk1::make_unique<mbgl::Map, mbgl::android::NativeMapView&, mbgl::Size, float&, mbgl::DefaultFileSource&, mbgl::ThreadPool&, mbgl::MapMode>(mbgl::android::NativeMapView&, mbgl::Size&&, float&, mbgl::DefaultFileSource&, mbgl::ThreadPool&, mbgl::MapMode&&) at /home/tvn/Mapbox/mapbox-gl-native/mason_packages/linux-x86_64/android-ndk/arm-9-r13b/bin/../lib/gcc/arm-linux-androideabi/4.9.x/../../../../include/c++/4.9.x/memory:3149
Stack frame #06 pc 0004d70d  /data/app/com.mapbox.mapboxsdk.testapp-1/lib/arm/libmapbox-gl.so: Routine (anonymous namespace)::nativeOnCreate(_JNIEnv*, jni::jobject*, long long, float) at /home/tvn/Mapbox/mapbox-gl-native/build/android-arm-v7/Debug/../../../platform/android/src/jni.cpp:314
Stack frame #07 pc 005163b9  /data/app/com.mapbox.mapboxsdk.testapp-1/oat/arm/base.odex (offset 0x4ed000)

@ntomizawa
Copy link

Thanks the team

Is this problem already fixed in 5.0.0 release? If not, could you please tell me the release plan?

(We met the problem which has similar stack trace)

@zugaldia
Copy link
Member

@ntomizawa Unfortunately GlSurfaceView didn't make it to 5.0 but we remain committed to implementing this or a similar approach to improve rendering performance in the shorter term. We'll post an update when we have more concrete plans to share.

@tobrun
Copy link
Member

tobrun commented Apr 26, 2017

Next steps for introducing rendering on a separate thread in #8820.

@zugaldia zugaldia mentioned this issue Apr 26, 2017
28 tasks
@kkaefer
Copy link
Member

kkaefer commented May 9, 2017

A GLSurfaceView has a Renderer, which looks like this:

public class GLMapViewRenderer implements GLSurfaceView.Renderer {

  private static final String TAG = "MyGLRenderer";

  @Override
  public void onSurfaceCreated(GL10 unused, EGLConfig config) {
    GLES20.glClearColor(1.0f, 0.0f, 0.0f, 1.0f);
  }

  @Override
  public void onDrawFrame(GL10 unused) {
    GLES20.glClear(GLES20.GL_COLOR_BUFFER_BIT | GLES20.GL_DEPTH_BUFFER_BIT);
  }

  @Override
  public void onSurfaceChanged(GL10 unused, int width, int height) {
    GLES20.glViewport(0, 0, width, height);
  }
}

onSurfaceCreated is called when a view backing surface (framebuffer) is created. onSurfaceChanged is called when the backing surface is resized.

The Android Activity Lifecycle supports "pausing" an activity. This happens when the activity goes into the background (e.g. when another activity is shown), but also when e.g. a dialog is shown(!). By default, GLSurfaceView destroys the underlying EGLContext and all OpenGL objects derived from it. This means that any OpenGL objects created will not be available anymore and the handles are dangling.

The API supports a call named setPreserveEGLContextOnPause(true);, but the docs don't give a guarantee that the EGLContext won't be destroyed.

Here are a few things that we have to be aware of:

  • The "hammer" would be to destroy the Map object when an activity is paused. However, this leads to an awkward API because users would have to re-initialize the Map object when the activity is resumed. We could e.g. serialize the state in the Map object, but that gets tricky quickly with runtime styling, annotations etc. Ideally, we'd have the Map object continue to exist when the activity is paused.
  • There are no events that we can hook into on the GLThread of the GLSurfaceView that are called when the activity is paused or destroyed. This means that we should ideally only have OpenGL handles be owned by the GLThread, and no other resources.
  • When we have a render tree on the GLThread, we need to abandon all of the OpenGL handles when onSurfaceCreated is called (unique_resource::release()), and re-upload the handles from the source data. That also means we need to retain the data that we upload to OpenGL so that we can upload it again later on. This might be interesting for iOS as well, e.g. we could trash all OpenGL handles when the application goes to the background, and recreate them when it returns to foreground.

I'm starting to believe that the best architecture for this is to have an "orchestration" thread that holds the Painter object. When the GLThread kicks in, it pauses/blocks the orchestration thread and uses the Painter object to perform the render, potentially creating OpenGL objects in the process. When the orchestration thread abandons OpenGL handles, they're added to the Context's list of abandoned handles. Only the GLThread calls Context::performCleanup, which actually deletes the OpenGL handles. If onSurfaceCreated is called, we'll empty the list of abandoned handles without cleaning up (since the handles have been destroyed by the system already).

Potential issues I'm seeing:

  • The orchestration thread is owned by Map object (which lives on the main thread). When that is GCed, can the render thread potentially render things (and try to access Painter) when it's already been destructed?

Useful Links

@tobrun
Copy link
Member

tobrun commented Oct 2, 2017

done with #9576

@tobrun tobrun closed this as completed Oct 2, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Android Mapbox Maps SDK for Android performance Speed, stability, CPU usage, memory usage, or power usage
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants