[core] Fix SymbolAnnotation coordinate system conversions

7 files changed, 44 insertions, 47 deletions

diff --git a/src/mbgl/annotation/annotation_manager.cpp b/src/mbgl/annotation/annotation_manager.cpp
index dbd5f1f433..ada580eb84 100644
--- a/src/mbgl/annotation/annotation_manager.cpp
+++ b/src/mbgl/annotation/annotation_manager.cpp
@@ -151,7 +151,7 @@ void AnnotationManager::removeAndAdd(const AnnotationID& id, const Annotation& a
     });
 }
 
-std::unique_ptr<AnnotationTileData> AnnotationManager::getTileData(const CanonicalTileID& tileID) {
+std::unique_ptr<AnnotationTileData> AnnotationManager::getTileData(const CanonicalTileID& tileID, const TransformState& state) {
     if (symbolAnnotations.empty() && shapeAnnotations.empty())
         return nullptr;
 
@@ -159,11 +159,9 @@ std::unique_ptr<AnnotationTileData> AnnotationManager::getTileData(const Canonic
 
     AnnotationTileLayer& pointLayer = tileData->layers.emplace(PointLayerID, PointLayerID).first->second;
 
-    LatLngBounds tileBounds(tileID);
-
-    symbolTree.query(boost::geometry::index::intersects(tileBounds),
+    symbolTree.query(bgi::intersects(LatLngBounds { tileID }),
         boost::make_function_output_iterator([&](const auto& val){
-            val->updateLayer(tileID, pointLayer);
+            val->updateLayer(tileID, pointLayer, state);
         }));
 
     for (const auto& shape : shapeAnnotations) {
@@ -205,15 +203,15 @@ void AnnotationManager::updateStyle(Style& style) {
     obsoleteShapeAnnotationLayers.clear();
 }
 
-void AnnotationManager::updateData() {
+void AnnotationManager::updateData(const TransformState& state) {
     for (auto& tile : tiles) {
-        tile->setData(getTileData(tile->id.canonical));
+        tile->setData(getTileData(tile->id.canonical, state));
     }
 }
 
-void AnnotationManager::addTile(AnnotationTile& tile) {
+void AnnotationManager::addTile(AnnotationTile& tile, const TransformState& state) {
     tiles.insert(&tile);
-    tile.setData(getTileData(tile.id.canonical));
+    tile.setData(getTileData(tile.id.canonical, state));
 }
 
 void AnnotationManager::removeTile(AnnotationTile& tile) {
diff --git a/src/mbgl/annotation/annotation_manager.hpp b/src/mbgl/annotation/annotation_manager.hpp
index 856aeeed72..eece1efd4e 100644
--- a/src/mbgl/annotation/annotation_manager.hpp
+++ b/src/mbgl/annotation/annotation_manager.hpp
@@ -18,11 +18,14 @@ class AnnotationTile;
 class AnnotationTileData;
 class SymbolAnnotationImpl;
 class ShapeAnnotationImpl;
+class TransformState;
 
 namespace style {
 class Style;
 } // namespace style
 
+namespace bgi = boost::geometry::index;
+
 class AnnotationManager : private util::noncopyable {
 public:
     AnnotationManager(float pixelRatio);
@@ -38,9 +41,9 @@ public:
     SpriteAtlas& getSpriteAtlas() { return spriteAtlas; }
 
     void updateStyle(style::Style&);
-    void updateData();
+    void updateData(const TransformState&);
 
-    void addTile(AnnotationTile&);
+    void addTile(AnnotationTile&, const TransformState&);
     void removeTile(AnnotationTile&);
 
     static const std::string SourceID;
@@ -59,11 +62,11 @@ private:
 
     void removeAndAdd(const AnnotationID&, const Annotation&, const uint8_t);
 
-    std::unique_ptr<AnnotationTileData> getTileData(const CanonicalTileID&);
+    std::unique_ptr<AnnotationTileData> getTileData(const CanonicalTileID&, const TransformState&);
 
     AnnotationID nextID = 0;
 
-    using SymbolAnnotationTree = boost::geometry::index::rtree<std::shared_ptr<const SymbolAnnotationImpl>, boost::geometry::index::rstar<16, 4>>;
+    using SymbolAnnotationTree = bgi::rtree<std::shared_ptr<const SymbolAnnotationImpl>, boost::geometry::index::rstar<16, 4>>;
     // Unlike std::unordered_map, std::map is guaranteed to sort by AnnotationID, ensuring that older annotations are below newer annotations.
     // <https://github.com/mapbox/mapbox-gl-native/issues/5691>
     using SymbolAnnotationMap = std::map<AnnotationID, std::shared_ptr<SymbolAnnotationImpl>>;
diff --git a/src/mbgl/annotation/annotation_tile.cpp b/src/mbgl/annotation/annotation_tile.cpp
index 9d28f67785..5f191b04b7 100644
--- a/src/mbgl/annotation/annotation_tile.cpp
+++ b/src/mbgl/annotation/annotation_tile.cpp
@@ -12,7 +12,7 @@ AnnotationTile::AnnotationTile(const OverscaledTileID& overscaledTileID,
                                const style::UpdateParameters& parameters)
     : GeometryTile(overscaledTileID, AnnotationManager::SourceID, parameters),
       annotationManager(parameters.annotationManager) {
-    annotationManager.addTile(*this);
+    annotationManager.addTile(*this, parameters.transformState);
 }
 
 AnnotationTile::~AnnotationTile() {
diff --git a/src/mbgl/annotation/symbol_annotation_impl.cpp b/src/mbgl/annotation/symbol_annotation_impl.cpp
index 040de21214..f67bf565e7 100644
--- a/src/mbgl/annotation/symbol_annotation_impl.cpp
+++ b/src/mbgl/annotation/symbol_annotation_impl.cpp
@@ -2,6 +2,7 @@
 #include <mbgl/annotation/annotation_tile.hpp>
 #include <mbgl/tile/tile_id.hpp>
 #include <mbgl/math/clamp.hpp>
+#include <mbgl/util/tile_coordinate.hpp>
 
 namespace mbgl {
 
@@ -10,30 +11,14 @@ SymbolAnnotationImpl::SymbolAnnotationImpl(AnnotationID id_, SymbolAnnotation an
   annotation(std::move(annotation_)) {
 }
 
-void SymbolAnnotationImpl::updateLayer(const CanonicalTileID& tileID, AnnotationTileLayer& layer) const {
+void SymbolAnnotationImpl::updateLayer(const CanonicalTileID& tileID, AnnotationTileLayer& layer, const TransformState& state) const {
     std::unordered_map<std::string, std::string> featureProperties;
     featureProperties.emplace("sprite", annotation.icon.empty() ? std::string("default_marker") : annotation.icon);
 
-    const Point<double>& p = annotation.geometry;
-
-    // Clamp to the latitude limits of Web Mercator.
-    const double constrainedLatitude = util::clamp(p.y, -util::LATITUDE_MAX, util::LATITUDE_MAX);
-
-    // Project a coordinate into unit space in a square map.
-    const double sine = std::sin(constrainedLatitude * util::DEG2RAD);
-    const double x = p.x / util::DEGREES_MAX + 0.5;
-    const double y = 0.5 - 0.25 * std::log((1.0 + sine) / (1.0 - sine)) / M_PI;
-
-    Point<double> projected(x, y);
-    projected *= std::pow(2, tileID.z);
-    projected.x = std::fmod(projected.x, 1);
-    projected.y = std::fmod(projected.y, 1);
-    projected *= double(util::EXTENT);
-
-    layer.features.emplace_back(id,
-                                FeatureType::Point,
-                                GeometryCollection {{ {{ convertPoint<int16_t>(projected) }} }},
-                                featureProperties);
+    LatLng latLng { annotation.geometry.y, annotation.geometry.x };
+    TileCoordinate coordinate = TileCoordinate::fromLatLng(state, 0, latLng);
+    GeometryCoordinate tilePoint = TileCoordinate::toGeometryCoordinate(UnwrappedTileID(0, tileID), coordinate.p);
+    layer.features.emplace_back(id, FeatureType::Point, GeometryCollection {{ {{ tilePoint }} }}, featureProperties);
 }
 
 } // namespace mbgl
diff --git a/src/mbgl/annotation/symbol_annotation_impl.hpp b/src/mbgl/annotation/symbol_annotation_impl.hpp
index 2e98f2414c..45cbfa2215 100644
--- a/src/mbgl/annotation/symbol_annotation_impl.hpp
+++ b/src/mbgl/annotation/symbol_annotation_impl.hpp
@@ -1,6 +1,7 @@
 #pragma once
 
 #include <mbgl/annotation/annotation.hpp>
+#include <mbgl/map/transform_state.hpp>
 #include <mbgl/util/geo.hpp>
 
 #include <string>
@@ -34,12 +35,13 @@ namespace mbgl {
 
 class AnnotationTileLayer;
 class CanonicalTileID;
+class TransformState;
 
 class SymbolAnnotationImpl {
 public:
     SymbolAnnotationImpl(AnnotationID, SymbolAnnotation);
 
-    void updateLayer(const CanonicalTileID&, AnnotationTileLayer&) const;
+    void updateLayer(const CanonicalTileID&, AnnotationTileLayer&, const TransformState&) const;
 
     const AnnotationID id;
     const SymbolAnnotation annotation;
diff --git a/src/mbgl/map/map.cpp b/src/mbgl/map/map.cpp
index bf2462e2ab..424406c527 100644
--- a/src/mbgl/map/map.cpp
+++ b/src/mbgl/map/map.cpp
@@ -212,7 +212,7 @@ void Map::Impl::update() {
     }
 
     if (updateFlags & Update::AnnotationData) {
-        annotationManager->updateData();
+        annotationManager->updateData(transform.getState());
     }
 
     if (updateFlags & Update::Layout) {
diff --git a/test/api/annotations.test.cpp b/test/api/annotations.test.cpp
index dc4b00170a..6d6734a10f 100644
--- a/test/api/annotations.test.cpp
+++ b/test/api/annotations.test.cpp
@@ -49,10 +49,7 @@ TEST(Annotations, SymbolAnnotation) {
     EXPECT_EQ(features.size(), 1u);
 
     test.map.setZoom(test.map.getMaxZoom());
-    // FIXME: https://github.com/mapbox/mapbox-gl-native/issues/5419
-    //test.map.setZoom(test.map.getMaxZoom());
-    //test.checkRendering("point_annotation");
-    test::render(test.map);
+    test.checkRendering("point_annotation");
 
     features = test.map.queryPointAnnotations(screenBox);
     EXPECT_EQ(features.size(), 1u);
@@ -369,10 +366,17 @@ TEST(Annotations, QueryFractionalZoomLevels) {
     }
 
     test.map.setLatLngZoom({ 5, 5 }, 0);
-    for (uint16_t zoomSteps = 0; zoomSteps <= 20; ++zoomSteps) {
+    for (uint16_t zoomSteps = 10; zoomSteps <= 20; ++zoomSteps) {
         test.map.setZoom(zoomSteps / 10.0);
         test::render(test.map);
         auto features = test.map.queryRenderedFeatures(box);
+
+        // Filter out repeated features.
+        // See 'edge-cases/null-island' query-test for reference.
+        auto sortID = [](const Feature& lhs, const Feature& rhs) { return lhs.id < rhs.id; };
+        auto sameID = [](const Feature& lhs, const Feature& rhs) { return lhs.id == rhs.id; };
+        std::sort(features.begin(), features.end(), sortID);
+        features.erase(std::unique(features.begin(), features.end(), sameID), features.end());
         EXPECT_EQ(features.size(), ids.size());
     }
 }
@@ -385,26 +389,31 @@ TEST(Annotations, VisibleFeatures) {
 
     test.map.setStyleJSON(util::read_file("test/fixtures/api/empty.json"));
     test.map.addAnnotationIcon("default_marker", namedMarker("default_marker.png"));
-    test.map.setZoom(3);
+    test.map.setLatLngZoom({ 5, 5 }, 3);
 
     std::vector<mbgl::AnnotationID> ids;
-    for (int longitude = -5; longitude <= 5; ++longitude) {
-        for (int latitude = -5; latitude <= 5; ++latitude) {
+    for (int longitude = 0; longitude < 10; ++longitude) {
+        for (int latitude = 0; latitude <= 10; ++latitude) {
             ids.push_back(test.map.addAnnotation(SymbolAnnotation { { double(latitude), double(longitude) }, "default_marker" }));
         }
     }
 
-    // Change bearing *after* adding annotations cause them to be reordered,
-    // and some annotations become occluded by others.
+    // Change bearing *after* adding annotations causes them to be reordered.
     test.map.setBearing(45);
     test::render(test.map);
 
     auto features = test.map.queryRenderedFeatures(box);
+    auto sortID = [](const Feature& lhs, const Feature& rhs) { return lhs.id < rhs.id; };
+    auto sameID = [](const Feature& lhs, const Feature& rhs) { return lhs.id == rhs.id; };
+    std::sort(features.begin(), features.end(), sortID);
+    features.erase(std::unique(features.begin(), features.end(), sameID), features.end());
     EXPECT_EQ(features.size(), ids.size());
 
     test.map.setBearing(0);
     test.map.setZoom(4);
     test::render(test.map);
     features = test.map.queryRenderedFeatures(box);
+    std::sort(features.begin(), features.end(), sortID);
+    features.erase(std::unique(features.begin(), features.end(), sameID), features.end());
     EXPECT_EQ(features.size(), ids.size());
 }