[core] always return owning pointers for GeometryTileData::getLayer()

9 files changed, 78 insertions, 52 deletions

diff --git a/src/mbgl/annotation/annotation_manager.cpp b/src/mbgl/annotation/annotation_manager.cpp
index bc23a94adf..58bda6052e 100644
--- a/src/mbgl/annotation/annotation_manager.cpp
+++ b/src/mbgl/annotation/annotation_manager.cpp
@@ -133,13 +133,13 @@ std::unique_ptr<AnnotationTileData> AnnotationManager::getTileData(const Canonic
 
     auto tileData = std::make_unique<AnnotationTileData>();
 
-    AnnotationTileLayer& pointLayer = tileData->addLayer(PointLayerID);
+    auto pointLayer = tileData->addLayer(PointLayerID);
 
     LatLngBounds tileBounds(tileID);
 
     symbolTree.query(boost::geometry::index::intersects(tileBounds),
         boost::make_function_output_iterator([&](const auto& val){
-            val->updateLayer(tileID, pointLayer);
+            val->updateLayer(tileID, *pointLayer);
         }));
 
     for (const auto& shape : shapeAnnotations) {
diff --git a/src/mbgl/annotation/annotation_tile.cpp b/src/mbgl/annotation/annotation_tile.cpp
index e8c62effd8..0596d60f4f 100644
--- a/src/mbgl/annotation/annotation_tile.cpp
+++ b/src/mbgl/annotation/annotation_tile.cpp
@@ -66,7 +66,28 @@ GeometryCollection AnnotationTileFeature::getGeometries() const {
     return data->geometries;
 }
 
-AnnotationTileLayer::AnnotationTileLayer(std::string name_) : name(std::move(name_)) {
+class AnnotationTileLayerData {
+public:
+    AnnotationTileLayerData(const std::string& name_) : name(name_) {
+    }
+
+    const std::string name;
+    std::vector<std::shared_ptr<const AnnotationTileFeatureData>> features;
+};
+
+AnnotationTileLayer::AnnotationTileLayer(std::shared_ptr<AnnotationTileLayerData> layer_) : layer(std::move(layer_)) {
+}
+
+std::size_t AnnotationTileLayer::featureCount() const {
+    return layer->features.size();
+}
+
+std::unique_ptr<GeometryTileFeature> AnnotationTileLayer::getFeature(std::size_t i) const {
+    return std::make_unique<AnnotationTileFeature>(layer->features.at(i));
+}
+
+std::string AnnotationTileLayer::getName() const {
+    return layer->name;
 }
 
 void AnnotationTileLayer::addFeature(const AnnotationID id,
@@ -74,7 +95,7 @@ void AnnotationTileLayer::addFeature(const AnnotationID id,
                                      GeometryCollection geometries,
                                      std::unordered_map<std::string, std::string> properties) {
 
-    features.emplace_back(std::make_shared<AnnotationTileFeatureData>(
+    layer->features.emplace_back(std::make_shared<AnnotationTileFeatureData>(
         id, type, std::move(geometries), std::move(properties)));
 }
 
@@ -82,17 +103,21 @@ std::unique_ptr<GeometryTileData> AnnotationTileData::clone() const {
     return std::make_unique<AnnotationTileData>(*this);
 }
 
-const GeometryTileLayer* AnnotationTileData::getLayer(const std::string& name) const {
+std::unique_ptr<GeometryTileLayer> AnnotationTileData::getLayer(const std::string& name) const {
     auto it = layers.find(name);
     if (it != layers.end()) {
-        return &it->second;
+        return std::make_unique<AnnotationTileLayer>(it->second);
     }
     return nullptr;
 }
 
-AnnotationTileLayer& AnnotationTileData::addLayer(const std::string& name) {
+std::unique_ptr<AnnotationTileLayer> AnnotationTileData::addLayer(const std::string& name) {
     // Only constructs a new layer if it doesn't yet exist, otherwise, we'll use the existing one.
-    return layers.emplace(name, name).first->second;
+    auto it = layers.find(name);
+    if (it == layers.end()) {
+        it = layers.emplace(name, std::make_shared<AnnotationTileLayerData>(name)).first;
+    }
+    return std::make_unique<AnnotationTileLayer>(it->second);
 }
 
 } // namespace mbgl
diff --git a/src/mbgl/annotation/annotation_tile.hpp b/src/mbgl/annotation/annotation_tile.hpp
index 68a4d342ac..88505c50e3 100644
--- a/src/mbgl/annotation/annotation_tile.hpp
+++ b/src/mbgl/annotation/annotation_tile.hpp
@@ -36,21 +36,15 @@ private:
     std::shared_ptr<const AnnotationTileFeatureData> data;
 };
 
+class AnnotationTileLayerData;
+
 class AnnotationTileLayer : public GeometryTileLayer {
 public:
-    AnnotationTileLayer(std::string);
-
-    std::size_t featureCount() const override {
-        return features.size();
-    }
+    AnnotationTileLayer(std::shared_ptr<AnnotationTileLayerData>);
 
-    std::unique_ptr<GeometryTileFeature> getFeature(std::size_t i) const override {
-        return std::make_unique<AnnotationTileFeature>(features.at(i));
-    }
-
-    std::string getName() const override {
-        return name;
-    }
+    std::size_t featureCount() const override;
+    std::unique_ptr<GeometryTileFeature> getFeature(std::size_t i) const override;
+    std::string getName() const override;
 
     void addFeature(const AnnotationID,
                     FeatureType,
@@ -58,18 +52,18 @@ public:
                     std::unordered_map<std::string, std::string> properties = { {} });
 
 private:
-    std::vector<std::shared_ptr<const AnnotationTileFeatureData>> features;
-    std::string name;
+    std::shared_ptr<AnnotationTileLayerData> layer;
 };
 
 class AnnotationTileData : public GeometryTileData {
 public:
     std::unique_ptr<GeometryTileData> clone() const override;
-    const GeometryTileLayer* getLayer(const std::string&) const override;
-    AnnotationTileLayer& addLayer(const std::string&);
+    std::unique_ptr<GeometryTileLayer> getLayer(const std::string&) const override;
+
+    std::unique_ptr<AnnotationTileLayer> addLayer(const std::string&);
 
 private:
-    std::unordered_map<std::string, AnnotationTileLayer> layers;
+    std::unordered_map<std::string, std::shared_ptr<AnnotationTileLayerData>> layers;
 };
 
 } // namespace mbgl
diff --git a/src/mbgl/annotation/shape_annotation_impl.cpp b/src/mbgl/annotation/shape_annotation_impl.cpp
index 05ffce382d..0c1a631ad8 100644
--- a/src/mbgl/annotation/shape_annotation_impl.cpp
+++ b/src/mbgl/annotation/shape_annotation_impl.cpp
@@ -38,7 +38,7 @@ void ShapeAnnotationImpl::updateTileData(const CanonicalTileID& tileID, Annotati
     if (shapeTile.features.empty())
         return;
 
-    AnnotationTileLayer& layer = data.addLayer(layerID);
+    auto layer = data.addLayer(layerID);
 
     ToGeometryCollection toGeometryCollection;
     ToFeatureType toFeatureType;
@@ -53,7 +53,7 @@ void ShapeAnnotationImpl::updateTileData(const CanonicalTileID& tileID, Annotati
             renderGeometry = fixupPolygons(renderGeometry);
         }
 
-        layer.addFeature(id, featureType, renderGeometry);
+        layer->addFeature(id, featureType, renderGeometry);
     }
 }
 
diff --git a/src/mbgl/tile/geojson_tile.cpp b/src/mbgl/tile/geojson_tile.cpp
index 622d35d6d4..e9865e8272 100644
--- a/src/mbgl/tile/geojson_tile.cpp
+++ b/src/mbgl/tile/geojson_tile.cpp
@@ -51,8 +51,29 @@ public:
     }
 };
 
-class GeoJSONTileData : public GeometryTileData,
-                        public GeometryTileLayer {
+class GeoJSONTileLayer : public GeometryTileLayer {
+public:
+    GeoJSONTileLayer(std::shared_ptr<const mapbox::geometry::feature_collection<int16_t>> features_)
+        : features(std::move(features_)) {
+    }
+
+    std::size_t featureCount() const override {
+        return features->size();
+    }
+
+    std::unique_ptr<GeometryTileFeature> getFeature(std::size_t i) const override {
+        return std::make_unique<GeoJSONTileFeature>((*features)[i]);
+    }
+
+    std::string getName() const override {
+        return "";
+    }
+
+private:
+    std::shared_ptr<const mapbox::geometry::feature_collection<int16_t>> features;
+};
+
+class GeoJSONTileData : public GeometryTileData {
 public:
     GeoJSONTileData(mapbox::geometry::feature_collection<int16_t> features_)
         : features(std::make_shared<mapbox::geometry::feature_collection<int16_t>>(
@@ -67,21 +88,10 @@ public:
         return std::make_unique<GeoJSONTileData>(features);
     }
 
-    const GeometryTileLayer* getLayer(const std::string&) const override {
-        return this;
-    }
-
-    std::string getName() const override {
-        return "";
+    std::unique_ptr<GeometryTileLayer> getLayer(const std::string&) const override {
+        return std::make_unique<GeoJSONTileLayer>(features);
     }
 
-    std::size_t featureCount() const override {
-        return features->size();
-    }
-
-    std::unique_ptr<GeometryTileFeature> getFeature(std::size_t i) const override {
-        return std::make_unique<GeoJSONTileFeature>((*features)[i]);
-    }
 
 private:
     std::shared_ptr<const mapbox::geometry::feature_collection<int16_t>> features;
diff --git a/src/mbgl/tile/geometry_tile_data.hpp b/src/mbgl/tile/geometry_tile_data.hpp
index 285f86cc7b..741c5149a9 100644
--- a/src/mbgl/tile/geometry_tile_data.hpp
+++ b/src/mbgl/tile/geometry_tile_data.hpp
@@ -59,7 +59,7 @@ class GeometryTileData {
 public:
     virtual ~GeometryTileData() = default;
     virtual std::unique_ptr<GeometryTileData> clone() const = 0;
-    virtual const GeometryTileLayer* getLayer(const std::string&) const = 0;
+    virtual std::unique_ptr<GeometryTileLayer> getLayer(const std::string&) const = 0;
 };
 
 // classifies an array of rings into polygons with outer rings and holes
diff --git a/src/mbgl/tile/vector_tile_data.cpp b/src/mbgl/tile/vector_tile_data.cpp
index bb8392f6d9..2d4a01bda3 100644
--- a/src/mbgl/tile/vector_tile_data.cpp
+++ b/src/mbgl/tile/vector_tile_data.cpp
@@ -67,19 +67,17 @@ std::unique_ptr<GeometryTileData> VectorTileData::clone() const {
     return std::make_unique<VectorTileData>(data);
 }
 
-const GeometryTileLayer* VectorTileData::getLayer(const std::string& name) const {
+std::unique_ptr<GeometryTileLayer> VectorTileData::getLayer(const std::string& name) const {
     if (!parsed) {
         // We're parsing this lazily so that we can construct VectorTileData objects on the main
         // thread without incurring the overhead of parsing immediately.
-        for (const auto& pair : mapbox::vector_tile::buffer(*data).getLayers()) {
-            layers.emplace(pair.first, VectorTileLayer{ data, pair.second });
-        }
+        layers = mapbox::vector_tile::buffer(*data).getLayers();
         parsed = true;
     }
 
     auto it = layers.find(name);
     if (it != layers.end()) {
-        return &it->second;
+        return std::make_unique<VectorTileLayer>(data, it->second);
     }
     return nullptr;
 }
diff --git a/src/mbgl/tile/vector_tile_data.hpp b/src/mbgl/tile/vector_tile_data.hpp
index 5ecb591273..48beaf9d07 100644
--- a/src/mbgl/tile/vector_tile_data.hpp
+++ b/src/mbgl/tile/vector_tile_data.hpp
@@ -41,14 +41,14 @@ public:
     VectorTileData(std::shared_ptr<const std::string> data);
 
     std::unique_ptr<GeometryTileData> clone() const override;
-    const GeometryTileLayer* getLayer(const std::string& name) const override;
+    std::unique_ptr<GeometryTileLayer> getLayer(const std::string& name) const override;
 
     std::vector<std::string> layerNames() const;
 
 private:
     std::shared_ptr<const std::string> data;
     mutable bool parsed = false;
-    mutable std::unordered_map<std::string, VectorTileLayer> layers;
+    mutable std::map<std::string, const protozero::data_view> layers;
 };
 
 } // namespace mbgl
diff --git a/test/tile/annotation_tile.test.cpp b/test/tile/annotation_tile.test.cpp
index 707f118fd5..a69735d23e 100644
--- a/test/tile/annotation_tile.test.cpp
+++ b/test/tile/annotation_tile.test.cpp
@@ -49,8 +49,7 @@ TEST(AnnotationTile, Issue8289) {
     AnnotationTile tile(OverscaledTileID(0, 0, 0), test.tileParameters);
 
     auto data = std::make_unique<AnnotationTileData>();
-    data->layers.emplace("test", AnnotationTileLayer("test"));
-    data->layers.at("test").features.emplace_back(0, FeatureType::Point, GeometryCollection());
+    data->addLayer("test")->addFeature(0, FeatureType::Point, GeometryCollection());
 
     // Simulate layout and placement of a symbol layer.
     tile.onLayout(GeometryTile::LayoutResult {