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

Commit fb29dc8

Browse files
1ec51ec5
committed
[core, android, ios, osx] 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.
1 parent 8ace0d3 commit fb29dc8

File tree

15 files changed

+75
-61
lines changed

15 files changed

+75
-61
lines changed

‎include/mbgl/map/map.hpp

+1-2
Original file line numberDiff line numberDiff line change
@@ -153,8 +153,6 @@ class Map : private util::noncopyable {
153153
void removeAnnotation(AnnotationID);
154154
void removeAnnotations(const AnnotationIDs&);
155155

156-
AnnotationIDs getPointAnnotationsInBounds(const LatLngBounds&);
157-
158156
void addCustomLayer(const std::string& id,
159157
CustomLayerInitializeFunction,
160158
CustomLayerRenderFunction,
@@ -166,6 +164,7 @@ class Map : private util::noncopyable {
166164
// Feature queries
167165
std::vector<Feature> queryRenderedFeatures(const ScreenCoordinate&, const optional<std::vector<std::string>>& layerIDs = {});
168166
std::vector<Feature> queryRenderedFeatures(const ScreenBox&, const optional<std::vector<std::string>>& layerIDs = {});
167+
AnnotationIDs queryPointAnnotations(const ScreenBox&);
169168

170169
// Memory
171170
void setSourceTileCacheSize(size_t);

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

+6-5
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package com.mapbox.mapboxsdk.annotations;
22

33
import android.graphics.PointF;
4+
import android.graphics.RectF;
45
import android.os.SystemClock;
56
import android.support.annotation.NonNull;
67
import android.support.annotation.Nullable;
@@ -130,7 +131,7 @@ public void onAnimationEnd() {
130131
public void addMarkerViewAdapter(@Nullable MapboxMap.MarkerViewAdapter markerViewAdapter) {
131132
if (!markerViewAdapters.contains(markerViewAdapter)) {
132133
markerViewAdapters.add(markerViewAdapter);
133-
invalidateViewMarkersInBounds();
134+
invalidateViewMarkersInVisibleRegion();
134135
}
135136
}
136137

@@ -148,14 +149,14 @@ public void invalidateViewMarkers() {
148149
if (currentTime < mViewMarkerBoundsUpdateTime) {
149150
return;
150151
}
151-
invalidateViewMarkersInBounds();
152+
invalidateViewMarkersInVisibleRegion();
152153
mViewMarkerBoundsUpdateTime = currentTime + 250;
153154
}
154155
}
155156

156-
public void invalidateViewMarkersInBounds() {
157-
Projection projection = mapboxMap.getProjection();
158-
List<MarkerView> markers = mapView.getMarkerViewsInBounds(projection.getVisibleRegion().latLngBounds);
157+
public void invalidateViewMarkersInVisibleRegion() {
158+
RectF mapViewRect = new RectF(0, 0, mapView.getWidth(), mapView.getHeight());
159+
List<MarkerView> markers = mapView.getMarkerViewsInRect(mapViewRect);
159160
View convertView;
160161

161162
// remove old markers

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

+9-15
Original file line numberDiff line numberDiff line change
@@ -1073,13 +1073,13 @@ void removeAnnotations(@NonNull long[] ids) {
10731073
mNativeMapView.removeAnnotations(ids);
10741074
}
10751075

1076-
List<Marker> getMarkersInBounds(@NonNull LatLngBounds bbox) {
1077-
if (mDestroyed || bbox == null) {
1076+
List<Marker> getMarkersInRect(@NonNull RectF rect) {
1077+
if (mDestroyed || rect == null) {
10781078
return new ArrayList<>();
10791079
}
10801080

1081-
// TODO: filter in JNI using C++ parameter to getAnnotationsInBounds
1082-
long[] ids = mNativeMapView.getAnnotationsInBounds(bbox);
1081+
// TODO: filter in JNI using C++ parameter to queryPointAnnotations
1082+
long[] ids = mNativeMapView.queryPointAnnotations(rect);
10831083

10841084
List<Long> idsList = new ArrayList<>(ids.length);
10851085
for (int i = 0; i < ids.length; i++) {
@@ -1099,13 +1099,13 @@ List<Marker> getMarkersInBounds(@NonNull LatLngBounds bbox) {
10991099
return new ArrayList<>(annotations);
11001100
}
11011101

1102-
public List<MarkerView> getMarkerViewsInBounds(@NonNull LatLngBounds bbox) {
1103-
if (mDestroyed || bbox == null) {
1102+
public List<MarkerView> getMarkerViewsInRect(@NonNull RectF rect) {
1103+
if (mDestroyed || rect == null) {
11041104
return new ArrayList<>();
11051105
}
11061106

1107-
// TODO: filter in JNI using C++ parameter to getAnnotationsInBounds
1108-
long[] ids = mNativeMapView.getAnnotationsInBounds(bbox);
1107+
// TODO: filter in JNI using C++ parameter to queryPointAnnotations
1108+
long[] ids = mNativeMapView.queryPointAnnotations(rect);
11091109

11101110
List<Long> idsList = new ArrayList<>(ids.length);
11111111
for (int i = 0; i < ids.length; i++) {
@@ -1627,13 +1627,7 @@ public boolean onSingleTapConfirmed(MotionEvent e) {
16271627
tapPoint.x + mAverageIconWidth / 2 + toleranceSides,
16281628
tapPoint.y + mAverageIconHeight / 2 + toleranceTopBottom);
16291629

1630-
LatLngBounds.Builder builder = new LatLngBounds.Builder();
1631-
builder.include(fromScreenLocation(new PointF(tapRect.left, tapRect.bottom)));
1632-
builder.include(fromScreenLocation(new PointF(tapRect.left, tapRect.top)));
1633-
builder.include(fromScreenLocation(new PointF(tapRect.right, tapRect.top)));
1634-
builder.include(fromScreenLocation(new PointF(tapRect.right, tapRect.bottom)));
1635-
1636-
List<Marker> nearbyMarkers = getMarkersInBounds(builder.build());
1630+
List<Marker> nearbyMarkers = getMarkersInRect(tapRect);
16371631
long newSelectedMarkerId = -1;
16381632

16391633
if (nearbyMarkers != null && nearbyMarkers.size() > 0) {

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

+3-3
Original file line numberDiff line numberDiff line change
@@ -382,8 +382,8 @@ public void removeAnnotations(long[] ids) {
382382
nativeRemoveAnnotations(mNativeMapViewPtr, ids);
383383
}
384384

385-
public long[] getAnnotationsInBounds(LatLngBounds bbox) {
386-
return nativeGetAnnotationsInBounds(mNativeMapViewPtr, bbox);
385+
public long[] queryPointAnnotations(RectF rect) {
386+
return nativeQueryPointAnnotations(mNativeMapViewPtr, rect);
387387
}
388388

389389
public void addAnnotationIcon(String symbol, int width, int height, float scale, byte[] pixels) {
@@ -607,7 +607,7 @@ private native void nativeSetBearingXY(long nativeMapViewPtr, double degrees,
607607

608608
private native void nativeRemoveAnnotations(long nativeMapViewPtr, long[] id);
609609

610-
private native long[] nativeGetAnnotationsInBounds(long mNativeMapViewPtr, LatLngBounds bbox);
610+
private native long[] nativeQueryPointAnnotations(long mNativeMapViewPtr, RectF rect);
611611

612612
private native void nativeAddAnnotationIcon(long nativeMapViewPtr, String symbol,
613613
int width, int height, float scale, byte[] pixels);

‎platform/android/src/jni.cpp

+13-9
Original file line numberDiff line numberDiff line change
@@ -960,20 +960,24 @@ void nativeRemoveAnnotations(JNIEnv *env, jni::jobject* obj, jlong nativeMapView
960960
nativeMapView->getMap().removeAnnotations(ids);
961961
}
962962

963-
jni::jarray<jlong>* nativeGetAnnotationsInBounds(JNIEnv *env, jni::jobject* obj, jlong nativeMapViewPtr, jni::jobject* latLngBounds_) {
964-
mbgl::Log::Debug(mbgl::Event::JNI, "nativeGetAnnotationsInBounds");
963+
jni::jarray<jlong>* nativeQueryPointAnnotations(JNIEnv *env, jni::jobject* obj, jlong nativeMapViewPtr, jni::jobject* rect) {
964+
mbgl::Log::Debug(mbgl::Event::JNI, "nativeQueryPointAnnotations");
965965
assert(nativeMapViewPtr != 0);
966966
NativeMapView *nativeMapView = reinterpret_cast<NativeMapView *>(nativeMapViewPtr);
967967

968968
// Conversion
969-
mbgl::LatLngBounds latLngBounds = latlngbounds_from_java(env, latLngBounds_);
970-
if (latLngBounds.isEmpty()) {
971-
return nullptr;
972-
}
969+
jfloat left = jni::GetField<jfloat>(*env, rect, *rectFLeftId);
970+
jfloat right = jni::GetField<jfloat>(*env, rect, *rectFRightId);
971+
jfloat top = jni::GetField<jfloat>(*env, rect, *rectFTopId);
972+
jfloat bottom = jni::GetField<jfloat>(*env, rect, *rectFBottomId);
973+
mbgl::ScreenBox box = {
974+
{ left, top },
975+
{ right, bottom },
976+
};
973977

974978
// Assume only points for now
975-
std::vector<uint32_t> annotations = nativeMapView->getMap().getPointAnnotationsInBounds(
976-
latLngBounds);
979+
std::vector<uint32_t> annotations = nativeMapView->getMap().queryPointAnnotations(
980+
box);
977981

978982
return std_vector_uint_to_jobject(env, annotations);
979983
}
@@ -1856,7 +1860,7 @@ extern "C" JNIEXPORT jint JNI_OnLoad(JavaVM *vm, void *reserved) {
18561860
MAKE_NATIVE_METHOD(nativeUpdateMarker, "(JLcom/mapbox/mapboxsdk/annotations/Marker;)V"),
18571861
MAKE_NATIVE_METHOD(nativeRemoveAnnotation, "(JJ)V"),
18581862
MAKE_NATIVE_METHOD(nativeRemoveAnnotations, "(J[J)V"),
1859-
MAKE_NATIVE_METHOD(nativeGetAnnotationsInBounds, "(JLcom/mapbox/mapboxsdk/geometry/LatLngBounds;)[J"),
1863+
MAKE_NATIVE_METHOD(nativeQueryPointAnnotations, "(JLandroid/graphics/RectF;)[J"),
18601864
MAKE_NATIVE_METHOD(nativeAddAnnotationIcon, "(JLjava/lang/String;IIF[B)V"),
18611865
MAKE_NATIVE_METHOD(nativeSetVisibleCoordinateBounds, "(J[Lcom/mapbox/mapboxsdk/geometry/LatLng;Landroid/graphics/RectF;DJ)V"),
18621866
MAKE_NATIVE_METHOD(nativeOnLowMemory, "(J)V"),

‎platform/ios/src/MGLMapView.mm

+4-2
Original file line numberDiff line numberDiff line change
@@ -3275,8 +3275,10 @@ - (MGLAnnotationTag)annotationTagAtPoint:(CGPoint)point persistingResults:(BOOL)
32753275
/// Returns the tags of the annotations coincident with the given rectangle.
32763276
- (std::vector<MGLAnnotationTag>)annotationTagsInRect:(CGRect)rect
32773277
{
3278-
mbgl::LatLngBounds queryBounds = [self convertRect:rect toLatLngBoundsFromView:self];
3279-
return _mbglMap->getPointAnnotationsInBounds(queryBounds);
3278+
return _mbglMap->queryPointAnnotations({
3279+
{ CGRectGetMinX(rect), CGRectGetMinY(rect) },
3280+
{ CGRectGetMaxX(rect), CGRectGetMaxY(rect) },
3281+
});
32803282
}
32813283

32823284
- (id <MGLAnnotation>)selectedAnnotation

‎platform/osx/src/MGLMapView.mm

+5-2
Original file line numberDiff line numberDiff line change
@@ -1910,8 +1910,11 @@ - (MGLAnnotationTag)annotationTagAtPoint:(NSPoint)point persistingResults:(BOOL)
19101910

19111911
/// Returns the tags of the annotations coincident with the given rectangle.
19121912
- (std::vector<MGLAnnotationTag>)annotationTagsInRect:(NSRect)rect {
1913-
mbgl::LatLngBounds queryBounds = [self convertRect:rect toLatLngBoundsFromView:self];
1914-
return _mbglMap->getPointAnnotationsInBounds(queryBounds);
1913+
// Cocoa origin is at the lower-left corner.
1914+
return _mbglMap->queryPointAnnotations({
1915+
{ NSMinX(rect), NSHeight(self.bounds) - NSMaxY(rect) },
1916+
{ NSMaxX(rect), NSHeight(self.bounds) - NSMinY(rect) },
1917+
});
19151918
}
19161919

19171920
- (id <MGLAnnotation>)selectedAnnotation {

‎src/mbgl/annotation/annotation_manager.cpp

-11
Original file line numberDiff line numberDiff line change
@@ -71,17 +71,6 @@ void AnnotationManager::removeAnnotations(const AnnotationIDs& ids) {
7171
}
7272
}
7373

74-
AnnotationIDs AnnotationManager::getPointAnnotationsInBounds(const LatLngBounds& bounds) const {
75-
AnnotationIDs result;
76-
77-
pointTree.query(boost::geometry::index::intersects(bounds),
78-
boost::make_function_output_iterator([&](const auto& val){
79-
result.push_back(val->id);
80-
}));
81-
82-
return result;
83-
}
84-
8574
std::unique_ptr<AnnotationTile> AnnotationManager::getTile(const CanonicalTileID& tileID) {
8675
if (pointAnnotations.empty() && shapeAnnotations.empty())
8776
return nullptr;

‎src/mbgl/annotation/annotation_manager.hpp

-2
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,6 @@ class AnnotationManager : private util::noncopyable {
3030
void updatePointAnnotation(const AnnotationID&, const PointAnnotation&, const uint8_t maxZoom);
3131
void removeAnnotations(const AnnotationIDs&);
3232

33-
AnnotationIDs getPointAnnotationsInBounds(const LatLngBounds&) const;
34-
3533
void addIcon(const std::string& name, std::shared_ptr<const SpriteImage>);
3634
void removeIcon(const std::string& name);
3735
double getTopOffsetPixelsForIcon(const std::string& name);

‎src/mbgl/annotation/annotation_tile.cpp

+5-3
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,11 @@
77

88
namespace mbgl {
99

10-
AnnotationTileFeature::AnnotationTileFeature(FeatureType type_, GeometryCollection geometries_,
11-
std::unordered_map<std::string, std::string> properties_)
12-
: type(type_),
10+
AnnotationTileFeature::AnnotationTileFeature(const AnnotationID id_,
11+
FeatureType type_, GeometryCollection geometries_,
12+
std::unordered_map<std::string, std::string> properties_)
13+
: id(id_),
14+
type(type_),
1315
properties(std::move(properties_)),
1416
geometries(std::move(geometries_)) {}
1517

‎src/mbgl/annotation/annotation_tile.hpp

+4-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
#pragma once
22

3+
#include <mbgl/annotation/annotation.hpp>
34
#include <mbgl/tile/geometry_tile.hpp>
45
#include <mbgl/tile/tile_id.hpp>
56

@@ -10,13 +11,15 @@ namespace mbgl {
1011

1112
class AnnotationTileFeature : public GeometryTileFeature {
1213
public:
13-
AnnotationTileFeature(FeatureType, GeometryCollection,
14+
AnnotationTileFeature(AnnotationID, FeatureType, GeometryCollection,
1415
std::unordered_map<std::string, std::string> properties = {{}});
1516

1617
FeatureType getType() const override { return type; }
1718
optional<Value> getValue(const std::string&) const override;
19+
optional<uint64_t> getID() const override { return id; }
1820
GeometryCollection getGeometries() const override { return geometries; }
1921

22+
const AnnotationID id;
2023
const FeatureType type;
2124
const std::unordered_map<std::string, std::string> properties;
2225
const GeometryCollection geometries;

‎src/mbgl/annotation/point_annotation_impl.cpp

+2-1
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,8 @@ void PointAnnotationImpl::updateLayer(const CanonicalTileID& tileID, AnnotationT
2828
projected *= double(util::EXTENT);
2929

3030
layer.features.emplace_back(
31-
std::make_shared<const AnnotationTileFeature>(FeatureType::Point,
31+
std::make_shared<const AnnotationTileFeature>(id,
32+
FeatureType::Point,
3233
GeometryCollection {{ {{ convertPoint<int16_t>(projected) }} }},
3334
featureProperties));
3435
}

‎src/mbgl/annotation/shape_annotation_impl.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ void ShapeAnnotationImpl::updateTile(const CanonicalTileID& tileID, AnnotationTi
144144
}
145145

146146
layer.features.emplace_back(
147-
std::make_shared<AnnotationTileFeature>(featureType, renderGeometry));
147+
std::make_shared<AnnotationTileFeature>(id, featureType, renderGeometry));
148148
}
149149
}
150150

‎src/mbgl/map/map.cpp

+12-4
Original file line numberDiff line numberDiff line change
@@ -728,10 +728,6 @@ void Map::removeAnnotations(const AnnotationIDs& annotations) {
728728
update(Update::Annotations);
729729
}
730730

731-
AnnotationIDs Map::getPointAnnotationsInBounds(const LatLngBounds& bounds) {
732-
return impl->annotationManager->getPointAnnotationsInBounds(bounds);
733-
}
734-
735731
#pragma mark - Feature query api
736732

737733
std::vector<Feature> Map::queryRenderedFeatures(const ScreenCoordinate& point, const optional<std::vector<std::string>>& layerIDs) {
@@ -760,6 +756,18 @@ std::vector<Feature> Map::queryRenderedFeatures(const ScreenBox& box, const opti
760756
});
761757
}
762758

759+
AnnotationIDs Map::queryPointAnnotations(const ScreenBox& box) {
760+
auto features = queryRenderedFeatures(box, {{ AnnotationManager::PointLayerID }});
761+
AnnotationIDs ids;
762+
ids.reserve(features.size());
763+
for (auto &feature : features) {
764+
assert(feature.id);
765+
assert(*feature.id <= std::numeric_limits<AnnotationID>::max());
766+
ids.push_back((AnnotationID(*feature.id)));
767+
}
768+
return ids;
769+
}
770+
763771
#pragma mark - Style API
764772

765773
void Map::addCustomLayer(const std::string& id,

‎test/api/annotations.cpp

+10
Original file line numberDiff line numberDiff line change
@@ -272,10 +272,20 @@ TEST(Annotations, QueryRenderedFeatures) {
272272
map.addAnnotationIcon("default_marker", namedMarker("default_marker.png"));
273273
const LatLng latLng(0, 0);
274274
map.addPointAnnotation(PointAnnotation(latLng, "default_marker"));
275+
const LatLng latLng2(50, 0);
276+
map.addPointAnnotation(PointAnnotation(latLng2, "default_marker"));
275277

276278
test::render(map);
277279

278280
auto point = map.pixelForLatLng(latLng);
279281
auto features = map.queryRenderedFeatures(point);
280282
EXPECT_EQ(features.size(), 1);
283+
EXPECT_TRUE(!!features[0].id);
284+
EXPECT_EQ(*features[0].id, 0);
285+
286+
auto point2 = map.pixelForLatLng(latLng2);
287+
auto features2 = map.queryRenderedFeatures(point2);
288+
EXPECT_EQ(features2.size(), 1);
289+
EXPECT_TRUE(!!features2[0].id);
290+
EXPECT_EQ(*features2[0].id, 1);
281291
}

0 commit comments

Comments
 (0)