From 36cc9abf27784f1f540342036e281d56800a6e6d Mon Sep 17 00:00:00 2001 From: Bruno de Oliveira Abinader Date: Sun, 9 Oct 2016 17:54:07 +0300 Subject: [PATCH] [core] Fix SymbolAnnotation coordinate system conversions --- src/mbgl/annotation/annotation_manager.cpp | 16 +++++----- src/mbgl/annotation/annotation_manager.hpp | 11 ++++--- src/mbgl/annotation/annotation_tile.cpp | 2 +- .../annotation/symbol_annotation_impl.cpp | 27 ++++------------- .../annotation/symbol_annotation_impl.hpp | 4 ++- src/mbgl/map/map.cpp | 2 +- test/api/annotations.test.cpp | 29 ++++++++++++------- 7 files changed, 44 insertions(+), 47 deletions(-) diff --git a/src/mbgl/annotation/annotation_manager.cpp b/src/mbgl/annotation/annotation_manager.cpp index dbd5f1f433a..ada580eb84e 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 AnnotationManager::getTileData(const CanonicalTileID& tileID) { +std::unique_ptr AnnotationManager::getTileData(const CanonicalTileID& tileID, const TransformState& state) { if (symbolAnnotations.empty() && shapeAnnotations.empty()) return nullptr; @@ -159,11 +159,9 @@ std::unique_ptr 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 856aeeed72e..eece1efd4e6 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 @@ 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 getTileData(const CanonicalTileID&); + std::unique_ptr getTileData(const CanonicalTileID&, const TransformState&); AnnotationID nextID = 0; - using SymbolAnnotationTree = boost::geometry::index::rtree, boost::geometry::index::rstar<16, 4>>; + using SymbolAnnotationTree = bgi::rtree, 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. // using SymbolAnnotationMap = std::map>; diff --git a/src/mbgl/annotation/annotation_tile.cpp b/src/mbgl/annotation/annotation_tile.cpp index 9d28f67785d..5f191b04b73 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 040de212142..f67bf565e77 100644 --- a/src/mbgl/annotation/symbol_annotation_impl.cpp +++ b/src/mbgl/annotation/symbol_annotation_impl.cpp @@ -2,6 +2,7 @@ #include #include #include +#include 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 featureProperties; featureProperties.emplace("sprite", annotation.icon.empty() ? std::string("default_marker") : annotation.icon); - const Point& 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 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(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 2e98f2414c3..45cbfa22156 100644 --- a/src/mbgl/annotation/symbol_annotation_impl.hpp +++ b/src/mbgl/annotation/symbol_annotation_impl.hpp @@ -1,6 +1,7 @@ #pragma once #include +#include #include #include @@ -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 bf2462e2ab4..424406c527b 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 dc4b00170ad..6d6734a10ff 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 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()); }