[core] Upload buckets only when there are feature state changes

7 files changed, 54 insertions, 28 deletions

diff --git a/src/mbgl/renderer/bucket.hpp b/src/mbgl/renderer/bucket.hpp
index 6da0f280e6..6b6f7be27c 100644
--- a/src/mbgl/renderer/bucket.hpp
+++ b/src/mbgl/renderer/bucket.hpp
@@ -47,10 +47,8 @@ public:
         return 0;
     };
 
-    bool needsUpload() const {
-        return hasData() && !uploaded;
-    }
-   
+    bool needsUpload() const { return (hasData() && !uploaded) || stateChanged; }
+
     // The following methods are implemented by buckets that require cross-tile indexing and placement.
 
     // Returns a pair, the first element of which is a bucket cross-tile id
@@ -67,6 +65,7 @@ public:
 protected:
     Bucket() = default;
     std::atomic<bool> uploaded { false };
+    std::atomic<bool> stateChanged{false};
 };
 
 } // namespace mbgl
diff --git a/src/mbgl/renderer/buckets/circle_bucket.cpp b/src/mbgl/renderer/buckets/circle_bucket.cpp
index 560a74781a..2e6051051b 100644
--- a/src/mbgl/renderer/buckets/circle_bucket.cpp
+++ b/src/mbgl/renderer/buckets/circle_bucket.cpp
@@ -35,6 +35,7 @@ void CircleBucket::upload(gfx::UploadPass& uploadPass) {
     }
 
     uploaded = true;
+    stateChanged = false;
 }
 
 bool CircleBucket::hasData() const {
@@ -115,10 +116,11 @@ float CircleBucket::getQueryRadius(const RenderLayer& layer) const {
 void CircleBucket::update(const FeatureStates& states, const GeometryTileLayer& layer, const std::string& layerID,
                           const ImagePositions& imagePositions) {
     auto it = paintPropertyBinders.find(layerID);
+    bool updated = false;
     if (it != paintPropertyBinders.end()) {
-        it->second.updateVertexVectors(states, layer, imagePositions);
-        uploaded = false;
+        updated |= it->second.updateVertexVectors(states, layer, imagePositions);
     }
+    stateChanged = updated;
 }
 
 } // namespace mbgl
diff --git a/src/mbgl/renderer/buckets/fill_bucket.cpp b/src/mbgl/renderer/buckets/fill_bucket.cpp
index 6660934f8d..898595239e 100644
--- a/src/mbgl/renderer/buckets/fill_bucket.cpp
+++ b/src/mbgl/renderer/buckets/fill_bucket.cpp
@@ -133,6 +133,7 @@ void FillBucket::upload(gfx::UploadPass& uploadPass) {
     }
 
     uploaded = true;
+    stateChanged = false;
 }
 
 bool FillBucket::hasData() const {
@@ -148,10 +149,11 @@ float FillBucket::getQueryRadius(const RenderLayer& layer) const {
 void FillBucket::update(const FeatureStates& states, const GeometryTileLayer& layer, const std::string& layerID,
                         const ImagePositions& imagePositions) {
     auto it = paintPropertyBinders.find(layerID);
+    bool updated = false;
     if (it != paintPropertyBinders.end()) {
-        it->second.updateVertexVectors(states, layer, imagePositions);
-        uploaded = false;
+        updated |= it->second.updateVertexVectors(states, layer, imagePositions);
     }
+    stateChanged = updated;
 }
 
 } // namespace mbgl
diff --git a/src/mbgl/renderer/buckets/fill_extrusion_bucket.cpp b/src/mbgl/renderer/buckets/fill_extrusion_bucket.cpp
index 698895fdcf..75c66fea7e 100644
--- a/src/mbgl/renderer/buckets/fill_extrusion_bucket.cpp
+++ b/src/mbgl/renderer/buckets/fill_extrusion_bucket.cpp
@@ -175,6 +175,7 @@ void FillExtrusionBucket::upload(gfx::UploadPass& uploadPass) {
     }
 
     uploaded = true;
+    stateChanged = false;
 }
 
 bool FillExtrusionBucket::hasData() const {
@@ -190,10 +191,11 @@ float FillExtrusionBucket::getQueryRadius(const RenderLayer& layer) const {
 void FillExtrusionBucket::update(const FeatureStates& states, const GeometryTileLayer& layer,
                                  const std::string& layerID, const ImagePositions& imagePositions) {
     auto it = paintPropertyBinders.find(layerID);
+    bool updated = false;
     if (it != paintPropertyBinders.end()) {
-        it->second.updateVertexVectors(states, layer, imagePositions);
-        uploaded = false;
+        updated |= it->second.updateVertexVectors(states, layer, imagePositions);
     }
+    stateChanged = updated;
 }
 
 } // namespace mbgl
diff --git a/src/mbgl/renderer/buckets/line_bucket.cpp b/src/mbgl/renderer/buckets/line_bucket.cpp
index e756722854..f9ce5980e2 100644
--- a/src/mbgl/renderer/buckets/line_bucket.cpp
+++ b/src/mbgl/renderer/buckets/line_bucket.cpp
@@ -522,6 +522,7 @@ void LineBucket::upload(gfx::UploadPass& uploadPass) {
     }
 
     uploaded = true;
+    stateChanged = false;
 }
 
 bool LineBucket::hasData() const {
@@ -554,10 +555,11 @@ float LineBucket::getQueryRadius(const RenderLayer& layer) const {
 void LineBucket::update(const FeatureStates& states, const GeometryTileLayer& layer, const std::string& layerID,
                         const ImagePositions& imagePositions) {
     auto it = paintPropertyBinders.find(layerID);
+    bool updated = false;
     if (it != paintPropertyBinders.end()) {
-        it->second.updateVertexVectors(states, layer, imagePositions);
-        uploaded = false;
+        updated |= it->second.updateVertexVectors(states, layer, imagePositions);
     }
+    stateChanged = updated;
 }
 
 } // namespace mbgl
diff --git a/src/mbgl/renderer/paint_property_binder.hpp b/src/mbgl/renderer/paint_property_binder.hpp
index db9f61411a..42dad26d56 100644
--- a/src/mbgl/renderer/paint_property_binder.hpp
+++ b/src/mbgl/renderer/paint_property_binder.hpp
@@ -109,7 +109,9 @@ public:
                                       const ImagePositions&, const optional<PatternDependency>&,
                                       const style::expression::Value&) = 0;
 
