use pimpl for collision in tile_parser

include/mbgl/map/tile_parser.hpp

@@ -4,7 +4,6 @@
 #include <mbgl/map/vector_tile.hpp>
 #include <mbgl/style/filter_expression.hpp>
 #include <mbgl/text/glyph.hpp>
-#include <mbgl/text/collision.hpp>
 
 #include <cstdint>
 #include <iosfwd>
@@ -28,6 +27,7 @@ class StyleBucketLine;
 class StyleBucketSymbol;
 class StyleLayerGroup;
 class VectorTileData;
+class Collision;
 
 class TileParser {
 public:
@@ -37,6 +37,7 @@ public:
                const std::shared_ptr<GlyphStore> &glyphStore,
                const std::shared_ptr<SpriteAtlas> &spriteAtlas,
                const std::shared_ptr<Sprite> &sprite);
+    ~TileParser();
 
 public:
     void parse();
@@ -65,7 +66,7 @@ private:
     std::shared_ptr<Sprite> sprite;
     std::shared_ptr<Texturepool> texturePool;
 
-    Collision collision;
+    std::unique_ptr<Collision> collision;
 };
 
 }

include/mbgl/text/collision.hpp

@@ -29,7 +29,6 @@ class Collision {
 
 public:
     Collision(float zoom, float tileExtent, float tileSize, float placementDepth = 1);
-    ~Collision();
 
     float getPlacementScale(const GlyphBoxes &glyphs, float minPlacementScale, bool avoidEdges);
     PlacementRange getPlacementRange(const GlyphBoxes &glyphs, float placementScale,
@@ -38,8 +37,8 @@ public:
                 const PlacementRange &placementRange, bool horizontal);
 
 private:
-    void *hTree;
-    void *cTree;
+    Tree hTree;
+    Tree cTree;
     PlacementValue leftEdge;
     PlacementValue topEdge;
     PlacementValue rightEdge;

src/map/tile_parser.cpp

@@ -13,6 +13,7 @@
 #include <mbgl/util/token.hpp>
 #include <mbgl/geometry/glyph_atlas.hpp>
 #include <mbgl/text/glyph_store.hpp>
+#include <mbgl/text/collision.hpp>
 #include <mbgl/text/glyph.hpp>
 #include <mbgl/map/map.hpp>
 
@@ -29,7 +30,12 @@ namespace regex_impl = boost;
 namespace regex_impl = std;
 #endif
 
-using namespace mbgl;
+namespace mbgl {
+
+// Note: This destructor is seemingly empty, but we need to declare it anyway
+// because this object has a std::unique_ptr<> of a forward-declare type in
+// its header file.
+TileParser::~TileParser() = default;
 
 TileParser::TileParser(const std::string &data, VectorTileData &tile,
                        const std::shared_ptr<const Style> &style,
@@ -44,7 +50,7 @@ TileParser::TileParser(const std::string &data, VectorTileData &tile,
       glyphStore(glyphStore),
       spriteAtlas(spriteAtlas),
       sprite(sprite),
-      collision(tile.id.z, 4096, tile.source.tile_size, tile.depth) {
+      collision(std::make_unique<Collision>(tile.id.z, 4096, tile.source.tile_size, tile.depth)) {
 }
 
 void TileParser::parse() {
@@ -163,7 +169,9 @@ std::unique_ptr<Bucket> TileParser::createLineBucket(const VectorTileLayer& laye
 }
 
 std::unique_ptr<Bucket> TileParser::createSymbolBucket(const VectorTileLayer& layer, const FilterExpression &filter, const StyleBucketSymbol &symbol) {
-    std::unique_ptr<SymbolBucket> bucket = std::make_unique<SymbolBucket>(symbol, collision);
+    std::unique_ptr<SymbolBucket> bucket = std::make_unique<SymbolBucket>(symbol, *collision);
     bucket->addFeatures(layer, filter, tile.id, *spriteAtlas, *sprite, *glyphAtlas, *glyphStore);
     return obsolete() ? nullptr : std::move(bucket);
 }
+
+}

src/renderer/symbol_bucket.cpp

@@ -240,7 +240,7 @@ void SymbolBucket::addFeature(const std::vector<Coordinate> &line, const Shaping
         Placement iconPlacement;
         float glyphScale = 0;
         float iconScale = 0;
-        bool inside = !(anchor.x < 0 || anchor.x > 4096 || anchor.y < 0 || anchor.y > 4096);
+        const bool inside = !(anchor.x < 0 || anchor.x > 4096 || anchor.y < 0 || anchor.y > 4096);
 
         if (avoidEdges && !inside) continue;
 

src/text/collision.cpp

@@ -19,17 +19,9 @@ Box getBox(const CollisionAnchor &anchor, const CollisionRect &bbox, float minSc
     };
 };
 
-Collision::~Collision() {
-    delete ((Tree *)cTree);
-    delete ((Tree *)hTree);
-}
-
 Collision::Collision(float zoom, float tileExtent, float tileSize, float placementDepth)
-    : hTree(new Tree()), // tree for horizontal labels
-      cTree(new Tree()), // tree for glyphs from curved labels
-
       // tile pixels per screen pixels at the tile's zoom level
-      tilePixelRatio(tileExtent / tileSize),
+    : tilePixelRatio(tileExtent / tileSize),
 
       zoom(zoom),
 
@@ -135,8 +127,8 @@ float Collision::getPlacementScale(const GlyphBoxes &glyphs, float minPlacementS
         const Box searchBox = getBox(anchor, bbox, minScale, maxScale);
 
         std::vector<PlacementValue> blocking;
-        ((Tree *)hTree)->query(bgi::intersects(searchBox), std::back_inserter(blocking));
-        ((Tree *)cTree)->query(bgi::intersects(searchBox), std::back_inserter(blocking));
+        hTree.query(bgi::intersects(searchBox), std::back_inserter(blocking));
+        cTree.query(bgi::intersects(searchBox), std::back_inserter(blocking));
 
         if (avoidEdges) {
             if (searchBox.min_corner().get<0>() < 0) blocking.emplace_back(leftEdge);
@@ -218,10 +210,10 @@ PlacementRange Collision::getPlacementRange(const GlyphBoxes &glyphs, float plac
         Box query_box{Point{minPlacedX, minPlacedY}, Point{maxPlacedX, maxPlacedY}};
 
         std::vector<PlacementValue> blocking;
-        ((Tree *)hTree)->query(bgi::intersects(query_box), std::back_inserter(blocking));
+        hTree.query(bgi::intersects(query_box), std::back_inserter(blocking));
 
         if (horizontal) {
-            ((Tree *)cTree)->query(bgi::intersects(query_box), std::back_inserter(blocking));
+            cTree.query(bgi::intersects(query_box), std::back_inserter(blocking));
         }
 
         for (const PlacementValue &value : blocking) {
@@ -298,8 +290,8 @@ void Collision::insert(const GlyphBoxes &glyphs, const CollisionAnchor &anchor,
 
     // Bulk-insert all glyph boxes
     if (horizontal) {
-        ((Tree *)hTree)->insert(allBounds.begin(), allBounds.end());
+        hTree.insert(allBounds.begin(), allBounds.end());
     } else {
-        ((Tree *)cTree)->insert(allBounds.begin(), allBounds.end());
+        cTree.insert(allBounds.begin(), allBounds.end());
     }
 }