1 files changed, 71 insertions, 46 deletions

diff --git a/src/mbgl/annotation/annotation_manager.cpp b/src/mbgl/annotation/annotation_manager.cpp
index 88153f5fb7..a69dba1bf2 100644
--- a/src/mbgl/annotation/annotation_manager.cpp
+++ b/src/mbgl/annotation/annotation_manager.cpp
@@ -4,8 +4,7 @@
 #include <mbgl/annotation/symbol_annotation_impl.hpp>
 #include <mbgl/annotation/line_annotation_impl.hpp>
 #include <mbgl/annotation/fill_annotation_impl.hpp>
-#include <mbgl/annotation/style_sourced_annotation_impl.hpp>
-#include <mbgl/style/style.hpp>
+#include <mbgl/style/style_impl.hpp>
 #include <mbgl/style/layers/symbol_layer.hpp>
 #include <mbgl/style/layers/symbol_layer_impl.hpp>
 #include <mbgl/storage/file_source.hpp>
@@ -19,16 +18,11 @@ using namespace style;
 const std::string AnnotationManager::SourceID = "com.mapbox.annotations";
 const std::string AnnotationManager::PointLayerID = "com.mapbox.annotations.points";
 
-AnnotationManager::AnnotationManager(float pixelRatio)
-    : spriteAtlas({ 1024, 1024 }, pixelRatio) {
-    // This is a special atlas, holding only images added via addIcon, so we always treat it as
-    // loaded.
-    spriteAtlas.markAsLoaded();
-}
-
+AnnotationManager::AnnotationManager() = default;
 AnnotationManager::~AnnotationManager() = default;
 
 AnnotationID AnnotationManager::addAnnotation(const Annotation& annotation, const uint8_t maxZoom) {
+    std::lock_guard<std::mutex> lock(mutex);
     AnnotationID id = nextID++;
     Annotation::visit(annotation, [&] (const auto& annotation_) {
         this->add(id, annotation_, maxZoom);
@@ -37,21 +31,15 @@ AnnotationID AnnotationManager::addAnnotation(const Annotation& annotation, cons
 }
 
 Update AnnotationManager::updateAnnotation(const AnnotationID& id, const Annotation& annotation, const uint8_t maxZoom) {
+    std::lock_guard<std::mutex> lock(mutex);
     return Annotation::visit(annotation, [&] (const auto& annotation_) {
         return this->update(id, annotation_, maxZoom);
     });
 }
 
 void AnnotationManager::removeAnnotation(const AnnotationID& id) {
-    if (symbolAnnotations.find(id) != symbolAnnotations.end()) {
-        symbolTree.remove(symbolAnnotations.at(id));
-        symbolAnnotations.erase(id);
-    } else if (shapeAnnotations.find(id) != shapeAnnotations.end()) {
-        obsoleteShapeAnnotationLayers.insert(shapeAnnotations.at(id)->layerID);
-        shapeAnnotations.erase(id);
-    } else {
-        assert(false); // Should never happen
-    }
+    std::lock_guard<std::mutex> lock(mutex);
+    remove(id);
 }
 
 void AnnotationManager::add(const AnnotationID& id, const SymbolAnnotation& annotation, const uint8_t) {
@@ -72,12 +60,6 @@ void AnnotationManager::add(const AnnotationID& id, const FillAnnotation& annota
     obsoleteShapeAnnotationLayers.erase(impl.layerID);
 }
 
-void AnnotationManager::add(const AnnotationID& id, const StyleSourcedAnnotation& annotation, const uint8_t maxZoom) {
-    ShapeAnnotationImpl& impl = *shapeAnnotations.emplace(id,
-        std::make_unique<StyleSourcedAnnotationImpl>(id, annotation, maxZoom)).first->second;
-    obsoleteShapeAnnotationLayers.erase(impl.layerID);
-}
-
 Update AnnotationManager::update(const AnnotationID& id, const SymbolAnnotation& annotation, const uint8_t maxZoom) {
     Update result = Update::Nothing;
 
@@ -110,6 +92,7 @@ Update AnnotationManager::update(const AnnotationID& id, const LineAnnotation& a
         assert(false); // Attempt to update a non-existent shape annotation
         return Update::Nothing;
     }
+
     removeAndAdd(id, annotation, maxZoom);
     return Update::AnnotationData | Update::AnnotationStyle;
 }
@@ -120,40 +103,43 @@ Update AnnotationManager::update(const AnnotationID& id, const FillAnnotation& a
         assert(false); // Attempt to update a non-existent shape annotation
         return Update::Nothing;
     }
-    removeAndAdd(id, annotation, maxZoom);
-    return Update::AnnotationData | Update::AnnotationStyle;
-}
 
-Update AnnotationManager::update(const AnnotationID& id, const StyleSourcedAnnotation& annotation, const uint8_t maxZoom) {
-    auto it = shapeAnnotations.find(id);
-    if (it == shapeAnnotations.end()) {
-        assert(false); // Attempt to update a non-existent shape annotation
-        return Update::Nothing;
-    }
     removeAndAdd(id, annotation, maxZoom);
     return Update::AnnotationData | Update::AnnotationStyle;
 }
 
 void AnnotationManager::removeAndAdd(const AnnotationID& id, const Annotation& annotation, const uint8_t maxZoom) {
-    removeAnnotation(id);
+    remove(id);
     Annotation::visit(annotation, [&] (const auto& annotation_) {
         this->add(id, annotation_, maxZoom);
     });
 }
 
+void AnnotationManager::remove(const AnnotationID& id) {
+    if (symbolAnnotations.find(id) != symbolAnnotations.end()) {
+        symbolTree.remove(symbolAnnotations.at(id));
+        symbolAnnotations.erase(id);
+    } else if (shapeAnnotations.find(id) != shapeAnnotations.end()) {
+        obsoleteShapeAnnotationLayers.insert(shapeAnnotations.at(id)->layerID);
+        shapeAnnotations.erase(id);
+    } else {
+        assert(false); // Should never happen
+    }
+}
+
 std::unique_ptr<AnnotationTileData> AnnotationManager::getTileData(const CanonicalTileID& tileID) {
     if (symbolAnnotations.empty() && shapeAnnotations.empty())
         return nullptr;
 
     auto tileData = std::make_unique<AnnotationTileData>();
 
-    AnnotationTileLayer& pointLayer = tileData->layers.emplace(PointLayerID, PointLayerID).first->second;
+    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) {
@@ -163,60 +149,99 @@ std::unique_ptr<AnnotationTileData> AnnotationManager::getTileData(const Canonic
     return tileData;
 }
 
-void AnnotationManager::updateStyle(Style& style) {
-    // Create annotation source, point layer, and point bucket
+void AnnotationManager::updateStyle(Style::Impl& style) {
+    // Create annotation source, point layer, and point bucket. We do everything via Style::Impl
+    // because we don't want annotation mutations to trigger Style::Impl::styleMutated to be set.
     if (!style.getSource(SourceID)) {
         style.addSource(std::make_unique<AnnotationSource>());
 
         std::unique_ptr<SymbolLayer> layer = std::make_unique<SymbolLayer>(PointLayerID, SourceID);
 
         layer->setSourceLayer(PointLayerID);
-        layer->setIconImage({"{sprite}"});
+        layer->setIconImage({SourceID + ".{sprite}"});
         layer->setIconAllowOverlap(true);
         layer->setIconIgnorePlacement(true);
 
         style.addLayer(std::move(layer));
     }
 
+    std::lock_guard<std::mutex> lock(mutex);
+
     for (const auto& shape : shapeAnnotations) {
         shape.second->updateStyle(style);
     }
 
+    for (const auto& image : images) {
+        // Call addImage even for images we may have previously added, because we must support
+        // addAnnotationImage being used to update an existing image. Creating a new image is
+        // relatively cheap, as it copies only the Immutable reference. (We can't keep track
+        // of which images need to be added because we don't know if the style is the same
+        // instance as in the last updateStyle call. If it's a new style, we need to add all
+        // images.)
+        style.addImage(std::make_unique<style::Image>(image.second));
+    }
+
     for (const auto& layer : obsoleteShapeAnnotationLayers) {
         if (style.getLayer(layer)) {
             style.removeLayer(layer);
         }
     }
 
+    for (const auto& image : obsoleteImages) {
+        if (style.getImage(image)) {
+            style.removeImage(image);
+        }
+    }
+
     obsoleteShapeAnnotationLayers.clear();
+    obsoleteImages.clear();
 }
 
 void AnnotationManager::updateData() {
+    std::lock_guard<std::mutex> lock(mutex);
     for (auto& tile : tiles) {
         tile->setData(getTileData(tile->id.canonical));
     }
 }
 
 void AnnotationManager::addTile(AnnotationTile& tile) {
+    std::lock_guard<std::mutex> lock(mutex);
     tiles.insert(&tile);
     tile.setData(getTileData(tile.id.canonical));
 }
 
 void AnnotationManager::removeTile(AnnotationTile& tile) {
+    std::lock_guard<std::mutex> lock(mutex);
     tiles.erase(&tile);
 }
 
-void AnnotationManager::addImage(const std::string& id, std::unique_ptr<style::Image> image) {
-    spriteAtlas.addImage(id, std::move(image));
+// To ensure that annotation images do not collide with images from the style,
+// we prefix input image IDs with "com.mapbox.annotations".
+static std::string prefixedImageID(const std::string& id) {
+    return AnnotationManager::SourceID + "." + id;
+}
+
+void AnnotationManager::addImage(std::unique_ptr<style::Image> image) {
+    std::lock_guard<std::mutex> lock(mutex);
+    const std::string id = prefixedImageID(image->getID());
+    images.erase(id);
+    images.emplace(id,
+        style::Image(id, image->getImage().clone(), image->getPixelRatio(), image->isSdf()));
+    obsoleteImages.erase(id);
 }
 
-void AnnotationManager::removeImage(const std::string& id) {
-    spriteAtlas.removeImage(id);
+void AnnotationManager::removeImage(const std::string& id_) {
+    std::lock_guard<std::mutex> lock(mutex);
+    const std::string id = prefixedImageID(id_);
+    images.erase(id);
+    obsoleteImages.insert(id);
 }
 
-double AnnotationManager::getTopOffsetPixelsForImage(const std::string& id) {
-    const style::Image* image = spriteAtlas.getImage(id);
-    return image ? -(image->image.size.height / image->pixelRatio) / 2 : 0;
+double AnnotationManager::getTopOffsetPixelsForImage(const std::string& id_) {
+    std::lock_guard<std::mutex> lock(mutex);
+    const std::string id = prefixedImageID(id_);
+    auto it = images.find(id);
+    return it != images.end() ? -(it->second.getImage().size.height / it->second.getPixelRatio()) / 2 : 0;
 }
 
 } // namespace mbgl