-    virtual void updateVertexVectors(const FeatureStates&, const GeometryTileLayer&, const ImagePositions&) {}
+    virtual bool updateVertexVectors(const FeatureStates&, const GeometryTileLayer&, const ImagePositions&) {
+        return false;
+    }
 
     virtual void updateVertexVector(std::size_t, std::size_t, const GeometryTileFeature&, const FeatureState&) = 0;
 
@@ -221,8 +223,10 @@ public:
         }
     }
 
-    void updateVertexVectors(const FeatureStates& states, const GeometryTileLayer& layer,
+    bool updateVertexVectors(const FeatureStates& states,
+                             const GeometryTileLayer& layer,
                              const ImagePositions&) override {
+        bool updated = false;
         for (const auto& it : states) {
             const auto positions = featureMap.find(it.first);
             if (positions == featureMap.end()) {
@@ -233,9 +237,11 @@ public:
                 std::unique_ptr<GeometryTileFeature> feature = layer.getFeature(pos.featureIndex);
                 if (feature) {
                     updateVertexVector(pos.start, pos.end, *feature, it.second);
+                    updated = true;
                 }
             }
         }
+        return updated;
     }
 
     void updateVertexVector(std::size_t start, std::size_t end, const GeometryTileFeature& feature,
@@ -322,8 +328,10 @@ public:
         }
     }
 
