[android] hook into surface holder to cleanup renderer on the right thread before the surface is destroyedupstream/ivd_fix_maprenderer_freeze_11685

7 files changed, 66 insertions, 5 deletions

diff --git a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/renderer/MapRenderer.java b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/renderer/MapRenderer.java
index a9bea97836..3eeb72f5ad 100644
--- a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/renderer/MapRenderer.java
+++ b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/renderer/MapRenderer.java
@@ -75,6 +75,11 @@ public abstract class MapRenderer implements MapRendererScheduler {
   }
 
   @CallSuper
+  protected void onSurfaceDestroyed() {
+    nativeOnSurfaceDestroyed();
+  }
+
+  @CallSuper
   protected void onDrawFrame(GL10 gl) {
     long startTime = System.nanoTime();
     try {
@@ -123,6 +128,8 @@ public abstract class MapRenderer implements MapRendererScheduler {
 
   private native void nativeOnSurfaceChanged(int width, int height);
 
+  private native void nativeOnSurfaceDestroyed();
+
   private native void nativeRender();
 
   private long timeElapsed;
diff --git a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/renderer/glsurfaceview/GLSurfaceViewMapRenderer.java b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/renderer/glsurfaceview/GLSurfaceViewMapRenderer.java
index 584c9dcf57..e757907ab8 100644
--- a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/renderer/glsurfaceview/GLSurfaceViewMapRenderer.java
+++ b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/renderer/glsurfaceview/GLSurfaceViewMapRenderer.java
@@ -4,6 +4,8 @@ import android.content.Context;
 import android.opengl.GLSurfaceView;
 
 import android.support.annotation.NonNull;
+import android.view.SurfaceHolder;
+
 import com.mapbox.mapboxsdk.maps.renderer.MapRenderer;
 import com.mapbox.mapboxsdk.maps.renderer.egl.EGLConfigChooser;
 
@@ -33,6 +35,15 @@ public class GLSurfaceViewMapRenderer extends MapRenderer implements GLSurfaceVi
     glSurfaceView.setRenderer(this);
     glSurfaceView.setRenderMode(RENDERMODE_WHEN_DIRTY);
     glSurfaceView.setPreserveEGLContextOnPause(true);
+
+    glSurfaceView.getHolder().addCallback(new SurfaceHolderCallbackAdapter() {
+
+      @Override
+      public void surfaceDestroyed(SurfaceHolder holder) {
+        GLSurfaceViewMapRenderer.this.onSurfaceDestroyed();
+      }
+
+    });
   }
 
   @Override
diff --git a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/renderer/glsurfaceview/SurfaceHolderCallbackAdapter.java b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/renderer/glsurfaceview/SurfaceHolderCallbackAdapter.java
new file mode 100644
index 0000000000..4c50df5029
--- /dev/null
+++ b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/renderer/glsurfaceview/SurfaceHolderCallbackAdapter.java
@@ -0,0 +1,21 @@
+package com.mapbox.mapboxsdk.maps.renderer.glsurfaceview;
+
+import android.view.SurfaceHolder;
+
+class SurfaceHolderCallbackAdapter implements SurfaceHolder.Callback {
+
+  @Override
+  public void surfaceCreated(SurfaceHolder holder) {
+
+  }
+
+  @Override
+  public void surfaceChanged(SurfaceHolder holder, int format, int width, int height) {
+
+  }
+
+  @Override
+  public void surfaceDestroyed(SurfaceHolder holder) {
+
+  }
+}
diff --git a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/renderer/textureview/TextureViewMapRenderer.java b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/renderer/textureview/TextureViewMapRenderer.java
index ad25dea0d3..46e6463fe8 100644
--- a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/renderer/textureview/TextureViewMapRenderer.java
+++ b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/renderer/textureview/TextureViewMapRenderer.java
@@ -57,6 +57,14 @@ public class TextureViewMapRenderer extends MapRenderer {
    * Overridden to provide package access
    */
   @Override
+  protected void onSurfaceDestroyed() {
+    super.onSurfaceDestroyed();
+  }
+
+  /**
+   * Overridden to provide package access
+   */
+  @Override
   protected void onDrawFrame(GL10 gl) {
     super.onDrawFrame(gl);
   }
diff --git a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/renderer/textureview/TextureViewRenderThread.java b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/renderer/textureview/TextureViewRenderThread.java
index 165b15a512..96d5e9e943 100644
--- a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/renderer/textureview/TextureViewRenderThread.java
+++ b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/renderer/textureview/TextureViewRenderThread.java
@@ -197,6 +197,7 @@ class TextureViewRenderThread extends Thread implements TextureView.SurfaceTextu
             if (destroySurface) {
               eglHolder.destroySurface();
               destroySurface = false;
+              mapRenderer.onSurfaceDestroyed();
               break;
             }
 
diff --git a/platform/android/src/map_renderer.cpp b/platform/android/src/map_renderer.cpp
index e9eb82864d..cff021afd2 100644
--- a/platform/android/src/map_renderer.cpp
+++ b/platform/android/src/map_renderer.cpp
@@ -33,9 +33,12 @@ MapRenderer::~MapRenderer() = default;
 
 void MapRenderer::reset() {
     destroyed = true;
-    // Make sure to destroy the renderer on the GL Thread
-    auto self = ActorRef<MapRenderer>(*this, mailbox);
-    self.ask(&MapRenderer::resetRenderer).wait();
+
+    if (renderer) {
+        // Make sure to destroy the renderer on the GL Thread
+        auto self = ActorRef<MapRenderer>(*this, mailbox);
+        self.ask(&MapRenderer::resetRenderer).wait();
+    }
 
     // Lock to make sure there is no concurrent initialisation on the gl thread
     std::lock_guard<std::mutex> lock(initialisationMutex);
@@ -113,7 +116,6 @@ void MapRenderer::requestSnapshot(SnapshotCallback callback) {
 // Called on OpenGL thread //
 
 void MapRenderer::resetRenderer() {
-    assert (renderer);
     renderer.reset();
 }
 
@@ -188,6 +190,12 @@ void MapRenderer::onSurfaceChanged(JNIEnv&, jint width, jint height) {
     requestRender();
 }
 
+void MapRenderer::onSurfaceDestroyed(JNIEnv&) {
+    // Make sure to destroy the renderer on the GL Thread
+    auto self = ActorRef<MapRenderer>(*this, mailbox);
+    self.ask(&MapRenderer::resetRenderer).wait();
+}
+
 // Static methods //
 
 void MapRenderer::registerNative(jni::JNIEnv& env) {
@@ -204,7 +212,9 @@ void MapRenderer::registerNative(jni::JNIEnv& env) {
                                          METHOD(&MapRenderer::onSurfaceCreated,
                                                 "nativeOnSurfaceCreated"),
                                          METHOD(&MapRenderer::onSurfaceChanged,
-                                                "nativeOnSurfaceChanged"));
+                                                "nativeOnSurfaceChanged"),
+                                         METHOD(&MapRenderer::onSurfaceDestroyed,
+                                                "nativeOnSurfaceDestroyed"));
 }
 
 MapRenderer& MapRenderer::getNativePeer(JNIEnv& env, const jni::Object<MapRenderer>& jObject) {
diff --git a/platform/android/src/map_renderer.hpp b/platform/android/src/map_renderer.hpp
index d60447e0ec..8c2f28a4df 100644
--- a/platform/android/src/map_renderer.hpp
+++ b/platform/android/src/map_renderer.hpp
@@ -94,6 +94,9 @@ private:
 
     void onSurfaceChanged(JNIEnv&, jint width, jint height);
 
+    // Called on Main thread
+    void onSurfaceDestroyed(JNIEnv&);
+
 private:
     jni::WeakReference<jni::Object<MapRenderer>, jni::EnvAttachingDeleter> javaPeer;