Fix CollisionTile::queryRenderedSymbols query geometry

cmake/test-files.cmake

@@ -91,6 +91,7 @@ set(MBGL_TEST_FILES
     test/tile/raster_tile.test.cpp
     test/tile/tile_id.test.cpp
     test/tile/vector_tile.test.cpp
+    test/tile/tile_point.test.cpp
 
     # util
     test/util/async_task.test.cpp

package.json

@@ -22,7 +22,7 @@
     "express": "^4.11.1",
     "mapbox-gl-shaders": "mapbox/mapbox-gl-shaders#de2ab007455aa2587c552694c68583f94c9f2747",
     "mapbox-gl-style-spec": "mapbox/mapbox-gl-style-spec#7f62a4fc9f21e619824d68abbc4b03cbc1685572",
-    "mapbox-gl-test-suite": "mapbox/mapbox-gl-test-suite#694f0d0728f229d64d1639dc5ee7aff7f4b8ca41",
+    "mapbox-gl-test-suite": "mapbox/mapbox-gl-test-suite#9f6cbba2e4dcb64517fcbad091f311035f40862c",
     "mkdirp": "^0.5.1",
     "node-cmake": "^1.2.1",
     "request": "^2.72.0",

platform/node/src/node_map.cpp

@@ -61,6 +61,8 @@ void NodeMap::Init(v8::Local<v8::Object> target) {
     Nan::SetPrototypeMethod(tpl, "setLayoutProperty", SetLayoutProperty);
     Nan::SetPrototypeMethod(tpl, "setPaintProperty", SetPaintProperty);
     Nan::SetPrototypeMethod(tpl, "setFilter", SetFilter);
+    Nan::SetPrototypeMethod(tpl, "setCenter", SetCenter);
+    Nan::SetPrototypeMethod(tpl, "setBearing", SetBearing);
 
     Nan::SetPrototypeMethod(tpl, "dumpDebugLogs", DumpDebugLogs);
     Nan::SetPrototypeMethod(tpl, "queryRenderedFeatures", QueryRenderedFeatures);
@@ -674,6 +676,46 @@ void NodeMap::SetFilter(const Nan::FunctionCallbackInfo<v8::Value>& info) {
     layer->accept(SetFilterVisitor { filter });
 }
 
+void NodeMap::SetCenter(const Nan::FunctionCallbackInfo<v8::Value>& info) {
+    auto nodeMap = Nan::ObjectWrap::Unwrap<NodeMap>(info.Holder());
+    if (!nodeMap->map) return Nan::ThrowError(releasedMessage());
+
+    if (info.Length() <= 0 || !info[0]->IsArray()) {
+        return Nan::ThrowTypeError("First argument must be an array");
+    }
+
+    auto center = info[0].As<v8::Array>();
+    double latitude = 0;
+    double longitude = 0;
+    if (center->Length() > 0) { longitude = Nan::Get(center, 0).ToLocalChecked()->NumberValue(); }
+    if (center->Length() > 1) { latitude = Nan::Get(center, 1).ToLocalChecked()->NumberValue(); }
+
+    try {
+        nodeMap->map->setLatLng(mbgl::LatLng { latitude, longitude });
+    } catch (const std::exception &ex) {
+        return Nan::ThrowError(ex.what());
+    }
+
+    info.GetReturnValue().SetUndefined();
+}
+
+void NodeMap::SetBearing(const Nan::FunctionCallbackInfo<v8::Value>& info) {
+    auto nodeMap = Nan::ObjectWrap::Unwrap<NodeMap>(info.Holder());
+    if (!nodeMap->map) return Nan::ThrowError(releasedMessage());
+
+    if (info.Length() <= 0 || !info[0]->IsNumber()) {
+        return Nan::ThrowTypeError("First argument must be a number");
+    }
+
+    try {
+        nodeMap->map->setBearing(info[0]->NumberValue());
+    } catch (const std::exception &ex) {
+        return Nan::ThrowError(ex.what());
+    }
+
+    info.GetReturnValue().SetUndefined();
+}
+
 void NodeMap::DumpDebugLogs(const Nan::FunctionCallbackInfo<v8::Value>& info) {
     auto nodeMap = Nan::ObjectWrap::Unwrap<NodeMap>(info.Holder());
     if (!nodeMap->map) return Nan::ThrowError(releasedMessage());

platform/node/src/node_map.hpp

@@ -37,6 +37,8 @@ class NodeMap : public Nan::ObjectWrap,
     static void SetLayoutProperty(const Nan::FunctionCallbackInfo<v8::Value>&);
     static void SetPaintProperty(const Nan::FunctionCallbackInfo<v8::Value>&);
     static void SetFilter(const Nan::FunctionCallbackInfo<v8::Value>&);
+    static void SetCenter(const Nan::FunctionCallbackInfo<v8::Value>&);
+    static void SetBearing(const Nan::FunctionCallbackInfo<v8::Value>&);
     static void DumpDebugLogs(const Nan::FunctionCallbackInfo<v8::Value>&);
     static void QueryRenderedFeatures(const Nan::FunctionCallbackInfo<v8::Value>&);
 

platform/node/test/js/map.test.js

@@ -114,6 +114,8 @@ test('Map', function(t) {
             'setLayoutProperty',
             'setPaintProperty',
             'setFilter',
+            'setCenter',
+            'setBearing',
             'dumpDebugLogs',
             'queryRenderedFeatures'
         ]);

src/mbgl/annotation/annotation_manager.cpp

@@ -161,7 +161,15 @@ std::unique_ptr<AnnotationTileData> AnnotationManager::getTileData(const Canonic
 
     LatLngBounds tileBounds(tileID);
 
-    symbolTree.query(boost::geometry::index::intersects(tileBounds),
+    // Edge cases e.g. a point in null island (0, 0) makes the geometry
+    // intersectable by the four surrounding tiles. This makes sure only the
+    // tile that produces a valid projection in updateLayer() continues.
+    auto skipTileEdges = [&] (const auto& val) -> bool {
+        return val->annotation.geometry.x != tileBounds.east() &&
+               val->annotation.geometry.y != tileBounds.south();
+    };
+
+    symbolTree.query(bgi::intersects(tileBounds) && bgi::satisfies(skipTileEdges),
         boost::make_function_output_iterator([&](const auto& val){
             val->updateLayer(tileID, pointLayer);
         }));
@@ -185,6 +193,7 @@ void AnnotationManager::updateStyle(Style& style) {
         layer->setSourceLayer(PointLayerID);
         layer->setIconImage({"{sprite}"});
         layer->setIconAllowOverlap(true);
+        layer->setIconIgnorePlacement(true);
 
         layer->impl->spriteAtlas = &spriteAtlas;
 

src/mbgl/annotation/annotation_manager.hpp

@@ -23,6 +23,8 @@ namespace style {
 class Style;
 } // namespace style
 
+namespace bgi = boost::geometry::index;
+
 class AnnotationManager : private util::noncopyable {
 public:
     AnnotationManager(float pixelRatio);
@@ -63,7 +65,7 @@ class AnnotationManager : private util::noncopyable {
 
     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>, bgi::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/geometry/feature_index.cpp

@@ -83,7 +83,7 @@ void FeatureIndex::query(
         return;
     }
 
-    std::vector<IndexedSubfeature> symbolFeatures = collisionTile->queryRenderedSymbols(box, scale);
+    std::vector<IndexedSubfeature> symbolFeatures = collisionTile->queryRenderedSymbols(queryGeometry, scale);
     std::sort(symbolFeatures.begin(), symbolFeatures.end(), topDownSymbols);
     for (const auto& symbolFeature : symbolFeatures) {
         addFeature(result, symbolFeature, queryGeometry, filterLayerIDs, geometryTileData, tileID, style, bearing, pixelsToTileUnits);

src/mbgl/map/map.cpp

@@ -422,7 +422,7 @@ void Map::moveBy(const ScreenCoordinate& point, const Duration& duration) {
 }
 
 void Map::setLatLng(const LatLng& latLng, const Duration& duration) {
-    setLatLng(latLng, ScreenCoordinate {}, duration);
+    setLatLng(latLng, optional<ScreenCoordinate> {}, duration);
 }
 
 void Map::setLatLng(const LatLng& latLng, optional<EdgeInsets> padding, const Duration& duration) {

src/mbgl/style/source_impl.cpp

@@ -22,8 +22,12 @@
 namespace mbgl {
 namespace style {
 
+namespace {
+
 static SourceObserver nullObserver;
 
+} // anonymous namespace
+
 Source::Impl::Impl(SourceType type_, std::string id_, Source& base_)
     : type(type_),
       id(std::move(id_)),
@@ -168,7 +172,8 @@ void Source::Impl::updateTiles(const UpdateParameters& parameters) {
                                    parameters.debugOptions & MapDebugOptions::Collision };
 
     for (auto& pair : tiles) {
-        pair.second->setPlacementConfig(config);
+        auto& tile = pair.second;
+        tile->setPlacementConfig(config);
     }
 }
 
@@ -181,56 +186,40 @@ void Source::Impl::reloadTiles() {
     }
 }
 
-static Point<int16_t> coordinateToTilePoint(const UnwrappedTileID& tileID, const Point<double>& p) {
-    auto zoomedCoord = TileCoordinate { p, 0 }.zoomTo(tileID.canonical.z);
-    return {
-        int16_t(util::clamp<int64_t>((zoomedCoord.p.x - tileID.canonical.x - tileID.wrap * std::pow(2, tileID.canonical.z)) * util::EXTENT,
-                    std::numeric_limits<int16_t>::min(),
-                    std::numeric_limits<int16_t>::max())),
-        int16_t(util::clamp<int64_t>((zoomedCoord.p.y - tileID.canonical.y) * util::EXTENT,
-                    std::numeric_limits<int16_t>::min(),
-                    std::numeric_limits<int16_t>::max()))
-    };
-}
-
 std::unordered_map<std::string, std::vector<Feature>> Source::Impl::queryRenderedFeatures(const QueryParameters& parameters) const {
     std::unordered_map<std::string, std::vector<Feature>> result;
-    if (renderTiles.empty()) {
+    if (renderTiles.empty() || parameters.geometry.empty()) {
         return result;
     }
 
     LineString<double> queryGeometry;
-
     for (const auto& p : parameters.geometry) {
         queryGeometry.push_back(TileCoordinate::fromScreenCoordinate(
             parameters.transformState, 0, { p.x, parameters.transformState.getHeight() - p.y }).p);
     }
 
-    if (queryGeometry.empty()) {
-        return result;
-    }
-
     mapbox::geometry::box<double> box = mapbox::geometry::envelope(queryGeometry);
 
     for (const auto& tilePtr : renderTiles) {
-        const RenderTile& tile = tilePtr.second;
+        const RenderTile& renderTile = tilePtr.second;
 
-        Point<int16_t> tileSpaceBoundsMin = coordinateToTilePoint(tile.id, box.min);
-        Point<int16_t> tileSpaceBoundsMax = coordinateToTilePoint(tile.id, box.max);
+        TilePoint tileSpaceBoundsMin = Tile::pointFromTileCoordinate(renderTile.id, box.min);
+        TilePoint tileSpaceBoundsMax = Tile::pointFromTileCoordinate(renderTile.id, box.max);
 
         if (tileSpaceBoundsMin.x >= util::EXTENT || tileSpaceBoundsMin.y >= util::EXTENT ||
             tileSpaceBoundsMax.x < 0 || tileSpaceBoundsMax.y < 0) continue;
 
         GeometryCoordinates tileSpaceQueryGeometry;
 
         for (const auto& c : queryGeometry) {
-            tileSpaceQueryGeometry.push_back(coordinateToTilePoint(tile.id, c));
+            tileSpaceQueryGeometry.push_back(Tile::pointFromTileCoordinate(renderTile.id, c));
         }
 
-        tile.tile.queryRenderedFeatures(result,
-                                        tileSpaceQueryGeometry,
-                                        parameters.transformState,
-                                        parameters.layerIDs);
+        auto& tile = renderTile.tile;
+        tile.queryRenderedFeatures(result,
+                                   tileSpaceQueryGeometry,
+                                   parameters.transformState,
+                                   parameters.layerIDs);
     }
 
     return result;

src/mbgl/text/collision_tile.cpp

@@ -3,6 +3,8 @@
 #include <mbgl/util/constants.hpp>
 #include <mbgl/util/math.hpp>
 
+#include <mapbox/geometry/for_each_point.hpp>
+
 #include <cmath>
 
 namespace mbgl {
@@ -153,34 +155,56 @@ Box CollisionTile::getTreeBox(const Point<float>& anchor, const CollisionBox& bo
     };
 }
 
-std::vector<IndexedSubfeature> CollisionTile::queryRenderedSymbols(const mapbox::geometry::box<int16_t>& box, const float scale) {
+std::vector<IndexedSubfeature> CollisionTile::queryRenderedSymbols(const GeometryCollection& queryGeometry, const float scale) {
 
     std::vector<IndexedSubfeature> result;
+    if (queryGeometry.empty()) return result;
+
     std::unordered_map<std::string, std::unordered_set<std::size_t>> sourceLayerFeatures;
 
-    auto anchor = util::matrixMultiply(rotationMatrix, convertPoint<float>(box.min));
+    mapbox::geometry::box<float> box {
+        { std::numeric_limits<float>::max(), std::numeric_limits<float>::max() },
+        { std::numeric_limits<float>::lowest(), std::numeric_limits<float>::lowest() }
+    };
+
+    mapbox::geometry::for_each_point(queryGeometry, [&] (const mapbox::geometry::point<int16_t>& point) {
+        auto rotated = util::matrixMultiply(rotationMatrix, convertPoint<float>(point));
+        if (box.min.x > rotated.x) box.min.x = rotated.x;
+        if (box.min.y > rotated.y) box.min.y = rotated.y;
+        if (box.max.x < rotated.x) box.max.x = rotated.x;
+        if (box.max.y < rotated.y) box.max.y = rotated.y;
+    });
+
+    const auto& anchor = box.min;
     CollisionBox queryBox(anchor, 0, 0, box.max.x - box.min.x, box.max.y - box.min.y, scale);
     auto predicates = bgi::intersects(getTreeBox(anchor, queryBox));
 
-    auto fn = [&] (const Tree& tree_) {
+    auto fn = [&] (const Tree& tree_, bool ignorePlacement) {
         for (auto it = tree_.qbegin(predicates); it != tree_.qend(); ++it) {
             const CollisionBox& blocking = std::get<1>(*it);
             const IndexedSubfeature& indexedFeature = std::get<2>(*it);
 
             auto& seenFeatures = sourceLayerFeatures[indexedFeature.sourceLayerName];
             if (seenFeatures.find(indexedFeature.index) == seenFeatures.end()) {
-                auto blockingAnchor = util::matrixMultiply(rotationMatrix, blocking.anchor);
-                float minPlacementScale = findPlacementScale(minScale, anchor, queryBox, blockingAnchor, blocking);
-                if (minPlacementScale >= scale) {
+                if (ignorePlacement) {
                     seenFeatures.insert(indexedFeature.index);
                     result.push_back(indexedFeature);
+                } else {
+                    auto blockingAnchor = util::matrixMultiply(rotationMatrix, blocking.anchor);
+                    float minPlacementScale = findPlacementScale(minScale, anchor, queryBox, blockingAnchor, blocking);
+                    if (minPlacementScale >= scale) {
+                        seenFeatures.insert(indexedFeature.index);
+                        result.push_back(indexedFeature);
+                    }
                 }
             }
         }
     };
 
-    fn(tree);
-    fn(ignoredTree);
+    bool ignorePlacement = false;
+    fn(tree, ignorePlacement);
+    ignorePlacement = true;
+    fn(ignoredTree, ignorePlacement);
 
     return result;
 }

src/mbgl/text/collision_tile.hpp

@@ -2,6 +2,7 @@
 
 #include <mbgl/text/collision_feature.hpp>
 #include <mbgl/text/placement_config.hpp>
+#include <mbgl/tile/geometry_tile_data.hpp>
 
 #pragma GCC diagnostic push
 #pragma GCC diagnostic ignored "-Wunused-function"
@@ -41,7 +42,7 @@ class CollisionTile {
     float placeFeature(const CollisionFeature&, const bool allowOverlap, const bool avoidEdges);
     void insertFeature(CollisionFeature&, const float minPlacementScale, const bool ignorePlacement);
 
-    std::vector<IndexedSubfeature> queryRenderedSymbols(const mapbox::geometry::box<int16_t>&, const float scale);
+    std::vector<IndexedSubfeature> queryRenderedSymbols(const GeometryCollection&, const float scale);
 
     const PlacementConfig config;
 

src/mbgl/tile/tile.hpp

@@ -4,10 +4,12 @@
 #include <mbgl/util/chrono.hpp>
 #include <mbgl/util/optional.hpp>
 #include <mbgl/util/feature.hpp>
+#include <mbgl/util/tile_coordinate.hpp>
 #include <mbgl/tile/tile_id.hpp>
 #include <mbgl/renderer/bucket.hpp>
 #include <mbgl/tile/geometry_tile_data.hpp>
 #include <mbgl/storage/resource.hpp>
+#include <mbgl/math/clamp.hpp>
 
 #include <string>
 #include <memory>
@@ -25,8 +27,22 @@ namespace style {
 class Layer;
 } // namespace style
 
+using TilePoint = Point<int16_t>;
+
 class Tile : private util::noncopyable {
 public:
+    static TilePoint pointFromTileCoordinate(const UnwrappedTileID& tileID, const TileCoordinatePoint& p) {
+        auto zoomed = TileCoordinate { p, 0 }.zoomTo(tileID.canonical.z);
+        return {
+            util::clamp(int16_t((zoomed.p.x - tileID.canonical.x - tileID.wrap * std::pow(2.0, tileID.canonical.z)) * util::EXTENT),
+                        std::numeric_limits<int16_t>::min(),
+                        std::numeric_limits<int16_t>::max()),
+            util::clamp(int16_t((zoomed.p.y - tileID.canonical.y) * util::EXTENT),
+                        std::numeric_limits<int16_t>::min(),
+                        std::numeric_limits<int16_t>::max())
+        };
+    }
+
     Tile(OverscaledTileID);
     virtual ~Tile();
 

src/mbgl/util/tile_coordinate.hpp

@@ -5,20 +5,22 @@
 
 namespace mbgl {
 
+using TileCoordinatePoint = Point<double>;
+
 // Has floating point x/y coordinates.
 // Used for computing the tiles that need to be visible in the viewport.
 class TileCoordinate {
 public:
-    Point<double> p;
+    TileCoordinatePoint p;
     double z;
 
     static TileCoordinate fromLatLng(const TransformState& state, double zoom, const LatLng& latLng) {
         const double scale = std::pow(2, zoom - state.getZoom());
         return { state.project(latLng) * scale / double(util::tileSize), zoom };
     }
 
-    static TileCoordinate fromScreenCoordinate(const TransformState& state, double zoom, const ScreenCoordinate& point) {
-        return fromLatLng(state, zoom, state.screenCoordinateToLatLng(point));
+    static TileCoordinate fromScreenCoordinate(const TransformState& state, double zoom, const ScreenCoordinate& screenCoordinate) {
+        return fromLatLng(state, zoom, state.screenCoordinateToLatLng(screenCoordinate));
     }
 
     TileCoordinate zoomTo(double zoom) const {