-    void updateVertexVectors(const FeatureStates& states, const GeometryTileLayer& layer,
+    bool updateVertexVectors(const FeatureStates& states,
+                             const GeometryTileLayer& layer,
                              const ImagePositions&) override {
+        bool updated = false;
         for (const auto& it : states) {
             const auto positions = featureMap.find(it.first);
             if (positions == featureMap.end()) {
@@ -334,9 +342,11 @@ public:
                 std::unique_ptr<GeometryTileFeature> feature = layer.getFeature(pos.featureIndex);
                 if (feature) {
                     updateVertexVector(pos.start, pos.end, *feature, it.second);
+                    updated = true;
                 }
             }
         }
+        return updated;
     }
 
     void updateVertexVector(std::size_t start, std::size_t end, const GeometryTileFeature& feature,
@@ -359,13 +369,12 @@ public:
         vertexBuffer = uploadPass.createVertexBuffer(std::move(vertexVector));
     }
 
-    std::tuple<optional<gfx::AttributeBinding>> attributeBinding(const PossiblyEvaluatedPropertyValue<T>& currentValue) const override {
+    std::tuple<optional<gfx::AttributeBinding>> attributeBinding(
+        const PossiblyEvaluatedPropertyValue<T>& currentValue) const override {
         if (currentValue.isConstant()) {
             return {};
         } else {
-            return std::tuple<optional<gfx::AttributeBinding>>{
-                gfx::attributeBinding(*vertexBuffer)
-            };
+            return std::tuple<optional<gfx::AttributeBinding>>{gfx::attributeBinding(*vertexBuffer)};
         }
     }
 
@@ -584,35 +593,44 @@ public:
     PaintPropertyBinders(const EvaluatedProperties& properties, float z)
         : binders(Binder<Ps>::create(properties.template get<Ps>(), z, Ps::defaultValue())...) {
         (void)z; // Workaround for https://gcc.gnu.org/bugzilla/show_bug.cgi?id=56958
+        needsUpload = false;
     }
 
     PaintPropertyBinders(PaintPropertyBinders&&) = default;
     PaintPropertyBinders(const PaintPropertyBinders&) = delete;
 
-    void populateVertexVectors(const GeometryTileFeature& feature, std::size_t length, std::size_t index,
+    void populateVertexVectors(const GeometryTileFeature& feature,
+                               std::size_t length,
+                               std::size_t index,
                                const ImagePositions& patternPositions,
                                const optional<PatternDependency>& patternDependencies,
                                const style::expression::Value& formattedSection = {}) {
-        util::ignore({(binders.template get<Ps>()->populateVertexVector(feature, length, index, patternPositions,
-                                                                        patternDependencies, formattedSection),
+        util::ignore({(binders.template get<Ps>()->populateVertexVector(
+                           feature, length, index, patternPositions, patternDependencies, formattedSection),
                        0)...});
+        needsUpload = true;
     }
 
-    void updateVertexVectors(const FeatureStates& states, const GeometryTileLayer& layer,
+    bool updateVertexVectors(const FeatureStates& states,
+                             const GeometryTileLayer& layer,
                              const ImagePositions& imagePositions) {
-        util::ignore({(binders.template get<Ps>()->updateVertexVectors(states, layer, imagePositions), 0)...});
+        util::ignore(
+            {(needsUpload |= binders.template get<Ps>()->updateVertexVectors(states, layer, imagePositions), 0)...});
+        return needsUpload;
     }
 
     void setPatternParameters(const optional<ImagePosition>& posA, const optional<ImagePosition>& posB, const CrossfadeParameters& crossfade) const {
-        util::ignore({
-            (binders.template get<Ps>()->setPatternParameters(posA, posB, crossfade), 0)...
-        });
+        util::ignore({(binders.template get<Ps>()->setPatternParameters(posA, posB, crossfade), 0)...});
     }
 
     void upload(gfx::UploadPass& uploadPass) {
+        if (!needsUpload) {
+            return;
+        }
         util::ignore({
             (binders.template get<Ps>()->upload(uploadPass), 0)...
         });
+        needsUpload = false;
     }
 
     template <class P>
@@ -652,6 +670,7 @@ public:
 
 private:
     Binders binders;
+    bool needsUpload = false;
 };
 
 } // namespace mbgl
diff --git a/src/mbgl/tile/geometry_tile.cpp b/src/mbgl/tile/geometry_tile.cpp
index 4a3d91455a..47c11575e1 100644
--- a/src/mbgl/tile/geometry_tile.cpp
+++ b/src/mbgl/tile/geometry_tile.cpp
@@ -75,7 +75,7 @@ optional<ImagePosition> GeometryTileRenderData::getPattern(const std::string& pa
 void GeometryTileRenderData::upload(gfx::UploadPass& uploadPass) {
     if (!layoutResult) return;
 
-    auto uploadFn = [&] (Bucket& bucket) {
+    auto uploadFn = [&](Bucket& bucket) {
         if (bucket.needsUpload()) {
             bucket.upload(uploadPass);
         }