From 12724f8d2cecff52a812857a8311726bfa7c3926 Mon Sep 17 00:00:00 2001 From: Mikhail Pozdnyakov Date: Tue, 16 Jul 2019 15:12:41 +0300 Subject: [PATCH 1/4] [core] Render sources keep the existing tiles until the new tileset is ready In Continuous map mode, keep the existing tiles if the new tileset is not yet available, thus providing smart style transitions without flickering. --- .../renderer/sources/render_raster_dem_source.cpp | 12 ++++++++---- src/mbgl/renderer/sources/render_raster_source.cpp | 12 ++++++++---- src/mbgl/renderer/sources/render_vector_source.cpp | 12 ++++++++---- 3 files changed, 24 insertions(+), 12 deletions(-) diff --git a/src/mbgl/renderer/sources/render_raster_dem_source.cpp b/src/mbgl/renderer/sources/render_raster_dem_source.cpp index 841124fa4a2..06ddf3e7e05 100644 --- a/src/mbgl/renderer/sources/render_raster_dem_source.cpp +++ b/src/mbgl/renderer/sources/render_raster_dem_source.cpp @@ -4,6 +4,7 @@ #include #include #include +#include namespace mbgl { @@ -27,16 +28,19 @@ void RenderRasterDEMSource::update(Immutable baseImpl_, enabled = needsRendering; optional _tileset = impl().getTileset(); - - if (tileset != _tileset) { + // In Continuous mode, keep the existing tiles if the new tileset is not + // yet available, thus providing smart style transitions without flickering. + // In other modes, allow clearing the tile pyramid first, before the early + // return in order to avoid render tests being flaky. + bool canUpdateTileset = _tileset || parameters.mode != MapMode::Continuous; + if (canUpdateTileset && tileset != _tileset) { tileset = _tileset; maxzoom = tileset->zoomRange.max; // TODO: this removes existing buckets, and will cause flickering. // Should instead refresh tile data in place. tilePyramid.clearAll(); } - // Allow clearing the tile pyramid first, before the early return in case - // the new tileset is not yet available or has an error in loading + if (!_tileset) { return; } diff --git a/src/mbgl/renderer/sources/render_raster_source.cpp b/src/mbgl/renderer/sources/render_raster_source.cpp index 06eb36795bf..7c7a987c85d 100644 --- a/src/mbgl/renderer/sources/render_raster_source.cpp +++ b/src/mbgl/renderer/sources/render_raster_source.cpp @@ -2,6 +2,7 @@ #include #include #include +#include namespace mbgl { @@ -25,16 +26,19 @@ void RenderRasterSource::update(Immutable baseImpl_, enabled = needsRendering; optional _tileset = impl().getTileset(); - - if (tileset != _tileset) { + // In Continuous mode, keep the existing tiles if the new tileset is not + // yet available, thus providing smart style transitions without flickering. + // In other modes, allow clearing the tile pyramid first, before the early + // return in order to avoid render tests being flaky. + bool canUpdateTileset = _tileset || parameters.mode != MapMode::Continuous; + if (canUpdateTileset && tileset != _tileset) { tileset = _tileset; // TODO: this removes existing buckets, and will cause flickering. // Should instead refresh tile data in place. tilePyramid.clearAll(); } - // Allow clearing the tile pyramid first, before the early return in case - // the new tileset is not yet available or has an error in loading + if (!_tileset) { return; } diff --git a/src/mbgl/renderer/sources/render_vector_source.cpp b/src/mbgl/renderer/sources/render_vector_source.cpp index 53fc4c82992..3990cb86fa6 100644 --- a/src/mbgl/renderer/sources/render_vector_source.cpp +++ b/src/mbgl/renderer/sources/render_vector_source.cpp @@ -2,6 +2,7 @@ #include #include #include +#include namespace mbgl { @@ -25,16 +26,19 @@ void RenderVectorSource::update(Immutable baseImpl_, enabled = needsRendering; optional _tileset = impl().getTileset(); - - if (tileset != _tileset) { + // In Continuous mode, keep the existing tiles if the new tileset is not + // yet available, thus providing smart style transitions without flickering. + // In other modes, allow clearing the tile pyramid first, before the early + // return in order to avoid render tests being flaky. + bool canUpdateTileset = _tileset || parameters.mode != MapMode::Continuous; + if (canUpdateTileset && tileset != _tileset) { tileset = _tileset; // TODO: this removes existing buckets, and will cause flickering. // Should instead refresh tile data in place. tilePyramid.clearAll(); } - // Allow clearing the tile pyramid first, before the early return in case - // the new tileset is not yet available or has an error in loading + if (!_tileset) { return; } From c724484bb70a2669e890831a0eef96d520fe5e90 Mon Sep 17 00:00:00 2001 From: Mikhail Pozdnyakov Date: Tue, 16 Jul 2019 15:41:13 +0300 Subject: [PATCH 2/4] [core] Avoid unneeded tile sets copying at sources code --- src/mbgl/renderer/sources/render_raster_dem_source.cpp | 10 +++++----- src/mbgl/renderer/sources/render_raster_source.cpp | 10 +++++----- src/mbgl/renderer/sources/render_vector_source.cpp | 10 +++++----- src/mbgl/style/sources/raster_source.cpp | 2 +- src/mbgl/style/sources/raster_source_impl.cpp | 8 ++------ src/mbgl/style/sources/raster_source_impl.hpp | 4 ++-- src/mbgl/style/sources/vector_source.cpp | 2 +- src/mbgl/style/sources/vector_source_impl.cpp | 4 ---- src/mbgl/style/sources/vector_source_impl.hpp | 5 +---- 9 files changed, 22 insertions(+), 33 deletions(-) diff --git a/src/mbgl/renderer/sources/render_raster_dem_source.cpp b/src/mbgl/renderer/sources/render_raster_dem_source.cpp index 06ddf3e7e05..b86b0bba7bb 100644 --- a/src/mbgl/renderer/sources/render_raster_dem_source.cpp +++ b/src/mbgl/renderer/sources/render_raster_dem_source.cpp @@ -27,21 +27,21 @@ void RenderRasterDEMSource::update(Immutable baseImpl_, enabled = needsRendering; - optional _tileset = impl().getTileset(); + const optional& implTileset = impl().tileset; // In Continuous mode, keep the existing tiles if the new tileset is not // yet available, thus providing smart style transitions without flickering. // In other modes, allow clearing the tile pyramid first, before the early // return in order to avoid render tests being flaky. - bool canUpdateTileset = _tileset || parameters.mode != MapMode::Continuous; - if (canUpdateTileset && tileset != _tileset) { - tileset = _tileset; + bool canUpdateTileset = implTileset || parameters.mode != MapMode::Continuous; + if (canUpdateTileset && tileset != implTileset) { + tileset = implTileset; maxzoom = tileset->zoomRange.max; // TODO: this removes existing buckets, and will cause flickering. // Should instead refresh tile data in place. tilePyramid.clearAll(); } - if (!_tileset) { + if (!implTileset) { return; } diff --git a/src/mbgl/renderer/sources/render_raster_source.cpp b/src/mbgl/renderer/sources/render_raster_source.cpp index 7c7a987c85d..310a4cbc57b 100644 --- a/src/mbgl/renderer/sources/render_raster_source.cpp +++ b/src/mbgl/renderer/sources/render_raster_source.cpp @@ -25,21 +25,21 @@ void RenderRasterSource::update(Immutable baseImpl_, enabled = needsRendering; - optional _tileset = impl().getTileset(); + const optional& implTileset = impl().tileset; // In Continuous mode, keep the existing tiles if the new tileset is not // yet available, thus providing smart style transitions without flickering. // In other modes, allow clearing the tile pyramid first, before the early // return in order to avoid render tests being flaky. - bool canUpdateTileset = _tileset || parameters.mode != MapMode::Continuous; - if (canUpdateTileset && tileset != _tileset) { - tileset = _tileset; + bool canUpdateTileset = implTileset || parameters.mode != MapMode::Continuous; + if (canUpdateTileset && tileset != implTileset) { + tileset = implTileset; // TODO: this removes existing buckets, and will cause flickering. // Should instead refresh tile data in place. tilePyramid.clearAll(); } - if (!_tileset) { + if (!implTileset) { return; } diff --git a/src/mbgl/renderer/sources/render_vector_source.cpp b/src/mbgl/renderer/sources/render_vector_source.cpp index 3990cb86fa6..7035be9cf29 100644 --- a/src/mbgl/renderer/sources/render_vector_source.cpp +++ b/src/mbgl/renderer/sources/render_vector_source.cpp @@ -25,21 +25,21 @@ void RenderVectorSource::update(Immutable baseImpl_, enabled = needsRendering; - optional _tileset = impl().getTileset(); + const optional& implTileset = impl().tileset; // In Continuous mode, keep the existing tiles if the new tileset is not // yet available, thus providing smart style transitions without flickering. // In other modes, allow clearing the tile pyramid first, before the early // return in order to avoid render tests being flaky. - bool canUpdateTileset = _tileset || parameters.mode != MapMode::Continuous; - if (canUpdateTileset && tileset != _tileset) { - tileset = _tileset; + bool canUpdateTileset = implTileset || parameters.mode != MapMode::Continuous; + if (canUpdateTileset && tileset != implTileset) { + tileset = implTileset; // TODO: this removes existing buckets, and will cause flickering. // Should instead refresh tile data in place. tilePyramid.clearAll(); } - if (!_tileset) { + if (!implTileset) { return; } diff --git a/src/mbgl/style/sources/raster_source.cpp b/src/mbgl/style/sources/raster_source.cpp index 2d08e4be80a..b4fbe22ae1a 100644 --- a/src/mbgl/style/sources/raster_source.cpp +++ b/src/mbgl/style/sources/raster_source.cpp @@ -65,7 +65,7 @@ void RasterSource::loadDescription(FileSource& fileSource) { } util::mapbox::canonicalizeTileset(*tileset, url, getType(), getTileSize()); - bool changed = impl().getTileset() != *tileset; + bool changed = impl().tileset != *tileset; baseImpl = makeMutable(impl(), *tileset); loaded = true; diff --git a/src/mbgl/style/sources/raster_source_impl.cpp b/src/mbgl/style/sources/raster_source_impl.cpp index 4db25aafd12..4201fd05783 100644 --- a/src/mbgl/style/sources/raster_source_impl.cpp +++ b/src/mbgl/style/sources/raster_source_impl.cpp @@ -10,18 +10,14 @@ RasterSource::Impl::Impl(SourceType sourceType, std::string id_, uint16_t tileSi RasterSource::Impl::Impl(const Impl& other, Tileset tileset_) : Source::Impl(other), - tileSize(other.tileSize), - tileset(std::move(tileset_)) { + tileset(std::move(tileset_)), + tileSize(other.tileSize) { } uint16_t RasterSource::Impl::getTileSize() const { return tileSize; } -optional RasterSource::Impl::getTileset() const { - return tileset; -} - optional RasterSource::Impl::getAttribution() const { if (!tileset) { return {}; diff --git a/src/mbgl/style/sources/raster_source_impl.hpp b/src/mbgl/style/sources/raster_source_impl.hpp index 96f59a21596..bb58455140b 100644 --- a/src/mbgl/style/sources/raster_source_impl.hpp +++ b/src/mbgl/style/sources/raster_source_impl.hpp @@ -11,14 +11,14 @@ class RasterSource::Impl : public Source::Impl { Impl(SourceType sourceType, std::string id, uint16_t tileSize); Impl(const Impl&, Tileset); - optional getTileset() const; uint16_t getTileSize() const; optional getAttribution() const final; + const optional tileset; + private: uint16_t tileSize; - optional tileset; }; } // namespace style diff --git a/src/mbgl/style/sources/vector_source.cpp b/src/mbgl/style/sources/vector_source.cpp index d141d291e12..8fa694e4d00 100644 --- a/src/mbgl/style/sources/vector_source.cpp +++ b/src/mbgl/style/sources/vector_source.cpp @@ -62,7 +62,7 @@ void VectorSource::loadDescription(FileSource& fileSource) { } util::mapbox::canonicalizeTileset(*tileset, url, getType(), util::tileSize); - bool changed = impl().getTileset() != *tileset; + bool changed = impl().tileset != *tileset; baseImpl = makeMutable(impl(), *tileset); loaded = true; diff --git a/src/mbgl/style/sources/vector_source_impl.cpp b/src/mbgl/style/sources/vector_source_impl.cpp index b06f0557bf5..8f85a41ddbf 100644 --- a/src/mbgl/style/sources/vector_source_impl.cpp +++ b/src/mbgl/style/sources/vector_source_impl.cpp @@ -12,10 +12,6 @@ VectorSource::Impl::Impl(const Impl& other, Tileset tileset_) tileset(std::move(tileset_)) { } -optional VectorSource::Impl::getTileset() const { - return tileset; -} - optional VectorSource::Impl::getAttribution() const { if (!tileset) { return {}; diff --git a/src/mbgl/style/sources/vector_source_impl.hpp b/src/mbgl/style/sources/vector_source_impl.hpp index 5e559b92660..4526fbe3566 100644 --- a/src/mbgl/style/sources/vector_source_impl.hpp +++ b/src/mbgl/style/sources/vector_source_impl.hpp @@ -11,12 +11,9 @@ class VectorSource::Impl : public Source::Impl { Impl(std::string id); Impl(const Impl&, Tileset); - optional getTileset() const; - optional getAttribution() const final; -private: - optional tileset; + const optional tileset; }; } // namespace style From 1d8a9a379e3a24eef6f9ee01842540574c48e734 Mon Sep 17 00:00:00 2001 From: Mikhail Pozdnyakov Date: Wed, 17 Jul 2019 13:00:45 +0300 Subject: [PATCH 3/4] [core] Introduce RenderTileSetSource class Encapsulates the tiles update logic based on the given tile set. --- .../sources/render_raster_dem_source.cpp | 42 +++++------------- .../sources/render_raster_dem_source.hpp | 22 ++++------ .../renderer/sources/render_raster_source.cpp | 44 ++++++------------- .../renderer/sources/render_raster_source.hpp | 18 ++++---- .../renderer/sources/render_tile_source.cpp | 42 ++++++++++++++++++ .../renderer/sources/render_tile_source.hpp | 29 +++++++++++- .../renderer/sources/render_vector_source.cpp | 44 +++++-------------- .../renderer/sources/render_vector_source.hpp | 16 +++---- 8 files changed, 132 insertions(+), 125 deletions(-) diff --git a/src/mbgl/renderer/sources/render_raster_dem_source.cpp b/src/mbgl/renderer/sources/render_raster_dem_source.cpp index b86b0bba7bb..fb6e3436709 100644 --- a/src/mbgl/renderer/sources/render_raster_dem_source.cpp +++ b/src/mbgl/renderer/sources/render_raster_dem_source.cpp @@ -11,50 +11,32 @@ namespace mbgl { using namespace style; RenderRasterDEMSource::RenderRasterDEMSource(Immutable impl_) - : RenderTileSource(std::move(impl_)) { + : RenderTileSetSource(std::move(impl_)) { } const style::RasterSource::Impl& RenderRasterDEMSource::impl() const { return static_cast(*baseImpl); } -void RenderRasterDEMSource::update(Immutable baseImpl_, - const std::vector>& layers, - const bool needsRendering, - const bool needsRelayout, - const TileParameters& parameters) { - std::swap(baseImpl, baseImpl_); - - enabled = needsRendering; - - const optional& implTileset = impl().tileset; - // In Continuous mode, keep the existing tiles if the new tileset is not - // yet available, thus providing smart style transitions without flickering. - // In other modes, allow clearing the tile pyramid first, before the early - // return in order to avoid render tests being flaky. - bool canUpdateTileset = implTileset || parameters.mode != MapMode::Continuous; - if (canUpdateTileset && tileset != implTileset) { - tileset = implTileset; - maxzoom = tileset->zoomRange.max; - // TODO: this removes existing buckets, and will cause flickering. - // Should instead refresh tile data in place. - tilePyramid.clearAll(); - } - - if (!implTileset) { - return; - } +const optional& RenderRasterDEMSource::getTileset() const { + return impl().tileset; +} +void RenderRasterDEMSource::updateInternal(const Tileset& tileset, + const std::vector>& layers, + const bool needsRendering, + const bool needsRelayout, + const TileParameters& parameters) { tilePyramid.update(layers, needsRendering, needsRelayout, parameters, SourceType::RasterDEM, impl().getTileSize(), - tileset->zoomRange, - tileset->bounds, + tileset.zoomRange, + tileset.bounds, [&] (const OverscaledTileID& tileID) { - return std::make_unique(tileID, parameters, *tileset); + return std::make_unique(tileID, parameters, tileset); }); algorithm::updateTileMasks(tilePyramid.getRenderedTiles()); } diff --git a/src/mbgl/renderer/sources/render_raster_dem_source.hpp b/src/mbgl/renderer/sources/render_raster_dem_source.hpp index dd74f4d7e77..72a3779e994 100644 --- a/src/mbgl/renderer/sources/render_raster_dem_source.hpp +++ b/src/mbgl/renderer/sources/render_raster_dem_source.hpp @@ -2,20 +2,13 @@ #include #include -#include namespace mbgl { -class RenderRasterDEMSource final : public RenderTileSource { +class RenderRasterDEMSource final : public RenderTileSetSource { public: explicit RenderRasterDEMSource(Immutable); - void update(Immutable, - const std::vector>&, - bool needsRendering, - bool needsRelayout, - const TileParameters&) final; - std::unordered_map> queryRenderedFeatures(const ScreenLineString& geometry, const TransformState& transformState, @@ -26,13 +19,16 @@ class RenderRasterDEMSource final : public RenderTileSource { std::vector querySourceFeatures(const SourceQueryOptions&) const override; - uint8_t getMaxZoom() const override { return maxzoom; } - private: - const style::RasterSource::Impl& impl() const; + // RenderTileSetSource overrides + void updateInternal(const Tileset&, + const std::vector>&, + bool needsRendering, + bool needsRelayout, + const TileParameters&) override; + const optional& getTileset() const override; - optional tileset; - uint8_t maxzoom = util::TERRAIN_RGB_MAXZOOM; + const style::RasterSource::Impl& impl() const; void onTileChanged(Tile&) override; }; diff --git a/src/mbgl/renderer/sources/render_raster_source.cpp b/src/mbgl/renderer/sources/render_raster_source.cpp index 310a4cbc57b..408f8a4e119 100644 --- a/src/mbgl/renderer/sources/render_raster_source.cpp +++ b/src/mbgl/renderer/sources/render_raster_source.cpp @@ -9,50 +9,32 @@ namespace mbgl { using namespace style; RenderRasterSource::RenderRasterSource(Immutable impl_) - : RenderTileSource(std::move(impl_)) { + : RenderTileSetSource(std::move(impl_)) { } -const style::RasterSource::Impl& RenderRasterSource::impl() const { +inline const style::RasterSource::Impl& RenderRasterSource::impl() const { return static_cast(*baseImpl); } -void RenderRasterSource::update(Immutable baseImpl_, - const std::vector>& layers, - const bool needsRendering, - const bool needsRelayout, - const TileParameters& parameters) { - std::swap(baseImpl, baseImpl_); - - enabled = needsRendering; - - const optional& implTileset = impl().tileset; - // In Continuous mode, keep the existing tiles if the new tileset is not - // yet available, thus providing smart style transitions without flickering. - // In other modes, allow clearing the tile pyramid first, before the early - // return in order to avoid render tests being flaky. - bool canUpdateTileset = implTileset || parameters.mode != MapMode::Continuous; - if (canUpdateTileset && tileset != implTileset) { - tileset = implTileset; - - // TODO: this removes existing buckets, and will cause flickering. - // Should instead refresh tile data in place. - tilePyramid.clearAll(); - } - - if (!implTileset) { - return; - } +const optional& RenderRasterSource::getTileset() const { + return impl().tileset; +} +void RenderRasterSource::updateInternal(const Tileset& tileset, + const std::vector>& layers, + const bool needsRendering, + const bool needsRelayout, + const TileParameters& parameters) { tilePyramid.update(layers, needsRendering, needsRelayout, parameters, SourceType::Raster, impl().getTileSize(), - tileset->zoomRange, - tileset->bounds, + tileset.zoomRange, + tileset.bounds, [&] (const OverscaledTileID& tileID) { - return std::make_unique(tileID, parameters, *tileset); + return std::make_unique(tileID, parameters, tileset); }); algorithm::updateTileMasks(tilePyramid.getRenderedTiles()); } diff --git a/src/mbgl/renderer/sources/render_raster_source.hpp b/src/mbgl/renderer/sources/render_raster_source.hpp index 0071dddec97..0760b7fa2b7 100644 --- a/src/mbgl/renderer/sources/render_raster_source.hpp +++ b/src/mbgl/renderer/sources/render_raster_source.hpp @@ -5,15 +5,11 @@ namespace mbgl { -class RenderRasterSource final : public RenderTileSource { +class RenderRasterSource final : public RenderTileSetSource { public: explicit RenderRasterSource(Immutable); - void update(Immutable, - const std::vector>&, - bool needsRendering, - bool needsRelayout, - const TileParameters&) final; +private: void prepare(const SourcePrepareParameters&) final; std::unordered_map> @@ -26,9 +22,15 @@ class RenderRasterSource final : public RenderTileSource { std::vector querySourceFeatures(const SourceQueryOptions&) const override; -private: + // RenderTileSetSource overrides + void updateInternal(const Tileset&, + const std::vector>&, + bool needsRendering, + bool needsRelayout, + const TileParameters&) override; + const optional& getTileset() const override; + const style::RasterSource::Impl& impl() const; - optional tileset; }; } // namespace mbgl diff --git a/src/mbgl/renderer/sources/render_tile_source.cpp b/src/mbgl/renderer/sources/render_tile_source.cpp index 6ee2eca1b1c..ef3f34f595a 100644 --- a/src/mbgl/renderer/sources/render_tile_source.cpp +++ b/src/mbgl/renderer/sources/render_tile_source.cpp @@ -4,8 +4,10 @@ #include #include #include +#include #include #include +#include #include namespace mbgl { @@ -139,4 +141,44 @@ void RenderTileSource::dumpDebugLogs() const { tilePyramid.dumpDebugLogs(); } +// RenderTileSetSource implementation + +RenderTileSetSource::RenderTileSetSource(Immutable impl_) + : RenderTileSource(std::move(impl_)) { +} + +RenderTileSetSource::~RenderTileSetSource() = default; + +uint8_t RenderTileSetSource::getMaxZoom() const { + return cachedTileset ? cachedTileset->zoomRange.max : util::TERRAIN_RGB_MAXZOOM; +} + +void RenderTileSetSource::update(Immutable baseImpl_, + const std::vector>& layers, + const bool needsRendering, + const bool needsRelayout, + const TileParameters& parameters) { + std::swap(baseImpl, baseImpl_); + + enabled = needsRendering; + + const optional& implTileset = getTileset(); + // In Continuous mode, keep the existing tiles if the new cachedTileset is not + // yet available, thus providing smart style transitions without flickering. + // In other modes, allow clearing the tile pyramid first, before the early + // return in order to avoid render tests being flaky. + bool canUpdateTileset = implTileset || parameters.mode != MapMode::Continuous; + if (canUpdateTileset && cachedTileset != implTileset) { + cachedTileset = implTileset; + + // TODO: this removes existing buckets, and will cause flickering. + // Should instead refresh tile data in place. + tilePyramid.clearAll(); + } + + if (!implTileset) return; + + updateInternal(*cachedTileset, layers, needsRendering, needsRelayout, parameters); +} + } // namespace mbgl diff --git a/src/mbgl/renderer/sources/render_tile_source.hpp b/src/mbgl/renderer/sources/render_tile_source.hpp index f961c205616..7edff726d50 100644 --- a/src/mbgl/renderer/sources/render_tile_source.hpp +++ b/src/mbgl/renderer/sources/render_tile_source.hpp @@ -11,7 +11,6 @@ namespace mbgl { */ class RenderTileSource : public RenderSource { public: - RenderTileSource(Immutable); ~RenderTileSource() override; bool isLoaded() const override; @@ -39,6 +38,7 @@ class RenderTileSource : public RenderSource { void dumpDebugLogs() const override; protected: + RenderTileSource(Immutable); TilePyramid tilePyramid; Immutable> renderTiles; mutable RenderTiles filteredRenderTiles; @@ -46,4 +46,31 @@ class RenderTileSource : public RenderSource { float bearing = 0.0f; }; +/** + * @brief Base class for render sources that use tile sets. + */ +class RenderTileSetSource : public RenderTileSource { +protected: + RenderTileSetSource(Immutable); + ~RenderTileSetSource() override; + + virtual void updateInternal(const Tileset&, + const std::vector>&, + bool needsRendering, + bool needsRelayout, + const TileParameters&) = 0; + // Returns tileset from the current impl. + virtual const optional& getTileset() const = 0; + +private: + uint8_t getMaxZoom() const final; + void update(Immutable, + const std::vector>&, + bool needsRendering, + bool needsRelayout, + const TileParameters&) final; + + optional cachedTileset; +}; + } // namespace mbgl diff --git a/src/mbgl/renderer/sources/render_vector_source.cpp b/src/mbgl/renderer/sources/render_vector_source.cpp index 7035be9cf29..6e4fdede10d 100644 --- a/src/mbgl/renderer/sources/render_vector_source.cpp +++ b/src/mbgl/renderer/sources/render_vector_source.cpp @@ -9,50 +9,28 @@ namespace mbgl { using namespace style; RenderVectorSource::RenderVectorSource(Immutable impl_) - : RenderTileSource(impl_) { + : RenderTileSetSource(std::move(impl_)) { } -const style::VectorSource::Impl& RenderVectorSource::impl() const { - return static_cast(*baseImpl); +const optional& RenderVectorSource::getTileset() const { + return static_cast(*baseImpl).tileset; } -void RenderVectorSource::update(Immutable baseImpl_, - const std::vector>& layers, - const bool needsRendering, - const bool needsRelayout, - const TileParameters& parameters) { - std::swap(baseImpl, baseImpl_); - - enabled = needsRendering; - - const optional& implTileset = impl().tileset; - // In Continuous mode, keep the existing tiles if the new tileset is not - // yet available, thus providing smart style transitions without flickering. - // In other modes, allow clearing the tile pyramid first, before the early - // return in order to avoid render tests being flaky. - bool canUpdateTileset = implTileset || parameters.mode != MapMode::Continuous; - if (canUpdateTileset && tileset != implTileset) { - tileset = implTileset; - - // TODO: this removes existing buckets, and will cause flickering. - // Should instead refresh tile data in place. - tilePyramid.clearAll(); - } - - if (!implTileset) { - return; - } - +void RenderVectorSource::updateInternal(const Tileset& tileset, + const std::vector>& layers, + const bool needsRendering, + const bool needsRelayout, + const TileParameters& parameters) { tilePyramid.update(layers, needsRendering, needsRelayout, parameters, SourceType::Vector, util::tileSize, - tileset->zoomRange, - tileset->bounds, + tileset.zoomRange, + tileset.bounds, [&] (const OverscaledTileID& tileID) { - return std::make_unique(tileID, impl().id, parameters, *tileset); + return std::make_unique(tileID, baseImpl->id, parameters, tileset); }); } diff --git a/src/mbgl/renderer/sources/render_vector_source.hpp b/src/mbgl/renderer/sources/render_vector_source.hpp index d5ac443e1c9..b83402ddb4a 100644 --- a/src/mbgl/renderer/sources/render_vector_source.hpp +++ b/src/mbgl/renderer/sources/render_vector_source.hpp @@ -6,18 +6,16 @@ namespace mbgl { -class RenderVectorSource final : public RenderTileSource { +class RenderVectorSource final : public RenderTileSetSource { public: explicit RenderVectorSource(Immutable); - - void update(Immutable, - const std::vector>&, - bool needsRendering, - bool needsRelayout, - const TileParameters&) final; private: - const style::VectorSource::Impl& impl() const; - optional tileset; + void updateInternal(const Tileset&, + const std::vector>&, + bool needsRendering, + bool needsRelayout, + const TileParameters&) override; + const optional& getTileset() const override; }; } // namespace mbgl From f4fd3a6d980a6a49a889fd1e45ebd4452b6bd142 Mon Sep 17 00:00:00 2001 From: Mikhail Pozdnyakov Date: Fri, 19 Jul 2019 13:59:00 +0300 Subject: [PATCH 4/4] [ios][andoid] Add change log entry --- platform/android/CHANGELOG.md | 3 +++ platform/ios/CHANGELOG.md | 9 ++++++++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/platform/android/CHANGELOG.md b/platform/android/CHANGELOG.md index 9d9ba8c35a6..cf9e1b5819a 100644 --- a/platform/android/CHANGELOG.md +++ b/platform/android/CHANGELOG.md @@ -4,6 +4,9 @@ Mapbox welcomes participation and contributions from everyone. If you'd like to ## master +### Bug fixes + - Fixed flickering on style change for the same tile set [#15127](https://github.com/mapbox/mapbox-gl-native/pull/15127) + ## 8.2.0-beta.1 - July 18, 2019 [Changes](https://github.com/mapbox/mapbox-gl-native/compare/android-v8.2.0-alpha.3...android-v8.2.0-beta.1) since [Mapbox Maps SDK for Android v8.2.0-alpha.3](https://github.com/mapbox/mapbox-gl-native/releases/tag/android-v8.2.0-alpha.3): diff --git a/platform/ios/CHANGELOG.md b/platform/ios/CHANGELOG.md index 8932e88d975..e529607a48b 100644 --- a/platform/ios/CHANGELOG.md +++ b/platform/ios/CHANGELOG.md @@ -2,6 +2,13 @@ Mapbox welcomes participation and contributions from everyone. Please read [CONTRIBUTING.md](../../CONTRIBUTING.md) to get started. +## master + +### Styles and rendering + +* Fixed flickering on style change for the same tile set ([#15127](https://github.com/mapbox/mapbox-gl-native/pull/15127)) + + ## 5.2.0 ### Networking @@ -22,7 +29,7 @@ Mapbox welcomes participation and contributions from everyone. Please read [CONT * Fixed style change transition regression caused by delayed setting of the updated layer properties. ([#15016](https://github.com/mapbox/mapbox-gl-native/pull/15016)) * Fixed an issue where layers with fill extrusions would be incorrectly rendered above other layers. ([#15065](https://github.com/mapbox/mapbox-gl-native/pull/15065)) * Improved feature querying performance. ([#14930](https://github.com/mapbox/mapbox-gl-native/pull/14930)) -* Fixed a custom geometry source bug caused by using the outdated tiles after style update [#15112](https://github.com/mapbox/mapbox-gl-native/pull/15112) +* Fixed a custom geometry source bug caused by using the outdated tiles after style update ([#15112](https://github.com/mapbox/mapbox-gl-native/pull/15112)) ### User interaction