diff options
author | John Firebaugh <john.firebaugh@gmail.com> | 2016-06-16 17:31:52 -0700 |
---|---|---|
committer | John Firebaugh <john.firebaugh@gmail.com> | 2016-06-17 11:38:04 -0700 |
commit | 2921127bdbe6cb7583c097675fe67416c8dff38e (patch) | |
tree | fc1243f56ab93eabaddfb3a979ddee31ebffc4a7 | |
parent | e2f52a1dd8020e8665c55650c75d4e5a5e1423a6 (diff) | |
download | qtlocation-mapboxgl-2921127bdbe6cb7583c097675fe67416c8dff38e.tar.gz |
[core] Avoid unnecessary work when a symbol annotation is updated
In particular, if only the geometry changes, don't cascade and recalculate the style.
-rw-r--r-- | include/mbgl/map/update.hpp | 3 | ||||
-rw-r--r-- | src/mbgl/annotation/annotation_manager.cpp | 56 | ||||
-rw-r--r-- | src/mbgl/annotation/annotation_manager.hpp | 13 | ||||
-rw-r--r-- | src/mbgl/map/map.cpp | 15 | ||||
-rw-r--r-- | src/mbgl/tile/geojson_tile.cpp | 4 | ||||
-rw-r--r-- | src/mbgl/tile/vector_tile.cpp | 4 | ||||
-rw-r--r-- | test/api/annotations.cpp | 20 | ||||
-rw-r--r-- | test/fixtures/annotations/update_icon/expected.png | bin | 2949 -> 2545 bytes | |||
-rw-r--r-- | test/fixtures/annotations/update_point/expected.png | bin | 2504 -> 2554 bytes |
9 files changed, 87 insertions, 28 deletions
diff --git a/include/mbgl/map/update.hpp b/include/mbgl/map/update.hpp index 3915545bb0..1c1270ac70 100644 --- a/include/mbgl/map/update.hpp +++ b/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) { diff --git a/src/mbgl/annotation/annotation_manager.cpp b/src/mbgl/annotation/annotation_manager.cpp index e3c6cc56d4..dd2467e34f 100644 --- a/src/mbgl/annotation/annotation_manager.cpp +++ b/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)); } diff --git a/src/mbgl/annotation/annotation_manager.hpp b/src/mbgl/annotation/annotation_manager.hpp index d2e0813be2..bdce1a8c3a 100644 --- a/src/mbgl/annotation/annotation_manager.hpp +++ b/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; diff --git a/src/mbgl/map/map.cpp b/src/mbgl/map/map.cpp index b599268bf3..11bc89a1a0 100644 --- a/src/mbgl/map/map.cpp +++ b/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) { diff --git a/src/mbgl/tile/geojson_tile.cpp b/src/mbgl/tile/geojson_tile.cpp index 9334cf2fec..e2f8b69e4d 100644 --- a/src/mbgl/tile/geojson_tile.cpp +++ b/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))); } diff --git a/src/mbgl/tile/vector_tile.cpp b/src/mbgl/tile/vector_tile.cpp index 2184fb24dd..1f924a45e1 100644 --- a/src/mbgl/tile/vector_tile.cpp +++ b/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) { } diff --git a/test/api/annotations.cpp b/test/api/annotations.cpp index abe3862f4e..5b588a9e5c 100644 --- a/test/api/annotations.cpp +++ b/test/api/annotations.cpp @@ -106,23 +106,21 @@ TEST(Annotations, NonImmediateAdd) { test.checkRendering("non_immediate_add"); } -TEST(Annotations, UpdateIcon) { +TEST(Annotations, UpdateSymbolAnnotationGeometry) { 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.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.removeAnnotationIcon("flipped_marker"); - test.map.addAnnotationIcon("flipped_marker", namedMarker("flipped_marker.png")); - test.map.update(Update::Annotations); - - test.checkRendering("update_icon"); + test.map.updateAnnotation(point, SymbolAnnotation { Point<double> { -10, 0 }, "default_marker" }); + test.checkRendering("update_point"); } -TEST(Annotations, UpdatePoint) { +TEST(Annotations, UpdateSymbolAnnotationIcon) { AnnotationTest test; test.map.setStyleJSON(util::read_file("test/fixtures/api/empty.json")); @@ -132,8 +130,8 @@ TEST(Annotations, UpdatePoint) { test::render(test.map); - test.map.updateAnnotation(point, SymbolAnnotation { Point<double> { -10, 0 }, "flipped_marker" }); - test.checkRendering("update_point"); + test.map.updateAnnotation(point, SymbolAnnotation { Point<double> { 0, 0 }, "flipped_marker" }); + test.checkRendering("update_icon"); } TEST(Annotations, RemovePoint) { diff --git a/test/fixtures/annotations/update_icon/expected.png b/test/fixtures/annotations/update_icon/expected.png Binary files differindex 3b6ca22747..408d681ede 100644 --- a/test/fixtures/annotations/update_icon/expected.png +++ b/test/fixtures/annotations/update_icon/expected.png diff --git a/test/fixtures/annotations/update_point/expected.png b/test/fixtures/annotations/update_point/expected.png Binary files differindex 02cf77df8d..30aa39e380 100644 --- a/test/fixtures/annotations/update_point/expected.png +++ b/test/fixtures/annotations/update_point/expected.png |