[core] Fix SymbolAnnotation coordinate system conversions

2 files changed, 24 insertions, 30 deletions

diff --git a/src/mbgl/annotation/symbol_annotation_impl.cpp b/src/mbgl/annotation/symbol_annotation_impl.cpp
index 040de21214..e5ae5f4b91 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 {
 
@@ -14,26 +15,10 @@ void SymbolAnnotationImpl::updateLayer(const CanonicalTileID& tileID, Annotation
     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(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/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());
 }