From 591af0021bfb8b9fdfd803b55fb6c18a24c46943 Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Tue, 30 May 2017 13:40:36 -0700 Subject: [core] Refactor RenderSource updates * Eliminate updateBatch in favor of diffing layers and detecting changes to properties upon which layout depends. * Replace RenderSource::{update,remove,invalidate,reload}Tiles with a single update method * Replace TilePyramid::{update,remove,invalidate,reload}Tiles with a single update method * Remove Style& dependency TODO from GeometryTile and TileParameters --- cmake/core-files.cmake | 1 - include/mbgl/style/data_driven_property_value.hpp | 4 + include/mbgl/style/property_value.hpp | 4 + src/mbgl/annotation/annotation_tile.cpp | 5 +- src/mbgl/annotation/render_annotation_source.cpp | 39 +++--- src/mbgl/annotation/render_annotation_source.hpp | 18 +-- src/mbgl/renderer/render_source.cpp | 9 +- src/mbgl/renderer/render_source.hpp | 28 ++--- .../renderer/sources/render_geojson_source.cpp | 53 ++++---- .../renderer/sources/render_geojson_source.hpp | 18 +-- src/mbgl/renderer/sources/render_image_source.cpp | 9 +- src/mbgl/renderer/sources/render_image_source.hpp | 12 +- src/mbgl/renderer/sources/render_raster_source.cpp | 58 ++++----- src/mbgl/renderer/sources/render_raster_source.hpp | 18 +-- src/mbgl/renderer/sources/render_vector_source.cpp | 60 ++++----- src/mbgl/renderer/sources/render_vector_source.hpp | 18 +-- src/mbgl/renderer/style_diff.cpp | 11 +- src/mbgl/renderer/style_diff.hpp | 6 +- src/mbgl/renderer/tile_parameters.hpp | 28 +---- src/mbgl/renderer/tile_pyramid.cpp | 58 +++++---- src/mbgl/renderer/tile_pyramid.hpp | 26 ++-- src/mbgl/style/layer_impl.hpp | 4 + src/mbgl/style/layers/background_layer_impl.cpp | 4 + src/mbgl/style/layers/background_layer_impl.hpp | 1 + src/mbgl/style/layers/circle_layer_impl.cpp | 8 ++ src/mbgl/style/layers/circle_layer_impl.hpp | 1 + src/mbgl/style/layers/custom_layer_impl.cpp | 4 + src/mbgl/style/layers/custom_layer_impl.hpp | 1 + .../style/layers/fill_extrusion_layer_impl.cpp | 8 ++ .../style/layers/fill_extrusion_layer_impl.hpp | 1 + src/mbgl/style/layers/fill_layer_impl.cpp | 8 ++ src/mbgl/style/layers/fill_layer_impl.hpp | 1 + src/mbgl/style/layers/line_layer_impl.cpp | 9 ++ src/mbgl/style/layers/line_layer_impl.hpp | 1 + src/mbgl/style/layers/raster_layer_impl.cpp | 4 + src/mbgl/style/layers/raster_layer_impl.hpp | 1 + src/mbgl/style/layers/symbol_layer_impl.cpp | 9 ++ src/mbgl/style/layers/symbol_layer_impl.hpp | 1 + src/mbgl/style/properties.hpp | 6 + src/mbgl/style/style.cpp | 135 +++++++++------------ src/mbgl/style/style.hpp | 2 - src/mbgl/style/update_batch.hpp | 15 --- src/mbgl/tile/geojson_tile.cpp | 5 +- src/mbgl/tile/geometry_tile.cpp | 33 +++-- src/mbgl/tile/geometry_tile.hpp | 13 +- src/mbgl/tile/tile.hpp | 2 +- src/mbgl/tile/vector_tile.cpp | 5 +- test/style/source.test.cpp | 106 +++++++++++++--- test/tile/annotation_tile.test.cpp | 7 +- test/tile/geojson_tile.test.cpp | 14 ++- test/tile/raster_tile.test.cpp | 9 +- test/tile/vector_tile.test.cpp | 9 +- 52 files changed, 486 insertions(+), 424 deletions(-) delete mode 100644 src/mbgl/style/update_batch.hpp diff --git a/cmake/core-files.cmake b/cmake/core-files.cmake index 53873af7b1..fb3a49c6f8 100644 --- a/cmake/core-files.cmake +++ b/cmake/core-files.cmake @@ -353,7 +353,6 @@ set(MBGL_CORE_FILES src/mbgl/style/style.cpp src/mbgl/style/style.hpp src/mbgl/style/types.cpp - src/mbgl/style/update_batch.hpp # style/conversion include/mbgl/style/conversion/constant.hpp diff --git a/include/mbgl/style/data_driven_property_value.hpp b/include/mbgl/style/data_driven_property_value.hpp index 12a369abb1..090ee0f59b 100644 --- a/include/mbgl/style/data_driven_property_value.hpp +++ b/include/mbgl/style/data_driven_property_value.hpp @@ -59,6 +59,10 @@ public: auto evaluate(const Evaluator& evaluator, TimePoint = {}) const { return Value::visit(value, evaluator); } + + bool hasDataDrivenPropertyDifference(const DataDrivenPropertyValue& other) const { + return *this != other && (isDataDriven() || other.isDataDriven()); + } }; } // namespace style diff --git a/include/mbgl/style/property_value.hpp b/include/mbgl/style/property_value.hpp index 5259eecf2a..02d3a31148 100644 --- a/include/mbgl/style/property_value.hpp +++ b/include/mbgl/style/property_value.hpp @@ -38,6 +38,10 @@ public: auto evaluate(const Evaluator& evaluator, TimePoint = {}) const { return Value::visit(value, evaluator); } + + bool hasDataDrivenPropertyDifference(const PropertyValue&) const { + return false; + } }; } // namespace style diff --git a/src/mbgl/annotation/annotation_tile.cpp b/src/mbgl/annotation/annotation_tile.cpp index 1b34026f74..67a2d128ba 100644 --- a/src/mbgl/annotation/annotation_tile.cpp +++ b/src/mbgl/annotation/annotation_tile.cpp @@ -3,7 +3,6 @@ #include #include #include -#include #include @@ -11,9 +10,7 @@ namespace mbgl { AnnotationTile::AnnotationTile(const OverscaledTileID& overscaledTileID, const TileParameters& parameters) - : GeometryTile(overscaledTileID, AnnotationManager::SourceID, parameters, - *parameters.style.glyphAtlas, - *parameters.style.spriteAtlas), + : GeometryTile(overscaledTileID, AnnotationManager::SourceID, parameters), annotationManager(parameters.annotationManager) { annotationManager.addTile(*this); } diff --git a/src/mbgl/annotation/render_annotation_source.cpp b/src/mbgl/annotation/render_annotation_source.cpp index dfb78fb8b1..dce2914e7f 100644 --- a/src/mbgl/annotation/render_annotation_source.cpp +++ b/src/mbgl/annotation/render_annotation_source.cpp @@ -22,8 +22,25 @@ bool RenderAnnotationSource::isLoaded() const { return tilePyramid.isLoaded(); } -void RenderAnnotationSource::invalidateTiles() { - tilePyramid.invalidateTiles(); +void RenderAnnotationSource::update(Immutable baseImpl_, + const std::vector>& layers, + const bool needsRendering, + const bool needsRelayout, + const TileParameters& parameters) { + std::swap(baseImpl, baseImpl_); + + enabled = needsRendering; + + tilePyramid.update(layers, + needsRendering, + needsRelayout, + parameters, + SourceType::Annotations, + util::tileSize, + { 0, 22 }, + [&] (const OverscaledTileID& tileID) { + return std::make_unique(tileID, parameters); + }); } void RenderAnnotationSource::startRender(algorithm::ClipIDGenerator& generator, const mat4& projMatrix, const mat4& clipMatrix, const TransformState& transform) { @@ -39,24 +56,6 @@ std::map& RenderAnnotationSource::getRenderTiles() return tilePyramid.getRenderTiles(); } -void RenderAnnotationSource::updateTiles(const TileParameters& parameters) { - tilePyramid.updateTiles(parameters, - SourceType::Annotations, - util::tileSize, - { 0, 22 }, - [&] (const OverscaledTileID& tileID) { - return std::make_unique(tileID, parameters); - }); -} - -void RenderAnnotationSource::removeTiles() { - tilePyramid.removeTiles(); -} - -void RenderAnnotationSource::reloadTiles() { - tilePyramid.reloadTiles(); -} - std::unordered_map> RenderAnnotationSource::queryRenderedFeatures(const ScreenLineString& geometry, const TransformState& transformState, diff --git a/src/mbgl/annotation/render_annotation_source.hpp b/src/mbgl/annotation/render_annotation_source.hpp index c93cfbd7d7..ed30e62381 100644 --- a/src/mbgl/annotation/render_annotation_source.hpp +++ b/src/mbgl/annotation/render_annotation_source.hpp @@ -12,19 +12,11 @@ public: bool isLoaded() const final; - // Called when the camera has changed. May load new tiles, unload obsolete tiles, or - // trigger re-placement of existing complete tiles. - void updateTiles(const TileParameters&) final; - - // Removes all tiles (by putting them into the cache). - void removeTiles() final; - - // Remove all tiles and clear the cache. - void invalidateTiles() final; - - // Request that all loaded tiles re-run the layout operation on the existing source - // data with fresh style information. - void reloadTiles() final; + void update(Immutable, + const std::vector>&, + bool needsRendering, + bool needsRelayout, + const TileParameters&) final; void startRender(algorithm::ClipIDGenerator&, const mat4& projMatrix, diff --git a/src/mbgl/renderer/render_source.cpp b/src/mbgl/renderer/render_source.cpp index c59f767d26..7723a1c7ca 100644 --- a/src/mbgl/renderer/render_source.cpp +++ b/src/mbgl/renderer/render_source.cpp @@ -3,6 +3,7 @@ #include #include #include +#include #include #include #include @@ -44,10 +45,6 @@ void RenderSource::setObserver(RenderSourceObserver* observer_) { observer = observer_; } -void RenderSource::setImpl(Immutable impl) { - baseImpl = impl; -} - void RenderSource::onTileChanged(Tile& tile) { observer->onTileChanged(*this, tile.id); } @@ -56,4 +53,8 @@ void RenderSource::onTileError(Tile& tile, std::exception_ptr error) { observer->onTileError(*this, tile.id, error); } +bool RenderSource::isEnabled() const { + return enabled; +} + } // namespace mbgl diff --git a/src/mbgl/renderer/render_source.hpp b/src/mbgl/renderer/render_source.hpp index df44ea70fd..b229d8a0bd 100644 --- a/src/mbgl/renderer/render_source.hpp +++ b/src/mbgl/renderer/render_source.hpp @@ -6,6 +6,7 @@ #include #include #include +#include #include #include @@ -17,6 +18,7 @@ namespace mbgl { class Painter; class TransformState; class RenderTile; +class RenderLayer; class RenderedQueryOptions; class SourceQueryOptions; class Tile; @@ -35,8 +37,6 @@ class RenderSource : protected TileObserver { public: static std::unique_ptr create(Immutable); - ~RenderSource() override = default; - // Check whether this source is of the given subtype. template bool is() const; @@ -52,21 +52,14 @@ public: return is() ? reinterpret_cast(this) : nullptr; } + bool isEnabled() const; virtual bool isLoaded() const = 0; - // Called when the camera has changed. May load new tiles, unload obsolete tiles, or - // trigger re-placement of existing complete tiles. - virtual void updateTiles(const TileParameters&) = 0; - - // Removes all tiles (by putting them into the cache). - virtual void removeTiles() = 0; - - // Remove all tiles and clear the cache. - virtual void invalidateTiles() = 0; - - // Request that all loaded tiles re-run the layout operation on the existing source - // data with fresh style information. - virtual void reloadTiles() = 0; + virtual void update(Immutable, + const std::vector>&, + bool needsRendering, + bool needsRelayout, + const TileParameters&) = 0; virtual void startRender(algorithm::ClipIDGenerator&, const mat4& projMatrix, @@ -93,14 +86,13 @@ public: void setObserver(RenderSourceObserver*); Immutable baseImpl; - void setImpl(Immutable); - - bool enabled = false; protected: RenderSource(Immutable); RenderSourceObserver* observer; + bool enabled = false; + void onTileChanged(Tile&) final; void onTileError(Tile&, std::exception_ptr) final; }; diff --git a/src/mbgl/renderer/sources/render_geojson_source.cpp b/src/mbgl/renderer/sources/render_geojson_source.cpp index ca0c6c5244..19e93ca2ba 100644 --- a/src/mbgl/renderer/sources/render_geojson_source.cpp +++ b/src/mbgl/renderer/sources/render_geojson_source.cpp @@ -22,24 +22,15 @@ bool RenderGeoJSONSource::isLoaded() const { return tilePyramid.isLoaded(); } -void RenderGeoJSONSource::invalidateTiles() { - tilePyramid.invalidateTiles(); -} - -void RenderGeoJSONSource::startRender(algorithm::ClipIDGenerator& generator, const mat4& projMatrix, const mat4& clipMatrix, const TransformState& transform) { - generator.update(tilePyramid.getRenderTiles()); - tilePyramid.startRender(projMatrix, clipMatrix, transform); -} +void RenderGeoJSONSource::update(Immutable baseImpl_, + const std::vector>& layers, + const bool needsRendering, + const bool needsRelayout, + const TileParameters& parameters) { + std::swap(baseImpl, baseImpl_); -void RenderGeoJSONSource::finishRender(Painter& painter) { - tilePyramid.finishRender(painter); -} - -std::map& RenderGeoJSONSource::getRenderTiles() { - return tilePyramid.getRenderTiles(); -} + enabled = needsRendering; -void RenderGeoJSONSource::updateTiles(const TileParameters& parameters) { GeoJSONData* data_ = impl().getData(); if (!data_) { @@ -55,21 +46,29 @@ void RenderGeoJSONSource::updateTiles(const TileParameters& parameters) { } } - tilePyramid.updateTiles(parameters, - SourceType::GeoJSON, - util::tileSize, - impl().getZoomRange(), - [&] (const OverscaledTileID& tileID) { - return std::make_unique(tileID, impl().id, parameters, data->getTile(tileID.canonical)); - }); + tilePyramid.update(layers, + needsRendering, + needsRelayout, + parameters, + SourceType::GeoJSON, + util::tileSize, + impl().getZoomRange(), + [&] (const OverscaledTileID& tileID) { + return std::make_unique(tileID, impl().id, parameters, data->getTile(tileID.canonical)); + }); } -void RenderGeoJSONSource::removeTiles() { - tilePyramid.removeTiles(); +void RenderGeoJSONSource::startRender(algorithm::ClipIDGenerator& generator, const mat4& projMatrix, const mat4& clipMatrix, const TransformState& transform) { + generator.update(tilePyramid.getRenderTiles()); + tilePyramid.startRender(projMatrix, clipMatrix, transform); } -void RenderGeoJSONSource::reloadTiles() { - tilePyramid.reloadTiles(); +void RenderGeoJSONSource::finishRender(Painter& painter) { + tilePyramid.finishRender(painter); +} + +std::map& RenderGeoJSONSource::getRenderTiles() { + return tilePyramid.getRenderTiles(); } std::unordered_map> diff --git a/src/mbgl/renderer/sources/render_geojson_source.hpp b/src/mbgl/renderer/sources/render_geojson_source.hpp index 6571750347..c3783af642 100644 --- a/src/mbgl/renderer/sources/render_geojson_source.hpp +++ b/src/mbgl/renderer/sources/render_geojson_source.hpp @@ -16,19 +16,11 @@ public: bool isLoaded() const final; - // Called when the camera has changed. May load new tiles, unload obsolete tiles, or - // trigger re-placement of existing complete tiles. - void updateTiles(const TileParameters&) final; - - // Removes all tiles (by putting them into the cache). - void removeTiles() final; - - // Remove all tiles and clear the cache. - void invalidateTiles() final; - - // Request that all loaded tiles re-run the layout operation on the existing source - // data with fresh style information. - void reloadTiles() final; + void update(Immutable, + const std::vector>&, + bool needsRendering, + bool needsRelayout, + const TileParameters&) final; void startRender(algorithm::ClipIDGenerator&, const mat4& projMatrix, diff --git a/src/mbgl/renderer/sources/render_image_source.cpp b/src/mbgl/renderer/sources/render_image_source.cpp index a55e97fd9c..5f9887cdb8 100644 --- a/src/mbgl/renderer/sources/render_image_source.cpp +++ b/src/mbgl/renderer/sources/render_image_source.cpp @@ -72,7 +72,14 @@ void RenderImageSource::upload(gl::Context& context) { } } -void RenderImageSource::updateTiles(const TileParameters& parameters) { +void RenderImageSource::update(Immutable baseImpl_, + const std::vector>&, + const bool needsRendering, + const bool, + const TileParameters& parameters) { + std::swap(baseImpl, baseImpl_); + + enabled = needsRendering; auto transformState = parameters.transformState; auto size = transformState.getSize(); diff --git a/src/mbgl/renderer/sources/render_image_source.hpp b/src/mbgl/renderer/sources/render_image_source.hpp index f8581f079a..dc5c32876b 100644 --- a/src/mbgl/renderer/sources/render_image_source.hpp +++ b/src/mbgl/renderer/sources/render_image_source.hpp @@ -30,13 +30,11 @@ public: void finishRender(Painter&) final; - void updateTiles(const TileParameters&) final; - void removeTiles() final { - } - void invalidateTiles() final { - } - void reloadTiles() final { - } + void update(Immutable, + const std::vector>&, + bool needsRendering, + bool needsRelayout, + const TileParameters&) final; std::map& getRenderTiles() final { return tiles; diff --git a/src/mbgl/renderer/sources/render_raster_source.cpp b/src/mbgl/renderer/sources/render_raster_source.cpp index 182fe47549..239bac9dd6 100644 --- a/src/mbgl/renderer/sources/render_raster_source.cpp +++ b/src/mbgl/renderer/sources/render_raster_source.cpp @@ -19,23 +19,15 @@ bool RenderRasterSource::isLoaded() const { return tilePyramid.isLoaded(); } -void RenderRasterSource::invalidateTiles() { - tilePyramid.invalidateTiles(); -} +void RenderRasterSource::update(Immutable baseImpl_, + const std::vector>& layers, + const bool needsRendering, + const bool needsRelayout, + const TileParameters& parameters) { + std::swap(baseImpl, baseImpl_); -void RenderRasterSource::startRender(algorithm::ClipIDGenerator&, const mat4& projMatrix, const mat4& clipMatrix, const TransformState& transform) { - tilePyramid.startRender(projMatrix, clipMatrix, transform); -} + enabled = needsRendering; -void RenderRasterSource::finishRender(Painter& painter) { - tilePyramid.finishRender(painter); -} - -std::map& RenderRasterSource::getRenderTiles() { - return tilePyramid.getRenderTiles(); -} - -void RenderRasterSource::updateTiles(const TileParameters& parameters) { optional tileset = impl().getTileset(); if (!tileset) { @@ -44,24 +36,36 @@ void RenderRasterSource::updateTiles(const TileParameters& parameters) { if (tileURLTemplates != tileset->tiles) { tileURLTemplates = tileset->tiles; - tilePyramid.invalidateTiles(); + + // TODO: this removes existing buckets, and will cause flickering. + // Should instead refresh tile data in place. + tilePyramid.tiles.clear(); + tilePyramid.renderTiles.clear(); + tilePyramid.cache.clear(); } - tilePyramid.updateTiles(parameters, - SourceType::Raster, - impl().getTileSize(), - tileset->zoomRange, - [&] (const OverscaledTileID& tileID) { - return std::make_unique(tileID, parameters, *tileset); - }); + tilePyramid.update(layers, + needsRendering, + needsRelayout, + parameters, + SourceType::Raster, + impl().getTileSize(), + tileset->zoomRange, + [&] (const OverscaledTileID& tileID) { + return std::make_unique(tileID, parameters, *tileset); + }); +} + +void RenderRasterSource::startRender(algorithm::ClipIDGenerator&, const mat4& projMatrix, const mat4& clipMatrix, const TransformState& transform) { + tilePyramid.startRender(projMatrix, clipMatrix, transform); } -void RenderRasterSource::removeTiles() { - tilePyramid.removeTiles(); +void RenderRasterSource::finishRender(Painter& painter) { + tilePyramid.finishRender(painter); } -void RenderRasterSource::reloadTiles() { - tilePyramid.reloadTiles(); +std::map& RenderRasterSource::getRenderTiles() { + return tilePyramid.getRenderTiles(); } std::unordered_map> diff --git a/src/mbgl/renderer/sources/render_raster_source.hpp b/src/mbgl/renderer/sources/render_raster_source.hpp index d68c6d5dd3..cbe16b967e 100644 --- a/src/mbgl/renderer/sources/render_raster_source.hpp +++ b/src/mbgl/renderer/sources/render_raster_source.hpp @@ -12,19 +12,11 @@ public: bool isLoaded() const final; - // Called when the camera has changed. May load new tiles, unload obsolete tiles, or - // trigger re-placement of existing complete tiles. - void updateTiles(const TileParameters&) final; - - // Removes all tiles (by putting them into the cache). - void removeTiles() final; - - // Remove all tiles and clear the cache. - void invalidateTiles() final; - - // Request that all loaded tiles re-run the layout operation on the existing source - // data with fresh style information. - void reloadTiles() final; + void update(Immutable, + const std::vector>&, + bool needsRendering, + bool needsRelayout, + const TileParameters&) final; void startRender(algorithm::ClipIDGenerator&, const mat4& projMatrix, diff --git a/src/mbgl/renderer/sources/render_vector_source.cpp b/src/mbgl/renderer/sources/render_vector_source.cpp index 732969db64..869d87b0b7 100644 --- a/src/mbgl/renderer/sources/render_vector_source.cpp +++ b/src/mbgl/renderer/sources/render_vector_source.cpp @@ -22,24 +22,15 @@ bool RenderVectorSource::isLoaded() const { return tilePyramid.isLoaded(); } -void RenderVectorSource::invalidateTiles() { - tilePyramid.invalidateTiles(); -} - -void RenderVectorSource::startRender(algorithm::ClipIDGenerator& generator, const mat4& projMatrix, const mat4& clipMatrix, const TransformState& transform) { - generator.update(tilePyramid.getRenderTiles()); - tilePyramid.startRender(projMatrix, clipMatrix, transform); -} +void RenderVectorSource::update(Immutable baseImpl_, + const std::vector>& layers, + const bool needsRendering, + const bool needsRelayout, + const TileParameters& parameters) { + std::swap(baseImpl, baseImpl_); -void RenderVectorSource::finishRender(Painter& painter) { - tilePyramid.finishRender(painter); -} + enabled = needsRendering; -std::map& RenderVectorSource::getRenderTiles() { - return tilePyramid.getRenderTiles(); -} - -void RenderVectorSource::updateTiles(const TileParameters& parameters) { optional tileset = impl().getTileset(); if (!tileset) { @@ -48,24 +39,37 @@ void RenderVectorSource::updateTiles(const TileParameters& parameters) { if (tileURLTemplates != tileset->tiles) { tileURLTemplates = tileset->tiles; - tilePyramid.invalidateTiles(); + + // TODO: this removes existing buckets, and will cause flickering. + // Should instead refresh tile data in place. + tilePyramid.tiles.clear(); + tilePyramid.renderTiles.clear(); + tilePyramid.cache.clear(); } - tilePyramid.updateTiles(parameters, - SourceType::Vector, - util::tileSize, - tileset->zoomRange, - [&] (const OverscaledTileID& tileID) { - return std::make_unique(tileID, impl().id, parameters, *tileset); - }); + tilePyramid.update(layers, + needsRendering, + needsRelayout, + parameters, + SourceType::Vector, + util::tileSize, + tileset->zoomRange, + [&] (const OverscaledTileID& tileID) { + return std::make_unique(tileID, impl().id, parameters, *tileset); + }); } -void RenderVectorSource::removeTiles() { - tilePyramid.removeTiles(); +void RenderVectorSource::startRender(algorithm::ClipIDGenerator& generator, const mat4& projMatrix, const mat4& clipMatrix, const TransformState& transform) { + generator.update(tilePyramid.getRenderTiles()); + tilePyramid.startRender(projMatrix, clipMatrix, transform); } -void RenderVectorSource::reloadTiles() { - tilePyramid.reloadTiles(); +void RenderVectorSource::finishRender(Painter& painter) { + tilePyramid.finishRender(painter); +} + +std::map& RenderVectorSource::getRenderTiles() { + return tilePyramid.getRenderTiles(); } std::unordered_map> diff --git a/src/mbgl/renderer/sources/render_vector_source.hpp b/src/mbgl/renderer/sources/render_vector_source.hpp index d67df4ca2c..c1357141a6 100644 --- a/src/mbgl/renderer/sources/render_vector_source.hpp +++ b/src/mbgl/renderer/sources/render_vector_source.hpp @@ -12,19 +12,11 @@ public: bool isLoaded() const final; - // Called when the camera has changed. May load new tiles, unload obsolete tiles, or - // trigger re-placement of existing complete tiles. - void updateTiles(const TileParameters&) final; - - // Removes all tiles (by putting them into the cache). - void removeTiles() final; - - // Remove all tiles and clear the cache. - void invalidateTiles() final; - - // Request that all loaded tiles re-run the layout operation on the existing source - // data with fresh style information. - void reloadTiles() final; + void update(Immutable, + const std::vector>&, + bool needsRendering, + bool needsRelayout, + const TileParameters&) final; void startRender(algorithm::ClipIDGenerator&, const mat4& projMatrix, diff --git a/src/mbgl/renderer/style_diff.cpp b/src/mbgl/renderer/style_diff.cpp index e2be95432f..e250dfa8d5 100644 --- a/src/mbgl/renderer/style_diff.cpp +++ b/src/mbgl/renderer/style_diff.cpp @@ -27,7 +27,7 @@ StyleDifference diff(const std::vector& a, const std::vector& b, const bIt++; } else { if (aIt->get() != bIt->get()) { - result.changed.emplace((*bIt)->id, *bIt); + result.changed.emplace((*bIt)->id, std::array {{ *aIt, *bIt }}); } aIt++; bIt++; @@ -61,4 +61,13 @@ LayerDifference diffLayers(const std::vector& a, }); } +bool hasLayoutDifference(const LayerDifference& layerDiff, const std::string& layerID) { + if (layerDiff.added.count(layerID)) + return true; + const auto it = layerDiff.changed.find(layerID); + if (it == layerDiff.changed.end()) + return false; + return it->second[0]->hasLayoutDifference(*it->second[1]); +} + } // namespace mbgl diff --git a/src/mbgl/renderer/style_diff.hpp b/src/mbgl/renderer/style_diff.hpp index 0ebb1fbfd6..285d1e12ed 100644 --- a/src/mbgl/renderer/style_diff.hpp +++ b/src/mbgl/renderer/style_diff.hpp @@ -1,3 +1,5 @@ +#pragma once + #include #include #include @@ -13,7 +15,7 @@ class StyleDifference { public: std::unordered_map added; std::unordered_map removed; - std::unordered_map changed; + std::unordered_map> changed; }; using ImmutableImage = Immutable; @@ -34,4 +36,6 @@ using LayerDifference = StyleDifference; LayerDifference diffLayers(const std::vector&, const std::vector&); +bool hasLayoutDifference(const LayerDifference&, const std::string& layerID); + } // namespace mbgl diff --git a/src/mbgl/renderer/tile_parameters.hpp b/src/mbgl/renderer/tile_parameters.hpp index 8f04baaec5..b4a84ec6c8 100644 --- a/src/mbgl/renderer/tile_parameters.hpp +++ b/src/mbgl/renderer/tile_parameters.hpp @@ -8,30 +8,11 @@ class TransformState; class Scheduler; class FileSource; class AnnotationManager; - -namespace style { -class Style; -} // namespace style +class SpriteAtlas; +class GlyphAtlas; class TileParameters { public: - TileParameters(float pixelRatio_, - MapDebugOptions debugOptions_, - const TransformState& transformState_, - Scheduler& workerScheduler_, - FileSource& fileSource_, - const MapMode mode_, - AnnotationManager& annotationManager_, - style::Style& style_) - : pixelRatio(pixelRatio_), - debugOptions(debugOptions_), - transformState(transformState_), - workerScheduler(workerScheduler_), - fileSource(fileSource_), - mode(mode_), - annotationManager(annotationManager_), - style(style_) {} - float pixelRatio; MapDebugOptions debugOptions; const TransformState& transformState; @@ -39,9 +20,8 @@ public: FileSource& fileSource; const MapMode mode; AnnotationManager& annotationManager; - - // TODO: remove - style::Style& style; + SpriteAtlas& spriteAtlas; + GlyphAtlas& glyphAtlas; }; } // namespace mbgl diff --git a/src/mbgl/renderer/tile_pyramid.cpp b/src/mbgl/renderer/tile_pyramid.cpp index d55f7938dd..7d9d7c88ca 100644 --- a/src/mbgl/renderer/tile_pyramid.cpp +++ b/src/mbgl/renderer/tile_pyramid.cpp @@ -39,12 +39,6 @@ bool TilePyramid::isLoaded() const { return true; } -void TilePyramid::invalidateTiles() { - tiles.clear(); - renderTiles.clear(); - cache.clear(); -} - void TilePyramid::startRender(const mat4& projMatrix, const mat4& clipMatrix, const TransformState& transform) { @@ -67,11 +61,34 @@ std::map& TilePyramid::getRenderTiles() { return renderTiles; } -void TilePyramid::updateTiles(const TileParameters& parameters, - const SourceType type, - const uint16_t tileSize, - const Range zoomRange, - std::function (const OverscaledTileID&)> createTile) { +void TilePyramid::update(const std::vector>& layers, + const bool needsRendering, + const bool needsRelayout, + const TileParameters& parameters, + const SourceType type, + const uint16_t tileSize, + const Range zoomRange, + std::function (const OverscaledTileID&)> createTile) { + // If we need a relayout, abandon any cached tiles; they're now stale. + if (needsRelayout) { + cache.clear(); + } + + // If we're not going to render anything, move our existing tiles into + // the cache (if they're not stale) or abandon them, and return. + if (!needsRendering) { + if (!needsRelayout) { + for (auto& entry : tiles) { + cache.add(entry.first, std::move(entry.second)); + } + } + + tiles.clear(); + renderTiles.clear(); + + return; + } + // Determine the overzooming/underzooming amounts and required tiles. int32_t overscaledZoom = util::coveringZoomLevel(parameters.transformState.getZoom(), type, tileSize); int32_t tileZoom = overscaledZoom; @@ -97,6 +114,9 @@ void TilePyramid::updateTiles(const TileParameters& parameters, auto retainTileFn = [&](Tile& tile, Resource::Necessity necessity) -> void { retain.emplace(tile.id); tile.setNecessity(necessity); + if (needsRelayout) { + tile.setLayers(layers); + } }; auto getTileFn = [&](const OverscaledTileID& tileID) -> Tile* { auto it = tiles.find(tileID); @@ -108,6 +128,7 @@ void TilePyramid::updateTiles(const TileParameters& parameters, tile = createTile(tileID); if (tile) { tile->setObserver(observer); + tile->setLayers(layers); } } if (!tile) { @@ -163,21 +184,6 @@ void TilePyramid::removeStaleTiles(const std::set& retain) { } } -void TilePyramid::removeTiles() { - renderTiles.clear(); - if (!tiles.empty()) { - removeStaleTiles({}); - } -} - -void TilePyramid::reloadTiles() { - cache.clear(); - - for (auto& pair : tiles) { - pair.second->redoLayout(); - } -} - std::unordered_map> TilePyramid::queryRenderedFeatures(const ScreenLineString& geometry, const TransformState& transformState, const style::Style& style, diff --git a/src/mbgl/renderer/tile_pyramid.hpp b/src/mbgl/renderer/tile_pyramid.hpp index a2657c92f2..b99c94d065 100644 --- a/src/mbgl/renderer/tile_pyramid.hpp +++ b/src/mbgl/renderer/tile_pyramid.hpp @@ -5,6 +5,7 @@ #include #include #include +#include #include #include @@ -35,23 +36,14 @@ public: bool isLoaded() const; - // Called when the camera has changed. May load new tiles, unload obsolete tiles, or - // trigger re-placement of existing complete tiles. - void updateTiles(const TileParameters&, - SourceType type, - uint16_t tileSize, - Range zoomRange, - std::function (const OverscaledTileID&)> createTile); - - // Removes all tiles (by putting them into the cache). - void removeTiles(); - - // Remove all tiles and clear the cache. - void invalidateTiles(); - - // Request that all loaded tiles re-run the layout operation on the existing source - // data with fresh style information. - void reloadTiles(); + void update(const std::vector>&, + bool needsRendering, + bool needsRelayout, + const TileParameters&, + SourceType type, + uint16_t tileSize, + Range zoomRange, + std::function (const OverscaledTileID&)> createTile); void startRender(const mat4& projMatrix, const mat4& clipMatrix, diff --git a/src/mbgl/style/layer_impl.hpp b/src/mbgl/style/layer_impl.hpp index 569ef65906..f350044925 100644 --- a/src/mbgl/style/layer_impl.hpp +++ b/src/mbgl/style/layer_impl.hpp @@ -34,6 +34,10 @@ public: Impl& operator=(const Impl&) = delete; + // Returns true buckets if properties affecting layout have changed: i.e. filter, + // visibility, layout properties, or data-driven paint properties. + virtual bool hasLayoutDifference(const Layer::Impl&) const = 0; + // Utility function for automatic layer grouping. virtual void stringifyLayout(rapidjson::Writer&) const = 0; diff --git a/src/mbgl/style/layers/background_layer_impl.cpp b/src/mbgl/style/layers/background_layer_impl.cpp index 4b4f44a8d8..a59a84fbe9 100644 --- a/src/mbgl/style/layers/background_layer_impl.cpp +++ b/src/mbgl/style/layers/background_layer_impl.cpp @@ -3,5 +3,9 @@ namespace mbgl { namespace style { +bool BackgroundLayer::Impl::hasLayoutDifference(const Layer::Impl&) const { + return false; +} + } // namespace style } // namespace mbgl diff --git a/src/mbgl/style/layers/background_layer_impl.hpp b/src/mbgl/style/layers/background_layer_impl.hpp index 4f14662658..248a751027 100644 --- a/src/mbgl/style/layers/background_layer_impl.hpp +++ b/src/mbgl/style/layers/background_layer_impl.hpp @@ -11,6 +11,7 @@ class BackgroundLayer::Impl : public Layer::Impl { public: using Layer::Impl::Impl; + bool hasLayoutDifference(const Layer::Impl&) const override; void stringifyLayout(rapidjson::Writer&) const override; BackgroundPaintProperties::Transitionable paint; diff --git a/src/mbgl/style/layers/circle_layer_impl.cpp b/src/mbgl/style/layers/circle_layer_impl.cpp index cdb371bd58..69f574cd6b 100644 --- a/src/mbgl/style/layers/circle_layer_impl.cpp +++ b/src/mbgl/style/layers/circle_layer_impl.cpp @@ -3,5 +3,13 @@ namespace mbgl { namespace style { +bool CircleLayer::Impl::hasLayoutDifference(const Layer::Impl& other) const { + assert(dynamic_cast(&other)); + const auto& impl = static_cast(other); + return filter != impl.filter || + visibility != impl.visibility || + paint.hasDataDrivenPropertyDifference(impl.paint); +} + } // namespace style } // namespace mbgl diff --git a/src/mbgl/style/layers/circle_layer_impl.hpp b/src/mbgl/style/layers/circle_layer_impl.hpp index 79ef879ab9..4b148cdc42 100644 --- a/src/mbgl/style/layers/circle_layer_impl.hpp +++ b/src/mbgl/style/layers/circle_layer_impl.hpp @@ -11,6 +11,7 @@ class CircleLayer::Impl : public Layer::Impl { public: using Layer::Impl::Impl; + bool hasLayoutDifference(const Layer::Impl&) const override; void stringifyLayout(rapidjson::Writer&) const override; CirclePaintProperties::Transitionable paint; diff --git a/src/mbgl/style/layers/custom_layer_impl.cpp b/src/mbgl/style/layers/custom_layer_impl.cpp index e2ed00a10d..10dd058861 100644 --- a/src/mbgl/style/layers/custom_layer_impl.cpp +++ b/src/mbgl/style/layers/custom_layer_impl.cpp @@ -16,6 +16,10 @@ CustomLayer::Impl::Impl(const std::string& id_, context = context_; } +bool CustomLayer::Impl::hasLayoutDifference(const Layer::Impl&) const { + return false; +} + void CustomLayer::Impl::stringifyLayout(rapidjson::Writer&) const { } diff --git a/src/mbgl/style/layers/custom_layer_impl.hpp b/src/mbgl/style/layers/custom_layer_impl.hpp index 26a75f5834..466d1f3e3f 100644 --- a/src/mbgl/style/layers/custom_layer_impl.hpp +++ b/src/mbgl/style/layers/custom_layer_impl.hpp @@ -22,6 +22,7 @@ public: void render(const TransformState&) const; private: + bool hasLayoutDifference(const Layer::Impl&) const override; void stringifyLayout(rapidjson::Writer&) const override; CustomLayerInitializeFunction initializeFn = nullptr; diff --git a/src/mbgl/style/layers/fill_extrusion_layer_impl.cpp b/src/mbgl/style/layers/fill_extrusion_layer_impl.cpp index 07df4cd331..d37c2ad29b 100644 --- a/src/mbgl/style/layers/fill_extrusion_layer_impl.cpp +++ b/src/mbgl/style/layers/fill_extrusion_layer_impl.cpp @@ -3,5 +3,13 @@ namespace mbgl { namespace style { +bool FillExtrusionLayer::Impl::hasLayoutDifference(const Layer::Impl& other) const { + assert(dynamic_cast(&other)); + const auto& impl = static_cast(other); + return filter != impl.filter || + visibility != impl.visibility || + paint.hasDataDrivenPropertyDifference(impl.paint); +} + } // namespace style } // namespace mbgl diff --git a/src/mbgl/style/layers/fill_extrusion_layer_impl.hpp b/src/mbgl/style/layers/fill_extrusion_layer_impl.hpp index 73c65469d1..9abc6fc4b3 100644 --- a/src/mbgl/style/layers/fill_extrusion_layer_impl.hpp +++ b/src/mbgl/style/layers/fill_extrusion_layer_impl.hpp @@ -11,6 +11,7 @@ class FillExtrusionLayer::Impl : public Layer::Impl { public: using Layer::Impl::Impl; + bool hasLayoutDifference(const Layer::Impl&) const override; void stringifyLayout(rapidjson::Writer&) const override; FillExtrusionPaintProperties::Transitionable paint; diff --git a/src/mbgl/style/layers/fill_layer_impl.cpp b/src/mbgl/style/layers/fill_layer_impl.cpp index 38672c5bfe..0dc7ed14d5 100644 --- a/src/mbgl/style/layers/fill_layer_impl.cpp +++ b/src/mbgl/style/layers/fill_layer_impl.cpp @@ -3,5 +3,13 @@ namespace mbgl { namespace style { +bool FillLayer::Impl::hasLayoutDifference(const Layer::Impl& other) const { + assert(dynamic_cast(&other)); + const auto& impl = static_cast(other); + return filter != impl.filter || + visibility != impl.visibility || + paint.hasDataDrivenPropertyDifference(impl.paint); +} + } // namespace style } // namespace mbgl diff --git a/src/mbgl/style/layers/fill_layer_impl.hpp b/src/mbgl/style/layers/fill_layer_impl.hpp index 72ea8ab352..2673cd7443 100644 --- a/src/mbgl/style/layers/fill_layer_impl.hpp +++ b/src/mbgl/style/layers/fill_layer_impl.hpp @@ -11,6 +11,7 @@ class FillLayer::Impl : public Layer::Impl { public: using Layer::Impl::Impl; + bool hasLayoutDifference(const Layer::Impl&) const override; void stringifyLayout(rapidjson::Writer&) const override; FillPaintProperties::Transitionable paint; diff --git a/src/mbgl/style/layers/line_layer_impl.cpp b/src/mbgl/style/layers/line_layer_impl.cpp index 16a164eb9d..bee88d6a47 100644 --- a/src/mbgl/style/layers/line_layer_impl.cpp +++ b/src/mbgl/style/layers/line_layer_impl.cpp @@ -3,5 +3,14 @@ namespace mbgl { namespace style { +bool LineLayer::Impl::hasLayoutDifference(const Layer::Impl& other) const { + assert(dynamic_cast(&other)); + const auto& impl = static_cast(other); + return filter != impl.filter || + visibility != impl.visibility || + layout != impl.layout || + paint.hasDataDrivenPropertyDifference(impl.paint); +} + } // namespace style } // namespace mbgl diff --git a/src/mbgl/style/layers/line_layer_impl.hpp b/src/mbgl/style/layers/line_layer_impl.hpp index 333821dc17..04adc0e85c 100644 --- a/src/mbgl/style/layers/line_layer_impl.hpp +++ b/src/mbgl/style/layers/line_layer_impl.hpp @@ -11,6 +11,7 @@ class LineLayer::Impl : public Layer::Impl { public: using Layer::Impl::Impl; + bool hasLayoutDifference(const Layer::Impl&) const override; void stringifyLayout(rapidjson::Writer&) const override; LineLayoutProperties::Unevaluated layout; diff --git a/src/mbgl/style/layers/raster_layer_impl.cpp b/src/mbgl/style/layers/raster_layer_impl.cpp index 1c2204c5d7..a995f50dd3 100644 --- a/src/mbgl/style/layers/raster_layer_impl.cpp +++ b/src/mbgl/style/layers/raster_layer_impl.cpp @@ -3,5 +3,9 @@ namespace mbgl { namespace style { +bool RasterLayer::Impl::hasLayoutDifference(const Layer::Impl&) const { + return false; +} + } // namespace style } // namespace mbgl diff --git a/src/mbgl/style/layers/raster_layer_impl.hpp b/src/mbgl/style/layers/raster_layer_impl.hpp index 2178fd69d4..adbe703e92 100644 --- a/src/mbgl/style/layers/raster_layer_impl.hpp +++ b/src/mbgl/style/layers/raster_layer_impl.hpp @@ -11,6 +11,7 @@ class RasterLayer::Impl : public Layer::Impl { public: using Layer::Impl::Impl; + bool hasLayoutDifference(const Layer::Impl&) const override; void stringifyLayout(rapidjson::Writer&) const override; RasterPaintProperties::Transitionable paint; diff --git a/src/mbgl/style/layers/symbol_layer_impl.cpp b/src/mbgl/style/layers/symbol_layer_impl.cpp index 3f0ab9b8fb..b59768725d 100644 --- a/src/mbgl/style/layers/symbol_layer_impl.cpp +++ b/src/mbgl/style/layers/symbol_layer_impl.cpp @@ -3,5 +3,14 @@ namespace mbgl { namespace style { +bool SymbolLayer::Impl::hasLayoutDifference(const Layer::Impl& other) const { + assert(dynamic_cast(&other)); + const auto& impl = static_cast(other); + return filter != impl.filter || + visibility != impl.visibility || + layout != impl.layout || + paint.hasDataDrivenPropertyDifference(impl.paint); +} + } // namespace style } // namespace mbgl diff --git a/src/mbgl/style/layers/symbol_layer_impl.hpp b/src/mbgl/style/layers/symbol_layer_impl.hpp index 7b9415436d..f8ef87dcdf 100644 --- a/src/mbgl/style/layers/symbol_layer_impl.hpp +++ b/src/mbgl/style/layers/symbol_layer_impl.hpp @@ -11,6 +11,7 @@ class SymbolLayer::Impl : public Layer::Impl { public: using Layer::Impl::Impl; + bool hasLayoutDifference(const Layer::Impl&) const override; void stringifyLayout(rapidjson::Writer&) const override; SymbolLayoutProperties::Unevaluated layout; diff --git a/src/mbgl/style/properties.hpp b/src/mbgl/style/properties.hpp index 678d38563a..24ac030541 100644 --- a/src/mbgl/style/properties.hpp +++ b/src/mbgl/style/properties.hpp @@ -205,6 +205,12 @@ public: .transition(parameters, std::move(prior.template get()))... }; } + + bool hasDataDrivenPropertyDifference(const Transitionable& other) const { + bool result = false; + util::ignore({ (result |= this->template get().value.hasDataDrivenPropertyDifference(other.template get().value))... }); + return result; + } }; }; diff --git a/src/mbgl/style/style.cpp b/src/mbgl/style/style.cpp index 74ce401292..4acafba0ee 100644 --- a/src/mbgl/style/style.cpp +++ b/src/mbgl/style/style.cpp @@ -54,21 +54,6 @@ namespace style { static Observer nullObserver; -struct QueueSourceReloadVisitor { - UpdateBatch& updateBatch; - - // No need to reload sources for these types; their visibility can change but - // they don't participate in layout. - void operator()(CustomLayer&) {} - void operator()(RasterLayer&) {} - void operator()(BackgroundLayer&) {} - - template - void operator()(VectorLayer& layer) { - updateBatch.sourceIDs.insert(layer.getSourceID()); - } -}; - Style::Style(Scheduler& scheduler_, FileSource& fileSource_, float pixelRatio) : scheduler(scheduler_), fileSource(fileSource_), @@ -105,7 +90,6 @@ void Style::setJSON(const std::string& json) { renderSources.clear(); layers.clear(); transitionOptions = {}; - updateBatch = {}; Parser parser; auto error = parser.parse(json); @@ -193,7 +177,6 @@ std::unique_ptr Style::removeSource(const std::string& id) { auto source = std::move(*it); source->setObserver(nullptr); sources.erase(it); - updateBatch.sourceIDs.erase(id); return source; } @@ -244,7 +227,6 @@ Layer* Style::addLayer(std::unique_ptr layer, optional befor } layer->setObserver(this); - layer->accept(QueueSourceReloadVisitor { updateBatch }); return layers.emplace(before ? findLayer(*before) : layers.end(), std::move(layer))->get(); } @@ -339,14 +321,17 @@ void Style::update(const UpdateParameters& parameters) { parameters.mode == MapMode::Continuous ? util::DEFAULT_FADE_DURATION : Duration::zero() }; - const TileParameters tileParameters(parameters.pixelRatio, - parameters.debugOptions, - parameters.transformState, - parameters.scheduler, - parameters.fileSource, - parameters.mode, - parameters.annotationManager, - *this); + const TileParameters tileParameters { + parameters.pixelRatio, + parameters.debugOptions, + parameters.transformState, + parameters.scheduler, + parameters.fileSource, + parameters.mode, + parameters.annotationManager, + *spriteAtlas, + *glyphAtlas + }; // Update light. const bool lightChanged = renderLight.impl != light->impl; @@ -382,7 +367,7 @@ void Style::update(const UpdateParameters& parameters) { // Update changed images. for (const auto& entry : imageDiff.changed) { - spriteAtlas->updateImage(entry.second); + spriteAtlas->updateImage(entry.second[1]); } if (spriteLoaded && !spriteAtlas->isLoaded()) { @@ -396,26 +381,6 @@ void Style::update(const UpdateParameters& parameters) { newSourceImpls.push_back(source->baseImpl); } - const SourceDifference sourceDiff = diffSources(sourceImpls, newSourceImpls); - sourceImpls = std::move(newSourceImpls); - - // Remove render layers for removed sources. - for (const auto& entry : sourceDiff.removed) { - renderLayers.erase(entry.first); - } - - // Create render sources for newly added sources. - for (const auto& entry : sourceDiff.added) { - std::unique_ptr renderSource = RenderSource::create(entry.second); - renderSource->setObserver(this); - renderSources.emplace(entry.first, std::move(renderSource)); - } - - // Update render sources for changed sources. - for (const auto& entry : sourceDiff.changed) { - renderSources.at(entry.first)->setImpl(entry.second); - } - std::vector> newLayerImpls; newLayerImpls.reserve(layers.size()); @@ -438,7 +403,7 @@ void Style::update(const UpdateParameters& parameters) { // Update render layers for changed layers. for (const auto& entry : layerDiff.changed) { - renderLayers.at(entry.first)->setImpl(entry.second); + renderLayers.at(entry.first)->setImpl(entry.second[1]); } // Update layers for class and zoom changes. @@ -457,35 +422,51 @@ void Style::update(const UpdateParameters& parameters) { } - // Update tiles for each source. - for (const auto& entry : renderSources) { - entry.second->enabled = false; + const SourceDifference sourceDiff = diffSources(sourceImpls, newSourceImpls); + sourceImpls = std::move(newSourceImpls); + + // Remove render layers for removed sources. + for (const auto& entry : sourceDiff.removed) { + renderSources.erase(entry.first); } - for (const auto& entry : renderLayers) { - RenderLayer& layer = *entry.second; - if (layer.needsRendering(zoomHistory.lastZoom)) { - if (RenderSource* source = getRenderSource(layer.baseImpl->source)) { - source->enabled = true; - } - } + // Create render sources for newly added sources. + for (const auto& entry : sourceDiff.added) { + std::unique_ptr renderSource = RenderSource::create(entry.second); + renderSource->setObserver(this); + renderSources.emplace(entry.first, std::move(renderSource)); } - for (const auto& entry : renderSources) { - bool updated = updateBatch.sourceIDs.count(entry.first); - if (entry.second->enabled) { - if (updated) { - entry.second->reloadTiles(); + // Update all sources. + for (const auto& source : sourceImpls) { + std::vector> filteredLayers; + bool needsRendering = false; + bool needsRelayout = false; + + for (const auto& layer : layerImpls) { + if (layer->type == LayerType::Background || + layer->type == LayerType::Custom || + layer->source != source->id) { + continue; + } + + if (getRenderLayer(layer->id)->needsRendering(zoomHistory.lastZoom)) { + needsRendering = true; + } + + if (hasLayoutDifference(layerDiff, layer->id)) { + needsRelayout = true; } - entry.second->updateTiles(tileParameters); - } else if (updated) { - entry.second->invalidateTiles(); - } else { - entry.second->removeTiles(); + + filteredLayers.push_back(layer); } - } - updateBatch.sourceIDs.clear(); + renderSources.at(source->id)->update(source, + filteredLayers, + needsRendering, + needsRelayout, + tileParameters); + } } std::vector Style::getSources() const { @@ -580,7 +561,7 @@ RenderData Style::getRenderData(MapDebugOptions debugOptions, float angle) const RenderData result; for (const auto& entry : renderSources) { - if (entry.second->enabled) { + if (entry.second->isEnabled()) { result.sources.insert(entry.second.get()); } } @@ -801,13 +782,11 @@ void Style::onSpriteError(std::exception_ptr error) { observer->onResourceError(error); } -void Style::onLayerFilterChanged(Layer& layer) { - layer.accept(QueueSourceReloadVisitor { updateBatch }); +void Style::onLayerFilterChanged(Layer&) { observer->onUpdate(Update::Repaint); } -void Style::onLayerVisibilityChanged(Layer& layer) { - layer.accept(QueueSourceReloadVisitor { updateBatch }); +void Style::onLayerVisibilityChanged(Layer&) { observer->onUpdate(Update::Repaint); } @@ -815,13 +794,11 @@ void Style::onLayerPaintPropertyChanged(Layer&) { observer->onUpdate(Update::Repaint); } -void Style::onLayerDataDrivenPaintPropertyChanged(Layer& layer) { - layer.accept(QueueSourceReloadVisitor { updateBatch }); +void Style::onLayerDataDrivenPaintPropertyChanged(Layer&) { observer->onUpdate(Update::Repaint); } -void Style::onLayerLayoutPropertyChanged(Layer& layer, const char *) { - layer.accept(QueueSourceReloadVisitor { updateBatch }); +void Style::onLayerLayoutPropertyChanged(Layer&, const char *) { observer->onUpdate(Update::Repaint); } diff --git a/src/mbgl/style/style.hpp b/src/mbgl/style/style.hpp index 2d0b9eddbb..9138637894 100644 --- a/src/mbgl/style/style.hpp +++ b/src/mbgl/style/style.hpp @@ -5,7 +5,6 @@ #include #include #include -#include #include #include #include @@ -179,7 +178,6 @@ private: std::exception_ptr lastError; - UpdateBatch updateBatch; ZoomHistory zoomHistory; bool spriteLoaded = false; diff --git a/src/mbgl/style/update_batch.hpp b/src/mbgl/style/update_batch.hpp deleted file mode 100644 index 562df52afa..0000000000 --- a/src/mbgl/style/update_batch.hpp +++ /dev/null @@ -1,15 +0,0 @@ -#pragma once - -#include -#include - -namespace mbgl { -namespace style { - -class UpdateBatch { -public: - std::unordered_set sourceIDs; -}; - -} // namespace style -} // namespace mbgl diff --git a/src/mbgl/tile/geojson_tile.cpp b/src/mbgl/tile/geojson_tile.cpp index 3c4939a0a6..17067571da 100644 --- a/src/mbgl/tile/geojson_tile.cpp +++ b/src/mbgl/tile/geojson_tile.cpp @@ -1,7 +1,6 @@ #include #include #include -#include #include #include @@ -86,9 +85,7 @@ GeoJSONTile::GeoJSONTile(const OverscaledTileID& overscaledTileID, std::string sourceID_, const TileParameters& parameters, mapbox::geometry::feature_collection features) - : GeometryTile(overscaledTileID, sourceID_, parameters, - *parameters.style.glyphAtlas, - *parameters.style.spriteAtlas) { + : GeometryTile(overscaledTileID, sourceID_, parameters) { updateData(std::move(features)); } diff --git a/src/mbgl/tile/geometry_tile.cpp b/src/mbgl/tile/geometry_tile.cpp index df58294e90..1a60549eb0 100644 --- a/src/mbgl/tile/geometry_tile.cpp +++ b/src/mbgl/tile/geometry_tile.cpp @@ -10,7 +10,6 @@ #include #include #include -#include #include #include #include @@ -29,12 +28,9 @@ using namespace style; GeometryTile::GeometryTile(const OverscaledTileID& id_, std::string sourceID_, - const TileParameters& parameters, - GlyphAtlas& glyphAtlas_, - SpriteAtlas& spriteAtlas_) + const TileParameters& parameters) : Tile(id_), sourceID(std::move(sourceID_)), - style(parameters.style), mailbox(std::make_shared(*util::RunLoop::Get())), worker(parameters.workerScheduler, ActorRef(*this, mailbox), @@ -42,8 +38,8 @@ GeometryTile::GeometryTile(const OverscaledTileID& id_, obsolete, parameters.mode, parameters.pixelRatio), - glyphAtlas(glyphAtlas_), - spriteAtlas(spriteAtlas_), + glyphAtlas(parameters.glyphAtlas), + spriteAtlas(parameters.spriteAtlas), placementThrottler(Milliseconds(300), [this] { invokePlacement(); }) { } @@ -74,7 +70,6 @@ void GeometryTile::setData(std::unique_ptr data_) { ++correlationID; worker.invoke(&GeometryTileWorker::setData, std::move(data_), correlationID); - redoLayout(); } void GeometryTile::setPlacementConfig(const PlacementConfig& desiredConfig) { @@ -97,25 +92,25 @@ void GeometryTile::invokePlacement() { } } -void GeometryTile::redoLayout() { +void GeometryTile::setLayers(const std::vector>& layers) { // Mark the tile as pending again if it was complete before to prevent signaling a complete // state despite pending parse operations. pending = true; std::vector> impls; - for (const Layer* layer : style.getLayers()) { + for (const auto& layer : layers) { // Skip irrelevant layers. - if (layer->is() || - layer->is() || - layer->baseImpl->source != sourceID || - id.overscaledZ < std::floor(layer->baseImpl->minZoom) || - id.overscaledZ >= std::ceil(layer->baseImpl->maxZoom) || - layer->baseImpl->visibility == VisibilityType::None) { + if (layer->type == LayerType::Background || + layer->type == LayerType::Custom || + layer->source != sourceID || + id.overscaledZ < std::floor(layer->minZoom) || + id.overscaledZ >= std::ceil(layer->maxZoom) || + layer->visibility == VisibilityType::None) { continue; } - impls.push_back(layer->baseImpl); + impls.push_back(layer); } ++correlationID; @@ -181,7 +176,7 @@ void GeometryTile::queryRenderedFeatures( std::unordered_map>& result, const GeometryCoordinates& queryGeometry, const TransformState& transformState, - const style::Style& style_, + const style::Style& style, const RenderedQueryOptions& options) { if (!featureIndex || !data) return; @@ -194,7 +189,7 @@ void GeometryTile::queryRenderedFeatures( options, *data, id.canonical, - style_, + style, collisionTile.get(), *this); } diff --git a/src/mbgl/tile/geometry_tile.hpp b/src/mbgl/tile/geometry_tile.hpp index 807c8474de..7fd8d8bc77 100644 --- a/src/mbgl/tile/geometry_tile.hpp +++ b/src/mbgl/tile/geometry_tile.hpp @@ -23,17 +23,11 @@ class RenderLayer; class SourceQueryOptions; class TileParameters; -namespace style { -class Style; -} // namespace style - class GeometryTile : public Tile, public GlyphRequestor, IconRequestor { public: GeometryTile(const OverscaledTileID&, std::string sourceID, - const TileParameters&, - GlyphAtlas&, - SpriteAtlas&); + const TileParameters&); ~GeometryTile() override; @@ -41,7 +35,7 @@ public: void setData(std::unique_ptr); void setPlacementConfig(const PlacementConfig&) override; - void redoLayout() override; + void setLayers(const std::vector>&) override; void onGlyphsAvailable(GlyphPositionMap) override; void onIconsAvailable(IconMap) override; @@ -94,9 +88,6 @@ private: const std::string sourceID; - // TODO: remove - style::Style& style; - // Used to signal the worker that it should abandon parsing this tile as soon as possible. std::atomic obsolete { false }; diff --git a/src/mbgl/tile/tile.hpp b/src/mbgl/tile/tile.hpp index dfd35cde9a..91ed4aadaa 100644 --- a/src/mbgl/tile/tile.hpp +++ b/src/mbgl/tile/tile.hpp @@ -51,7 +51,7 @@ public: virtual Bucket* getBucket(const style::Layer::Impl&) const = 0; virtual void setPlacementConfig(const PlacementConfig&) {} - virtual void redoLayout() {} + virtual void setLayers(const std::vector>&) {} virtual void queryRenderedFeatures( std::unordered_map>& result, diff --git a/src/mbgl/tile/vector_tile.cpp b/src/mbgl/tile/vector_tile.cpp index 67b97c1c05..f6f8de72d1 100644 --- a/src/mbgl/tile/vector_tile.cpp +++ b/src/mbgl/tile/vector_tile.cpp @@ -1,7 +1,6 @@ #include #include #include -#include #include #include @@ -85,9 +84,7 @@ VectorTile::VectorTile(const OverscaledTileID& id_, std::string sourceID_, const TileParameters& parameters, const Tileset& tileset) - : GeometryTile(id_, sourceID_, parameters, - *parameters.style.glyphAtlas, - *parameters.style.spriteAtlas), + : GeometryTile(id_, sourceID_, parameters), loader(*this, id_, parameters, tileset) { } diff --git a/test/style/source.test.cpp b/test/style/source.test.cpp index 84929178d0..0ab98a63b1 100644 --- a/test/style/source.test.cpp +++ b/test/style/source.test.cpp @@ -8,6 +8,8 @@ #include #include #include +#include +#include #include #include @@ -27,12 +29,10 @@ #include #include -#include -#include #include #include - -#include +#include +#include #include @@ -48,7 +48,8 @@ public: TransformState transformState; ThreadPool threadPool { 1 }; AnnotationManager annotationManager; - style::Style style { threadPool, fileSource, 1.0 }; + SpriteAtlas spriteAtlas; + GlyphAtlas glyphAtlas { { 512, 512, }, fileSource }; TileParameters tileParameters { 1.0, @@ -58,7 +59,8 @@ public: fileSource, MapMode::Continuous, annotationManager, - style + spriteAtlas, + glyphAtlas }; SourceTest() { @@ -137,6 +139,9 @@ TEST(Source, RasterTileEmpty) { return response; }; + RasterLayer layer("id", "source"); + std::vector> layers {{ layer.baseImpl }}; + Tileset tileset; tileset.tiles = { "tiles" }; @@ -154,7 +159,11 @@ TEST(Source, RasterTileEmpty) { auto renderSource = RenderSource::create(source.baseImpl); renderSource->setObserver(&test.renderSourceObserver); - renderSource->updateTiles(test.tileParameters); + renderSource->update(source.baseImpl, + layers, + true, + true, + test.tileParameters); test.run(); } @@ -168,6 +177,11 @@ TEST(Source, VectorTileEmpty) { return response; }; + LineLayer layer("id", "source"); + layer.setSourceLayer("water"); + + std::vector> layers {{ layer.baseImpl }}; + Tileset tileset; tileset.tiles = { "tiles" }; @@ -185,7 +199,11 @@ TEST(Source, VectorTileEmpty) { auto renderSource = RenderSource::create(source.baseImpl); renderSource->setObserver(&test.renderSourceObserver); - renderSource->updateTiles(test.tileParameters); + renderSource->update(source.baseImpl, + layers, + true, + true, + test.tileParameters); test.run(); } @@ -201,6 +219,9 @@ TEST(Source, RasterTileFail) { return response; }; + RasterLayer layer("id", "source"); + std::vector> layers {{ layer.baseImpl }}; + Tileset tileset; tileset.tiles = { "tiles" }; @@ -216,7 +237,11 @@ TEST(Source, RasterTileFail) { auto renderSource = RenderSource::create(source.baseImpl); renderSource->setObserver(&test.renderSourceObserver); - renderSource->updateTiles(test.tileParameters); + renderSource->update(source.baseImpl, + layers, + true, + true, + test.tileParameters); test.run(); } @@ -232,6 +257,11 @@ TEST(Source, VectorTileFail) { return response; }; + LineLayer layer("id", "source"); + layer.setSourceLayer("water"); + + std::vector> layers {{ layer.baseImpl }}; + Tileset tileset; tileset.tiles = { "tiles" }; @@ -247,7 +277,11 @@ TEST(Source, VectorTileFail) { auto renderSource = RenderSource::create(source.baseImpl); renderSource->setObserver(&test.renderSourceObserver); - renderSource->updateTiles(test.tileParameters); + renderSource->update(source.baseImpl, + layers, + true, + true, + test.tileParameters); test.run(); } @@ -261,6 +295,9 @@ TEST(Source, RasterTileCorrupt) { return response; }; + RasterLayer layer("id", "source"); + std::vector> layers {{ layer.baseImpl }}; + Tileset tileset; tileset.tiles = { "tiles" }; @@ -277,7 +314,11 @@ TEST(Source, RasterTileCorrupt) { auto renderSource = RenderSource::create(source.baseImpl); renderSource->setObserver(&test.renderSourceObserver); - renderSource->updateTiles(test.tileParameters); + renderSource->update(source.baseImpl, + layers, + true, + true, + test.tileParameters); test.run(); } @@ -291,10 +332,10 @@ TEST(Source, VectorTileCorrupt) { return response; }; - // Need to have at least one layer that uses the source. - auto layer = std::make_unique("id", "source"); - layer->setSourceLayer("water"); - test.style.addLayer(std::move(layer)); + LineLayer layer("id", "source"); + layer.setSourceLayer("water"); + + std::vector> layers {{ layer.baseImpl }}; Tileset tileset; tileset.tiles = { "tiles" }; @@ -311,7 +352,11 @@ TEST(Source, VectorTileCorrupt) { auto renderSource = RenderSource::create(source.baseImpl); renderSource->setObserver(&test.renderSourceObserver); - renderSource->updateTiles(test.tileParameters); + renderSource->update(source.baseImpl, + layers, + true, + true, + test.tileParameters); test.run(); } @@ -324,6 +369,9 @@ TEST(Source, RasterTileCancel) { return optional(); }; + RasterLayer layer("id", "source"); + std::vector> layers {{ layer.baseImpl }}; + Tileset tileset; tileset.tiles = { "tiles" }; @@ -340,7 +388,11 @@ TEST(Source, RasterTileCancel) { auto renderSource = RenderSource::create(source.baseImpl); renderSource->setObserver(&test.renderSourceObserver); - renderSource->updateTiles(test.tileParameters); + renderSource->update(source.baseImpl, + layers, + true, + true, + test.tileParameters); test.run(); } @@ -353,6 +405,11 @@ TEST(Source, VectorTileCancel) { return optional(); }; + LineLayer layer("id", "source"); + layer.setSourceLayer("water"); + + std::vector> layers {{ layer.baseImpl }}; + Tileset tileset; tileset.tiles = { "tiles" }; @@ -369,7 +426,11 @@ TEST(Source, VectorTileCancel) { auto renderSource = RenderSource::create(source.baseImpl); renderSource->setObserver(&test.renderSourceObserver); - renderSource->updateTiles(test.tileParameters); + renderSource->update(source.baseImpl, + layers, + true, + true, + test.tileParameters); test.run(); } @@ -377,6 +438,9 @@ TEST(Source, VectorTileCancel) { TEST(Source, RasterTileAttribution) { SourceTest test; + RasterLayer layer("id", "source"); + std::vector> layers {{ layer.baseImpl }}; + std::string mapboxOSM = ("© Mapbox " "©️ OpenStreetMap"); @@ -406,7 +470,11 @@ TEST(Source, RasterTileAttribution) { source.loadDescription(test.fileSource); auto renderSource = RenderSource::create(source.baseImpl); - renderSource->updateTiles(test.tileParameters); + renderSource->update(source.baseImpl, + layers, + true, + true, + test.tileParameters); test.run(); } diff --git a/test/tile/annotation_tile.test.cpp b/test/tile/annotation_tile.test.cpp index 205a315db7..bfd991c504 100644 --- a/test/tile/annotation_tile.test.cpp +++ b/test/tile/annotation_tile.test.cpp @@ -12,6 +12,8 @@ #include #include #include +#include +#include #include @@ -25,6 +27,8 @@ public: ThreadPool threadPool { 1 }; AnnotationManager annotationManager; style::Style style { threadPool, fileSource, 1.0 }; + SpriteAtlas spriteAtlas; + GlyphAtlas glyphAtlas { { 512, 512, }, fileSource }; TileParameters tileParameters { 1.0, @@ -34,7 +38,8 @@ public: fileSource, MapMode::Continuous, annotationManager, - style + spriteAtlas, + glyphAtlas }; }; diff --git a/test/tile/geojson_tile.test.cpp b/test/tile/geojson_tile.test.cpp index dad4aef2ee..92068d5e43 100644 --- a/test/tile/geojson_tile.test.cpp +++ b/test/tile/geojson_tile.test.cpp @@ -7,10 +7,11 @@ #include #include #include -#include #include #include #include +#include +#include #include @@ -24,7 +25,8 @@ public: util::RunLoop loop; ThreadPool threadPool { 1 }; AnnotationManager annotationManager; - style::Style style { threadPool, fileSource, 1.0 }; + SpriteAtlas spriteAtlas; + GlyphAtlas glyphAtlas { { 512, 512, }, fileSource }; Tileset tileset { { "https://example.com" }, { 0, 22 }, "none" }; TileParameters tileParameters { @@ -35,14 +37,15 @@ public: fileSource, MapMode::Continuous, annotationManager, - style + spriteAtlas, + glyphAtlas }; }; TEST(GeoJSONTile, Issue7648) { GeoJSONTileTest test; - test.style.addLayer(std::make_unique("circle", "source")); + CircleLayer layer("circle", "source"); mapbox::geometry::feature_collection features; features.push_back(mapbox::geometry::feature { @@ -55,9 +58,10 @@ TEST(GeoJSONTile, Issue7648) { observer.tileChanged = [&] (const Tile&) { // Once present, the bucket should never "disappear", which would cause // flickering. - ASSERT_NE(nullptr, tile.getBucket(*test.style.getLayer("circle")->baseImpl)); + ASSERT_NE(nullptr, tile.getBucket(*layer.baseImpl)); }; + tile.setLayers({{ layer.baseImpl }}); tile.setObserver(&observer); tile.setPlacementConfig({}); diff --git a/test/tile/raster_tile.test.cpp b/test/tile/raster_tile.test.cpp index ee6e31d845..1bd35700b3 100644 --- a/test/tile/raster_tile.test.cpp +++ b/test/tile/raster_tile.test.cpp @@ -6,10 +6,11 @@ #include #include #include -#include #include #include #include +#include +#include using namespace mbgl; @@ -20,7 +21,8 @@ public: util::RunLoop loop; ThreadPool threadPool { 1 }; AnnotationManager annotationManager; - style::Style style { threadPool, fileSource, 1.0 }; + SpriteAtlas spriteAtlas; + GlyphAtlas glyphAtlas { { 512, 512, }, fileSource }; Tileset tileset { { "https://example.com" }, { 0, 22 }, "none" }; TileParameters tileParameters { @@ -31,7 +33,8 @@ public: fileSource, MapMode::Continuous, annotationManager, - style + spriteAtlas, + glyphAtlas }; }; diff --git a/test/tile/vector_tile.test.cpp b/test/tile/vector_tile.test.cpp index 205d001f72..a44eeb9001 100644 --- a/test/tile/vector_tile.test.cpp +++ b/test/tile/vector_tile.test.cpp @@ -7,13 +7,14 @@ #include #include #include -#include #include #include #include #include #include #include +#include +#include #include @@ -26,7 +27,8 @@ public: util::RunLoop loop; ThreadPool threadPool { 1 }; AnnotationManager annotationManager; - style::Style style { threadPool, fileSource, 1.0 }; + SpriteAtlas spriteAtlas; + GlyphAtlas glyphAtlas { { 512, 512, }, fileSource }; Tileset tileset { { "https://example.com" }, { 0, 22 }, "none" }; TileParameters tileParameters { @@ -37,7 +39,8 @@ public: fileSource, MapMode::Continuous, annotationManager, - style + spriteAtlas, + glyphAtlas }; }; -- cgit v1.2.1