[core] Fix updates of line and fill annotations

11 files changed, 110 insertions, 33 deletions

diff --git a/src/mbgl/annotation/annotation_manager.cpp b/src/mbgl/annotation/annotation_manager.cpp
index 4e837d370d..f1ddf99602 100644
--- a/src/mbgl/annotation/annotation_manager.cpp
+++ b/src/mbgl/annotation/annotation_manager.cpp
@@ -42,7 +42,7 @@ void AnnotationManager::removeAnnotation(const AnnotationID& id) {
         symbolTree.remove(symbolAnnotations.at(id));
         symbolAnnotations.erase(id);
     } else if (shapeAnnotations.find(id) != shapeAnnotations.end()) {
-        obsoleteShapeAnnotationLayers.push_back(shapeAnnotations.at(id)->layerID);
+        obsoleteShapeAnnotationLayers.insert(shapeAnnotations.at(id)->layerID);
         shapeAnnotations.erase(id);
     }
 }
@@ -54,18 +54,21 @@ void AnnotationManager::add(const AnnotationID& id, const SymbolAnnotation& anno
 }
 
 void AnnotationManager::add(const AnnotationID& id, const LineAnnotation& annotation, const uint8_t maxZoom) {
-    shapeAnnotations.emplace(id,
-        std::make_unique<LineAnnotationImpl>(id, annotation, maxZoom));
+    ShapeAnnotationImpl& impl = *shapeAnnotations.emplace(id,
+        std::make_unique<LineAnnotationImpl>(id, annotation, maxZoom)).first->second;
+    obsoleteShapeAnnotationLayers.erase(impl.layerID);
 }
 
 void AnnotationManager::add(const AnnotationID& id, const FillAnnotation& annotation, const uint8_t maxZoom) {
-    shapeAnnotations.emplace(id,
-        std::make_unique<FillAnnotationImpl>(id, annotation, maxZoom));
+    ShapeAnnotationImpl& impl = *shapeAnnotations.emplace(id,
+        std::make_unique<FillAnnotationImpl>(id, annotation, maxZoom)).first->second;
+    obsoleteShapeAnnotationLayers.erase(impl.layerID);
 }
 
 void AnnotationManager::add(const AnnotationID& id, const StyleSourcedAnnotation& annotation, const uint8_t maxZoom) {
-    shapeAnnotations.emplace(id,
-        std::make_unique<StyleSourcedAnnotationImpl>(id, annotation, 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) {
diff --git a/src/mbgl/annotation/annotation_manager.hpp b/src/mbgl/annotation/annotation_manager.hpp
index a0f25fdf57..a9afa0d52d 100644
--- a/src/mbgl/annotation/annotation_manager.hpp
+++ b/src/mbgl/annotation/annotation_manager.hpp
@@ -73,7 +73,7 @@ private:
     SymbolAnnotationTree symbolTree;
     SymbolAnnotationMap symbolAnnotations;
     ShapeAnnotationMap shapeAnnotations;
-    std::vector<std::string> obsoleteShapeAnnotationLayers;
+    std::set<std::string> obsoleteShapeAnnotationLayers;
     std::set<AnnotationTile*> tiles;
 
     SpriteStore spriteStore;
diff --git a/src/mbgl/annotation/fill_annotation_impl.cpp b/src/mbgl/annotation/fill_annotation_impl.cpp
index fe520451f7..0c69bc4fe5 100644
--- a/src/mbgl/annotation/fill_annotation_impl.cpp
+++ b/src/mbgl/annotation/fill_annotation_impl.cpp
@@ -13,16 +13,19 @@ FillAnnotationImpl::FillAnnotationImpl(AnnotationID id_, FillAnnotation annotati
 }
 
 void FillAnnotationImpl::updateStyle(Style& style) const {
-    if (style.getLayer(layerID))
-        return;
-
-    std::unique_ptr<FillLayer> layer = std::make_unique<FillLayer>(layerID, AnnotationManager::SourceID);
-    layer->setSourceLayer(layerID);
-    layer->setFillOpacity(annotation.opacity);
-    layer->setFillColor(annotation.color);
-    layer->setFillOutlineColor(annotation.outlineColor);
-
-    style.addLayer(std::move(layer), AnnotationManager::PointLayerID);
+    Layer* layer = style.getLayer(layerID);
+    FillLayer* fillLayer = layer ? layer->as<FillLayer>() : nullptr;
+
+    if (!fillLayer) {
+        fillLayer = style.addLayer(
+            std::make_unique<FillLayer>(layerID, AnnotationManager::SourceID),
+            AnnotationManager::PointLayerID)->as<FillLayer>();
+        fillLayer->setSourceLayer(layerID);
+    }
+
+    fillLayer->setFillOpacity(annotation.opacity);
+    fillLayer->setFillColor(annotation.color);
+    fillLayer->setFillOutlineColor(annotation.outlineColor);
 }
 
 const ShapeAnnotationGeometry& FillAnnotationImpl::geometry() const {
diff --git a/src/mbgl/annotation/line_annotation_impl.cpp b/src/mbgl/annotation/line_annotation_impl.cpp
index f18ca9fadc..5ec5a2a49f 100644
--- a/src/mbgl/annotation/line_annotation_impl.cpp
+++ b/src/mbgl/annotation/line_annotation_impl.cpp
@@ -13,17 +13,20 @@ LineAnnotationImpl::LineAnnotationImpl(AnnotationID id_, LineAnnotation annotati
 }
 
 void LineAnnotationImpl::updateStyle(Style& style) const {
-    if (style.getLayer(layerID))
-        return;
-
-    std::unique_ptr<LineLayer> layer = std::make_unique<LineLayer>(layerID, AnnotationManager::SourceID);
-    layer->setSourceLayer(layerID);
-    layer->setLineJoin(LineJoinType::Round);
-    layer->setLineOpacity(annotation.opacity);
-    layer->setLineWidth(annotation.width);
-    layer->setLineColor(annotation.color);
-
-    style.addLayer(std::move(layer), AnnotationManager::PointLayerID);
+    Layer* layer = style.getLayer(layerID);
+    LineLayer* lineLayer = layer ? layer->as<LineLayer>() : nullptr;
+
+    if (!lineLayer) {
+        lineLayer = style.addLayer(
+            std::make_unique<LineLayer>(layerID, AnnotationManager::SourceID),
+            AnnotationManager::PointLayerID)->as<LineLayer>();
+        lineLayer->setSourceLayer(layerID);
+    }
+
+    lineLayer->setLineJoin(LineJoinType::Round);
+    lineLayer->setLineOpacity(annotation.opacity);
+    lineLayer->setLineWidth(annotation.width);
+    lineLayer->setLineColor(annotation.color);
 }
 
 const ShapeAnnotationGeometry& LineAnnotationImpl::geometry() const {
diff --git a/src/mbgl/style/style.cpp b/src/mbgl/style/style.cpp
index e04384a96e..ae6bc14b8b 100644
--- a/src/mbgl/style/style.cpp
+++ b/src/mbgl/style/style.cpp
@@ -154,7 +154,7 @@ Layer* Style::getLayer(const std::string& id) const {
     return it != layers.end() ? it->get() : nullptr;
 }
 
-void Style::addLayer(std::unique_ptr<Layer> layer, optional<std::string> before) {
+Layer* Style::addLayer(std::unique_ptr<Layer> layer, optional<std::string> before) {
     // TODO: verify source
 
     if (SymbolLayer* symbolLayer = layer->as<SymbolLayer>()) {
@@ -167,7 +167,7 @@ void Style::addLayer(std::unique_ptr<Layer> layer, optional<std::string> before)
         customLayer->impl->initialize();
     }
 
-    layers.emplace(before ? findLayer(*before) : layers.end(), std::move(layer));
+    return layers.emplace(before ? findLayer(*before) : layers.end(), std::move(layer))->get();
 }
 
 void Style::removeLayer(const std::string& id) {
diff --git a/src/mbgl/style/style.hpp b/src/mbgl/style/style.hpp
index 149a0f0803..06a25385b6 100644
--- a/src/mbgl/style/style.hpp
+++ b/src/mbgl/style/style.hpp
@@ -65,8 +65,8 @@ public:
 
     std::vector<const Layer*> getLayers() const;
     Layer* getLayer(const std::string& id) const;
-    void addLayer(std::unique_ptr<Layer>,
-                  optional<std::string> beforeLayerID = {});
+    Layer* addLayer(std::unique_ptr<Layer>,
+                    optional<std::string> beforeLayerID = {});
     void removeLayer(const std::string& layerID);
 
     std::string getName() const;
diff --git a/test/api/annotations.cpp b/test/api/annotations.cpp
index b6f861f900..5b9fe7502e 100644
--- a/test/api/annotations.cpp
+++ b/test/api/annotations.cpp
@@ -201,6 +201,74 @@ TEST(Annotations, UpdateSymbolAnnotationIcon) {
     test.checkRendering("update_icon");
 }
 
+TEST(Annotations, UpdateLineAnnotationGeometry) {
+    AnnotationTest test;
+
+    LineAnnotation annotation { LineString<double> {{ { 0, 0 }, { 45, 45 }, { 30, 0 } }} };
+    annotation.color = { { 255, 0, 0, 1 } };
+    annotation.width = { 5 };
+
+    test.map.setStyleJSON(util::read_file("test/fixtures/api/empty.json"));
+    AnnotationID line = test.map.addAnnotation(annotation);
+
+    test::render(test.map);
+
+    annotation.geometry = LineString<double> {{ { 0, 0 }, { -45, -45 } }};
+    test.map.updateAnnotation(line, annotation);
+    test.checkRendering("update_line_geometry");
+}
+
+TEST(Annotations, UpdateLineAnnotationStyle) {
+    AnnotationTest test;
+
+    LineAnnotation annotation { LineString<double> {{ { 0, 0 }, { 45, 45 }, { 30, 0 } }} };
+    annotation.color = { { 255, 0, 0, 1 } };
+    annotation.width = { 5 };
+
+    test.map.setStyleJSON(util::read_file("test/fixtures/api/empty.json"));
+    AnnotationID line = test.map.addAnnotation(annotation);
+
+    test::render(test.map);
+
+    annotation.color = { { 0, 255, 0, 1 } };
+    annotation.width = { 2 };
+    test.map.updateAnnotation(line, annotation);
+    test.checkRendering("update_line_style");
+}
+
+TEST(Annotations, UpdateFillAnnotationGeometry) {
+    AnnotationTest test;
+
+    FillAnnotation annotation { Polygon<double> {{ {{ { 0, 0 }, { 0, 45 }, { 45, 45 }, { 45, 0 } }} }} };
+    annotation.color = { { 255, 0, 0, 1 } };
+
+    test.map.setStyleJSON(util::read_file("test/fixtures/api/empty.json"));
+    AnnotationID fill = test.map.addAnnotation(annotation);
+
+    test::render(test.map);
+
+    annotation.geometry = Polygon<double> {{ {{ { 0, 0 }, { 0, 45 }, { 45, 0 } }} }};
+    test.map.updateAnnotation(fill, annotation);
+    test.checkRendering("update_fill_geometry");
+}
+
+TEST(Annotations, UpdateFillAnnotationStyle) {
+    AnnotationTest test;
+
+    Polygon<double> polygon = {{ {{ { 0, 0 }, { 0, 45 }, { 45, 45 }, { 45, 0 } }} }};
+    FillAnnotation annotation { polygon };
+    annotation.color = { { 255, 0, 0, 1 } };
+
+    test.map.setStyleJSON(util::read_file("test/fixtures/api/empty.json"));
+    AnnotationID fill = test.map.addAnnotation(annotation);
+
+    test::render(test.map);
+
+    annotation.color = { { 0, 255, 0, 1 } };
+    test.map.updateAnnotation(fill, annotation);
+    test.checkRendering("update_fill_style");
+}
+
 TEST(Annotations, RemovePoint) {
     AnnotationTest test;
 
diff --git a/test/fixtures/annotations/update_fill_geometry/expected.png b/test/fixtures/annotations/update_fill_geometry/expected.png
new file mode 100644
index 0000000000..8dbcc7795c
--- /dev/null
+++ b/test/fixtures/annotations/update_fill_geometry/expected.png
diff --git a/test/fixtures/annotations/update_fill_style/expected.png b/test/fixtures/annotations/update_fill_style/expected.png
new file mode 100644
index 0000000000..84b854fd36
--- /dev/null
+++ b/test/fixtures/annotations/update_fill_style/expected.png
diff --git a/test/fixtures/annotations/update_line_geometry/expected.png b/test/fixtures/annotations/update_line_geometry/expected.png
new file mode 100644
index 0000000000..d3b9399968
--- /dev/null
+++ b/test/fixtures/annotations/update_line_geometry/expected.png
diff --git a/test/fixtures/annotations/update_line_style/expected.png b/test/fixtures/annotations/update_line_style/expected.png
new file mode 100644
index 0000000000..2295493c71
--- /dev/null
+++ b/test/fixtures/annotations/update_line_style/expected.png