[android] reset the native renderer only when the GL thread exitsupstream/lp-detach-gl-thread

7 files changed, 81 insertions, 66 deletions

diff --git a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/MapView.java b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/MapView.java
index 238c79392b..81cd1830e6 100644
--- a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/MapView.java
+++ b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/MapView.java
@@ -4,7 +4,6 @@ import android.content.Context;
 import android.graphics.Bitmap;
 import android.graphics.PointF;
 import android.graphics.drawable.ColorDrawable;
-import android.opengl.GLSurfaceView;
 import android.os.Bundle;
 import android.support.annotation.CallSuper;
 import android.support.annotation.NonNull;
@@ -19,6 +18,7 @@ import android.view.TextureView;
 import android.view.View;
 import android.widget.FrameLayout;
 import android.widget.ImageView;
+
 import com.mapbox.android.gestures.AndroidGesturesManager;
 import com.mapbox.mapboxsdk.MapStrictMode;
 import com.mapbox.mapboxsdk.Mapbox;
@@ -29,18 +29,20 @@ import com.mapbox.mapboxsdk.exceptions.MapboxConfigurationException;
 import com.mapbox.mapboxsdk.location.LocationComponent;
 import com.mapbox.mapboxsdk.maps.renderer.MapRenderer;
 import com.mapbox.mapboxsdk.maps.renderer.glsurfaceview.GLSurfaceViewMapRenderer;
+import com.mapbox.mapboxsdk.maps.renderer.glsurfaceview.MapboxGLSurfaceView;
 import com.mapbox.mapboxsdk.maps.renderer.textureview.TextureViewMapRenderer;
 import com.mapbox.mapboxsdk.maps.widgets.CompassView;
 import com.mapbox.mapboxsdk.net.ConnectivityReceiver;
 import com.mapbox.mapboxsdk.storage.FileSource;
 import com.mapbox.mapboxsdk.utils.BitmapUtils;
 
-import javax.microedition.khronos.egl.EGLConfig;
-import javax.microedition.khronos.opengles.GL10;
 import java.util.ArrayList;
 import java.util.Iterator;
 import java.util.List;
 
+import javax.microedition.khronos.egl.EGLConfig;
+import javax.microedition.khronos.opengles.GL10;
+
 import static com.mapbox.mapboxsdk.maps.widgets.CompassView.TIME_MAP_NORTH_ANIMATION;
 import static com.mapbox.mapboxsdk.maps.widgets.CompassView.TIME_WAIT_IDLE;
 
