[core] Avoid unnecessary work when a symbol annotation is updated

In particular, if only the geometry changes, don't cascade and recalculate the style.

include/mbgl/map/update.hpp

@@ -13,7 +13,8 @@ enum class Update : uint8_t {
     RecalculateStyle          = 1 << 3,
     RenderStill               = 1 << 4,
     Repaint                   = 1 << 5,
-    Annotations               = 1 << 6,
+    AnnotationStyle           = 1 << 6,
+    AnnotationData            = 1 << 7,
 };
 
 inline Update operator| (const Update& lhs, const Update& rhs) {

src/mbgl/annotation/annotation_manager.cpp

@@ -31,10 +31,9 @@ AnnotationID AnnotationManager::addAnnotation(const Annotation& annotation, cons
     return id;
 }
 
-void AnnotationManager::updateAnnotation(const AnnotationID& id, const Annotation& annotation, const uint8_t maxZoom) {
-    removeAnnotation(id);
-    Annotation::visit(annotation, [&] (const auto& annotation_) {
-        this->add(id, annotation_, maxZoom);
+Update AnnotationManager::updateAnnotation(const AnnotationID& id, const Annotation& annotation, const uint8_t maxZoom) {
+    return Annotation::visit(annotation, [&] (const auto& annotation_) {
+        return this->update(id, annotation_, maxZoom);
     });
 }
 
@@ -69,6 +68,53 @@ void AnnotationManager::add(const AnnotationID& id, const StyleSourcedAnnotation
         std::make_unique<StyleSourcedAnnotationImpl>(id, annotation, maxZoom));
 }
 
+Update AnnotationManager::update(const AnnotationID& id, const SymbolAnnotation& annotation, const uint8_t maxZoom) {
+    auto it = symbolAnnotations.find(id);
+    if (it == symbolAnnotations.end()) {
+        removeAndAdd(id, annotation, maxZoom);
+        return Update::AnnotationData | Update::AnnotationStyle;
+    }
+
+    Update result = Update::Nothing;
+    const SymbolAnnotation& existing = it->second->annotation;
+
+    if (existing.geometry != annotation.geometry) {
+        result |= Update::AnnotationData;
+    }
+
+    if (existing.icon != annotation.icon) {
+        result |= Update::AnnotationData | Update::AnnotationStyle;
+    }
+
+    if (result != Update::Nothing) {
+        removeAndAdd(id, annotation, maxZoom);
+    }
+
+    return result;
+}
+
+Update AnnotationManager::update(const AnnotationID& id, const LineAnnotation& annotation, const uint8_t maxZoom) {
+    removeAndAdd(id, annotation, maxZoom);
+    return Update::AnnotationData | Update::AnnotationStyle;
+}
+
+Update AnnotationManager::update(const AnnotationID& id, const FillAnnotation& annotation, const uint8_t maxZoom) {
+    removeAndAdd(id, annotation, maxZoom);
+    return Update::AnnotationData | Update::AnnotationStyle;
+}
+
+Update AnnotationManager::update(const AnnotationID& id, const StyleSourcedAnnotation& annotation, const uint8_t maxZoom) {
+    removeAndAdd(id, annotation, maxZoom);
+    return Update::AnnotationData | Update::AnnotationStyle;
+}
+
+void AnnotationManager::removeAndAdd(const AnnotationID& id, const Annotation& annotation, const uint8_t maxZoom) {
+    removeAnnotation(id);
+    Annotation::visit(annotation, [&] (const auto& annotation_) {
+        this->add(id, annotation_, maxZoom);
+    });
+}
+
 AnnotationIDs AnnotationManager::getPointAnnotationsInBounds(const LatLngBounds& bounds) const {
     AnnotationIDs result;
 
@@ -133,7 +179,9 @@ void AnnotationManager::updateStyle(Style& style) {
     }
 
     obsoleteShapeAnnotationLayers.clear();
+}
 
+void AnnotationManager::updateData() {
     for (auto& tile : tiles) {
         tile->setData(getTileData(tile->id.canonical));
     }

src/mbgl/annotation/annotation_manager.hpp

@@ -4,7 +4,7 @@
 #include <mbgl/annotation/symbol_annotation_impl.hpp>
 #include <mbgl/sprite/sprite_store.hpp>
 #include <mbgl/sprite/sprite_atlas.hpp>
-#include <mbgl/util/geo.hpp>
+#include <mbgl/map/update.hpp>
 #include <mbgl/util/noncopyable.hpp>
 
 #include <string>
@@ -14,6 +14,7 @@
 
 namespace mbgl {
 
+class LatLngBounds;
 class AnnotationTile;
 class AnnotationTileData;
 class SymbolAnnotationImpl;
@@ -29,7 +30,7 @@ public:
     ~AnnotationManager();
 
     AnnotationID addAnnotation(const Annotation&, const uint8_t maxZoom);
-    void updateAnnotation(const AnnotationID&, const Annotation&, const uint8_t maxZoom);
+    Update updateAnnotation(const AnnotationID&, const Annotation&, const uint8_t maxZoom);
     void removeAnnotation(const AnnotationID&);
 
     AnnotationIDs getPointAnnotationsInBounds(const LatLngBounds&) const;
@@ -40,6 +41,7 @@ public:
     SpriteAtlas& getSpriteAtlas() { return spriteAtlas; }
 
     void updateStyle(style::Style&);
+    void updateData();
 
     void addTile(AnnotationTile&);
     void removeTile(AnnotationTile&);
@@ -53,6 +55,13 @@ private:
     void add(const AnnotationID&, const FillAnnotation&, const uint8_t);
     void add(const AnnotationID&, const StyleSourcedAnnotation&, const uint8_t);
 
+    Update update(const AnnotationID&, const SymbolAnnotation&, const uint8_t);
+    Update update(const AnnotationID&, const LineAnnotation&, const uint8_t);
+    Update update(const AnnotationID&, const FillAnnotation&, const uint8_t);
+    Update update(const AnnotationID&, const StyleSourcedAnnotation&, const uint8_t);
+
+    void removeAndAdd(const AnnotationID&, const Annotation&, const uint8_t);
+
     std::unique_ptr<AnnotationTileData> getTileData(const CanonicalTileID&);
 
     AnnotationID nextID = 0;

src/mbgl/map/map.cpp

@@ -211,11 +211,15 @@ void Map::Impl::update() {
     // - Hint style sources to notify when all its tiles are loaded;
     timePoint = Clock::now();
 
-    if (style->loaded && updateFlags & Update::Annotations) {
+    if (style->loaded && updateFlags & Update::AnnotationStyle) {
         annotationManager->updateStyle(*style);
         updateFlags |= Update::Classes;
     }
 
+    if (updateFlags & Update::AnnotationData) {
+        annotationManager->updateData();
+    }
+
     if (updateFlags & Update::Classes) {
         style->cascade(timePoint, mode);
     }
@@ -338,7 +342,7 @@ void Map::Impl::loadStyleJSON(const std::string& json) {
     // force style cascade, causing all pending transitions to complete.
     style->cascade(Clock::now(), mode);
 
-    updateFlags |= Update::Classes | Update::RecalculateStyle | Update::Annotations;
+    updateFlags |= Update::Classes | Update::RecalculateStyle | Update::AnnotationStyle;
     asyncUpdate.send();
 }
 
@@ -689,18 +693,17 @@ double Map::getTopOffsetPixelsForAnnotationIcon(const std::string& name) {
 
 AnnotationID Map::addAnnotation(const Annotation& annotation) {
     auto result = impl->annotationManager->addAnnotation(annotation, getMaxZoom());
-    update(Update::Annotations);
+    update(Update::AnnotationStyle | Update::AnnotationData);
     return result;
 }
 
 void Map::updateAnnotation(AnnotationID id, const Annotation& annotation) {
-    impl->annotationManager->updateAnnotation(id, annotation, getMaxZoom());
-    update(Update::Annotations);
+    update(impl->annotationManager->updateAnnotation(id, annotation, getMaxZoom()));
 }
 
 void Map::removeAnnotation(AnnotationID annotation) {
     impl->annotationManager->removeAnnotation(annotation);
-    update(Update::Annotations);
+    update(Update::AnnotationStyle | Update::AnnotationData);
 }
 
 AnnotationIDs Map::getPointAnnotationsInBounds(const LatLngBounds& bounds) {

src/mbgl/tile/geojson_tile.cpp

@@ -107,10 +107,10 @@ std::unique_ptr<GeoJSONTileData> convertTile(const mapbox::geojsonvt::Tile& tile
 }
 
 GeoJSONTile::GeoJSONTile(const OverscaledTileID& overscaledTileID,
-                         std::string sourceID,
+                         std::string sourceID_,
                          const style::UpdateParameters& parameters,
                          mapbox::geojsonvt::GeoJSONVT& geojsonvt)
-    : GeometryTile(overscaledTileID, sourceID, parameters.style, parameters.mode) {
+    : GeometryTile(overscaledTileID, sourceID_, parameters.style, parameters.mode) {
     setData(convertTile(geojsonvt.getTile(id.canonical.z, id.canonical.x, id.canonical.y)));
 }
 

src/mbgl/tile/vector_tile.cpp

@@ -69,10 +69,10 @@ private:
 };
 
 VectorTile::VectorTile(const OverscaledTileID& id_,
-                       std::string sourceID,
+                       std::string sourceID_,
                        const style::UpdateParameters& parameters,
                        const Tileset& tileset)
-    : GeometryTile(id_, sourceID, parameters.style, parameters.mode),
+    : GeometryTile(id_, sourceID_, parameters.style, parameters.mode),
       loader(*this, id_, parameters, tileset) {
 }
 

test/api/annotations.cpp

@@ -106,23 +106,7 @@ TEST(Annotations, NonImmediateAdd) {
     test.checkRendering("non_immediate_add");
 }
 
-TEST(Annotations, UpdateIcon) {
-    AnnotationTest test;
-
-    test.map.setStyleJSON(util::read_file("test/fixtures/api/empty.json"));
-    test.map.addAnnotationIcon("flipped_marker", namedMarker("default_marker.png"));
-    test.map.addAnnotation(SymbolAnnotation { Point<double> { 0, 0 }, "flipped_marker" });
-
-    test::render(test.map);
-
-    test.map.removeAnnotationIcon("flipped_marker");
-    test.map.addAnnotationIcon("flipped_marker", namedMarker("flipped_marker.png"));
-    test.map.update(Update::Annotations);
-
-    test.checkRendering("update_icon");
-}
-
-TEST(Annotations, UpdatePoint) {
+TEST(Annotations, UpdateSymbolAnnotationGeometry) {
     AnnotationTest test;
 
     test.map.setStyleJSON(util::read_file("test/fixtures/api/empty.json"));
@@ -132,10 +116,24 @@ TEST(Annotations, UpdatePoint) {
 
     test::render(test.map);
 
-    test.map.updateAnnotation(point, SymbolAnnotation { Point<double> { -10, 0 }, "flipped_marker" });
+    test.map.updateAnnotation(point, SymbolAnnotation { Point<double> { -10, 0 }, "default_marker" });
     test.checkRendering("update_point");
 }
 
+TEST(Annotations, UpdateSymbolAnnotationIcon) {
+    AnnotationTest test;
+
+    test.map.setStyleJSON(util::read_file("test/fixtures/api/empty.json"));
+    test.map.addAnnotationIcon("default_marker", namedMarker("default_marker.png"));
+    test.map.addAnnotationIcon("flipped_marker", namedMarker("flipped_marker.png"));
+    AnnotationID point = test.map.addAnnotation(SymbolAnnotation { Point<double> { 0, 0 }, "default_marker" });
+
+    test::render(test.map);
+
+    test.map.updateAnnotation(point, SymbolAnnotation { Point<double> { 0, 0 }, "flipped_marker" });
+    test.checkRendering("update_icon");
+}
+
 TEST(Annotations, RemovePoint) {
     AnnotationTest test;