[core] Don't share util::ptr<StyleLayer> across threads

11 files changed, 81 insertions, 78 deletions

diff --git a/src/mbgl/annotation/shape_annotation_impl.cpp b/src/mbgl/annotation/shape_annotation_impl.cpp
index 4b60c667aa..62b55908f9 100644
--- a/src/mbgl/annotation/shape_annotation_impl.cpp
+++ b/src/mbgl/annotation/shape_annotation_impl.cpp
@@ -72,6 +72,7 @@ void ShapeAnnotationImpl::updateStyle(Style& style) {
             : ProjectedFeatureType::Polygon;
 
         layer->id = layerID;
+        layer->ref = "";
         layer->source = AnnotationManager::SourceID;
         layer->sourceLayer = layer->id;
         layer->visibility = VisibilityType::Visible;
diff --git a/src/mbgl/map/tile_worker.cpp b/src/mbgl/map/tile_worker.cpp
index f591d138d9..932bc0f689 100644
--- a/src/mbgl/map/tile_worker.cpp
+++ b/src/mbgl/map/tile_worker.cpp
@@ -30,7 +30,7 @@ TileWorker::~TileWorker() {
     glyphAtlas.removeGlyphs(reinterpret_cast<uintptr_t>(this));
 }
 
-TileParseResult TileWorker::parseAllLayers(std::vector<util::ptr<StyleLayer>> layers,
+TileParseResult TileWorker::parseAllLayers(std::vector<std::unique_ptr<StyleLayer>> layers,
                                            const GeometryTile& geometryTile,
                                            PlacementConfig config) {
     // We're doing a fresh parse of the tile, because the underlying data has changed.
@@ -45,10 +45,10 @@ TileParseResult TileWorker::parseAllLayers(std::vector<util::ptr<StyleLayer>> la
     std::set<std::string> parsed;
 
     for (auto i = layers.rbegin(); i != layers.rend(); i++) {
-        const StyleLayer& layer = **i;
-        if (parsed.find(layer.bucketName()) == parsed.end()) {
-            parsed.emplace(layer.bucketName());
-            parseLayer(layer, geometryTile);
+        std::unique_ptr<StyleLayer> layer = std::move(*i);
+        if (parsed.find(layer->bucketName()) == parsed.end()) {
+            parsed.emplace(layer->bucketName());
+            parseLayer(std::move(layer), geometryTile);
         }
     }
 
@@ -60,7 +60,7 @@ TileParseResult TileWorker::parsePendingLayers() {
     // Try parsing the remaining layers that we couldn't parse in the first step due to missing
     // dependencies.
     for (auto it = pending.begin(); it != pending.end();) {
-        auto& layer = it->first;
+        auto& layer = *it->first;
         auto& bucket = it->second;
         assert(bucket);
 
@@ -88,7 +88,7 @@ TileParseResult TileWorker::parsePendingLayers() {
 }
 
 void TileWorker::redoPlacement(
-    std::vector<util::ptr<StyleLayer>> layers,
+    std::vector<std::unique_ptr<StyleLayer>> layers,
     const std::unordered_map<std::string, std::unique_ptr<Bucket>>* buckets,
     PlacementConfig config) {
 
@@ -103,29 +103,29 @@ void TileWorker::redoPlacement(
     }
 }
 
-void TileWorker::parseLayer(const StyleLayer& layer, const GeometryTile& geometryTile) {
+void TileWorker::parseLayer(std::unique_ptr<StyleLayer> layer, const GeometryTile& geometryTile) {
     // Cancel early when parsing.
     if (state == TileData::State::obsolete)
         return;
 
     // Background is a special case.
-    if (layer.type == StyleLayerType::Background)
+    if (layer->type == StyleLayerType::Background)
         return;
 
     // Skip this bucket if we are to not render this
-    if ((layer.source != sourceID) ||
-        (id.z < std::floor(layer.minZoom)) ||
-        (id.z >= std::ceil(layer.maxZoom)) ||
-        (layer.visibility == VisibilityType::None)) {
+    if ((layer->source != sourceID) ||
+        (id.z < std::floor(layer->minZoom)) ||
+        (id.z >= std::ceil(layer->maxZoom)) ||
+        (layer->visibility == VisibilityType::None)) {
         return;
     }
 
-    auto geometryLayer = geometryTile.getLayer(layer.sourceLayer);
+    auto geometryLayer = geometryTile.getLayer(layer->sourceLayer);
     if (!geometryLayer) {
         // The layer specified in the bucket does not exist. Do nothing.
         if (debug::tileParseWarnings) {
             Log::Warning(Event::ParseTile, "layer '%s' does not exist in tile %d/%d/%d",
-                    layer.sourceLayer.c_str(), id.z, id.x, id.y);
+                    layer->sourceLayer.c_str(), id.z, id.x, id.y);
         }
         return;
     }
@@ -140,13 +140,13 @@ void TileWorker::parseLayer(const StyleLayer& layer, const GeometryTile& geometr
                                      glyphStore,
                                      *collisionTile);
 
-    std::unique_ptr<Bucket> bucket = layer.createBucket(parameters);
+    std::unique_ptr<Bucket> bucket = layer->createBucket(parameters);
 
-    if (layer.type == StyleLayerType::Symbol && partialParse) {
+    if (layer->type == StyleLayerType::Symbol && partialParse) {
         // We cannot parse this bucket yet. Instead, we're saving it for later.
-        pending.emplace_back(layer, std::move(bucket));
+        pending.emplace_back(std::move(layer), std::move(bucket));
     } else {
-        insertBucket(layer.bucketName(), std::move(bucket));
+        insertBucket(layer->bucketName(), std::move(bucket));
     }
 }
 
diff --git a/src/mbgl/map/tile_worker.hpp b/src/mbgl/map/tile_worker.hpp
index 110948b169..21e0ec8a32 100644
--- a/src/mbgl/map/tile_worker.hpp
+++ b/src/mbgl/map/tile_worker.hpp
@@ -45,18 +45,18 @@ public:
                const std::atomic<TileData::State>&);
     ~TileWorker();
 
-    TileParseResult parseAllLayers(std::vector<util::ptr<StyleLayer>>,
+    TileParseResult parseAllLayers(std::vector<std::unique_ptr<StyleLayer>>,
                                    const GeometryTile&,
                                    PlacementConfig);
 
     TileParseResult parsePendingLayers();
 
-    void redoPlacement(std::vector<util::ptr<StyleLayer>>,
+    void redoPlacement(std::vector<std::unique_ptr<StyleLayer>>,
                        const std::unordered_map<std::string, std::unique_ptr<Bucket>>*,
                        PlacementConfig);
 
 private:
-    void parseLayer(const StyleLayer&, const GeometryTile&);
+    void parseLayer(std::unique_ptr<StyleLayer>, const GeometryTile&);
     void insertBucket(const std::string& name, std::unique_ptr<Bucket>);
 
     const TileID id;
@@ -73,7 +73,7 @@ private:
 
     // Contains buckets that we couldn't parse so far due to missing resources.
     // They will be attempted on subsequent parses.
-    std::list<std::pair<const StyleLayer&, std::unique_ptr<Bucket>>> pending;
+    std::list<std::pair<std::unique_ptr<StyleLayer>, std::unique_ptr<Bucket>>> pending;
 
     // Temporary holder
     TileParseResultBuckets result;
diff --git a/src/mbgl/map/vector_tile_data.cpp b/src/mbgl/map/vector_tile_data.cpp
index 53bd941be5..fd27614488 100644
--- a/src/mbgl/map/vector_tile_data.cpp
+++ b/src/mbgl/map/vector_tile_data.cpp
@@ -62,7 +62,7 @@ VectorTileData::VectorTileData(const TileID& id_,
         // when tile data changed. Replacing the workdRequest will cancel a pending work
         // request in case there is one.
         workRequest.reset();
-        workRequest = worker.parseGeometryTile(tileWorker, style.layers, std::move(tile), targetConfig, [callback, this, config = targetConfig] (TileParseResult result) {
+        workRequest = worker.parseGeometryTile(tileWorker, style.getLayers(), std::move(tile), targetConfig, [callback, this, config = targetConfig] (TileParseResult result) {
             workRequest.reset();
             if (state == State::obsolete) {
                 return;
@@ -166,7 +166,7 @@ void VectorTileData::redoPlacement(const PlacementConfig newConfig) {
 
 void VectorTileData::redoPlacement() {
     workRequest.reset();
-    workRequest = worker.redoPlacement(tileWorker, style.layers, buckets, targetConfig, [this, config = targetConfig] {
+    workRequest = worker.redoPlacement(tileWorker, style.getLayers(), buckets, targetConfig, [this, config = targetConfig] {
         workRequest.reset();
 
         // Persist the configuration we just placed so that we can later check whether we need to
diff --git a/src/mbgl/style/style.cpp b/src/mbgl/style/style.cpp
index aa514de40c..7faa4b254a 100644
--- a/src/mbgl/style/style.cpp
+++ b/src/mbgl/style/style.cpp
@@ -48,16 +48,16 @@ void Style::setJSON(const std::string& json, const std::string&) {
     StyleParser parser;
     parser.parse(doc);
 
-    for (auto& source : parser.getSources()) {
+    for (auto& source : parser.sources) {
         addSource(std::move(source));
     }
 
-    for (auto& layer : parser.getLayers()) {
+    for (auto& layer : parser.layers) {
         addLayer(std::move(layer));
     }
 
-    glyphStore->setURL(parser.getGlyphURL());
-    spriteStore->setURL(parser.getSpriteURL());
+    glyphStore->setURL(parser.glyphURL);
+    spriteStore->setURL(parser.spriteURL);
 
     loaded = true;
 }
@@ -77,7 +77,16 @@ void Style::addSource(std::unique_ptr<Source> source) {
     sources.emplace_back(std::move(source));
 }
 
-std::vector<util::ptr<StyleLayer>>::const_iterator Style::findLayer(const std::string& id) const {
+std::vector<std::unique_ptr<StyleLayer>> Style::getLayers() const {
+    std::vector<std::unique_ptr<StyleLayer>> result;
+    result.reserve(layers.size());
+    for (const auto& layer : layers) {
+        result.push_back(layer->clone());
+    }
+    return result;
+}
+
+std::vector<std::unique_ptr<StyleLayer>>::const_iterator Style::findLayer(const std::string& id) const {
     return std::find_if(layers.begin(), layers.end(), [&](const auto& layer) {
         return layer->id == id;
     });
@@ -88,7 +97,7 @@ StyleLayer* Style::getLayer(const std::string& id) const {
     return it != layers.end() ? it->get() : nullptr;
 }
 
-void Style::addLayer(util::ptr<StyleLayer> layer) {
+void Style::addLayer(std::unique_ptr<StyleLayer> layer) {
     if (SymbolLayer* symbolLayer = dynamic_cast<SymbolLayer*>(layer.get())) {
         if (!symbolLayer->spriteAtlas) {
             symbolLayer->spriteAtlas = spriteAtlas.get();
@@ -98,7 +107,7 @@ void Style::addLayer(util::ptr<StyleLayer> layer) {
     layers.emplace_back(std::move(layer));
 }
 
-void Style::addLayer(util::ptr<StyleLayer> layer, const std::string& before) {
+void Style::addLayer(std::unique_ptr<StyleLayer> layer, const std::string& before) {
     layers.emplace(findLayer(before), std::move(layer));
 }
 
diff --git a/src/mbgl/style/style.hpp b/src/mbgl/style/style.hpp
index c2cb9e397c..d9cd137767 100644
--- a/src/mbgl/style/style.hpp
+++ b/src/mbgl/style/style.hpp
@@ -65,11 +65,12 @@ public:
     }
 
     Source* getSource(const std::string& id) const;
-    StyleLayer* getLayer(const std::string& id) const;
-
     void addSource(std::unique_ptr<Source>);
-    void addLayer(util::ptr<StyleLayer>);
-    void addLayer(util::ptr<StyleLayer>, const std::string& beforeLayerID);
+
+    std::vector<std::unique_ptr<StyleLayer>> getLayers() const;
+    StyleLayer* getLayer(const std::string& id) const;
+    void addLayer(std::unique_ptr<StyleLayer>);
+    void addLayer(std::unique_ptr<StyleLayer>, const std::string& beforeLayerID);
     void removeLayer(const std::string& layerID);
 
     void dumpDebugLogs() const;
@@ -82,10 +83,10 @@ public:
     std::unique_ptr<LineAtlas> lineAtlas;
 
     std::vector<std::unique_ptr<Source>> sources;
-    std::vector<util::ptr<StyleLayer>> layers;
+    std::vector<std::unique_ptr<StyleLayer>> layers;
 
 private:
-    std::vector<util::ptr<StyleLayer>>::const_iterator findLayer(const std::string& layerID) const;
+    std::vector<std::unique_ptr<StyleLayer>>::const_iterator findLayer(const std::string& layerID) const;
 
     // GlyphStore::Observer implementation.
     void onGlyphRangeLoaded() override;
diff --git a/src/mbgl/style/style_layer.cpp b/src/mbgl/style/style_layer.cpp
index 1b3d1a3c34..75209afcce 100644
--- a/src/mbgl/style/style_layer.cpp
+++ b/src/mbgl/style/style_layer.cpp
@@ -36,6 +36,8 @@ bool StyleLayer::hasRenderPass(RenderPass pass) const {
 }
 
 void StyleLayer::copy(const StyleLayer& src) {
+    id = src.id;
+    ref = src.ref;
     type = src.type;
     source = src.source;
     sourceLayer = src.sourceLayer;
diff --git a/src/mbgl/style/style_parser.cpp b/src/mbgl/style/style_parser.cpp
index dc14827dc6..dd2f6e0f73 100644
--- a/src/mbgl/style/style_parser.cpp
+++ b/src/mbgl/style/style_parser.cpp
@@ -1,5 +1,4 @@
 #include <mbgl/style/style_parser.hpp>
-#include <mbgl/style/style_layer.hpp>
 
 #include <mbgl/platform/log.hpp>
 
@@ -7,9 +6,11 @@
 
 namespace mbgl {
 
+StyleParser::~StyleParser() = default;
+
 void StyleParser::parse(const JSVal& document) {
     if (document.HasMember("version")) {
-        version = document["version"].GetInt();
+        int version = document["version"].GetInt();
         if (version != 8) {
             Log::Warning(Event::ParseStyle, "current renderer implementation only supports style spec version 8; using an outdated style will cause rendering errors");
         }
@@ -134,7 +135,7 @@ void StyleParser::parseLayers(const JSVal& value) {
             continue;
         }
 
-        layersMap.emplace(layerID, std::pair<const JSVal&, util::ptr<StyleLayer>> { layerValue, nullptr });
+        layersMap.emplace(layerID, std::pair<const JSVal&, std::unique_ptr<StyleLayer>> { layerValue, nullptr });
         ids.push_back(layerID);
     }
 
@@ -144,14 +145,18 @@ void StyleParser::parseLayers(const JSVal& value) {
         parseLayer(it->first,
                    it->second.first,
                    it->second.second);
+    }
+
+    for (const auto& id : ids) {
+        auto it = layersMap.find(id);
 
         if (it->second.second) {
-            layers.emplace_back(it->second.second);
+            layers.emplace_back(std::move(it->second.second));
         }
     }
 }
 
-void StyleParser::parseLayer(const std::string& id, const JSVal& value, util::ptr<StyleLayer>& layer) {
+void StyleParser::parseLayer(const std::string& id, const JSVal& value, std::unique_ptr<StyleLayer>& layer) {
     if (layer) {
         // Skip parsing this again. We already have a valid layer definition.
         return;
@@ -185,7 +190,7 @@ void StyleParser::parseLayer(const std::string& id, const JSVal& value, util::pt
                    it->second.second);
         stack.pop_front();
 
-        util::ptr<StyleLayer> reference = it->second.second;
+        StyleLayer* reference = it->second.second.get();
         if (!reference) {
             return;
         }
diff --git a/src/mbgl/style/style_parser.hpp b/src/mbgl/style/style_parser.hpp
index 489c9959f2..5ee2415abd 100644
--- a/src/mbgl/style/style_parser.hpp
+++ b/src/mbgl/style/style_parser.hpp
@@ -1,8 +1,8 @@
 #ifndef MBGL_STYLE_STYLE_PARSER
 #define MBGL_STYLE_STYLE_PARSER
 
+#include <mbgl/style/style_layer.hpp>
 #include <mbgl/map/source.hpp>
-#include <mbgl/util/ptr.hpp>
 
 #include <rapidjson/document.h>
 
@@ -21,43 +21,27 @@ using JSVal = rapidjson::Value;
 
 class StyleParser {
 public:
-    void parse(const JSVal&);
-
-    std::vector<std::unique_ptr<Source>>&& getSources() {
-        return std::move(sources);
-    }
+    ~StyleParser();
 
-    std::vector<util::ptr<StyleLayer>> getLayers() {
-        return layers;
-    }
+    void parse(const JSVal&);
 
-    std::string getSpriteURL() const {
-        return spriteURL;
-    }
+    std::string spriteURL;
+    std::string glyphURL;
 
-    std::string getGlyphURL() const {
-        return glyphURL;
-    }
+    std::vector<std::unique_ptr<Source>> sources;
+    std::vector<std::unique_ptr<StyleLayer>> layers;
 
 private:
     void parseSources(const JSVal&);
     void parseLayers(const JSVal&);
-    void parseLayer(const std::string& id, const JSVal&, util::ptr<StyleLayer>&);
+    void parseLayer(const std::string& id, const JSVal&, std::unique_ptr<StyleLayer>&);
     void parseVisibility(StyleLayer&, const JSVal& value);
 
-    std::uint8_t version;
-
-    std::vector<std::unique_ptr<Source>> sources;
-    std::vector<util::ptr<StyleLayer>> layers;
-
     std::unordered_map<std::string, const Source*> sourcesMap;
-    std::unordered_map<std::string, std::pair<const JSVal&, util::ptr<StyleLayer>>> layersMap;
+    std::unordered_map<std::string, std::pair<const JSVal&, std::unique_ptr<StyleLayer>>> layersMap;
 
     // Store a stack of layer IDs we're parsing right now. This is to prevent reference cycles.
     std::forward_list<std::string> stack;
-
-    std::string spriteURL;
-    std::string glyphURL;
 };
 
 }
diff --git a/src/mbgl/util/worker.cpp b/src/mbgl/util/worker.cpp
index 00bdb5baee..054f1df242 100644
--- a/src/mbgl/util/worker.cpp
+++ b/src/mbgl/util/worker.cpp
@@ -4,6 +4,7 @@
 #include <mbgl/platform/platform.hpp>
 #include <mbgl/renderer/raster_bucket.hpp>
 #include <mbgl/map/geometry_tile.hpp>
+#include <mbgl/style/style_layer.hpp>
 
 #include <cassert>
 #include <future>
@@ -33,12 +34,12 @@ public:
     }
 
     void parseGeometryTile(TileWorker* worker,
-                           std::vector<util::ptr<StyleLayer>> layers,
+                           std::vector<std::unique_ptr<StyleLayer>> layers,
                            std::unique_ptr<GeometryTile> tile,
                            PlacementConfig config,
                            std::function<void(TileParseResult)> callback) {
         try {
-            callback(worker->parseAllLayers(layers, *tile, config));
+            callback(worker->parseAllLayers(std::move(layers), *tile, config));
         } catch (const std::exception& ex) {
             callback(TileParseResult(ex.what()));
         }
@@ -54,11 +55,11 @@ public:
     }
 
     void redoPlacement(TileWorker* worker,
-                       std::vector<util::ptr<StyleLayer>> layers,
+                       std::vector<std::unique_ptr<StyleLayer>> layers,
                        const std::unordered_map<std::string, std::unique_ptr<Bucket>>* buckets,
                        PlacementConfig config,
                        std::function<void()> callback) {
-        worker->redoPlacement(layers, buckets, config);
+        worker->redoPlacement(std::move(layers), buckets, config);
         callback();
     }
 };
@@ -83,7 +84,7 @@ Worker::parseRasterTile(std::unique_ptr<RasterBucket> bucket,
 
 std::unique_ptr<WorkRequest>
 Worker::parseGeometryTile(TileWorker& worker,
-                          std::vector<util::ptr<StyleLayer>> layers,
+                          std::vector<std::unique_ptr<StyleLayer>> layers,
                           std::unique_ptr<GeometryTile> tile,
                           PlacementConfig config,
                           std::function<void(TileParseResult)> callback) {
@@ -102,13 +103,13 @@ Worker::parsePendingGeometryTileLayers(TileWorker& worker,
 
 std::unique_ptr<WorkRequest>
 Worker::redoPlacement(TileWorker& worker,
-                      std::vector<util::ptr<StyleLayer>> layers,
+                      std::vector<std::unique_ptr<StyleLayer>> layers,
                       const std::unordered_map<std::string, std::unique_ptr<Bucket>>& buckets,
                       PlacementConfig config,
                       std::function<void()> callback) {
     current = (current + 1) % threads.size();
     return threads[current]->invokeWithCallback(&Worker::Impl::redoPlacement, callback, &worker,
-                                                layers, &buckets, config);
+                                                std::move(layers), &buckets, config);
 }
 
 } // end namespace mbgl
diff --git a/src/mbgl/util/worker.hpp b/src/mbgl/util/worker.hpp
index f0f291e24c..cd89f0e964 100644
--- a/src/mbgl/util/worker.hpp
+++ b/src/mbgl/util/worker.hpp
@@ -40,7 +40,7 @@ public:
                             std::function<void(RasterTileParseResult)> callback);
 
     Request parseGeometryTile(TileWorker&,
-                              std::vector<util::ptr<StyleLayer>>,
+                              std::vector<std::unique_ptr<StyleLayer>>,
                               std::unique_ptr<GeometryTile>,
                               PlacementConfig,
                               std::function<void(TileParseResult)> callback);
@@ -49,7 +49,7 @@ public:
                                            std::function<void(TileParseResult)> callback);
 
     Request redoPlacement(TileWorker&,
-                          std::vector<util::ptr<StyleLayer>>,
+                          std::vector<std::unique_ptr<StyleLayer>>,
                           const std::unordered_map<std::string, std::unique_ptr<Bucket>>&,
                           PlacementConfig config,
                           std::function<void()> callback);