[core] Remove shape layers from the style (fixes #2405)

src/mbgl/annotation/annotation_manager.cpp

@@ -51,7 +51,7 @@ void AnnotationManager::removeAnnotations(const AnnotationIDs& ids) {
             pointTree.remove(pointAnnotations.at(id));
             pointAnnotations.erase(id);
         } else if (shapeAnnotations.find(id) != shapeAnnotations.end()) {
-            // TODO: remove style layer
+            obsoleteShapeAnnotationLayers.push_back(shapeAnnotations.at(id)->layerID);
             shapeAnnotations.erase(id);
         }
     }
@@ -132,6 +132,11 @@ void AnnotationManager::updateStyle(Style& style) {
         shape.second->updateStyle(style);
     }
 
+    for (const auto& layer : obsoleteShapeAnnotationLayers) {
+        style.removeLayer(layer);
+    }
+
+    obsoleteShapeAnnotationLayers.clear();
     style.getSource(SourceID)->invalidateTiles();
 }
 

src/mbgl/annotation/annotation_manager.hpp

@@ -40,6 +40,7 @@ private:
     PointAnnotationImpl::Tree pointTree;
     PointAnnotationImpl::Map pointAnnotations;
     ShapeAnnotationImpl::Map shapeAnnotations;
+    std::vector<std::string> obsoleteShapeAnnotationLayers;
 };
 
 }

src/mbgl/annotation/shape_annotation_impl.hpp

@@ -24,10 +24,11 @@ public:
     void updateStyle(Style&);
     void updateTile(const TileID&, AnnotationTile&);
 
-private:
     const AnnotationID id;
     const std::string layerID;
     const ShapeAnnotation shape;
+
+private:
     mapbox::util::geojsonvt::GeoJSONVT shapeTiler;
 };
 

src/mbgl/style/style.cpp

@@ -77,12 +77,30 @@ 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 {
+    return std::find_if(layers.begin(), layers.end(), [&](const auto& layer) {
+        return layer->id == id;
+    });
+}
+
+StyleLayer* Style::getLayer(const std::string& id) const {
+    auto it = findLayer(id);
+    return it != layers.end() ? it->get() : nullptr;
+}
+
 void Style::addLayer(util::ptr<StyleLayer> layer) {
     layers.emplace_back(std::move(layer));
 }
 
 void Style::addLayer(util::ptr<StyleLayer> layer, const std::string& before) {
-    layers.emplace(std::find_if(layers.begin(), layers.end(), [&](const auto& l) { return l->id == before; }), std::move(layer));
+    layers.emplace(findLayer(before), std::move(layer));
+}
+
+void Style::removeLayer(const std::string& id) {
+    auto it = findLayer(id);
+    if (it == layers.end())
+        throw std::runtime_error("no such layer");
+    layers.erase(it);
 }
 
 void Style::update(const TransformState& transform,
@@ -141,14 +159,6 @@ Source* Style::getSource(const std::string& id) const {
     return it != sources.end() ? it->get() : nullptr;
 }
 
-StyleLayer* Style::getLayer(const std::string& id) const {
-    const auto it = std::find_if(layers.begin(), layers.end(), [&](const auto& layer) {
-        return layer->id == id;
-    });
-
-    return it != layers.end() ? it->get() : nullptr;
-}
-
 bool Style::hasTransitions() const {
     for (const auto& layer : layers) {
         if (layer->hasTransitions()) {

src/mbgl/style/style.hpp

@@ -67,6 +67,7 @@ public:
     void addSource(std::unique_ptr<Source>);
     void addLayer(util::ptr<StyleLayer>);
     void addLayer(util::ptr<StyleLayer>, const std::string& beforeLayerID);
+    void removeLayer(const std::string& layerID);
 
     MapData& data;
     std::unique_ptr<GlyphStore> glyphStore;
@@ -80,6 +81,8 @@ public:
     std::vector<util::ptr<StyleLayer>> layers;
 
 private:
+    std::vector<util::ptr<StyleLayer>>::const_iterator findLayer(const std::string& layerID) const;
+
     // GlyphStore::Observer implementation.
     void onGlyphRangeLoaded() override;
     void onGlyphRangeLoadingFailed(std::exception_ptr error) override;

test/api/annotations.cpp

@@ -120,7 +120,7 @@ TEST(Annotations, NonImmediateAdd) {
     util::write_file("test/output/non_immediate_add.png", renderPNG(map));
 }
 
-TEST(Annotations, Remove) {
+TEST(Annotations, RemovePoint) {
     auto display = std::make_shared<mbgl::HeadlessDisplay>();
     HeadlessView view(display, 1);
     DefaultFileSource fileSource(nullptr);
@@ -133,7 +133,32 @@ TEST(Annotations, Remove) {
 
     map.removeAnnotation(point);
 
-    util::write_file("test/output/remove.png", renderPNG(map));
+    util::write_file("test/output/remove_point.png", renderPNG(map));
+}
+
+TEST(Annotations, RemoveShape) {
+    auto display = std::make_shared<mbgl::HeadlessDisplay>();
+    HeadlessView view(display, 1);
+    DefaultFileSource fileSource(nullptr);
+
+    AnnotationSegments segments = {{ {{ { 0, 0 }, { 45, 45 } }} }};
+
+    LineProperties lineProperties;
+    lineProperties.color = {{ 255, 0, 0, 1 }};
+    lineProperties.width = 5;
+
+    StyleProperties styleProperties;
+    styleProperties.set<LineProperties>(lineProperties);
+
+    Map map(view, fileSource, MapMode::Still);
+    map.setStyleJSON(util::read_file("test/fixtures/api/empty.json"), "");
+    uint32_t shape = map.addShapeAnnotation(ShapeAnnotation(segments, styleProperties));
+
+    renderPNG(map);
+
+    map.removeAnnotation(shape);
+
+    util::write_file("test/output/remove_shape.png", renderPNG(map));
 }
 
 TEST(Annotations, SwitchStyle) {