[core] Fix SymbolAnnotation coordinate system conversions

src/mbgl/annotation/annotation_manager.cpp

@@ -151,19 +151,17 @@ 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;
 
  auto tileData = std::make_unique<AnnotationTileData>();
 
  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) {

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 @@ class AnnotationManager : private util::noncopyable {
  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 @@ class AnnotationManager : private util::noncopyable {
 
  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>>;

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() {

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

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;

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) {

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());
 }