@@ -295,7 +297,7 @@ public class MapView extends FrameLayout implements NativeMapView.ViewCallback {
 
       addView(textureView, 0);
     } else {
-      GLSurfaceView glSurfaceView = new GLSurfaceView(getContext());
+      MapboxGLSurfaceView glSurfaceView = new MapboxGLSurfaceView(getContext());
       glSurfaceView.setZOrderMediaOverlay(mapboxMapOptions.getRenderSurfaceOnTop());
       mapRenderer = new GLSurfaceViewMapRenderer(getContext(), glSurfaceView, localFontFamily) {
         @Override
@@ -505,7 +507,7 @@ public class MapView extends FrameLayout implements NativeMapView.ViewCallback {
    */
   @UiThread
   public void onLowMemory() {
-    if (nativeMapView != null && mapboxMap != null && !destroyed ) {
+    if (nativeMapView != null && mapboxMap != null && !destroyed) {
       nativeMapView.onLowMemory();
     }
   }
@@ -745,7 +747,6 @@ public class MapView extends FrameLayout implements NativeMapView.ViewCallback {
   }
 
   /**
-   *
    * Set a callback that's invoked when the style has finished loading.
    *
    * @param listener The callback that's invoked when the style has finished loading
@@ -801,12 +802,14 @@ public class MapView extends FrameLayout implements NativeMapView.ViewCallback {
 
   /**
    * Set a callback that's invoked when map needs to release unused image resources.
-   *
+   * <p>
    * A callback will be called only for unused images that were provided by the client via
    * {@link OnStyleImageMissingListener#onStyleImageMissing(String)} listener interface.
-   *
+   * </p>
+   * <p>
    * By default, platform will remove unused images from the style. By adding listener, default
    * behavior can be overridden and client can control whether to release unused resources.
+   * </p>
    *
    * @param listener The callback that's invoked when map needs to release unused image resources
    */
@@ -816,9 +819,10 @@ public class MapView extends FrameLayout implements NativeMapView.ViewCallback {
 
   /**
    * Removes a callback that's invoked when map needs to release unused image resources.
-   *
+   * <p>
    * When all listeners are removed, platform will fallback to default behavior, which is to remove
    * unused images from the style.
+   * </p>
    *
    * @param listener The callback that's invoked when map needs to release unused image resources
    */
diff --git a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/NativeMapView.java b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/NativeMapView.java
index 3980790fd1..4e774d9f2c 100755
--- a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/NativeMapView.java
+++ b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/NativeMapView.java
@@ -532,7 +532,7 @@ final class NativeMapView implements NativeMap {
   @Override
   @NonNull
   public long[] queryPointAnnotations(RectF rect) {
-    if (checkState("queryPointAnnotations") || !mapRenderer.hasSurface()) {
+    if (checkState("queryPointAnnotations")) {
       return new long[] {};
     }
     return nativeQueryPointAnnotations(rect);
@@ -541,7 +541,7 @@ final class NativeMapView implements NativeMap {
   @Override
   @NonNull
   public long[] queryShapeAnnotations(RectF rectF) {
-    if (checkState("queryShapeAnnotations") || !mapRenderer.hasSurface()) {
+    if (checkState("queryShapeAnnotations")) {
       return new long[] {};
     }
     return nativeQueryShapeAnnotations(rectF);
@@ -573,7 +573,7 @@ final class NativeMapView implements NativeMap {
 
   @Override
   public void onLowMemory() {
-    if (checkState("onLowMemory") || !mapRenderer.hasSurface()) {
+    if (checkState("onLowMemory")) {
       return;
     }
     nativeOnLowMemory();
@@ -890,7 +890,7 @@ final class NativeMapView implements NativeMap {
   public List<Feature> queryRenderedFeatures(@NonNull PointF coordinates,
                                              @Nullable String[] layerIds,
                                              @Nullable Expression filter) {
-    if (checkState("queryRenderedFeatures") || !mapRenderer.hasSurface()) {
+    if (checkState("queryRenderedFeatures")) {
       return new ArrayList<>();
     }
     Feature[] features = nativeQueryRenderedFeaturesForPoint(coordinates.x / pixelRatio,
@@ -903,7 +903,7 @@ final class NativeMapView implements NativeMap {
   public List<Feature> queryRenderedFeatures(@NonNull RectF coordinates,
                                              @Nullable String[] layerIds,
                                              @Nullable Expression filter) {
-    if (checkState("queryRenderedFeatures") || !mapRenderer.hasSurface()) {
+    if (checkState("queryRenderedFeatures")) {
       return new ArrayList<>();
     }
     Feature[] features = nativeQueryRenderedFeaturesForBox(
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 715d05df6f..833a875a09 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
@@ -4,13 +4,12 @@ import android.content.Context;
 import android.support.annotation.CallSuper;
 import android.support.annotation.Keep;
 import android.support.annotation.NonNull;
+
 import com.mapbox.mapboxsdk.LibraryLoader;
 import com.mapbox.mapboxsdk.log.Logger;
 import com.mapbox.mapboxsdk.maps.MapboxMap;
 import com.mapbox.mapboxsdk.storage.FileSource;
 
-import java.util.concurrent.atomic.AtomicBoolean;
-
 import javax.microedition.khronos.egl.EGLConfig;
 import javax.microedition.khronos.opengles.GL10;
 
@@ -34,7 +33,6 @@ public abstract class MapRenderer implements MapRendererScheduler {
   private long nativePtr = 0;
   private double expectedRenderTime = 0;
   private MapboxMap.OnFpsChangedListener onFpsChangedListener;
-  protected AtomicBoolean hasSurface = new AtomicBoolean();
 
   public MapRenderer(@NonNull Context context, String localIdeographFontFamily) {
     float pixelRatio = context.getResources().getDisplayMetrics().density;
@@ -160,13 +158,4 @@ public abstract class MapRenderer implements MapRendererScheduler {
     }
     expectedRenderTime = 1E9 / maximumFps;
   }
-
-  /**
-   * Returns true if renderer has a surface to draw on.
-   *
-   * @return returns if renderer has a surface, false otherwise
-   */
-  public boolean hasSurface() {
-    return hasSurface.get();
-  }
 }
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 9ddee8d1ad..2d0c0ed69b 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
@@ -3,7 +3,6 @@ package com.mapbox.mapboxsdk.maps.renderer.glsurfaceview;
 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;
@@ -22,10 +21,10 @@ import static android.opengl.GLSurfaceView.RENDERMODE_WHEN_DIRTY;
 public class GLSurfaceViewMapRenderer extends MapRenderer implements GLSurfaceView.Renderer {
 
   @NonNull
-  private final GLSurfaceView glSurfaceView;
+  private final MapboxGLSurfaceView glSurfaceView;
 
   public GLSurfaceViewMapRenderer(Context context,
-                                  GLSurfaceView glSurfaceView,
+                                  MapboxGLSurfaceView glSurfaceView,
                                   String localIdeographFontFamily) {
     super(context, localIdeographFontFamily);
     this.glSurfaceView = glSurfaceView;
@@ -34,18 +33,13 @@ 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 surfaceCreated(SurfaceHolder holder) {
-        super.surfaceCreated(holder);
-        hasSurface.set(true);
-      }
-
+    glSurfaceView.setDetachedListener(new MapboxGLSurfaceView.OnGLSurfaceViewDetachedListener() {
       @Override
-      public void surfaceDestroyed(SurfaceHolder holder) {
-        super.surfaceDestroyed(holder);
-        hasSurface.set(false);
+      public void onGLSurfaceViewDetached() {
+        // because the GL thread is destroyed when the view is detached from window,
+        // we need to ensure releasing the native renderer as well.
+        // This avoids releasing it only when the view is being recreated, which is already on a new GL thread,
+        // and leads to JNI crashes like https://github.com/mapbox/mapbox-gl-native/issues/14618
         nativeReset();
       }
     });
@@ -103,9 +97,6 @@ public class GLSurfaceViewMapRenderer extends MapRenderer implements GLSurfaceVi
    */
   @Override
   public void requestRender() {
-    if (!hasSurface.get()) {
-      return;
-    }
     glSurfaceView.requestRender();
   }
 
diff --git a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/renderer/glsurfaceview/MapboxGLSurfaceView.java b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/renderer/glsurfaceview/MapboxGLSurfaceView.java
new file mode 100644
index 0000000000..ccdbc58220
--- /dev/null
+++ b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/renderer/glsurfaceview/MapboxGLSurfaceView.java
@@ -0,0 +1,54 @@
+package com.mapbox.mapboxsdk.maps.renderer.glsurfaceview;
+
+import android.content.Context;
+import android.opengl.GLSurfaceView;
+import android.support.annotation.NonNull;
+import android.util.AttributeSet;
+
+/**
+ * {@link GLSurfaceView} extension that notifies a listener when the view is detached from window,
+ * which is the point of destruction of the GL thread.
+ */
+public class MapboxGLSurfaceView extends GLSurfaceView {
+
+  private OnGLSurfaceViewDetachedListener detachedListener;
+
+  public MapboxGLSurfaceView(Context context) {
+    super(context);
+  }
+
+  public MapboxGLSurfaceView(Context context, AttributeSet attrs) {
+    super(context, attrs);
+  }
+
+  @Override
+  protected void onDetachedFromWindow() {
+    if (detachedListener != null) {
+      detachedListener.onGLSurfaceViewDetached();
+    }
+    super.onDetachedFromWindow();
+  }
+
+  /**
+   * Set a listener that gets notified when the view is detached from window.
+   *
+   * @param detachedListener listener
+   */
+  public void setDetachedListener(@NonNull OnGLSurfaceViewDetachedListener detachedListener) {
+    if (this.detachedListener != null) {
+      throw new IllegalArgumentException("Detached from window listener has been already set.");
+    }
+    this.detachedListener = detachedListener;
+  }
+
+  /**
+   * Listener interface that notifies when a {@link MapboxGLSurfaceView} is detached from window.
+   */
+  public interface OnGLSurfaceViewDetachedListener {
+
+    /**
+     * Called when a {@link MapboxGLSurfaceView} is detached from window.
+     */
+    void onGLSurfaceViewDetached();
+  }
+}
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
deleted file mode 100644
index 4c50df5029..0000000000
--- a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/renderer/glsurfaceview/SurfaceHolderCallbackAdapter.java
+++ /dev/null
@@ -1,21 +0,0 @@
-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 b60316e586..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
@@ -43,7 +43,6 @@ public class TextureViewMapRenderer extends MapRenderer {
   @Override
   protected void onSurfaceCreated(GL10 gl, EGLConfig config) {
     super.onSurfaceCreated(gl, config);
-    hasSurface.set(true);
   }
 
   /**
@@ -59,7 +58,6 @@ public class TextureViewMapRenderer extends MapRenderer {
    */
   @Override
   protected void onSurfaceDestroyed() {
-    hasSurface.set(false);
     super.onSurfaceDestroyed();
   }