[android] - move shape annotation click handling to core

10 files changed, 129 insertions, 68 deletions

diff --git a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/AnnotationManager.java b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/AnnotationManager.java
index c09c926eb5..9f256c341b 100644
--- a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/AnnotationManager.java
+++ b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/AnnotationManager.java
@@ -21,7 +21,6 @@ import com.mapbox.mapboxsdk.annotations.Polygon;
 import com.mapbox.mapboxsdk.annotations.PolygonOptions;
 import com.mapbox.mapboxsdk.annotations.Polyline;
 import com.mapbox.mapboxsdk.annotations.PolylineOptions;
-import com.mapbox.services.commons.geojson.Feature;
 
 import java.util.ArrayList;
 import java.util.List;
@@ -41,7 +40,6 @@ import timber.log.Timber;
  */
 class AnnotationManager {
 
-  private static final String LAYER_ID_SHAPE_ANNOTATIONS = "com.mapbox.annotations.shape.";
   private static final long NO_ANNOTATION_ID = -1;
 
   private final MapView mapView;
@@ -50,7 +48,6 @@ class AnnotationManager {
   private final MarkerViewManager markerViewManager;
   private final LongSparseArray<Annotation> annotationsArray;
   private final List<Marker> selectedMarkers = new ArrayList<>();
-  private final List<String> shapeAnnotationIds = new ArrayList<>();
 
   private MapboxMap mapboxMap;
   private MapboxMap.OnMarkerClickListener onMarkerClickListener;
@@ -58,13 +55,14 @@ class AnnotationManager {
   private MapboxMap.OnPolylineClickListener onPolylineClickListener;
 
   private Annotations annotations;
+  private ShapeAnnotations shapeAnnotations;
   private Markers markers;
   private Polygons polygons;
   private Polylines polylines;
 
   AnnotationManager(NativeMapView view, MapView mapView, LongSparseArray<Annotation> annotationsArray,
                     MarkerViewManager markerViewManager, IconManager iconManager, Annotations annotations,
-                    Markers markers, Polygons polygons, Polylines polylines) {
+                    Markers markers, Polygons polygons, Polylines polylines, ShapeAnnotations shapeAnnotations) {
     this.mapView = mapView;
     this.annotationsArray = annotationsArray;
     this.markerViewManager = markerViewManager;
@@ -73,6 +71,7 @@ class AnnotationManager {
     this.markers = markers;
     this.polygons = polygons;
     this.polylines = polylines;
+    this.shapeAnnotations = shapeAnnotations;
     if (view != null) {
       // null checking needed for unit tests
       view.addOnMapChangedListener(markerViewManager);
@@ -122,9 +121,6 @@ class AnnotationManager {
         // do icon cleanup
         iconManager.iconCleanup(marker.getIcon());
       }
-    } else {
-      // instanceOf Polygon/Polyline
-      shapeAnnotationIds.remove(annotation.getId());
     }
     annotations.removeBy(annotation);
   }
@@ -143,9 +139,6 @@ class AnnotationManager {
         } else {
           iconManager.iconCleanup(marker.getIcon());
         }
-      } else {
-        // instanceOf Polygon/Polyline
-        shapeAnnotationIds.remove(annotation.getId());
       }
     }
     annotations.removeBy(annotationList);
@@ -167,9 +160,6 @@ class AnnotationManager {
         } else {
           iconManager.iconCleanup(marker.getIcon());
         }
-      } else {
-        // instanceOf Polygon/Polyline
-        shapeAnnotationIds.remove(annotation.getId());
       }
     }
     annotations.removeAll();
