[core, android, ios, macos] Replaced getPointAnnotationsInBounds() w/…

… queryPointAnnotations()

queryPointAnnotations() accepts a screen rectangle instead of a geographic bounding box, so marker hit testing works at the edges of a rotated, tilted map view.

Fixes #5151.

include/mbgl/map/map.hpp

@@ -148,8 +148,6 @@ class Map : private util::noncopyable {
     void updateAnnotation(AnnotationID, const Annotation&);
     void removeAnnotation(AnnotationID);
 
-    AnnotationIDs getPointAnnotationsInBounds(const LatLngBounds&);
-
     // Sources
     style::Source* getSource(const std::string& sourceID);
     void addSource(std::unique_ptr<style::Source>);
@@ -163,6 +161,7 @@ class Map : private util::noncopyable {
     // Feature queries
     std::vector<Feature> queryRenderedFeatures(const ScreenCoordinate&, const optional<std::vector<std::string>>& layerIDs = {});
     std::vector<Feature> queryRenderedFeatures(const ScreenBox&,        const optional<std::vector<std::string>>& layerIDs = {});
+    AnnotationIDs queryPointAnnotations(const ScreenBox&);
 
     // Memory
     void setSourceTileCacheSize(size_t);

platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/annotations/MarkerViewManager.java

@@ -2,6 +2,7 @@
 
 import android.content.Context;
 import android.graphics.PointF;
+import android.graphics.RectF;
 import android.os.SystemClock;
 import android.support.annotation.NonNull;
 import android.support.annotation.Nullable;
@@ -275,7 +276,7 @@ public void addMarkerViewAdapter(MapboxMap.MarkerViewAdapter markerViewAdapter)
 
         if (!markerViewAdapters.contains(markerViewAdapter)) {
             markerViewAdapters.add(markerViewAdapter);
-            invalidateViewMarkersInBounds();
+            invalidateViewMarkersInVisibleRegion();
         }
     }
 
@@ -300,7 +301,7 @@ public void setOnMarkerViewClickListener(@Nullable MapboxMap.OnMarkerViewClickLi
     /**
      * Schedule that ViewMarkers found in the viewport are invalidated.
      * <p>
-     * This method is rate limited, and {@link #invalidateViewMarkersInBounds} will only be called
+     * This method is rate limited, and {@link #invalidateViewMarkersInVisibleRegion} will only be called
      * once each 250 ms.
      * </p>
      */
@@ -310,7 +311,7 @@ public void scheduleViewMarkerInvalidation() {
             if (currentTime < viewMarkerBoundsUpdateTime) {
                 return;
             }
-            invalidateViewMarkersInBounds();
+            invalidateViewMarkersInVisibleRegion();
             viewMarkerBoundsUpdateTime = currentTime + 250;
         }
     }
@@ -322,9 +323,9 @@ public void scheduleViewMarkerInvalidation() {
      * ones for each found Marker in the changed viewport.
      * </p>
      */
-    public void invalidateViewMarkersInBounds() {
-        Projection projection = mapboxMap.getProjection();
-        List<MarkerView> markers = mapView.getMarkerViewsInBounds(projection.getVisibleRegion().latLngBounds);
+    public void invalidateViewMarkersInVisibleRegion() {
+        RectF mapViewRect = new RectF(0, 0, mapView.getWidth(), mapView.getHeight());
+        List<MarkerView> markers = mapView.getMarkerViewsInRect(mapViewRect);
         View convertView;
 
         // remove old markers

platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/MapView.java

@@ -1108,13 +1108,13 @@ void removeAnnotations(@NonNull long[] ids) {
         mNativeMapView.removeAnnotations(ids);
     }
 
-    List<Marker> getMarkersInBounds(@NonNull LatLngBounds bbox) {
-        if (mDestroyed || bbox == null) {
+    List<Marker> getMarkersInRect(@NonNull RectF rect) {
+        if (mDestroyed || rect == null) {
             return new ArrayList<>();
         }
 
-        // TODO: filter in JNI using C++ parameter to getAnnotationsInBounds
-        long[] ids = mNativeMapView.getAnnotationsInBounds(bbox);
+        // TODO: filter in JNI using C++ parameter to queryPointAnnotations
+        long[] ids = mNativeMapView.queryPointAnnotations(rect);
 
         List<Long> idsList = new ArrayList<>(ids.length);
         for (int i = 0; i < ids.length; i++) {
@@ -1134,13 +1134,13 @@ List<Marker> getMarkersInBounds(@NonNull LatLngBounds bbox) {
         return new ArrayList<>(annotations);
     }
 
-    public List<MarkerView> getMarkerViewsInBounds(@NonNull LatLngBounds bbox) {
-        if (mDestroyed || bbox == null) {
+    public List<MarkerView> getMarkerViewsInRect(@NonNull RectF rect) {
+        if (mDestroyed || rect == null) {
             return new ArrayList<>();
         }
 
-        // TODO: filter in JNI using C++ parameter to getAnnotationsInBounds
-        long[] ids = mNativeMapView.getAnnotationsInBounds(bbox);
+        // TODO: filter in JNI using C++ parameter to queryPointAnnotations
+        long[] ids = mNativeMapView.queryPointAnnotations(rect);
 
         List<Long> idsList = new ArrayList<>(ids.length);
         for (int i = 0; i < ids.length; i++) {
@@ -1642,13 +1642,7 @@ public boolean onSingleTapConfirmed(MotionEvent e) {
                     tapPoint.x + mAverageIconWidth / 2 + toleranceSides,
                     tapPoint.y + mAverageIconHeight / 2 + toleranceTopBottom);
 
-            LatLngBounds.Builder builder = new LatLngBounds.Builder();
-            builder.include(fromScreenLocation(new PointF(tapRect.left, tapRect.bottom)));
-            builder.include(fromScreenLocation(new PointF(tapRect.left, tapRect.top)));
-            builder.include(fromScreenLocation(new PointF(tapRect.right, tapRect.top)));
-            builder.include(fromScreenLocation(new PointF(tapRect.right, tapRect.bottom)));
-
-            List<Marker> nearbyMarkers = getMarkersInBounds(builder.build());
+            List<Marker> nearbyMarkers = getMarkersInRect(tapRect);
             long newSelectedMarkerId = -1;
 
             if (nearbyMarkers != null && nearbyMarkers.size() > 0) {

platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/MapboxMap.java

@@ -705,7 +705,7 @@ public MarkerView addMarker(@NonNull BaseMarkerViewOptions markerOptions) {
         long id = mMapView.addMarker(marker);
         marker.setId(id);
         mAnnotations.put(id, marker);
-        mMarkerViewManager.invalidateViewMarkersInBounds();
+        mMarkerViewManager.invalidateViewMarkersInVisibleRegion();
         return marker;
     }
 

platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/NativeMapView.java

@@ -385,8 +385,8 @@ public void removeAnnotations(long[] ids) {
         nativeRemoveAnnotations(mNativeMapViewPtr, ids);
     }
 
-    public long[] getAnnotationsInBounds(LatLngBounds bbox) {
-        return nativeGetAnnotationsInBounds(mNativeMapViewPtr, bbox);
+    public long[] queryPointAnnotations(RectF rect) {
+        return nativeQueryPointAnnotations(mNativeMapViewPtr, rect);
     }
 
     public void addAnnotationIcon(String symbol, int width, int height, float scale, byte[] pixels) {
@@ -601,7 +601,7 @@ private native void nativeSetBearingXY(long nativeMapViewPtr, double degrees,
 
     private native void nativeRemoveAnnotations(long nativeMapViewPtr, long[] id);
 
-    private native long[] nativeGetAnnotationsInBounds(long mNativeMapViewPtr, LatLngBounds bbox);
+    private native long[] nativeQueryPointAnnotations(long mNativeMapViewPtr, RectF rect);
 
     private native void nativeAddAnnotationIcon(long nativeMapViewPtr, String symbol,
                                                 int width, int height, float scale, byte[] pixels);

platform/android/src/jni.cpp

@@ -809,20 +809,24 @@ void nativeRemoveAnnotations(JNIEnv *env, jni::jobject* obj, jlong nativeMapView
     }
 }
 
-jni::jarray<jlong>* nativeGetAnnotationsInBounds(JNIEnv *env, jni::jobject* obj, jlong nativeMapViewPtr, jni::jobject* latLngBounds_) {
-    mbgl::Log::Debug(mbgl::Event::JNI, "nativeGetAnnotationsInBounds");
+jni::jarray<jlong>* nativeQueryPointAnnotations(JNIEnv *env, jni::jobject* obj, jlong nativeMapViewPtr, jni::jobject* rect) {
+    mbgl::Log::Debug(mbgl::Event::JNI, "nativeQueryPointAnnotations");
     assert(nativeMapViewPtr != 0);
     NativeMapView *nativeMapView = reinterpret_cast<NativeMapView *>(nativeMapViewPtr);
 
     // Conversion
-    mbgl::LatLngBounds latLngBounds = latlngbounds_from_java(env, latLngBounds_);
-    if (latLngBounds.isEmpty()) {
-        return nullptr;
-    }
+    jfloat left = jni::GetField<jfloat>(*env, rect, *rectFLeftId);
+    jfloat right = jni::GetField<jfloat>(*env, rect, *rectFRightId);
+    jfloat top = jni::GetField<jfloat>(*env, rect, *rectFTopId);
+    jfloat bottom = jni::GetField<jfloat>(*env, rect, *rectFBottomId);
+    mbgl::ScreenBox box = {
+        { left, top },
+        { right, bottom },
+    };
 
     // Assume only points for now
-    std::vector<uint32_t> annotations = nativeMapView->getMap().getPointAnnotationsInBounds(
-        latLngBounds);
+    std::vector<uint32_t> annotations = nativeMapView->getMap().queryPointAnnotations(
+        box);
 
     return std_vector_uint_to_jobject(env, annotations);
 }
@@ -1676,7 +1680,7 @@ extern "C" JNIEXPORT jint JNI_OnLoad(JavaVM *vm, void *reserved) {
         MAKE_NATIVE_METHOD(nativeAddPolygons, "(J[Lcom/mapbox/mapboxsdk/annotations/Polygon;)[J"),
         MAKE_NATIVE_METHOD(nativeUpdateMarker, "(JJDDLjava/lang/String;)V"),
         MAKE_NATIVE_METHOD(nativeRemoveAnnotations, "(J[J)V"),
-        MAKE_NATIVE_METHOD(nativeGetAnnotationsInBounds, "(JLcom/mapbox/mapboxsdk/geometry/LatLngBounds;)[J"),
+        MAKE_NATIVE_METHOD(nativeQueryPointAnnotations, "(JLandroid/graphics/RectF;)[J"),
         MAKE_NATIVE_METHOD(nativeAddAnnotationIcon, "(JLjava/lang/String;IIF[B)V"),
         MAKE_NATIVE_METHOD(nativeSetVisibleCoordinateBounds, "(J[Lcom/mapbox/mapboxsdk/geometry/LatLng;Landroid/graphics/RectF;DJ)V"),
         MAKE_NATIVE_METHOD(nativeOnLowMemory, "(J)V"),

platform/ios/src/MGLMapView.mm

@@ -3373,8 +3373,10 @@ - (MGLAnnotationTag)annotationTagAtPoint:(CGPoint)point persistingResults:(BOOL)
 /// Returns the tags of the annotations coincident with the given rectangle.
 - (std::vector<MGLAnnotationTag>)annotationTagsInRect:(CGRect)rect
 {
-    mbgl::LatLngBounds queryBounds = [self convertRect:rect toLatLngBoundsFromView:self];
-    return _mbglMap->getPointAnnotationsInBounds(queryBounds);
+    return _mbglMap->queryPointAnnotations({
+        { CGRectGetMinX(rect), CGRectGetMinY(rect) },
+        { CGRectGetMaxX(rect), CGRectGetMaxY(rect) },
+    });
 }
 
 - (id <MGLAnnotation>)selectedAnnotation

platform/macos/src/MGLMapView.mm

@@ -1905,8 +1905,11 @@ - (MGLAnnotationTag)annotationTagAtPoint:(NSPoint)point persistingResults:(BOOL)
 
 /// Returns the tags of the annotations coincident with the given rectangle.
 - (std::vector<MGLAnnotationTag>)annotationTagsInRect:(NSRect)rect {
-    mbgl::LatLngBounds queryBounds = [self convertRect:rect toLatLngBoundsFromView:self];
-    return _mbglMap->getPointAnnotationsInBounds(queryBounds);
+    // Cocoa origin is at the lower-left corner.
+    return _mbglMap->queryPointAnnotations({
+        { NSMinX(rect), NSHeight(self.bounds) - NSMaxY(rect) },
+        { NSMaxX(rect), NSHeight(self.bounds) - NSMinY(rect) },
+    });
 }
 
 - (id <MGLAnnotation>)selectedAnnotation {

src/mbgl/annotation/annotation_manager.cpp

@@ -115,17 +115,6 @@ void AnnotationManager::removeAndAdd(const AnnotationID& id, const Annotation& a
     });
 }
 
-AnnotationIDs AnnotationManager::getPointAnnotationsInBounds(const LatLngBounds& bounds) const {
-    AnnotationIDs result;
-
-    symbolTree.query(boost::geometry::index::intersects(bounds),
-        boost::make_function_output_iterator([&](const auto& val){
-            result.push_back(val->id);
-        }));
-
-    return result;
-}
-
 std::unique_ptr<AnnotationTileData> AnnotationManager::getTileData(const CanonicalTileID& tileID) {
     if (symbolAnnotations.empty() && shapeAnnotations.empty())
         return nullptr;

src/mbgl/annotation/annotation_manager.hpp

@@ -33,8 +33,6 @@ class AnnotationManager : private util::noncopyable {
     Update updateAnnotation(const AnnotationID&, const Annotation&, const uint8_t maxZoom);
     void removeAnnotation(const AnnotationID&);
 
-    AnnotationIDs getPointAnnotationsInBounds(const LatLngBounds&) const;
-
     void addIcon(const std::string& name, std::shared_ptr<const SpriteImage>);
     void removeIcon(const std::string& name);
     double getTopOffsetPixelsForIcon(const std::string& name);

src/mbgl/annotation/annotation_tile.cpp

@@ -21,9 +21,11 @@ AnnotationTile::~AnnotationTile() {
 
 void AnnotationTile::setNecessity(Necessity) {}
 
-AnnotationTileFeature::AnnotationTileFeature(FeatureType type_, GeometryCollection geometries_,
-                                 std::unordered_map<std::string, std::string> properties_)
-    : type(type_),
+AnnotationTileFeature::AnnotationTileFeature(const AnnotationID id_,
+                                             FeatureType type_, GeometryCollection geometries_,
+                                             std::unordered_map<std::string, std::string> properties_)
+    : id(id_),
+      type(type_),
       properties(std::move(properties_)),
       geometries(std::move(geometries_)) {}
 

src/mbgl/annotation/annotation_tile.hpp

@@ -1,5 +1,6 @@
 #pragma once
 
+#include <mbgl/annotation/annotation.hpp>
 #include <mbgl/tile/geometry_tile.hpp>
 #include <mbgl/tile/geometry_tile_data.hpp>
 
@@ -25,13 +26,15 @@ class AnnotationTile : public GeometryTile {
 
 class AnnotationTileFeature : public GeometryTileFeature {
 public:
-    AnnotationTileFeature(FeatureType, GeometryCollection,
+    AnnotationTileFeature(AnnotationID, FeatureType, GeometryCollection,
                           std::unordered_map<std::string, std::string> properties = {{}});
 
     FeatureType getType() const override { return type; }
     optional<Value> getValue(const std::string&) const override;
+    optional<FeatureIdentifier> getID() const override { return { id }; }
     GeometryCollection getGeometries() const override { return geometries; }
 
+    const AnnotationID id;
     const FeatureType type;
     const std::unordered_map<std::string, std::string> properties;
     const GeometryCollection geometries;

src/mbgl/annotation/shape_annotation_impl.cpp

@@ -55,7 +55,7 @@ void ShapeAnnotationImpl::updateTileData(const CanonicalTileID& tileID, Annotati
         }
 
         layer.features.emplace_back(
-            std::make_shared<AnnotationTileFeature>(featureType, renderGeometry));
+            std::make_shared<AnnotationTileFeature>(id, featureType, renderGeometry));
     }
 }
 

src/mbgl/annotation/symbol_annotation_impl.cpp

@@ -31,7 +31,8 @@ void SymbolAnnotationImpl::updateLayer(const CanonicalTileID& tileID, Annotation
     projected *= double(util::EXTENT);
 
     layer.features.emplace_back(
-        std::make_shared<const AnnotationTileFeature>(FeatureType::Point,
+        std::make_shared<const AnnotationTileFeature>(id,
+                                                      FeatureType::Point,
                                                       GeometryCollection {{ {{ convertPoint<int16_t>(projected) }} }},
                                                       featureProperties));
 }

src/mbgl/map/map.cpp

@@ -700,10 +700,6 @@ void Map::removeAnnotation(AnnotationID annotation) {
     update(Update::AnnotationStyle | Update::AnnotationData);
 }
 
-AnnotationIDs Map::getPointAnnotationsInBounds(const LatLngBounds& bounds) {
-    return impl->annotationManager->getPointAnnotationsInBounds(bounds);
-}
-
 #pragma mark - Feature query api
 
 std::vector<Feature> Map::queryRenderedFeatures(const ScreenCoordinate& point, const optional<std::vector<std::string>>& layerIDs) {
@@ -732,6 +728,18 @@ std::vector<Feature> Map::queryRenderedFeatures(const ScreenBox& box, const opti
     });
 }
 
+AnnotationIDs Map::queryPointAnnotations(const ScreenBox& box) {
+    auto features = queryRenderedFeatures(box, {{ AnnotationManager::PointLayerID }});
+    AnnotationIDs ids;
+    ids.reserve(features.size());
+    for (auto &feature : features) {
+        assert(feature.id);
+        assert(*feature.id <= std::numeric_limits<AnnotationID>::max());
+        ids.push_back(static_cast<AnnotationID>(feature.id->get<uint64_t>()));
+    }
+    return ids;
+}
+
 #pragma mark - Style API
 
 style::Source* Map::getSource(const std::string& sourceID) {

test/api/annotations.cpp

@@ -202,9 +202,17 @@ TEST(Annotations, QueryRenderedFeatures) {
     test.map.setStyleJSON(util::read_file("test/fixtures/api/empty.json"));
     test.map.addAnnotationIcon("default_marker", namedMarker("default_marker.png"));
     test.map.addAnnotation(SymbolAnnotation { Point<double> { 0, 0 }, "default_marker" });
+    test.map.addAnnotation(SymbolAnnotation { Point<double> { 0, 50 }, "default_marker" });
 
     test::render(test.map);
 
     auto features = test.map.queryRenderedFeatures(test.map.pixelForLatLng({ 0, 0 }));
     EXPECT_EQ(features.size(), 1u);
+    EXPECT_TRUE(!!features[0].id);
+    EXPECT_EQ(*features[0].id, 0);
+
+    auto features2 = test.map.queryRenderedFeatures(test.map.pixelForLatLng({ 50, 0 }));
+    EXPECT_EQ(features2.size(), 1);
+    EXPECT_TRUE(!!features2[0].id);
+    EXPECT_EQ(*features2[0].id, 1);
 }