[core] Tile updates the contained layer properties based on constants mask

include/mbgl/style/layer_properties.hpp

@@ -13,7 +13,8 @@ namespace style {
 class LayerProperties {
 public:
     virtual ~LayerProperties() = default;
-
+    // Returns constants mask for the data-driven properties.
+    virtual unsigned long constantsMask() const { return 0u; }
     Immutable<Layer::Impl> baseImpl;
 
 protected:

platform/android/CHANGELOG.md

@@ -2,6 +2,11 @@
 
 Mapbox welcomes participation and contributions from everyone.  If you'd like to do so please see the [`Contributing Guide`](https://github.com/mapbox/mapbox-gl-native/blob/master/CONTRIBUTING.md) first to get started.
 
+## master
+
+### Bugs
+ - Fixed style change transition regression caused by delayed setting of the updated layer properties [#15016](https://github.com/mapbox/mapbox-gl-native/pull/15016)
+
 ## 8.2.0 - June 26, 2019
 
 ### Bugs

platform/ios/CHANGELOG.md

@@ -2,6 +2,12 @@
 
 Mapbox welcomes participation and contributions from everyone. Please read [CONTRIBUTING.md](../../CONTRIBUTING.md) to get started.
 
+## master
+
+### Styles and rendering
+
+* Fixed style change transition regression caused by delayed setting of the updated layer properties. ([#15016](https://github.com/mapbox/mapbox-gl-native/pull/15016))
+
 ## 5.2.0
 
 ### Offline maps

src/mbgl/renderer/paint_property_binder.hpp

@@ -544,18 +544,6 @@ class PaintPropertyBinders<TypeList<Ps...>> {
         return binders.template get<P>()->statistics;
     }
 
-    using Bitset = std::bitset<sizeof...(Ps)>;
-
-    template <class EvaluatedProperties>
-    static Bitset constants(const EvaluatedProperties& currentProperties) {
-        Bitset result;
-        util::ignore({
-            result.set(TypeIndex<Ps, Ps...>::value,
-                       currentProperties.template get<Ps>().isConstant())...
-        });
-        return result;
-    }
-
 private:
     Binders binders;
 };

src/mbgl/renderer/render_orchestrator.cpp

@@ -241,11 +241,16 @@ std::unique_ptr<RenderTree> RenderOrchestrator::createRenderTree(const UpdatePar
     }
 
     // Update layers for class and zoom changes.
+    std::unordered_set<std::string> constantsMaskChanged;
     for (const auto& entry : renderLayers) {
         RenderLayer& layer = *entry.second;
         const bool layerAddedOrChanged = layerDiff.added.count(entry.first) || layerDiff.changed.count(entry.first);
         if (layerAddedOrChanged || zoomChanged || layer.hasTransition() || layer.hasCrossfade()) {
+            auto previousMask = layer.evaluatedProperties->constantsMask();
             layer.evaluate(evaluationParameters);
+            if (previousMask != layer.evaluatedProperties->constantsMask()) {
+                constantsMaskChanged.insert(layer.getID());
+            }
         }
     }
 
@@ -298,7 +303,7 @@ std::unique_ptr<RenderTree> RenderOrchestrator::createRenderTree(const UpdatePar
 
             if (layerInfo->source != LayerTypeInfo::Source::NotRequired) {
                 if (layerImpl->source == sourceImpl->id) {
-                    sourceNeedsRelayout = (sourceNeedsRelayout || hasImageDiff || hasLayoutDifference(layerDiff, layerImpl->id));
+                    sourceNeedsRelayout = (sourceNeedsRelayout || hasImageDiff || constantsMaskChanged.count(layerImpl->id) || hasLayoutDifference(layerDiff, layerImpl->id));
                     if (layerNeedsRendering) {
                         filteredLayersForSource.push_back(layer->evaluatedProperties);
                         if (zoomFitsLayer) {

src/mbgl/renderer/tile_pyramid.cpp

@@ -231,12 +231,7 @@ void TilePyramid::update(const std::vector<Immutable<style::LayerProperties>>& l
             if (holdForFade && typeInfo->fadingTiles == LayerTypeInfo::FadingTiles::NotRequired) {
                 continue;
             }
-            // Update layer properties for complete tiles; for incomplete just check the presence.
-            bool layerRenderableInTile = tile.isComplete() ? tile.updateLayerProperties(layerProperties)
-                                                           : static_cast<bool>(tile.getBucket(*layerProperties->baseImpl));
-            if (layerRenderableInTile) {
-                tile.usedByRenderedLayers = true;
-            }
+            tile.usedByRenderedLayers |= tile.layerPropertiesUpdated(layerProperties);
         }
     }
 }

src/mbgl/style/layers/background_layer_properties.cpp

@@ -21,6 +21,10 @@ BackgroundLayerProperties::BackgroundLayerProperties(
 
 BackgroundLayerProperties::~BackgroundLayerProperties() = default;
 
+unsigned long BackgroundLayerProperties::constantsMask() const {
+    return evaluated.constantsMask();
+}
+
 const BackgroundLayer::Impl& BackgroundLayerProperties::layerImpl() const {
     return static_cast<const BackgroundLayer::Impl&>(*baseImpl);
 }

src/mbgl/style/layers/background_layer_properties.hpp

@@ -41,6 +41,8 @@ class BackgroundLayerProperties final : public LayerProperties {
         BackgroundPaintProperties::PossiblyEvaluated);
     ~BackgroundLayerProperties() override;
 
+    unsigned long constantsMask() const override;
+
     const BackgroundLayer::Impl& layerImpl() const;
     // Data members.
     CrossfadeParameters crossfade;

src/mbgl/style/layers/circle_layer_properties.cpp

@@ -19,6 +19,10 @@ CircleLayerProperties::CircleLayerProperties(
 
 CircleLayerProperties::~CircleLayerProperties() = default;
 
+unsigned long CircleLayerProperties::constantsMask() const {
+    return evaluated.constantsMask();
+}
+
 const CircleLayer::Impl& CircleLayerProperties::layerImpl() const {
     return static_cast<const CircleLayer::Impl&>(*baseImpl);
 }

src/mbgl/style/layers/circle_layer_properties.hpp

@@ -80,6 +80,8 @@ class CircleLayerProperties final : public LayerProperties {
         CirclePaintProperties::PossiblyEvaluated);
     ~CircleLayerProperties() override;
 
+    unsigned long constantsMask() const override;
+
     const CircleLayer::Impl& layerImpl() const;
     // Data members.
     CirclePaintProperties::PossiblyEvaluated evaluated;

src/mbgl/style/layers/fill_extrusion_layer_properties.cpp

@@ -21,6 +21,10 @@ FillExtrusionLayerProperties::FillExtrusionLayerProperties(
 
 FillExtrusionLayerProperties::~FillExtrusionLayerProperties() = default;
 
+unsigned long FillExtrusionLayerProperties::constantsMask() const {
+    return evaluated.constantsMask();
+}
+
 const FillExtrusionLayer::Impl& FillExtrusionLayerProperties::layerImpl() const {
     return static_cast<const FillExtrusionLayer::Impl&>(*baseImpl);
 }

src/mbgl/style/layers/fill_extrusion_layer_properties.hpp

@@ -66,6 +66,8 @@ class FillExtrusionLayerProperties final : public LayerProperties {
         FillExtrusionPaintProperties::PossiblyEvaluated);
     ~FillExtrusionLayerProperties() override;
 
+    unsigned long constantsMask() const override;
+
     const FillExtrusionLayer::Impl& layerImpl() const;
     // Data members.
     CrossfadeParameters crossfade;

src/mbgl/style/layers/fill_layer_properties.cpp

@@ -21,6 +21,10 @@ FillLayerProperties::FillLayerProperties(
 
 FillLayerProperties::~FillLayerProperties() = default;
 
+unsigned long FillLayerProperties::constantsMask() const {
+    return evaluated.constantsMask();
+}
+
 const FillLayer::Impl& FillLayerProperties::layerImpl() const {
     return static_cast<const FillLayer::Impl&>(*baseImpl);
 }

src/mbgl/style/layers/fill_layer_properties.hpp

@@ -61,6 +61,8 @@ class FillLayerProperties final : public LayerProperties {
         FillPaintProperties::PossiblyEvaluated);
     ~FillLayerProperties() override;
 
+    unsigned long constantsMask() const override;
+
     const FillLayer::Impl& layerImpl() const;
     // Data members.
     CrossfadeParameters crossfade;

src/mbgl/style/layers/heatmap_layer_properties.cpp

@@ -19,6 +19,10 @@ HeatmapLayerProperties::HeatmapLayerProperties(
 
 HeatmapLayerProperties::~HeatmapLayerProperties() = default;
 
+unsigned long HeatmapLayerProperties::constantsMask() const {
+    return evaluated.constantsMask();
+}
+
 const HeatmapLayer::Impl& HeatmapLayerProperties::layerImpl() const {
     return static_cast<const HeatmapLayer::Impl&>(*baseImpl);
 }

src/mbgl/style/layers/heatmap_layer_properties.hpp

@@ -49,6 +49,8 @@ class HeatmapLayerProperties final : public LayerProperties {
         HeatmapPaintProperties::PossiblyEvaluated);
     ~HeatmapLayerProperties() override;
 
+    unsigned long constantsMask() const override;
+
     const HeatmapLayer::Impl& layerImpl() const;
     // Data members.
     HeatmapPaintProperties::PossiblyEvaluated evaluated;

src/mbgl/style/layers/hillshade_layer_properties.cpp

@@ -19,6 +19,10 @@ HillshadeLayerProperties::HillshadeLayerProperties(
 
 HillshadeLayerProperties::~HillshadeLayerProperties() = default;
 
+unsigned long HillshadeLayerProperties::constantsMask() const {
+    return evaluated.constantsMask();
+}
+
 const HillshadeLayer::Impl& HillshadeLayerProperties::layerImpl() const {
     return static_cast<const HillshadeLayer::Impl&>(*baseImpl);
 }

src/mbgl/style/layers/hillshade_layer_properties.hpp

@@ -55,6 +55,8 @@ class HillshadeLayerProperties final : public LayerProperties {
         HillshadePaintProperties::PossiblyEvaluated);
     ~HillshadeLayerProperties() override;
 
+    unsigned long constantsMask() const override;
+
     const HillshadeLayer::Impl& layerImpl() const;
     // Data members.
     HillshadePaintProperties::PossiblyEvaluated evaluated;

src/mbgl/style/layers/layer_properties.cpp.ejs

@@ -30,6 +30,10 @@ namespace style {
 
 <%- camelize(type) %>LayerProperties::~<%- camelize(type) %>LayerProperties() = default;
 
+unsigned long <%- camelize(type) %>LayerProperties::constantsMask() const {
+    return evaluated.constantsMask();
+}
+
 const <%- camelize(type) %>Layer::Impl& <%- camelize(type) %>LayerProperties::layerImpl() const {
     return static_cast<const <%- camelize(type) %>Layer::Impl&>(*baseImpl);
 }

src/mbgl/style/layers/layer_properties.hpp.ejs

@@ -72,6 +72,8 @@ public:
         <%- camelize(type) %>PaintProperties::PossiblyEvaluated);
     ~<%- camelize(type) %>LayerProperties() override;
 
+    unsigned long constantsMask() const override;
+
     const <%- camelize(type) %>Layer::Impl& layerImpl() const;
     // Data members.
 <% if (type === 'background' || type === 'fill' || type === 'line' || type === 'fill-extrusion') { -%>

src/mbgl/style/layers/line_layer_properties.cpp

@@ -21,6 +21,10 @@ LineLayerProperties::LineLayerProperties(
 
 LineLayerProperties::~LineLayerProperties() = default;
 
+unsigned long LineLayerProperties::constantsMask() const {
+    return evaluated.constantsMask();
+}
+
 const LineLayer::Impl& LineLayerProperties::layerImpl() const {
     return static_cast<const LineLayer::Impl&>(*baseImpl);
 }

src/mbgl/style/layers/line_layer_properties.hpp

@@ -113,6 +113,8 @@ class LineLayerProperties final : public LayerProperties {
         LinePaintProperties::PossiblyEvaluated);
     ~LineLayerProperties() override;
 
+    unsigned long constantsMask() const override;
+
     const LineLayer::Impl& layerImpl() const;
     // Data members.
     CrossfadeParameters crossfade;

src/mbgl/style/layers/raster_layer_properties.cpp

@@ -19,6 +19,10 @@ RasterLayerProperties::RasterLayerProperties(
 
 RasterLayerProperties::~RasterLayerProperties() = default;
 
+unsigned long RasterLayerProperties::constantsMask() const {
+    return evaluated.constantsMask();
+}
+
 const RasterLayer::Impl& RasterLayerProperties::layerImpl() const {
     return static_cast<const RasterLayer::Impl&>(*baseImpl);
 }

src/mbgl/style/layers/raster_layer_properties.hpp

@@ -65,6 +65,8 @@ class RasterLayerProperties final : public LayerProperties {
         RasterPaintProperties::PossiblyEvaluated);
     ~RasterLayerProperties() override;
 
+    unsigned long constantsMask() const override;
+
     const RasterLayer::Impl& layerImpl() const;
     // Data members.
     RasterPaintProperties::PossiblyEvaluated evaluated;

src/mbgl/style/layers/symbol_layer_properties.cpp

@@ -19,6 +19,10 @@ SymbolLayerProperties::SymbolLayerProperties(
 
 SymbolLayerProperties::~SymbolLayerProperties() = default;
 
+unsigned long SymbolLayerProperties::constantsMask() const {
+    return evaluated.constantsMask();
+}
+
 const SymbolLayer::Impl& SymbolLayerProperties::layerImpl() const {
     return static_cast<const SymbolLayer::Impl&>(*baseImpl);
 }

src/mbgl/style/layers/symbol_layer_properties.hpp

@@ -341,6 +341,8 @@ class SymbolLayerProperties final : public LayerProperties {
         SymbolPaintProperties::PossiblyEvaluated);
     ~SymbolLayerProperties() override;
 
+    unsigned long constantsMask() const override;
+
     const SymbolLayer::Impl& layerImpl() const;
     // Data members.
     SymbolPaintProperties::PossiblyEvaluated evaluated;

src/mbgl/style/properties.hpp

@@ -9,6 +9,8 @@
 #include <mbgl/util/indexed_tuple.hpp>
 #include <mbgl/util/ignore.hpp>
 
+#include <bitset>
+
 namespace mbgl {
 
 class GeometryTileFeature;
@@ -102,6 +104,22 @@ struct IsDataDriven : std::integral_constant<bool, P::IsDataDriven> {};
 template <class P>
 struct IsOverridable : std::integral_constant<bool, P::IsOverridable> {};
 
+template <class Ps>
+struct ConstantsMask;
+
+template <class... Ps>
+struct ConstantsMask<TypeList<Ps...>> {
+    template <class Properties>
+    static unsigned long getMask(const Properties& properties) {
+        std::bitset<sizeof... (Ps)> result;
+        util::ignore({
+            result.set(TypeIndex<Ps, Ps...>::value,
+                       properties.template get<Ps>().isConstant())...
+        });
+        return result.to_ulong();
+    }
+};
+
 template <class... Ps>
 class Properties {
 public:
@@ -172,6 +190,10 @@ class Properties {
                 evaluate<Ps>(z, feature)...
             };
         }
+
+        unsigned long constantsMask() const {
+            return ConstantsMask<DataDrivenProperties>::getMask(*this);
+        }
     };
 
     class Unevaluated : public Tuple<UnevaluatedTypes> {

src/mbgl/tile/geometry_tile.cpp

@@ -212,13 +212,14 @@ const LayerRenderData* GeometryTile::getLayerRenderData(const style::Layer::Impl
     return that->getMutableLayerRenderData(layerImpl);
 }
 
-bool GeometryTile::updateLayerProperties(const Immutable<style::LayerProperties>& layerProperties) {
+bool GeometryTile::layerPropertiesUpdated(const Immutable<style::LayerProperties>& layerProperties) {
     LayerRenderData* renderData = getMutableLayerRenderData(*layerProperties->baseImpl);
     if (!renderData) {
         return false;
     }
 
-    if (renderData->layerProperties != layerProperties) {
+    if (renderData->layerProperties != layerProperties &&
+        renderData->layerProperties->constantsMask() == layerProperties->constantsMask()) {
         renderData->layerProperties = layerProperties;
     }
 

src/mbgl/tile/geometry_tile.hpp

@@ -47,7 +47,7 @@ class GeometryTile : public Tile, public GlyphRequestor, public ImageRequestor {
     void upload(gfx::UploadPass&) override;
     Bucket* getBucket(const style::Layer::Impl&) const override;
     const LayerRenderData* getLayerRenderData(const style::Layer::Impl&) const override;
-    bool updateLayerProperties(const Immutable<style::LayerProperties>&) override;
+    bool layerPropertiesUpdated(const Immutable<style::LayerProperties>&) override;
 
     void queryRenderedFeatures(
             std::unordered_map<std::string, std::vector<Feature>>& result,

src/mbgl/tile/tile.hpp

@@ -58,10 +58,16 @@ class Tile {
         assert(false);
         return nullptr;
     }
-    // Updates the contained layer render data with the given properties.
+    // Notifies this tile of the updated layer properties.
+    //
+    // Tile implementation should update the contained layer
+    // render data with the given properties.
+    // 
     // Returns `true` if the corresponding render layer data is present in this tile (and i.e. it
     // was succesfully updated); returns `false` otherwise.
-    virtual bool updateLayerProperties(const Immutable<style::LayerProperties>&) { return true; }
+    virtual bool layerPropertiesUpdated(const Immutable<style::LayerProperties>& layerProperties) { 
+        return bool(getBucket(*layerProperties->baseImpl));
+    }
     virtual void setShowCollisionBoxes(const bool) {}
     virtual void setLayers(const std::vector<Immutable<style::LayerProperties>>&) {}
     virtual void setMask(TileMask&&) {}