External synchronization for AnnotationManager

6 files changed, 23 insertions, 32 deletions

diff --git a/src/mbgl/map/annotation.cpp b/src/mbgl/map/annotation.cpp
index 5b857e9b59..a0f65cd7b7 100644
--- a/src/mbgl/map/annotation.cpp
+++ b/src/mbgl/map/annotation.cpp
@@ -49,17 +49,14 @@ AnnotationManager::~AnnotationManager() {
 }
 
 void AnnotationManager::markStaleTiles(std::unordered_set<TileID, TileID::Hash> ids) {
-    std::lock_guard<std::mutex> lock(mtx);
     std::copy(ids.begin(), ids.end(), std::inserter(staleTiles, staleTiles.begin()));
 }
 
 std::unordered_set<TileID, TileID::Hash> AnnotationManager::resetStaleTiles() {
-    std::lock_guard<std::mutex> lock(mtx);
     return std::move(staleTiles);
 }
 
 void AnnotationManager::setDefaultPointAnnotationSymbol(const std::string& symbol) {
-    std::lock_guard<std::mutex> lock(mtx);
     defaultPointAnnotationSymbol = symbol;
 }
 
@@ -261,8 +258,6 @@ AnnotationManager::addTileFeature(const uint32_t annotationID,
 std::pair<std::unordered_set<TileID, TileID::Hash>, AnnotationIDs>
 AnnotationManager::addPointAnnotations(const std::vector<PointAnnotation>& points,
                                        const uint8_t maxZoom) {
-    std::lock_guard<std::mutex> lock(mtx);
-
     // We pre-generate tiles to contain each annotation up to the map's max zoom.
     // We do this for fast rendering without projection conversions on the fly, as well as
     // to simplify bounding box queries of annotations later. Tiles get invalidated when
@@ -310,8 +305,6 @@ AnnotationManager::addPointAnnotations(const std::vector<PointAnnotation>& point
 std::pair<std::unordered_set<TileID, TileID::Hash>, AnnotationIDs>
 AnnotationManager::addShapeAnnotations(const std::vector<ShapeAnnotation>& shapes,
                                        const uint8_t maxZoom) {
-    std::lock_guard<std::mutex> lock(mtx);
-
     // We pre-generate tiles to contain each annotation up to the map's max zoom.
     // We do this for fast rendering without projection conversions on the fly, as well as
     // to simplify bounding box queries of annotations later. Tiles get invalidated when
@@ -348,8 +341,6 @@ AnnotationManager::addShapeAnnotations(const std::vector<ShapeAnnotation>& shape
 
 std::unordered_set<TileID, TileID::Hash> AnnotationManager::removeAnnotations(const AnnotationIDs& ids,
                                                                               const uint8_t maxZoom) {
-    std::lock_guard<std::mutex> lock(mtx);
-
     std::unordered_set<TileID, TileID::Hash> affectedTiles;
 
     std::vector<uint32_t> z2s;
@@ -416,8 +407,6 @@ std::unordered_set<TileID, TileID::Hash> AnnotationManager::removeAnnotations(co
 }
 
 const StyleProperties AnnotationManager::getAnnotationStyleProperties(uint32_t annotationID) const {
-    std::lock_guard<std::mutex> lock(mtx);
-
     auto anno_it = annotations.find(annotationID);
     assert(anno_it != annotations.end());
 
@@ -427,8 +416,6 @@ const StyleProperties AnnotationManager::getAnnotationStyleProperties(uint32_t a
 AnnotationIDs AnnotationManager::getAnnotationsInBounds(const LatLngBounds& queryBounds,
                                                         const uint8_t maxZoom,
                                                         const AnnotationType& type) const {
-    std::lock_guard<std::mutex> lock(mtx);
-
     const uint8_t z = maxZoom;
     const uint32_t z2 = 1 << z;
     const vec2<double> swPoint = projectPoint(queryBounds.sw);
@@ -493,8 +480,6 @@ AnnotationIDs AnnotationManager::getAnnotationsInBounds(const LatLngBounds& quer
 }
 
 LatLngBounds AnnotationManager::getBoundsForAnnotations(const AnnotationIDs& ids) const {
-    std::lock_guard<std::mutex> lock(mtx);
-
     LatLngBounds bounds;
     for (auto id : ids) {
         const auto annotation_it = annotations.find(id);
@@ -507,8 +492,6 @@ LatLngBounds AnnotationManager::getBoundsForAnnotations(const AnnotationIDs& ids
 }
 
 const LiveTile* AnnotationManager::getTile(const TileID& id) {
-    std::lock_guard<std::mutex> lock(mtx);
-
     // look up any existing annotation tile
     LiveTile *renderTile = nullptr;
     const auto tile_lookup_it = tiles.find(id);
diff --git a/src/mbgl/map/annotation.hpp b/src/mbgl/map/annotation.hpp
index e75c57f1f0..745fc779cd 100644
--- a/src/mbgl/map/annotation.hpp
+++ b/src/mbgl/map/annotation.hpp
@@ -13,7 +13,6 @@
 
 #include <string>
 #include <vector>
-#include <mutex>
 #include <memory>
 #include <unordered_map>
 #include <unordered_set>
@@ -90,7 +89,6 @@ private:
         const uint8_t maxZoom);
 
 private:
-    mutable std::mutex mtx;
     std::string defaultPointAnnotationSymbol;
     std::unordered_map<uint32_t, std::unique_ptr<Annotation>> annotations;
     std::vector<uint32_t> orderedShapeAnnotations;
diff --git a/src/mbgl/map/map.cpp b/src/mbgl/map/map.cpp
index 91017d89c8..94d3e57d41 100644
--- a/src/mbgl/map/map.cpp
+++ b/src/mbgl/map/map.cpp
@@ -304,7 +304,7 @@ const LatLng Map::latLngForPixel(const vec2<double> pixel) const {
 #pragma mark - Annotations
 
 void Map::setDefaultPointAnnotationSymbol(const std::string& symbol) {
-    data->annotationManager.setDefaultPointAnnotationSymbol(symbol);
+    data->getAnnotationManager()->setDefaultPointAnnotationSymbol(symbol);
 }
 
 double Map::getTopOffsetPixelsForAnnotationSymbol(const std::string& symbol) {
@@ -316,7 +316,7 @@ uint32_t Map::addPointAnnotation(const PointAnnotation& annotation) {
 }
 
 AnnotationIDs Map::addPointAnnotations(const std::vector<PointAnnotation>& annotations) {
-    auto result = data->annotationManager.addPointAnnotations(annotations, getMaxZoom());
+    auto result = data->getAnnotationManager()->addPointAnnotations(annotations, getMaxZoom());
     context->invoke(&MapContext::updateAnnotationTiles, result.first);
     return result.second;
 }
@@ -326,7 +326,7 @@ uint32_t Map::addShapeAnnotation(const ShapeAnnotation& annotation) {
 }
 
 AnnotationIDs Map::addShapeAnnotations(const std::vector<ShapeAnnotation>& annotations) {
-    auto result = data->annotationManager.addShapeAnnotations(annotations, getMaxZoom());
+    auto result = data->getAnnotationManager()->addShapeAnnotations(annotations, getMaxZoom());
     context->invoke(&MapContext::updateAnnotationTiles, result.first);
     return result.second;
 }
@@ -336,16 +336,16 @@ void Map::removeAnnotation(uint32_t annotation) {
 }
 
 void Map::removeAnnotations(const std::vector<uint32_t>& annotations) {
-    auto result = data->annotationManager.removeAnnotations(annotations, getMaxZoom());
+    auto result = data->getAnnotationManager()->removeAnnotations(annotations, getMaxZoom());
     context->invoke(&MapContext::updateAnnotationTiles, result);
 }
 
 std::vector<uint32_t> Map::getAnnotationsInBounds(const LatLngBounds& bounds, const AnnotationType& type) {
-    return data->annotationManager.getAnnotationsInBounds(bounds, getMaxZoom(), type);
+    return data->getAnnotationManager()->getAnnotationsInBounds(bounds, getMaxZoom(), type);
 }
 
 LatLngBounds Map::getBoundsForAnnotations(const std::vector<uint32_t>& annotations) {
-    return data->annotationManager.getBoundsForAnnotations(annotations);
+    return data->getAnnotationManager()->getBoundsForAnnotations(annotations);
 }
 
 
diff --git a/src/mbgl/map/map_context.cpp b/src/mbgl/map/map_context.cpp
index fa06a42672..fc4766f3a5 100644
--- a/src/mbgl/map/map_context.cpp
+++ b/src/mbgl/map/map_context.cpp
@@ -136,7 +136,7 @@ void MapContext::loadStyleJSON(const std::string& json, const std::string& base)
     updated |= static_cast<UpdateType>(Update::Zoom);
     asyncUpdate->send();
 
-    auto staleTiles = data.annotationManager.resetStaleTiles();
+    auto staleTiles = data.getAnnotationManager()->resetStaleTiles();
     if (staleTiles.size()) {
         updateAnnotationTiles(staleTiles);
     }
@@ -145,7 +145,8 @@ void MapContext::loadStyleJSON(const std::string& json, const std::string& base)
 void MapContext::updateAnnotationTiles(const std::unordered_set<TileID, TileID::Hash>& ids) {
     assert(util::ThreadContext::currentlyOn(util::ThreadType::Map));
 
-    data.annotationManager.markStaleTiles(ids);
+    util::exclusive<AnnotationManager> annotationManager = data.getAnnotationManager();
+    annotationManager->markStaleTiles(ids);
 
     if (!style) return;
 
@@ -154,7 +155,7 @@ void MapContext::updateAnnotationTiles(const std::unordered_set<TileID, TileID::
     style->getSource(shapeID)->enabled = true;
 
     // create (if necessary) layers and buckets for each shape
-    for (const auto &shapeAnnotationID : data.annotationManager.getOrderedShapeAnnotations()) {
+    for (const auto &shapeAnnotationID : annotationManager->getOrderedShapeAnnotations()) {
         const std::string shapeLayerID = shapeID + "." + std::to_string(shapeAnnotationID);
 
         const auto layer_it = std::find_if(style->layers.begin(), style->layers.end(),
@@ -164,7 +165,7 @@ void MapContext::updateAnnotationTiles(const std::unordered_set<TileID, TileID::
 
         if (layer_it == style->layers.end()) {
             // query shape styling
-            auto& shapeStyle = data.annotationManager.getAnnotationStyleProperties(shapeAnnotationID);
+            auto& shapeStyle = annotationManager->getAnnotationStyleProperties(shapeAnnotationID);
 
             // apply shape paint properties
             ClassProperties paintProperties;
@@ -233,7 +234,7 @@ void MapContext::updateAnnotationTiles(const std::unordered_set<TileID, TileID::
     updated |= static_cast<UpdateType>(Update::Classes);
     asyncUpdate->send();
 
-    data.annotationManager.resetStaleTiles();
+    annotationManager->resetStaleTiles();
 }
 
 void MapContext::cascadeClasses() {
diff --git a/src/mbgl/map/map_data.hpp b/src/mbgl/map/map_data.hpp
index a1ee3c06dc..7e5955d744 100644
--- a/src/mbgl/map/map_data.hpp
+++ b/src/mbgl/map/map_data.hpp
@@ -12,6 +12,7 @@
 
 #include <mbgl/map/mode.hpp>
 #include <mbgl/map/annotation.hpp>
+#include <mbgl/util/exclusive.hpp>
 
 namespace mbgl {
 
@@ -78,11 +79,19 @@ public:
         defaultTransitionDuration = duration;
     };
 
+    util::exclusive<AnnotationManager> getAnnotationManager() {
+        return util::exclusive<AnnotationManager>(
+            &annotationManager,
+            std::make_unique<std::lock_guard<std::mutex>>(annotationManagerMutex));
+    }
+
 public:
-    AnnotationManager annotationManager;
     const MapMode mode;
 
 private:
+    mutable std::mutex annotationManagerMutex;
+    AnnotationManager annotationManager;
+
     mutable std::mutex mtx;
 
     std::vector<std::string> classes;
diff --git a/src/mbgl/map/source.cpp b/src/mbgl/map/source.cpp
index f596296fc3..58f3924ecd 100644
--- a/src/mbgl/map/source.cpp
+++ b/src/mbgl/map/source.cpp
@@ -300,7 +300,7 @@ TileData::State Source::addTile(MapData& data,
             new_tile.data = tileData;
         } else if (info.type == SourceType::Annotations) {
             new_tile.data = std::make_shared<LiveTileData>(normalized_id,
-                data.annotationManager.getTile(normalized_id), style, info, callback);
+                data.getAnnotationManager()->getTile(normalized_id), style, info, callback);
         } else {
             throw std::runtime_error("source type not implemented");
         }