[core] Update `style::LayerProperties` instances in tiles

11 files changed, 94 insertions, 85 deletions

diff --git a/src/mbgl/renderer/buckets/symbol_bucket.cpp b/src/mbgl/renderer/buckets/symbol_bucket.cpp
index 4b5207da39..c7eb7c6e5c 100644
--- a/src/mbgl/renderer/buckets/symbol_bucket.cpp
+++ b/src/mbgl/renderer/buckets/symbol_bucket.cpp
@@ -8,10 +8,6 @@ namespace mbgl {
 
 using namespace style;
 
-const SymbolLayerProperties& toSymbolLayerProperties(const Immutable<LayerProperties>& layer) {
-    return static_cast<const SymbolLayerProperties&>(*layer);
-}
-
 SymbolBucket::SymbolBucket(style::SymbolLayoutProperties::PossiblyEvaluated layout_,
                            const std::map<std::string, Immutable<style::LayerProperties>>& paintProperties_,
                            const style::PropertyValue<float>& textSize,
@@ -34,17 +30,13 @@ SymbolBucket::SymbolBucket(style::SymbolLayoutProperties::PossiblyEvaluated layo
       tilePixelRatio(tilePixelRatio_) {
 
     for (const auto& pair : paintProperties_) {
-        auto layerPaintProperties = toSymbolLayerProperties(pair.second).evaluated;
-        if (hasFormatSectionOverrides()) {
-            setPaintPropertyOverrides(layerPaintProperties);
-        }
+        const auto& evaluated = getEvaluated<SymbolLayerProperties>(pair.second);
         paintProperties.emplace(
             std::piecewise_construct,
             std::forward_as_tuple(pair.first),
             std::forward_as_tuple(PaintProperties {
-                layerPaintProperties,
-                { RenderSymbolLayer::iconPaintProperties(layerPaintProperties), zoom },
-                { RenderSymbolLayer::textPaintProperties(layerPaintProperties), zoom }
+                { RenderSymbolLayer::iconPaintProperties(evaluated), zoom },
+                { RenderSymbolLayer::textPaintProperties(evaluated), zoom }
             }));
     }
 }
@@ -246,19 +238,7 @@ void SymbolBucket::sortFeatures(const float angle) {
     }
 }
 
-void SymbolBucket::updatePaintProperties(const std::string& layerID,
-                                         style::SymbolPaintProperties::PossiblyEvaluated updated) {
-    if (hasFormatSectionOverrides()) {
-        SymbolLayerPaintPropertyOverrides::updateOverrides(paintProperties.at(layerID).evaluated, updated);
-    }
-    paintProperties.at(layerID).evaluated = updated;
-}
-
-void SymbolBucket::setPaintPropertyOverrides(style::SymbolPaintProperties::PossiblyEvaluated& paint) {
-    SymbolLayerPaintPropertyOverrides::setOverrides(layout, paint);
-}
-
-bool SymbolBucket::hasFormatSectionOverrides() {
+bool SymbolBucket::hasFormatSectionOverrides() const {
     if (!hasFormatSectionOverrides_) {
         hasFormatSectionOverrides_= SymbolLayerPaintPropertyOverrides::hasOverrides(layout.get<TextField>());
     }
diff --git a/src/mbgl/renderer/buckets/symbol_bucket.hpp b/src/mbgl/renderer/buckets/symbol_bucket.hpp
index ca740c5488..61b4435e77 100644
--- a/src/mbgl/renderer/buckets/symbol_bucket.hpp
+++ b/src/mbgl/renderer/buckets/symbol_bucket.hpp
@@ -61,10 +61,7 @@ public:
     bool hasIconData() const;
     bool hasCollisionBoxData() const;
     bool hasCollisionCircleData() const;
-    bool hasFormatSectionOverrides();
-    void updatePaintProperties(const std::string& layerID,
-                               style::SymbolPaintProperties::PossiblyEvaluated);
-    void setPaintPropertyOverrides(style::SymbolPaintProperties::PossiblyEvaluated&);
+    bool hasFormatSectionOverrides() const;
 
     void updateOpacity();
     void sortFeatures(const float angle);
@@ -86,7 +83,6 @@ public:
     std::vector<SymbolInstance> symbolInstances;
 
     struct PaintProperties {
-        style::SymbolPaintProperties::PossiblyEvaluated evaluated;
         SymbolIconProgram::Binders iconBinders;
         SymbolSDFTextProgram::Binders textBinders;
     };
@@ -136,7 +132,7 @@ public:
     const float tilePixelRatio;
     uint32_t bucketInstanceId = 0;
     bool justReloaded = false;
-    optional<bool> hasFormatSectionOverrides_;
+    mutable optional<bool> hasFormatSectionOverrides_;
 
     std::shared_ptr<std::vector<size_t>> featureSortOrder;
 };
diff --git a/src/mbgl/renderer/layers/render_symbol_layer.cpp b/src/mbgl/renderer/layers/render_symbol_layer.cpp
index 95df9435f9..7e2eb1aa86 100644
--- a/src/mbgl/renderer/layers/render_symbol_layer.cpp
+++ b/src/mbgl/renderer/layers/render_symbol_layer.cpp
@@ -72,21 +72,25 @@ using SegmentsWrapper = variant<SegmentWrapper, SegmentVectorWrapper>;
 struct RenderableSegment {
     RenderableSegment(SegmentWrapper segment_,
                       const RenderTile& tile_,
-                      SymbolBucket& bucket_,
+                      const LayerRenderData& renderData_,
                       const SymbolBucket::PaintProperties& bucketPaintProperties_,
                       float sortKey_) :
     segment(std::move(segment_)),
     tile(tile_),
-    bucket(bucket_),
+    renderData(renderData_),
     bucketPaintProperties(bucketPaintProperties_),
     sortKey(sortKey_) {}
 
     SegmentWrapper segment;
     const RenderTile& tile;
-    SymbolBucket& bucket;
+    const LayerRenderData& renderData;
     const SymbolBucket::PaintProperties& bucketPaintProperties;
     float sortKey;
 
+    bool hasIconData() const {
+        return static_cast<const SymbolBucket&>(*renderData.bucket).hasIconData();
+    }
+
     friend bool operator < (const RenderableSegment& lhs, const RenderableSegment& rhs) {
         return lhs.sortKey < rhs.sortKey;
     }
@@ -95,16 +99,17 @@ struct RenderableSegment {
 template <typename DrawFn>
 void drawIcon(const DrawFn& draw,
               const RenderTile& tile,
-              SymbolBucket& bucket,
+              const LayerRenderData& renderData,
               SegmentsWrapper iconSegments,
               const SymbolBucket::PaintProperties& bucketPaintProperties,
               const PaintParameters& parameters) {
     assert(tile.tile.kind == Tile::Kind::Geometry);
     auto& geometryTile = static_cast<GeometryTile&>(tile.tile);
-    const auto& evaluated_ = bucketPaintProperties.evaluated;
+    auto& bucket = static_cast<SymbolBucket&>(*renderData.bucket);
+    const auto& evaluated = getEvaluated<SymbolLayerProperties>(renderData.layerProperties);
     const auto& layout = bucket.layout;
-    auto values = iconPropertyValues(evaluated_, layout);
-    const auto& paintPropertyValues = RenderSymbolLayer::iconPaintProperties(evaluated_);
+    auto values = iconPropertyValues(evaluated, layout);
+    const auto& paintPropertyValues = RenderSymbolLayer::iconPaintProperties(evaluated);
 
     const bool alongLine = layout.get<SymbolPlacement>() != SymbolPlacementType::Point &&
         layout.get<IconRotationAlignment>() == AlignmentType::Map;
@@ -179,20 +184,21 @@ void drawIcon(const DrawFn& draw,
 template <typename DrawFn>
 void drawText(const DrawFn& draw,
               const RenderTile& tile,
-              SymbolBucket& bucket,
+              const LayerRenderData& renderData,
               SegmentsWrapper textSegments,
               const SymbolBucket::PaintProperties& bucketPaintProperties,
               const PaintParameters& parameters) {
     assert(tile.tile.kind == Tile::Kind::Geometry);
     auto& geometryTile = static_cast<GeometryTile&>(tile.tile);
-    const auto& evaluated_ = bucketPaintProperties.evaluated;
+    auto& bucket = static_cast<SymbolBucket&>(*renderData.bucket);
+    const auto& evaluated = getEvaluated<SymbolLayerProperties>(renderData.layerProperties);
     const auto& layout = bucket.layout;
 
     const gfx::TextureBinding textureBinding{ geometryTile.glyphAtlasTexture->getResource(),
                                               gfx::TextureFilterType::Linear };
 
-    auto values = textPropertyValues(evaluated_, layout);
-    const auto& paintPropertyValues = RenderSymbolLayer::textPaintProperties(evaluated_);
+    auto values = textPropertyValues(evaluated, layout);
+    const auto& paintPropertyValues = RenderSymbolLayer::textPaintProperties(evaluated);
     bool hasVariablePacement = false;
 
     const bool alongLine = layout.get<SymbolPlacement>() != SymbolPlacementType::Point &&
@@ -322,13 +328,20 @@ RenderSymbolLayer::~RenderSymbolLayer() = default;
 
 void RenderSymbolLayer::transition(const TransitionParameters& parameters) {
     unevaluated = impl(baseImpl).paint.transitioned(parameters, std::move(unevaluated));
+    hasFormatSectionOverrides = SymbolLayerPaintPropertyOverrides::hasOverrides(impl(baseImpl).layout.get<TextField>());
 }
 
 void RenderSymbolLayer::evaluate(const PropertyEvaluationParameters& parameters) {
     auto properties = makeMutable<SymbolLayerProperties>(
         staticImmutableCast<SymbolLayer::Impl>(baseImpl),
         unevaluated.evaluate(parameters));
-    const auto& evaluated = properties->evaluated;
+    auto& evaluated = properties->evaluated;
+    auto& layout = impl(baseImpl).layout;
+
+    if (hasFormatSectionOverrides) {
+        SymbolLayerPaintPropertyOverrides::setOverrides(layout, evaluated);
+    }
+
     auto hasIconOpacity = evaluated.get<style::IconColor>().constantOr(Color::black()).a > 0 ||
                           evaluated.get<style::IconHaloColor>().constantOr(Color::black()).a > 0;
     auto hasTextOpacity = evaluated.get<style::TextColor>().constantOr(Color::black()).a > 0 ||
@@ -442,17 +455,18 @@ void RenderSymbolLayer::render(PaintParameters& parameters, RenderSource*) {
     };
 
     for (const RenderTile& tile : renderTiles) {
-        auto bucket_ = tile.tile.getBucket<SymbolBucket>(*baseImpl);
-        if (!bucket_) {
+        const LayerRenderData* renderData = tile.tile.getLayerRenderData(*baseImpl);
+        if (!renderData) {
             continue;
         }
 
-        SymbolBucket& bucket = *bucket_;
+        auto& bucket = static_cast<SymbolBucket&>(*renderData->bucket);
         assert(bucket.paintProperties.find(getID()) != bucket.paintProperties.end());
         const auto& bucketPaintProperties = bucket.paintProperties.at(getID());
-        auto addRenderables = [&renderableSegments, &tile, &bucket, &bucketPaintProperties, it = renderableSegments.begin()] (auto& segments) mutable {
+
+        auto addRenderables = [&renderableSegments, &tile, renderData, &bucketPaintProperties, it = renderableSegments.begin()] (auto& segments) mutable {
             for (auto& segment : segments) {
-                it = renderableSegments.emplace_hint(it, SegmentWrapper{std::ref(segment)}, tile, bucket, bucketPaintProperties, segment.sortKey);
+                it = renderableSegments.emplace_hint(it, SegmentWrapper{std::ref(segment)}, tile, *renderData, bucketPaintProperties, segment.sortKey);
             }
         };
 
@@ -460,7 +474,7 @@ void RenderSymbolLayer::render(PaintParameters& parameters, RenderSource*) {
             if (sortFeaturesByKey) {
                 addRenderables(bucket.icon.segments);
             } else {
-                drawIcon(draw, tile, bucket, std::ref(bucket.icon.segments), bucketPaintProperties, parameters);
+                drawIcon(draw, tile, *renderData, std::ref(bucket.icon.segments), bucketPaintProperties, parameters);
             }
         }
 
@@ -468,7 +482,7 @@ void RenderSymbolLayer::render(PaintParameters& parameters, RenderSource*) {
             if (sortFeaturesByKey) {
                 addRenderables(bucket.text.segments);
             } else {
-                drawText(draw, tile, bucket, std::ref(bucket.text.segments), bucketPaintProperties, parameters);
+                drawText(draw, tile, *renderData, std::ref(bucket.text.segments), bucketPaintProperties, parameters);
             }
         }
 
@@ -551,10 +565,10 @@ void RenderSymbolLayer::render(PaintParameters& parameters, RenderSource*) {
 
     if (sortFeaturesByKey) {
         for (auto& renderable : renderableSegments) {
-            if (renderable.bucket.hasIconData()) {
-                drawIcon(draw, renderable.tile, renderable.bucket, renderable.segment, renderable.bucketPaintProperties, parameters);
+            if (renderable.hasIconData()) {
+                drawIcon(draw, renderable.tile, renderable.renderData, renderable.segment, renderable.bucketPaintProperties, parameters);
             } else {
-                drawText(draw, renderable.tile, renderable.bucket, renderable.segment, renderable.bucketPaintProperties, parameters);
+                drawText(draw, renderable.tile, renderable.renderData, renderable.segment, renderable.bucketPaintProperties, parameters);
             }
         }
     }
@@ -602,10 +616,4 @@ void RenderSymbolLayer::setRenderTiles(RenderTiles tiles, const TransformState&
     });
 }
 
-void RenderSymbolLayer::updateBucketPaintProperties(Bucket* bucket) const {
-    assert(bucket->supportsLayer(*baseImpl));
-    const auto& evaluated = static_cast<const SymbolLayerProperties&>(*evaluatedProperties).evaluated;
-    static_cast<SymbolBucket*>(bucket)->updatePaintProperties(getID(), evaluated);
-}
-
 } // namespace mbgl
diff --git a/src/mbgl/renderer/layers/render_symbol_layer.hpp b/src/mbgl/renderer/layers/render_symbol_layer.hpp
index 552dd30ca2..ad33a2ab0e 100644
--- a/src/mbgl/renderer/layers/render_symbol_layer.hpp
+++ b/src/mbgl/renderer/layers/render_symbol_layer.hpp
@@ -81,8 +81,7 @@ private:
     float iconSize = 1.0f;
     float textSize = 16.0f;
 
-protected:
-    void updateBucketPaintProperties(Bucket*) const final;
+    bool hasFormatSectionOverrides = false;
 };
 
 } // namespace mbgl
diff --git a/src/mbgl/renderer/render_layer.cpp b/src/mbgl/renderer/render_layer.cpp
index c2719fef91..b844c3f60d 100644
--- a/src/mbgl/renderer/render_layer.cpp
+++ b/src/mbgl/renderer/render_layer.cpp
@@ -54,24 +54,13 @@ optional<Color> RenderLayer::getSolidBackground() const {
 RenderLayer::RenderTiles RenderLayer::filterRenderTiles(RenderTiles tiles, FilterFunctionPtr filterFn) const {
     assert(filterFn != nullptr);
     RenderTiles filtered;
-    // We only need clipping when we're drawing fill or line layers.
-    const bool needsClipping_ =
-            baseImpl->getTypeInfo()->clipping == LayerTypeInfo::Clipping::Required;
 
     for (auto& tileRef : tiles) {
         auto& tile = tileRef.get();
         if (filterFn(tile)) {
             continue;
         }
-
-        if (Bucket* bucket = tile.tile.getBucket(*baseImpl)) {
-            tile.used = true;
-            tile.needsClipping |= needsClipping_;
-            filtered.emplace_back(tile);
-            if (tile.tile.isComplete()) {
-                updateBucketPaintProperties(bucket);
-            }
-        }
+        filtered.emplace_back(tile);
     }
     return filtered;
 }
@@ -80,10 +69,6 @@ void RenderLayer::markContextDestroyed() {
     // no-op
 }
 
-void RenderLayer::updateBucketPaintProperties(Bucket*) const {
-    // no-op
-}
-
 void RenderLayer::checkRenderability(const PaintParameters& parameters,
                                      const uint32_t activeBindingCount) {
     // Only warn once for every layer.
diff --git a/src/mbgl/renderer/render_layer.hpp b/src/mbgl/renderer/render_layer.hpp
index 3a762947db..692fc85f29 100644
--- a/src/mbgl/renderer/render_layer.hpp
+++ b/src/mbgl/renderer/render_layer.hpp
@@ -92,11 +92,6 @@ protected:
     // in the console to inform the developer.
     void checkRenderability(const PaintParameters&, uint32_t activeBindingCount);
 
-    // For some layers, we want Buckets to cache their corresponding paint properties, so that outdated buckets (and
-    // the cached paint properties) can be still in use while the tile is loading new buckets (which will
-    // correpond to the current paint properties of the layer).
-    virtual void updateBucketPaintProperties(Bucket*) const;
-
     using FilterFunctionPtr = bool (*)(RenderTile&);
     RenderTiles filterRenderTiles(RenderTiles, FilterFunctionPtr) const;
 
diff --git a/src/mbgl/renderer/tile_pyramid.cpp b/src/mbgl/renderer/tile_pyramid.cpp
index 6e1bddec0a..37925a54b7 100644
--- a/src/mbgl/renderer/tile_pyramid.cpp
+++ b/src/mbgl/renderer/tile_pyramid.cpp
@@ -238,6 +238,27 @@ void TilePyramid::update(const std::vector<Immutable<style::LayerProperties>>& l
     for (auto& pair : tiles) {
         pair.second->setShowCollisionBoxes(parameters.debugOptions & MapDebugOptions::Collision);
     }
+
+    // Initialize render tiles fields and update the tile contained layer render data.
+    for (RenderTile& renderTile : renderTiles) {
+        Tile& tile = renderTile.tile;
+        if (!tile.isRenderable()) continue;
+
+        const bool holdForFade = tile.holdForFade();
+        for (const auto& layerProperties : layers) {
+            const auto* typeInfo = layerProperties->baseImpl->getTypeInfo();
+            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) {
+                renderTile.used = true;
+                renderTile.needsClipping = (renderTile.needsClipping || typeInfo->clipping == LayerTypeInfo::Clipping::Required);
+            }
+        }
+    }
 }
 
 void TilePyramid::handleWrapJump(float lng) {
diff --git a/src/mbgl/renderer/tile_pyramid.hpp b/src/mbgl/renderer/tile_pyramid.hpp
index 594330e0c9..98eafc39e6 100644
--- a/src/mbgl/renderer/tile_pyramid.hpp
+++ b/src/mbgl/renderer/tile_pyramid.hpp
@@ -33,7 +33,7 @@ public:
 
     bool isLoaded() const;
 
-    void update(const std::vector<Immutable<style::LayerProperties>>&,
+    void update(const std::vector<Immutable<style::LayerProperties>>& visibleLayers,
                 bool needsRendering,
                 bool needsRelayout,
                 const TileParameters&,
diff --git a/src/mbgl/tile/geometry_tile.cpp b/src/mbgl/tile/geometry_tile.cpp
index 109b6377e1..792586e73d 100644
--- a/src/mbgl/tile/geometry_tile.cpp
+++ b/src/mbgl/tile/geometry_tile.cpp
@@ -206,16 +206,34 @@ Bucket* GeometryTile::getBucket(const Layer::Impl& layer) const {
 }
 
 const LayerRenderData* GeometryTile::getLayerRenderData(const style::Layer::Impl& layerImpl) const {
-    const auto it = layerIdToLayerRenderData.find(layerImpl.id);
+    auto* that = const_cast<GeometryTile*>(this);
+    return that->getMutableLayerRenderData(layerImpl);
+}
+
+bool GeometryTile::updateLayerProperties(const Immutable<style::LayerProperties>& layerProperties) {
+    LayerRenderData* renderData = getMutableLayerRenderData(*layerProperties->baseImpl);
+    if (!renderData) {
+        return false;
+    }
+
+    if (renderData->layerProperties != layerProperties) {
+        renderData->layerProperties = layerProperties;
+    }
+
+    return true;
+}
+
+LayerRenderData* GeometryTile::getMutableLayerRenderData(const style::Layer::Impl& layerImpl) {
+    auto it = layerIdToLayerRenderData.find(layerImpl.id);
     if (it == layerIdToLayerRenderData.end()) {
         return nullptr;
     }
-    const LayerRenderData& result = it->second;
+    LayerRenderData& result = it->second;
     if (result.layerProperties->baseImpl->getTypeInfo() != layerImpl.getTypeInfo()) {
         // Layer data might be outdated, see issue #12432.
         return nullptr;
     }
-    return &result;
+    return &result;   
 }
 
 float GeometryTile::getQueryPadding(const std::vector<const RenderLayer*>& layers) {
diff --git a/src/mbgl/tile/geometry_tile.hpp b/src/mbgl/tile/geometry_tile.hpp
index 231e71190b..18f94b9a1f 100644
--- a/src/mbgl/tile/geometry_tile.hpp
+++ b/src/mbgl/tile/geometry_tile.hpp
@@ -45,6 +45,7 @@ public:
     void upload(gfx::Context&) 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;
 
     void queryRenderedFeatures(
             std::unordered_map<std::string, std::vector<Feature>>& result,
@@ -96,6 +97,8 @@ protected:
         return latestFeatureIndex ? latestFeatureIndex->getData() : nullptr;
     }
 
+    LayerRenderData* getMutableLayerRenderData(const style::Layer::Impl&);
+
 private:
     void markObsolete();
 
diff --git a/src/mbgl/tile/tile.hpp b/src/mbgl/tile/tile.hpp
index 50fd67bedd..651bafe277 100644
--- a/src/mbgl/tile/tile.hpp
+++ b/src/mbgl/tile/tile.hpp
@@ -60,6 +60,10 @@ public:
     virtual const LayerRenderData* getLayerRenderData(const style::Layer::Impl&) const {
         return nullptr;
     }
+    // Updates 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; }
 
     template <class T>
     T* getBucket(const style::Layer::Impl& layer) const {