@@ -227,17 +217,11 @@ class AnnotationManager {
   //
 
   Polygon addPolygon(@NonNull PolygonOptions polygonOptions, @NonNull MapboxMap mapboxMap) {
-    Polygon polygon = polygons.addBy(polygonOptions, mapboxMap);
-    shapeAnnotationIds.add(LAYER_ID_SHAPE_ANNOTATIONS + polygon.getId());
-    return polygon;
+    return polygons.addBy(polygonOptions, mapboxMap);
   }
 
   List<Polygon> addPolygons(@NonNull List<PolygonOptions> polygonOptionsList, @NonNull MapboxMap mapboxMap) {
-    List<Polygon> polygonList = polygons.addBy(polygonOptionsList, mapboxMap);
-    for (Polygon polygon : polygonList) {
-      shapeAnnotationIds.add(LAYER_ID_SHAPE_ANNOTATIONS + polygon.getId());
-    }
-    return polygonList;
+    return polygons.addBy(polygonOptionsList, mapboxMap);
   }
 
   void updatePolygon(Polygon polygon) {
@@ -257,17 +241,11 @@ class AnnotationManager {
   //
 
   Polyline addPolyline(@NonNull PolylineOptions polylineOptions, @NonNull MapboxMap mapboxMap) {
-    Polyline polyline = polylines.addBy(polylineOptions, mapboxMap);
-    shapeAnnotationIds.add(LAYER_ID_SHAPE_ANNOTATIONS + polyline.getId());
-    return polyline;
+    return polylines.addBy(polylineOptions, mapboxMap);
   }
 
   List<Polyline> addPolylines(@NonNull List<PolylineOptions> polylineOptionsList, @NonNull MapboxMap mapboxMap) {
-    List<Polyline> polylineList = polylines.addBy(polylineOptionsList, mapboxMap);
-    for (Polyline polyline : polylineList) {
-      shapeAnnotationIds.add(LAYER_ID_SHAPE_ANNOTATIONS + polyline.getId());
-    }
-    return polylineList;
+    return polylines.addBy(polylineOptionsList, mapboxMap);
   }
 
   void updatePolyline(Polyline polyline) {
@@ -397,11 +375,11 @@ class AnnotationManager {
   //
 
   boolean onTap(PointF tapPoint) {
-    if (!shapeAnnotationIds.isEmpty()) {
-      ShapeAnnotationHit shapeAnnotationHit = getShapeAnnotationHitFromTap(tapPoint);
-      long shapeAnnotationId = new ShapeAnnotationHitResolver(mapboxMap).execute(shapeAnnotationHit);
-      if (shapeAnnotationId != NO_ANNOTATION_ID) {
-        handleClickForShapeAnnotation(shapeAnnotationId);
+    ShapeAnnotationHit shapeAnnotationHit = getShapeAnnotationHitFromTap(tapPoint);
+    Annotation annotation = new ShapeAnnotationHitResolver(shapeAnnotations).execute(shapeAnnotationHit);
+    if (annotation != null) {
+      if (handleClickForShapeAnnotation(annotation)) {
+        return true;
       }
     }
 
@@ -418,16 +396,18 @@ class AnnotationManager {
       tapPoint.x + touchTargetSide,
       tapPoint.y + touchTargetSide
     );
-    return new ShapeAnnotationHit(tapRect, shapeAnnotationIds.toArray(new String[shapeAnnotationIds.size()]));
+    return new ShapeAnnotationHit(tapRect);
   }
 
-  private void handleClickForShapeAnnotation(long shapeAnnotationId) {
-    Annotation annotation = getAnnotation(shapeAnnotationId);
+  private boolean handleClickForShapeAnnotation(Annotation annotation) {
     if (annotation instanceof Polygon && onPolygonClickListener != null) {
       onPolygonClickListener.onPolygonClick((Polygon) annotation);
+      return true;
     } else if (annotation instanceof Polyline && onPolylineClickListener != null) {
       onPolylineClickListener.onPolylineClick((Polyline) annotation);
+      return true;
     }
+    return false;
   }
 
   private MarkerHit getMarkerHitFromTouchArea(PointF tapPoint) {
@@ -470,28 +450,19 @@ class AnnotationManager {
 
   private static class ShapeAnnotationHitResolver {
 
-    private MapboxMap mapboxMap;
-
-    ShapeAnnotationHitResolver(MapboxMap mapboxMap) {
-      this.mapboxMap = mapboxMap;
-    }
+    private ShapeAnnotations shapeAnnotations;
 
-    public long execute(ShapeAnnotationHit shapeHit) {
-      long foundAnnotationId = NO_ANNOTATION_ID;
-      List<Feature> features = mapboxMap.queryRenderedFeatures(shapeHit.tapPoint, shapeHit.layerIds);
-      if (!features.isEmpty()) {
-        foundAnnotationId = getIdFromFeature(features.get(0));
-      }
-      return foundAnnotationId;
+    ShapeAnnotationHitResolver(ShapeAnnotations shapeAnnotations) {
+      this.shapeAnnotations = shapeAnnotations;
     }
 
-    private long getIdFromFeature(Feature feature) {
-      try {
-        return Long.valueOf(feature.getId());
-      } catch (NumberFormatException exception) {
-        Timber.e(exception, "Couldn't parse feature id to a long, with id: %s", feature.getId());
-        return NO_ANNOTATION_ID;
+    public Annotation execute(ShapeAnnotationHit shapeHit) {
+      Annotation foundAnnotation = null;
+      List<Annotation> annotations = shapeAnnotations.obtainAllIn(shapeHit.tapPoint);
+      if (annotations.size() > 0) {
+        foundAnnotation = annotations.get(0);
       }
+      return foundAnnotation;
     }
   }
 
@@ -567,11 +538,9 @@ class AnnotationManager {
 
   private static class ShapeAnnotationHit {
     private final RectF tapPoint;
-    private final String[] layerIds;
 
-    ShapeAnnotationHit(RectF tapRect, String[] layerIds) {
-      this.tapPoint = tapRect;
-      this.layerIds = layerIds;
+    ShapeAnnotationHit(RectF tapPoint) {
+      this.tapPoint = tapPoint;
     }
   }
 
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 f4198cdde7..de27964a5d 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
@@ -175,8 +175,9 @@ public class MapView extends FrameLayout {
     Markers markers = new MarkerContainer(nativeMapView, this, annotationsArray, iconManager, markerViewManager);
     Polygons polygons = new PolygonContainer(nativeMapView, annotationsArray);
     Polylines polylines = new PolylineContainer(nativeMapView, annotationsArray);
+    ShapeAnnotations shapeAnnotations = new ShapeAnnotationContainer(nativeMapView, annotationsArray);
     AnnotationManager annotationManager = new AnnotationManager(nativeMapView, this, annotationsArray,
-      markerViewManager, iconManager, annotations, markers, polygons, polylines);
+      markerViewManager, iconManager, annotations, markers, polygons, polylines, shapeAnnotations);
     Transform transform = new Transform(nativeMapView, annotationManager.getMarkerViewManager(), trackingSettings,
       cameraChangeDispatcher);
 
diff --git a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/MarkerContainer.java b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/MarkerContainer.java
index 072382ce07..2c2f07a112 100644
--- a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/MarkerContainer.java
+++ b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/MarkerContainer.java
@@ -98,15 +98,8 @@ class MarkerContainer implements Markers {
   @NonNull
   @Override
   public List<Marker> obtainAllIn(@NonNull RectF rectangle) {
-    // convert Rectangle to be density depedent
-    float pixelRatio = nativeMapView.getPixelRatio();
-    RectF rect = new RectF(rectangle.left / pixelRatio,
-      rectangle.top / pixelRatio,
-      rectangle.right / pixelRatio,
-      rectangle.bottom / pixelRatio);
-
+    RectF rect = nativeMapView.getDensityDependantRectangle(rectangle);
     long[] ids = nativeMapView.queryPointAnnotations(rect);
-
     List<Long> idsList = new ArrayList<>(ids.length);
     for (long id : ids) {
       idsList.add(id);
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 bd8a54783e..db0b14a4c7 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
@@ -468,6 +468,13 @@ final class NativeMapView {
     return nativeQueryPointAnnotations(rect);
   }
 
+  public long[] queryShapeAnnotations(RectF rectF) {
+    if (isDestroyedOn("queryShapeAnnotations")) {
+      return new long[] {};
+    }
+    return nativeQueryShapeAnnotations(rectF);
+  }
+
   public void addAnnotationIcon(String symbol, int width, int height, float scale, byte[] pixels) {
     if (isDestroyedOn("addAnnotationIcon")) {
       return;
@@ -825,6 +832,15 @@ final class NativeMapView {
     return pixelRatio;
   }
 
+  RectF getDensityDependantRectangle(final RectF rectangle) {
+    return new RectF(
+      rectangle.left / pixelRatio,
+      rectangle.top / pixelRatio,
+      rectangle.right / pixelRatio,
+      rectangle.bottom / pixelRatio
+    );
+  }
+
   //
   // Callbacks
   //
@@ -927,6 +943,8 @@ final class NativeMapView {
 
   private native long[] nativeQueryPointAnnotations(RectF rect);
 
+  private native long[] nativeQueryShapeAnnotations(RectF rect);
+
   private native void nativeAddAnnotationIcon(String symbol, int width, int height, float scale, byte[] pixels);
 
   private native void nativeRemoveAnnotationIcon(String symbol);
diff --git a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/ShapeAnnotationContainer.java b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/ShapeAnnotationContainer.java
new file mode 100644
index 0000000000..6ded2f32fb
--- /dev/null
+++ b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/ShapeAnnotationContainer.java
@@ -0,0 +1,38 @@
+package com.mapbox.mapboxsdk.maps;
+
+import android.graphics.RectF;
+import android.support.v4.util.LongSparseArray;
+
+import com.mapbox.mapboxsdk.annotations.Annotation;
+
+import java.util.ArrayList;
+import java.util.List;
+
+class ShapeAnnotationContainer implements ShapeAnnotations {
+
+  private final NativeMapView nativeMapView;
+  private final LongSparseArray<Annotation> annotations;
+
+  ShapeAnnotationContainer(NativeMapView nativeMapView, LongSparseArray<Annotation> annotations) {
+    this.nativeMapView = nativeMapView;
+    this.annotations = annotations;
+  }
+
+  @Override
+  public List<Annotation> obtainAllIn(RectF rectangle) {
+    RectF rect = nativeMapView.getDensityDependantRectangle(rectangle);
+    long[] annotationIds = nativeMapView.queryShapeAnnotations(rect);
+    return getAnnotationsFromIds(annotationIds);
+  }
+
+  private List<Annotation> getAnnotationsFromIds(long[] annotationIds) {
+    List<Annotation> shapeAnnotations = new ArrayList<>();
+    for (long annotationId : annotationIds) {
+      Annotation annotation = annotations.get(annotationId);
+      if (annotation != null) {
+        shapeAnnotations.add(annotation);
+      }
+    }
+    return shapeAnnotations;
+  }
+}
diff --git a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/ShapeAnnotations.java b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/ShapeAnnotations.java
new file mode 100644
index 0000000000..f9b2ca10db
--- /dev/null
+++ b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/ShapeAnnotations.java
@@ -0,0 +1,13 @@
+package com.mapbox.mapboxsdk.maps;
+
+import android.graphics.RectF;
+
+import com.mapbox.mapboxsdk.annotations.Annotation;
+
+import java.util.List;
+
+interface ShapeAnnotations {
+
+  List<Annotation> obtainAllIn(RectF rectF);
+
+}
diff --git a/platform/android/src/android_renderer_frontend.cpp b/platform/android/src/android_renderer_frontend.cpp
index b80e23e21f..afdb08a10e 100644
--- a/platform/android/src/android_renderer_frontend.cpp
+++ b/platform/android/src/android_renderer_frontend.cpp
@@ -112,6 +112,11 @@ AnnotationIDs AndroidRendererFrontend::queryPointAnnotations(const ScreenBox& bo
     return mapRenderer.actor().ask(&Renderer::queryPointAnnotations, box).get();
 }
 
+AnnotationIDs AndroidRendererFrontend::queryShapeAnnotations(const ScreenBox& box) const {
+    // Waits for the result from the orchestration thread and returns
+    return mapRenderer.actor().ask(&Renderer::queryShapeAnnotations, box).get();
+}
+
 } // namespace android
 } // namespace mbgl
 
diff --git a/platform/android/src/android_renderer_frontend.hpp b/platform/android/src/android_renderer_frontend.hpp
index 94508fd816..178870c452 100644
--- a/platform/android/src/android_renderer_frontend.hpp
+++ b/platform/android/src/android_renderer_frontend.hpp
@@ -36,6 +36,7 @@ public:
     std::vector<Feature> queryRenderedFeatures(const ScreenBox&, const RenderedQueryOptions&) const;
     std::vector<Feature> querySourceFeatures(const std::string& sourceID, const SourceQueryOptions&) const;
     AnnotationIDs queryPointAnnotations(const ScreenBox& box) const;
+    AnnotationIDs queryShapeAnnotations(const ScreenBox& box) const;
 
     // Memory
     void onLowMemory();
diff --git a/platform/android/src/native_map_view.cpp b/platform/android/src/native_map_view.cpp
index 24a35a7068..7372ecdf71 100755
--- a/platform/android/src/native_map_view.cpp
+++ b/platform/android/src/native_map_view.cpp
@@ -633,6 +633,26 @@ jni::Array<jlong> NativeMapView::queryPointAnnotations(JNIEnv& env, jni::Object<
     return result;
 }
 
+jni::Array<jlong> NativeMapView::queryShapeAnnotations(JNIEnv &env, jni::Object<RectF> rect) {
+    using namespace mbgl::style;
+    using namespace mbgl::style::conversion;
+
+    // Convert input
+    mbgl::ScreenBox box = {
+         {RectF::getLeft(env, rect),  RectF::getTop(env, rect)},
+         {RectF::getRight(env, rect), RectF::getBottom(env, rect)},
+    };
+
+    mbgl::AnnotationIDs ids = rendererFrontend->queryShapeAnnotations(box);
+
+    // Convert result
+    std::vector<jlong> longIds(ids.begin(), ids.end());
+    auto result = jni::Array<jni::jlong>::New(env, ids.size());
+    result.SetRegion<std::vector<jni::jlong>>(env, 0, longIds);
+
+    return result;
+}
+
 jni::Array<jni::Object<geojson::Feature>> NativeMapView::queryRenderedFeaturesForPoint(JNIEnv& env, jni::jfloat x, jni::jfloat y,
                                                                               jni::Array<jni::String> layerIds,
                                                                               jni::Array<jni::Object<>> jfilter) {
@@ -1000,6 +1020,7 @@ void NativeMapView::registerNative(jni::JNIEnv& env) {
             METHOD(&NativeMapView::getTransitionDelay, "nativeGetTransitionDelay"),
             METHOD(&NativeMapView::setTransitionDelay, "nativeSetTransitionDelay"),
             METHOD(&NativeMapView::queryPointAnnotations, "nativeQueryPointAnnotations"),
+            METHOD(&NativeMapView::queryShapeAnnotations, "nativeQueryShapeAnnotations"),
             METHOD(&NativeMapView::queryRenderedFeaturesForPoint, "nativeQueryRenderedFeaturesForPoint"),
             METHOD(&NativeMapView::queryRenderedFeaturesForBox, "nativeQueryRenderedFeaturesForBox"),
             METHOD(&NativeMapView::getLight, "nativeGetLight"),
diff --git a/platform/android/src/native_map_view.hpp b/platform/android/src/native_map_view.hpp
index 4d226d0fa9..11b12a9f6f 100755
--- a/platform/android/src/native_map_view.hpp
+++ b/platform/android/src/native_map_view.hpp
@@ -197,6 +197,8 @@ public:
 
     jni::Array<jlong> queryPointAnnotations(JNIEnv&, jni::Object<RectF>);
 
+    jni::Array<jlong> queryShapeAnnotations(JNIEnv&, jni::Object<RectF>);
+
     jni::Array<jni::Object<geojson::Feature>> queryRenderedFeaturesForPoint(JNIEnv&, jni::jfloat, jni::jfloat,
                                                                    jni::Array<jni::String>,
                                                                    jni::Array<jni::Object<>> jfilter);