[core] Fix flickering on style change

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):
 

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
 

src/mbgl/renderer/sources/render_raster_dem_source.cpp

@@ -4,53 +4,39 @@
 #include <mbgl/algorithm/update_tile_masks.hpp>
 #include <mbgl/geometry/dem_data.hpp>
 #include <mbgl/renderer/buckets/hillshade_bucket.hpp>
+#include <mbgl/renderer/tile_parameters.hpp>
 
 namespace mbgl {
 
 using namespace style;
 
 RenderRasterDEMSource::RenderRasterDEMSource(Immutable<style::RasterSource::Impl> impl_)
-    : RenderTileSource(std::move(impl_)) {
+    : RenderTileSetSource(std::move(impl_)) {
 }
 
 const style::RasterSource::Impl& RenderRasterDEMSource::impl() const {
     return static_cast<const style::RasterSource::Impl&>(*baseImpl);
 }
 
-void RenderRasterDEMSource::update(Immutable<style::Source::Impl> baseImpl_,
-                                const std::vector<Immutable<LayerProperties>>& layers,
-                                const bool needsRendering,
-                                const bool needsRelayout,
-                                const TileParameters& parameters) {
-    std::swap(baseImpl, baseImpl_);
-
-    enabled = needsRendering;
-
-    optional<Tileset> _tileset = impl().getTileset();
-
-    if (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;
-    }
+const optional<Tileset>& RenderRasterDEMSource::getTileset() const {
+    return impl().tileset;
+}
 
+void RenderRasterDEMSource::updateInternal(const Tileset& tileset,
+                                           const std::vector<Immutable<LayerProperties>>& 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<RasterDEMTile>(tileID, parameters, *tileset);
+                           return std::make_unique<RasterDEMTile>(tileID, parameters, tileset);
                        });
     algorithm::updateTileMasks(tilePyramid.getRenderedTiles());
 }

src/mbgl/renderer/sources/render_raster_dem_source.hpp

@@ -2,20 +2,13 @@
 
 #include <mbgl/renderer/sources/render_tile_source.hpp>
 #include <mbgl/style/sources/raster_source_impl.hpp>
-#include <mbgl/util/constants.hpp>
 
 namespace mbgl {
 
-class RenderRasterDEMSource final : public RenderTileSource {
+class RenderRasterDEMSource final : public RenderTileSetSource {
 public:
     explicit RenderRasterDEMSource(Immutable<style::RasterSource::Impl>);
 
-    void update(Immutable<style::Source::Impl>,
-                const std::vector<Immutable<style::LayerProperties>>&,
-                bool needsRendering,
-                bool needsRelayout,
-                const TileParameters&) final;
-
     std::unordered_map<std::string, std::vector<Feature>>
     queryRenderedFeatures(const ScreenLineString& geometry,
                           const TransformState& transformState,
@@ -26,13 +19,16 @@ class RenderRasterDEMSource final : public RenderTileSource {
     std::vector<Feature>
     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<Immutable<style::LayerProperties>>&,
+                        bool needsRendering,
+                        bool needsRelayout,
+                        const TileParameters&) override;
+    const optional<Tileset>& getTileset() const override;
 
-    optional<Tileset> tileset;
-    uint8_t maxzoom = util::TERRAIN_RGB_MAXZOOM;
+    const style::RasterSource::Impl& impl() const;
 
     void onTileChanged(Tile&) override;
 };

src/mbgl/renderer/sources/render_raster_source.cpp

@@ -2,53 +2,39 @@
 #include <mbgl/renderer/render_tile.hpp>
 #include <mbgl/tile/raster_tile.hpp>
 #include <mbgl/algorithm/update_tile_masks.hpp>
+#include <mbgl/renderer/tile_parameters.hpp>
 
 namespace mbgl {
 
 using namespace style;
 
 RenderRasterSource::RenderRasterSource(Immutable<style::RasterSource::Impl> 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<const style::RasterSource::Impl&>(*baseImpl);
 }
 
-void RenderRasterSource::update(Immutable<style::Source::Impl> baseImpl_,
-                                const std::vector<Immutable<LayerProperties>>& layers,
-                                const bool needsRendering,
-                                const bool needsRelayout,
-                                const TileParameters& parameters) {
-    std::swap(baseImpl, baseImpl_);
-
-    enabled = needsRendering;
-
-    optional<Tileset> _tileset = impl().getTileset();
-
-    if (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;
-    }
+const optional<Tileset>& RenderRasterSource::getTileset() const {
+    return impl().tileset;
+}
 
+void RenderRasterSource::updateInternal(const Tileset& tileset,
+                                        const std::vector<Immutable<LayerProperties>>& 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<RasterTile>(tileID, parameters, *tileset);
+                           return std::make_unique<RasterTile>(tileID, parameters, tileset);
                        });
     algorithm::updateTileMasks(tilePyramid.getRenderedTiles());
 }

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<style::RasterSource::Impl>);
 
-    void update(Immutable<style::Source::Impl>,
-                const std::vector<Immutable<style::LayerProperties>>&,
-                bool needsRendering,
-                bool needsRelayout,
-                const TileParameters&) final;
+private:
     void prepare(const SourcePrepareParameters&) final;
 
     std::unordered_map<std::string, std::vector<Feature>>
@@ -26,9 +22,15 @@ class RenderRasterSource final : public RenderTileSource {
     std::vector<Feature>
     querySourceFeatures(const SourceQueryOptions&) const override;
 
-private:
+    // RenderTileSetSource overrides
+    void updateInternal(const Tileset&,
+                        const std::vector<Immutable<style::LayerProperties>>&,
+                        bool needsRendering,
+                        bool needsRelayout,
+                        const TileParameters&) override;
+    const optional<Tileset>& getTileset() const override;
+
     const style::RasterSource::Impl& impl() const;
-    optional<Tileset> tileset;
 };
 
 } // namespace mbgl

src/mbgl/renderer/sources/render_tile_source.cpp

@@ -4,8 +4,10 @@
 #include <mbgl/renderer/render_tile.hpp>
 #include <mbgl/renderer/render_tree.hpp>
 #include <mbgl/renderer/paint_parameters.hpp>
+#include <mbgl/renderer/tile_parameters.hpp>
 #include <mbgl/renderer/tile_render_data.hpp>
 #include <mbgl/tile/vector_tile.hpp>
+#include <mbgl/util/constants.hpp>
 #include <mbgl/util/math.hpp>
 
 namespace mbgl {
@@ -139,4 +141,44 @@ void RenderTileSource::dumpDebugLogs() const {
     tilePyramid.dumpDebugLogs();
 }
 
+// RenderTileSetSource implementation
+
+RenderTileSetSource::RenderTileSetSource(Immutable<style::Source::Impl> 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<style::Source::Impl> baseImpl_,
+                                const std::vector<Immutable<style::LayerProperties>>& layers,
+                                const bool needsRendering,
+                                const bool needsRelayout,
+                                const TileParameters& parameters) {
+    std::swap(baseImpl, baseImpl_);
+
+    enabled = needsRendering;
+
+    const optional<Tileset>& 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

src/mbgl/renderer/sources/render_tile_source.hpp

@@ -11,7 +11,6 @@ namespace mbgl {
  */
 class RenderTileSource : public RenderSource {
 public:
-    RenderTileSource(Immutable<style::Source::Impl>);
     ~RenderTileSource() override;
 
     bool isLoaded() const override;
@@ -39,11 +38,39 @@ class RenderTileSource : public RenderSource {
     void dumpDebugLogs() const override;
 
 protected:
+    RenderTileSource(Immutable<style::Source::Impl>);
     TilePyramid tilePyramid;
     Immutable<std::vector<RenderTile>> renderTiles;
     mutable RenderTiles filteredRenderTiles;
     mutable RenderTiles renderTilesSortedByY;
     float bearing = 0.0f;      
 };
 
+/**
+ * @brief Base class for render sources that use tile sets.
+ */
+class RenderTileSetSource : public RenderTileSource {
+protected:
+    RenderTileSetSource(Immutable<style::Source::Impl>);
+    ~RenderTileSetSource() override;
+
+    virtual void updateInternal(const Tileset&,
+                                const std::vector<Immutable<style::LayerProperties>>&,
+                                bool needsRendering,
+                                bool needsRelayout,
+                                const TileParameters&) = 0;
+    // Returns tileset from the current impl.
+    virtual const optional<Tileset>& getTileset() const = 0;
+
+private:
+    uint8_t getMaxZoom() const final;
+    void update(Immutable<style::Source::Impl>,
+                const std::vector<Immutable<style::LayerProperties>>&,
+                bool needsRendering,
+                bool needsRelayout,
+                const TileParameters&) final;
+
+    optional<Tileset> cachedTileset;
+};
+
 } // namespace mbgl