From da4eb01bc7b74b09405f9911ef506c190b426f9d Mon Sep 17 00:00:00 2001
From: Andrew Hay Kurtz <andrew.hay.kurtz@gmail.com>
Date: Tue, 17 Sep 2019 17:35:08 -0700
Subject: [PATCH] [core] Enable 'line-sort-key' and 'fill-sort-key' layout
 properties (#15839)

- Generate style code for 'line-sort-key' and 'symbol-sort-key'

- Add new layout properties to FillLayer::Impl, FillBucket, and FillLayerFactory

- Fix consistency of paint and layout properties type alias usage in FillBucket, LineBucket

- Add optional feature sorting to fill and line Layout creation

- Enable node render tests for fill-sort-key and line-sort-key

- Fix FillBucket test construction

- Prefer emplace_back to push_back for PatternFeature container

- Fix buggy static_cast for PatternFeature indices

- Maintain sort of features as they are created

- Switch pattern layout features container to list from vector for better insert performance

- Fix formatting expected by sanity check

- Use subclass PatternLayoutSorted to work around lack of template functions

- Fix to retain source order for features with equivalent sort keys during sorting

- [core] Fix clang-format

- [core] Address review comments

- [core] Pass inserting strategy class at compile time

- [core] Use sorted strategy only if sort key is defined in layout

- [core] Update style generator

- [core] Merge PatternLayout and PatternLayoutSorted classes

  - Use static methods for inserter strategies
  - Merge PatternLayout and PatternLayoutSorted classes
---
 include/mbgl/style/layers/fill_layer.hpp      |   6 +
 include/mbgl/style/layers/line_layer.hpp      |   4 +
 scripts/generate-style-code.js                |   2 -
 src/mbgl/layermanager/fill_layer_factory.cpp  |  10 +-
 src/mbgl/layermanager/line_layer_factory.cpp  |  10 +-
 src/mbgl/layout/pattern_layout.hpp            |  59 +++++++--
 src/mbgl/renderer/buckets/fill_bucket.hpp     |   3 +-
 src/mbgl/renderer/buckets/line_bucket.cpp     |   4 +-
 src/mbgl/renderer/buckets/line_bucket.hpp     |   1 -
 src/mbgl/style/layers/fill_layer.cpp          |  37 +++++-
 src/mbgl/style/layers/fill_layer_impl.cpp     |   3 +-
 src/mbgl/style/layers/fill_layer_impl.hpp     |   2 +-
 .../style/layers/fill_layer_properties.hpp    |   9 ++
 src/mbgl/style/layers/layer.cpp.ejs           |   3 +-
 src/mbgl/style/layers/line_layer.cpp          |  40 ++++--
 .../style/layers/line_layer_properties.hpp    |   8 +-
 src/mbgl/style/layers/symbol_layer.cpp        | 123 ++++++------------
 test/gl/bucket.test.cpp                       |   4 +-
 18 files changed, 204 insertions(+), 124 deletions(-)

diff --git a/include/mbgl/style/layers/fill_layer.hpp b/include/mbgl/style/layers/fill_layer.hpp
index d5249c269..17dd66859 100644
--- a/include/mbgl/style/layers/fill_layer.hpp
+++ b/include/mbgl/style/layers/fill_layer.hpp
@@ -25,6 +25,12 @@ public:
 
     StyleProperty getProperty(const std::string& name) const final;
 
+    // Layout properties
+
+    static PropertyValue<float> getDefaultFillSortKey();
+    const PropertyValue<float>& getFillSortKey() const;
+    void setFillSortKey(const PropertyValue<float>&);
+
     // Paint properties
 
     static PropertyValue<bool> getDefaultFillAntialias();
diff --git a/include/mbgl/style/layers/line_layer.hpp b/include/mbgl/style/layers/line_layer.hpp
index 5b93d99ce..9f0882765 100644
--- a/include/mbgl/style/layers/line_layer.hpp
+++ b/include/mbgl/style/layers/line_layer.hpp
@@ -46,6 +46,10 @@ public:
     const PropertyValue<float>& getLineRoundLimit() const;
     void setLineRoundLimit(const PropertyValue<float>&);
 
+    static PropertyValue<float> getDefaultLineSortKey();
+    const PropertyValue<float>& getLineSortKey() const;
+    void setLineSortKey(const PropertyValue<float>&);
+
     // Paint properties
 
     static PropertyValue<float> getDefaultLineBlur();
diff --git a/scripts/generate-style-code.js b/scripts/generate-style-code.js
index db2548680..804d4a243 100755
--- a/scripts/generate-style-code.js
+++ b/scripts/generate-style-code.js
@@ -8,8 +8,6 @@ const colorParser = require('csscolorparser');
 
 // FIXME: https://github.com/mapbox/mapbox-gl-native/issues/15008
 delete spec.layout_circle["circle-sort-key"]
-delete spec.layout_line["line-sort-key"]
-delete spec.layout_fill["fill-sort-key"]
 
 require('./style-code');
 
diff --git a/src/mbgl/layermanager/fill_layer_factory.cpp b/src/mbgl/layermanager/fill_layer_factory.cpp
index 265fdc69f..0c7c270c9 100644
--- a/src/mbgl/layermanager/fill_layer_factory.cpp
+++ b/src/mbgl/layermanager/fill_layer_factory.cpp
@@ -29,8 +29,14 @@ FillLayerFactory::createLayout(const LayoutParameters& parameters,
                                std::unique_ptr<GeometryTileLayer> layer,
                                const std::vector<Immutable<style::LayerProperties>>& group) noexcept {
     using namespace style;
-    using LayoutType = PatternLayout<FillBucket, FillLayerProperties, FillPattern>;
-    return std::make_unique<LayoutType>(parameters.bucketParameters, group, std::move(layer), parameters);
+    using LayoutTypeUnsorted = PatternLayout<FillBucket, FillLayerProperties, FillPattern, FillLayoutProperties>;
+    using LayoutTypeSorted =
+        PatternLayout<FillBucket, FillLayerProperties, FillPattern, FillLayoutProperties, FillSortKey>;
+    auto layerProperties = staticImmutableCast<FillLayerProperties>(group.front());
+    if (layerProperties->layerImpl().layout.get<FillSortKey>().isUndefined()) {
+        return std::make_unique<LayoutTypeUnsorted>(parameters.bucketParameters, group, std::move(layer), parameters);
+    }
+    return std::make_unique<LayoutTypeSorted>(parameters.bucketParameters, group, std::move(layer), parameters);
 }
 
 std::unique_ptr<RenderLayer> FillLayerFactory::createRenderLayer(Immutable<style::Layer::Impl> impl) noexcept {
diff --git a/src/mbgl/layermanager/line_layer_factory.cpp b/src/mbgl/layermanager/line_layer_factory.cpp
index 5770b19f3..53b91a070 100644
--- a/src/mbgl/layermanager/line_layer_factory.cpp
+++ b/src/mbgl/layermanager/line_layer_factory.cpp
@@ -28,8 +28,14 @@ std::unique_ptr<Layout> LineLayerFactory::createLayout(const LayoutParameters& p
                                                        std::unique_ptr<GeometryTileLayer> layer,
                                                        const std::vector<Immutable<style::LayerProperties>>& group) noexcept {
     using namespace style;
-    using LayoutType = PatternLayout<LineBucket, LineLayerProperties, LinePattern, LineLayoutProperties::PossiblyEvaluated>;
-    return std::make_unique<LayoutType>(parameters.bucketParameters, group, std::move(layer), parameters);
+    using LayoutTypeUnsorted = PatternLayout<LineBucket, LineLayerProperties, LinePattern, LineLayoutProperties>;
+    using LayoutTypeSorted =
+        PatternLayout<LineBucket, LineLayerProperties, LinePattern, LineLayoutProperties, LineSortKey>;
+    auto layerProperties = staticImmutableCast<LineLayerProperties>(group.front());
+    if (layerProperties->layerImpl().layout.get<LineSortKey>().isUndefined()) {
+        return std::make_unique<LayoutTypeUnsorted>(parameters.bucketParameters, group, std::move(layer), parameters);
+    }
+    return std::make_unique<LayoutTypeSorted>(parameters.bucketParameters, group, std::move(layer), parameters);
 }
 
 std::unique_ptr<RenderLayer> LineLayerFactory::createRenderLayer(Immutable<style::Layer::Impl> impl) noexcept {
diff --git a/src/mbgl/layout/pattern_layout.hpp b/src/mbgl/layout/pattern_layout.hpp
index 48531e836..44729dea8 100644
--- a/src/mbgl/layout/pattern_layout.hpp
+++ b/src/mbgl/layout/pattern_layout.hpp
@@ -1,4 +1,5 @@
 #pragma once
+#include <list>
 #include <mbgl/geometry/feature_index.hpp>
 #include <mbgl/layout/layout.hpp>
 #include <mbgl/renderer/bucket_parameters.hpp>
@@ -19,15 +20,58 @@ using PatternLayerMap = std::map<std::string, PatternDependency>;
 
 class PatternFeature  {
 public:
-    const uint32_t i;
+    PatternFeature(std::size_t i_,
+                   std::unique_ptr<GeometryTileFeature> feature_,
+                   PatternLayerMap patterns_,
+                   float sortKey_ = 0.0f)
+        : i(i_), feature(std::move(feature_)), patterns(std::move(patterns_)), sortKey(sortKey_) {}
+
+    friend bool operator<(const PatternFeature& lhs, const PatternFeature& rhs) { return lhs.sortKey < rhs.sortKey; }
+
+    std::size_t i;
     std::unique_ptr<GeometryTileFeature> feature;
     PatternLayerMap patterns;
+    float sortKey;
+};
+
+template <typename SortKeyPropertyType>
+struct PatternFeatureInserter;
+
+template <>
+struct PatternFeatureInserter<void> {
+    template <typename PropertiesType>
+    static void insert(std::vector<PatternFeature>& features,
+                       std::size_t index,
+                       std::unique_ptr<GeometryTileFeature> feature,
+                       PatternLayerMap patternDependencyMap,
+                       float /*zoom*/,
+                       const PropertiesType&) {
+        features.emplace_back(index, std::move(feature), std::move(patternDependencyMap));
+    }
+};
+
+template <class SortKeyPropertyType>
+struct PatternFeatureInserter {
+    template <typename PropertiesType>
+    static void insert(std::vector<PatternFeature>& features,
+                       std::size_t index,
+                       std::unique_ptr<GeometryTileFeature> feature,
+                       PatternLayerMap patternDependencyMap,
+                       float zoom,
+                       const PropertiesType& properties) {
+        const auto& sortKeyProperty = properties.template get<SortKeyPropertyType>();
+        float sortKey = sortKeyProperty.evaluate(*feature, zoom, SortKeyPropertyType::defaultValue());
+        PatternFeature patternFeature{index, std::move(feature), std::move(patternDependencyMap), sortKey};
+        const auto lowerBound = std::lower_bound(features.cbegin(), features.cend(), patternFeature);
+        features.insert(lowerBound, std::move(patternFeature));
+    }
 };
 
 template <class BucketType,
           class LayerPropertiesType,
           class PatternPropertyType,
-          class PossiblyEvaluatedLayoutPropertiesType = typename style::Properties<>::PossiblyEvaluated>
+          class LayoutPropertiesType = typename style::Properties<>,
+          class SortKeyPropertyType = void>
 class PatternLayout : public Layout {
 public:
     PatternLayout(const BucketParameters& parameters,
@@ -98,12 +142,12 @@ public:
                     }
                 }
             }
-            features.push_back({static_cast<uint32_t>(i), std::move(feature), patternDependencyMap});
+
+            PatternFeatureInserter<SortKeyPropertyType>::insert(
+                features, i, std::move(feature), std::move(patternDependencyMap), zoom, layout);
         }
     };
 
-    ~PatternLayout() final = default;
-
     bool hasDependencies() const override {
         return hasPattern;
     }
@@ -126,13 +170,13 @@ public:
         }
     };
 
-private:
+protected:
     std::map<std::string, Immutable<style::LayerProperties>> layerPropertiesMap;
     std::string bucketLeaderID;
 
     const std::unique_ptr<GeometryTileLayer> sourceLayer;
     std::vector<PatternFeature> features;
-    PossiblyEvaluatedLayoutPropertiesType layout;
+    typename LayoutPropertiesType::PossiblyEvaluated layout;
 
     const float zoom;
     const uint32_t overscaling;
@@ -141,4 +185,3 @@ private:
 };
 
 } // namespace mbgl
-
diff --git a/src/mbgl/renderer/buckets/fill_bucket.hpp b/src/mbgl/renderer/buckets/fill_bucket.hpp
index 9f65e774c..7a3f68112 100644
--- a/src/mbgl/renderer/buckets/fill_bucket.hpp
+++ b/src/mbgl/renderer/buckets/fill_bucket.hpp
@@ -18,8 +18,7 @@ class RenderFillLayer;
 class FillBucket final : public Bucket {
 public:
     ~FillBucket() override;
-    using PossiblyEvaluatedPaintProperties = style::FillPaintProperties::PossiblyEvaluated;
-    using PossiblyEvaluatedLayoutProperties = style::Properties<>::PossiblyEvaluated;
+    using PossiblyEvaluatedLayoutProperties = style::FillLayoutProperties::PossiblyEvaluated;
 
     FillBucket(const PossiblyEvaluatedLayoutProperties layout,
                const std::map<std::string, Immutable<style::LayerProperties>>& layerPaintProperties,
diff --git a/src/mbgl/renderer/buckets/line_bucket.cpp b/src/mbgl/renderer/buckets/line_bucket.cpp
index e75672285..895d5aa1b 100644
--- a/src/mbgl/renderer/buckets/line_bucket.cpp
+++ b/src/mbgl/renderer/buckets/line_bucket.cpp
@@ -10,8 +10,8 @@ namespace mbgl {
 
 using namespace style;
 
-LineBucket::LineBucket(const style::LineLayoutProperties::PossiblyEvaluated layout_,
-                       const std::map<std::string, Immutable<style::LayerProperties>>& layerPaintProperties,
+LineBucket::LineBucket(const LineBucket::PossiblyEvaluatedLayoutProperties layout_,
+                       const std::map<std::string, Immutable<LayerProperties>>& layerPaintProperties,
                        const float zoom_,
                        const uint32_t overscaling_)
     : layout(layout_), zoom(zoom_), overscaling(overscaling_) {
diff --git a/src/mbgl/renderer/buckets/line_bucket.hpp b/src/mbgl/renderer/buckets/line_bucket.hpp
index 6343aab2b..a3237c9df 100644
--- a/src/mbgl/renderer/buckets/line_bucket.hpp
+++ b/src/mbgl/renderer/buckets/line_bucket.hpp
@@ -17,7 +17,6 @@ class RenderLineLayer;
 
 class LineBucket final : public Bucket {
 public:
-    using PossiblyEvaluatedPaintProperties = style::LinePaintProperties::PossiblyEvaluated;
     using PossiblyEvaluatedLayoutProperties = style::LineLayoutProperties::PossiblyEvaluated;
 
     LineBucket(const PossiblyEvaluatedLayoutProperties layout,
diff --git a/src/mbgl/style/layers/fill_layer.cpp b/src/mbgl/style/layers/fill_layer.cpp
index 9a4303787..757738dfe 100644
--- a/src/mbgl/style/layers/fill_layer.cpp
+++ b/src/mbgl/style/layers/fill_layer.cpp
@@ -55,11 +55,27 @@ std::unique_ptr<Layer> FillLayer::cloneRef(const std::string& id_) const {
     return std::make_unique<FillLayer>(std::move(impl_));
 }
 
-void FillLayer::Impl::stringifyLayout(rapidjson::Writer<rapidjson::StringBuffer>&) const {
+void FillLayer::Impl::stringifyLayout(rapidjson::Writer<rapidjson::StringBuffer>& writer) const {
+    layout.stringify(writer);
 }
 
 // Layout properties
 
+PropertyValue<float> FillLayer::getDefaultFillSortKey() {
+    return FillSortKey::defaultValue();
+}
+
+const PropertyValue<float>& FillLayer::getFillSortKey() const {
+    return impl().layout.get<FillSortKey>();
+}
+
+void FillLayer::setFillSortKey(const PropertyValue<float>& value) {
+    if (value == getFillSortKey()) return;
+    auto impl_ = mutableImpl();
+    impl_->layout.get<FillSortKey>() = value;
+    baseImpl = std::move(impl_);
+    observer->onLayerChanged(*this);
+}
 
 // Paint properties
 
@@ -271,6 +287,7 @@ enum class Property : uint8_t {
     FillPatternTransition,
     FillTranslateTransition,
     FillTranslateAnchorTransition,
+    FillSortKey,
 };
 
 template <typename T>
@@ -292,7 +309,8 @@ MAPBOX_ETERNAL_CONSTEXPR const auto layerProperties = mapbox::eternal::hash_map<
      {"fill-outline-color-transition", toUint8(Property::FillOutlineColorTransition)},
      {"fill-pattern-transition", toUint8(Property::FillPatternTransition)},
      {"fill-translate-transition", toUint8(Property::FillTranslateTransition)},
-     {"fill-translate-anchor-transition", toUint8(Property::FillTranslateAnchorTransition)}});
+     {"fill-translate-anchor-transition", toUint8(Property::FillTranslateAnchorTransition)},
+     {"fill-sort-key", toUint8(Property::FillSortKey)}});
 } // namespace
 
 optional<Error> FillLayer::setProperty(const std::string& name, const Convertible& value) {
@@ -331,15 +349,22 @@ optional<Error> FillLayer::setProperty(const std::string& name, const Convertibl
             return nullopt;
         }
     }
-    if (property == Property::FillOpacity) {
+    if (property == Property::FillOpacity || property == Property::FillSortKey) {
         Error error;
         const auto& typedValue = convert<PropertyValue<float>>(value, error, true, false);
         if (!typedValue) {
             return error;
         }
 
-        setFillOpacity(*typedValue);
-        return nullopt;
+        if (property == Property::FillOpacity) {
+            setFillOpacity(*typedValue);
+            return nullopt;
+        }
+
+        if (property == Property::FillSortKey) {
+            setFillSortKey(*typedValue);
+            return nullopt;
+        }
     }
     if (property == Property::FillPattern) {
         Error error;
@@ -451,6 +476,8 @@ StyleProperty FillLayer::getProperty(const std::string& name) const {
             return makeStyleProperty(getFillTranslateTransition());
         case Property::FillTranslateAnchorTransition:
             return makeStyleProperty(getFillTranslateAnchorTransition());
+        case Property::FillSortKey:
+            return makeStyleProperty(getFillSortKey());
     }
     return {};
 }
diff --git a/src/mbgl/style/layers/fill_layer_impl.cpp b/src/mbgl/style/layers/fill_layer_impl.cpp
index 45e350e00..ad994a06b 100644
--- a/src/mbgl/style/layers/fill_layer_impl.cpp
+++ b/src/mbgl/style/layers/fill_layer_impl.cpp
@@ -6,8 +6,7 @@ namespace style {
 bool FillLayer::Impl::hasLayoutDifference(const Layer::Impl& other) const {
     assert(other.getTypeInfo() == getTypeInfo());
     const auto& impl = static_cast<const style::FillLayer::Impl&>(other);
-    return filter     != impl.filter ||
-           visibility != impl.visibility ||
+    return filter != impl.filter || visibility != impl.visibility || layout != impl.layout ||
            paint.get<FillPattern>().value != impl.paint.get<FillPattern>().value ||
            paint.hasDataDrivenPropertyDifference(impl.paint);
 }
diff --git a/src/mbgl/style/layers/fill_layer_impl.hpp b/src/mbgl/style/layers/fill_layer_impl.hpp
index 92f3c9728..2a4fa44eb 100644
--- a/src/mbgl/style/layers/fill_layer_impl.hpp
+++ b/src/mbgl/style/layers/fill_layer_impl.hpp
@@ -14,7 +14,7 @@ public:
     bool hasLayoutDifference(const Layer::Impl&) const override;
     void stringifyLayout(rapidjson::Writer<rapidjson::StringBuffer>&) const override;
 
-    Properties<>::Unevaluated layout;
+    FillLayoutProperties::Unevaluated layout;
     FillPaintProperties::Transitionable paint;
 
     DECLARE_LAYER_TYPE_INFO;
diff --git a/src/mbgl/style/layers/fill_layer_properties.hpp b/src/mbgl/style/layers/fill_layer_properties.hpp
index ad8ed8489..7867fe8f0 100644
--- a/src/mbgl/style/layers/fill_layer_properties.hpp
+++ b/src/mbgl/style/layers/fill_layer_properties.hpp
@@ -16,6 +16,11 @@
 namespace mbgl {
 namespace style {
 
+struct FillSortKey : DataDrivenLayoutProperty<float> {
+    static constexpr const char *name() { return "fill-sort-key"; }
+    static float defaultValue() { return 0; }
+};
+
 struct FillAntialias : PaintProperty<bool> {
     static bool defaultValue() { return true; }
 };
@@ -44,6 +49,10 @@ struct FillTranslateAnchor : PaintProperty<TranslateAnchorType> {
     static TranslateAnchorType defaultValue() { return TranslateAnchorType::Map; }
 };
 
+class FillLayoutProperties : public Properties<
+    FillSortKey
+> {};
+
 class FillPaintProperties : public Properties<
     FillAntialias,
     FillColor,
diff --git a/src/mbgl/style/layers/layer.cpp.ejs b/src/mbgl/style/layers/layer.cpp.ejs
index bcec4098d..e74e7ab34 100644
--- a/src/mbgl/style/layers/layer.cpp.ejs
+++ b/src/mbgl/style/layers/layer.cpp.ejs
@@ -149,8 +149,7 @@ const <%- propertyValueType(property) %>& <%- camelize(type) %>Layer::get<%- cam
 }
 
 void <%- camelize(type) %>Layer::set<%- camelize(property.name) %>(const <%- propertyValueType(property) %>& value) {
-    if (value == get<%- camelize(property.name) %>())
-        return;
+    if (value == get<%- camelize(property.name) %>()) return;
     auto impl_ = mutableImpl();
     impl_->layout.get<<%- camelize(property.name) %>>() = value;
     baseImpl = std::move(impl_);
diff --git a/src/mbgl/style/layers/line_layer.cpp b/src/mbgl/style/layers/line_layer.cpp
index 0f3e49963..caa7f44e0 100644
--- a/src/mbgl/style/layers/line_layer.cpp
+++ b/src/mbgl/style/layers/line_layer.cpp
@@ -70,8 +70,7 @@ const PropertyValue<LineCapType>& LineLayer::getLineCap() const {
 }
 
 void LineLayer::setLineCap(const PropertyValue<LineCapType>& value) {
-    if (value == getLineCap())
-        return;
+    if (value == getLineCap()) return;
     auto impl_ = mutableImpl();
     impl_->layout.get<LineCap>() = value;
     baseImpl = std::move(impl_);
@@ -86,8 +85,7 @@ const PropertyValue<LineJoinType>& LineLayer::getLineJoin() const {
 }
 
 void LineLayer::setLineJoin(const PropertyValue<LineJoinType>& value) {
-    if (value == getLineJoin())
-        return;
+    if (value == getLineJoin()) return;
     auto impl_ = mutableImpl();
     impl_->layout.get<LineJoin>() = value;
     baseImpl = std::move(impl_);
@@ -102,8 +100,7 @@ const PropertyValue<float>& LineLayer::getLineMiterLimit() const {
 }
 
 void LineLayer::setLineMiterLimit(const PropertyValue<float>& value) {
-    if (value == getLineMiterLimit())
-        return;
+    if (value == getLineMiterLimit()) return;
     auto impl_ = mutableImpl();
     impl_->layout.get<LineMiterLimit>() = value;
     baseImpl = std::move(impl_);
@@ -118,13 +115,27 @@ const PropertyValue<float>& LineLayer::getLineRoundLimit() const {
 }
 
 void LineLayer::setLineRoundLimit(const PropertyValue<float>& value) {
-    if (value == getLineRoundLimit())
-        return;
+    if (value == getLineRoundLimit()) return;
     auto impl_ = mutableImpl();
     impl_->layout.get<LineRoundLimit>() = value;
     baseImpl = std::move(impl_);
     observer->onLayerChanged(*this);
 }
+PropertyValue<float> LineLayer::getDefaultLineSortKey() {
+    return LineSortKey::defaultValue();
+}
+
+const PropertyValue<float>& LineLayer::getLineSortKey() const {
+    return impl().layout.get<LineSortKey>();
+}
+
+void LineLayer::setLineSortKey(const PropertyValue<float>& value) {
+    if (value == getLineSortKey()) return;
+    auto impl_ = mutableImpl();
+    impl_->layout.get<LineSortKey>() = value;
+    baseImpl = std::move(impl_);
+    observer->onLayerChanged(*this);
+}
 
 // Paint properties
 
@@ -457,6 +468,7 @@ enum class Property : uint8_t {
     LineJoin,
     LineMiterLimit,
     LineRoundLimit,
+    LineSortKey,
 };
 
 template <typename T>
@@ -490,7 +502,8 @@ MAPBOX_ETERNAL_CONSTEXPR const auto layerProperties = mapbox::eternal::hash_map<
      {"line-cap", toUint8(Property::LineCap)},
      {"line-join", toUint8(Property::LineJoin)},
      {"line-miter-limit", toUint8(Property::LineMiterLimit)},
-     {"line-round-limit", toUint8(Property::LineRoundLimit)}});
+     {"line-round-limit", toUint8(Property::LineRoundLimit)},
+     {"line-sort-key", toUint8(Property::LineSortKey)}});
 } // namespace
 
 optional<Error> LineLayer::setProperty(const std::string& name, const Convertible& value) {
@@ -503,7 +516,7 @@ optional<Error> LineLayer::setProperty(const std::string& name, const Convertibl
     auto property = static_cast<Property>(it->second);
 
     if (property == Property::LineBlur || property == Property::LineGapWidth || property == Property::LineOffset ||
-        property == Property::LineOpacity || property == Property::LineWidth) {
+        property == Property::LineOpacity || property == Property::LineWidth || property == Property::LineSortKey) {
         Error error;
         const auto& typedValue = convert<PropertyValue<float>>(value, error, true, false);
         if (!typedValue) {
@@ -534,6 +547,11 @@ optional<Error> LineLayer::setProperty(const std::string& name, const Convertibl
             setLineWidth(*typedValue);
             return nullopt;
         }
+
+        if (property == Property::LineSortKey) {
+            setLineSortKey(*typedValue);
+            return nullopt;
+        }
     }
     if (property == Property::LineColor) {
         Error error;
@@ -756,6 +774,8 @@ StyleProperty LineLayer::getProperty(const std::string& name) const {
             return makeStyleProperty(getLineMiterLimit());
         case Property::LineRoundLimit:
             return makeStyleProperty(getLineRoundLimit());
+        case Property::LineSortKey:
+            return makeStyleProperty(getLineSortKey());
     }
     return {};
 }
diff --git a/src/mbgl/style/layers/line_layer_properties.hpp b/src/mbgl/style/layers/line_layer_properties.hpp
index 29ba9ec64..b2bd4aa40 100644
--- a/src/mbgl/style/layers/line_layer_properties.hpp
+++ b/src/mbgl/style/layers/line_layer_properties.hpp
@@ -36,6 +36,11 @@ struct LineRoundLimit : LayoutProperty<float> {
     static float defaultValue() { return 1; }
 };
 
+struct LineSortKey : DataDrivenLayoutProperty<float> {
+    static constexpr const char *name() { return "line-sort-key"; }
+    static float defaultValue() { return 0; }
+};
+
 struct LineBlur : DataDrivenPaintProperty<float, attributes::blur, uniforms::blur> {
     static float defaultValue() { return 0; }
 };
@@ -88,7 +93,8 @@ class LineLayoutProperties : public Properties<
     LineCap,
     LineJoin,
     LineMiterLimit,
-    LineRoundLimit
+    LineRoundLimit,
+    LineSortKey
 > {};
 
 class LinePaintProperties : public Properties<
diff --git a/src/mbgl/style/layers/symbol_layer.cpp b/src/mbgl/style/layers/symbol_layer.cpp
index fe9658b25..39c2f2a05 100644
--- a/src/mbgl/style/layers/symbol_layer.cpp
+++ b/src/mbgl/style/layers/symbol_layer.cpp
@@ -70,8 +70,7 @@ const PropertyValue<bool>& SymbolLayer::getIconAllowOverlap() const {
 }
 
 void SymbolLayer::setIconAllowOverlap(const PropertyValue<bool>& value) {
-    if (value == getIconAllowOverlap())
-        return;
+    if (value == getIconAllowOverlap()) return;
     auto impl_ = mutableImpl();
     impl_->layout.get<IconAllowOverlap>() = value;
     baseImpl = std::move(impl_);
@@ -86,8 +85,7 @@ const PropertyValue<SymbolAnchorType>& SymbolLayer::getIconAnchor() const {
 }
 
 void SymbolLayer::setIconAnchor(const PropertyValue<SymbolAnchorType>& value) {
-    if (value == getIconAnchor())
-        return;
+    if (value == getIconAnchor()) return;
     auto impl_ = mutableImpl();
     impl_->layout.get<IconAnchor>() = value;
     baseImpl = std::move(impl_);
@@ -102,8 +100,7 @@ const PropertyValue<bool>& SymbolLayer::getIconIgnorePlacement() const {
 }
 
 void SymbolLayer::setIconIgnorePlacement(const PropertyValue<bool>& value) {
-    if (value == getIconIgnorePlacement())
-        return;
+    if (value == getIconIgnorePlacement()) return;
     auto impl_ = mutableImpl();
     impl_->layout.get<IconIgnorePlacement>() = value;
     baseImpl = std::move(impl_);
@@ -118,8 +115,7 @@ const PropertyValue<expression::Image>& SymbolLayer::getIconImage() const {
 }
 
 void SymbolLayer::setIconImage(const PropertyValue<expression::Image>& value) {
-    if (value == getIconImage())
-        return;
+    if (value == getIconImage()) return;
     auto impl_ = mutableImpl();
     impl_->layout.get<IconImage>() = value;
     baseImpl = std::move(impl_);
@@ -134,8 +130,7 @@ const PropertyValue<bool>& SymbolLayer::getIconKeepUpright() const {
 }
 
 void SymbolLayer::setIconKeepUpright(const PropertyValue<bool>& value) {
-    if (value == getIconKeepUpright())
-        return;
+    if (value == getIconKeepUpright()) return;
     auto impl_ = mutableImpl();
     impl_->layout.get<IconKeepUpright>() = value;
     baseImpl = std::move(impl_);
@@ -150,8 +145,7 @@ const PropertyValue<std::array<float, 2>>& SymbolLayer::getIconOffset() const {
 }
 
 void SymbolLayer::setIconOffset(const PropertyValue<std::array<float, 2>>& value) {
-    if (value == getIconOffset())
-        return;
+    if (value == getIconOffset()) return;
     auto impl_ = mutableImpl();
     impl_->layout.get<IconOffset>() = value;
     baseImpl = std::move(impl_);
@@ -166,8 +160,7 @@ const PropertyValue<bool>& SymbolLayer::getIconOptional() const {
 }
 
 void SymbolLayer::setIconOptional(const PropertyValue<bool>& value) {
-    if (value == getIconOptional())
-        return;
+    if (value == getIconOptional()) return;
     auto impl_ = mutableImpl();
     impl_->layout.get<IconOptional>() = value;
     baseImpl = std::move(impl_);
@@ -182,8 +175,7 @@ const PropertyValue<float>& SymbolLayer::getIconPadding() const {
 }
 
 void SymbolLayer::setIconPadding(const PropertyValue<float>& value) {
-    if (value == getIconPadding())
-        return;
+    if (value == getIconPadding()) return;
     auto impl_ = mutableImpl();
     impl_->layout.get<IconPadding>() = value;
     baseImpl = std::move(impl_);
@@ -198,8 +190,7 @@ const PropertyValue<AlignmentType>& SymbolLayer::getIconPitchAlignment() const {
 }
 
 void SymbolLayer::setIconPitchAlignment(const PropertyValue<AlignmentType>& value) {
-    if (value == getIconPitchAlignment())
-        return;
+    if (value == getIconPitchAlignment()) return;
     auto impl_ = mutableImpl();
     impl_->layout.get<IconPitchAlignment>() = value;
     baseImpl = std::move(impl_);
@@ -214,8 +205,7 @@ const PropertyValue<float>& SymbolLayer::getIconRotate() const {
 }
 
 void SymbolLayer::setIconRotate(const PropertyValue<float>& value) {
-    if (value == getIconRotate())
-        return;
+    if (value == getIconRotate()) return;
     auto impl_ = mutableImpl();
     impl_->layout.get<IconRotate>() = value;
     baseImpl = std::move(impl_);
@@ -230,8 +220,7 @@ const PropertyValue<AlignmentType>& SymbolLayer::getIconRotationAlignment() cons
 }
 
 void SymbolLayer::setIconRotationAlignment(const PropertyValue<AlignmentType>& value) {
-    if (value == getIconRotationAlignment())
-        return;
+    if (value == getIconRotationAlignment()) return;
     auto impl_ = mutableImpl();
     impl_->layout.get<IconRotationAlignment>() = value;
     baseImpl = std::move(impl_);
@@ -246,8 +235,7 @@ const PropertyValue<float>& SymbolLayer::getIconSize() const {
 }
 
 void SymbolLayer::setIconSize(const PropertyValue<float>& value) {
-    if (value == getIconSize())
-        return;
+    if (value == getIconSize()) return;
     auto impl_ = mutableImpl();
     impl_->layout.get<IconSize>() = value;
     baseImpl = std::move(impl_);
@@ -262,8 +250,7 @@ const PropertyValue<IconTextFitType>& SymbolLayer::getIconTextFit() const {
 }
 
 void SymbolLayer::setIconTextFit(const PropertyValue<IconTextFitType>& value) {
-    if (value == getIconTextFit())
-        return;
+    if (value == getIconTextFit()) return;
     auto impl_ = mutableImpl();
     impl_->layout.get<IconTextFit>() = value;
     baseImpl = std::move(impl_);
@@ -278,8 +265,7 @@ const PropertyValue<std::array<float, 4>>& SymbolLayer::getIconTextFitPadding()
 }
 
 void SymbolLayer::setIconTextFitPadding(const PropertyValue<std::array<float, 4>>& value) {
-    if (value == getIconTextFitPadding())
-        return;
+    if (value == getIconTextFitPadding()) return;
     auto impl_ = mutableImpl();
     impl_->layout.get<IconTextFitPadding>() = value;
     baseImpl = std::move(impl_);
@@ -294,8 +280,7 @@ const PropertyValue<bool>& SymbolLayer::getSymbolAvoidEdges() const {
 }
 
 void SymbolLayer::setSymbolAvoidEdges(const PropertyValue<bool>& value) {
-    if (value == getSymbolAvoidEdges())
-        return;
+    if (value == getSymbolAvoidEdges()) return;
     auto impl_ = mutableImpl();
     impl_->layout.get<SymbolAvoidEdges>() = value;
     baseImpl = std::move(impl_);
@@ -310,8 +295,7 @@ const PropertyValue<SymbolPlacementType>& SymbolLayer::getSymbolPlacement() cons
 }
 
 void SymbolLayer::setSymbolPlacement(const PropertyValue<SymbolPlacementType>& value) {
-    if (value == getSymbolPlacement())
-        return;
+    if (value == getSymbolPlacement()) return;
     auto impl_ = mutableImpl();
     impl_->layout.get<SymbolPlacement>() = value;
     baseImpl = std::move(impl_);
@@ -326,8 +310,7 @@ const PropertyValue<float>& SymbolLayer::getSymbolSortKey() const {
 }
 
 void SymbolLayer::setSymbolSortKey(const PropertyValue<float>& value) {
-    if (value == getSymbolSortKey())
-        return;
+    if (value == getSymbolSortKey()) return;
     auto impl_ = mutableImpl();
     impl_->layout.get<SymbolSortKey>() = value;
     baseImpl = std::move(impl_);
@@ -342,8 +325,7 @@ const PropertyValue<float>& SymbolLayer::getSymbolSpacing() const {
 }
 
 void SymbolLayer::setSymbolSpacing(const PropertyValue<float>& value) {
-    if (value == getSymbolSpacing())
-        return;
+    if (value == getSymbolSpacing()) return;
     auto impl_ = mutableImpl();
     impl_->layout.get<SymbolSpacing>() = value;
     baseImpl = std::move(impl_);
@@ -358,8 +340,7 @@ const PropertyValue<SymbolZOrderType>& SymbolLayer::getSymbolZOrder() const {
 }
 
 void SymbolLayer::setSymbolZOrder(const PropertyValue<SymbolZOrderType>& value) {
-    if (value == getSymbolZOrder())
-        return;
+    if (value == getSymbolZOrder()) return;
     auto impl_ = mutableImpl();
     impl_->layout.get<SymbolZOrder>() = value;
     baseImpl = std::move(impl_);
@@ -374,8 +355,7 @@ const PropertyValue<bool>& SymbolLayer::getTextAllowOverlap() const {
 }
 
 void SymbolLayer::setTextAllowOverlap(const PropertyValue<bool>& value) {
-    if (value == getTextAllowOverlap())
-        return;
+    if (value == getTextAllowOverlap()) return;
     auto impl_ = mutableImpl();
     impl_->layout.get<TextAllowOverlap>() = value;
     baseImpl = std::move(impl_);
@@ -390,8 +370,7 @@ const PropertyValue<SymbolAnchorType>& SymbolLayer::getTextAnchor() const {
 }
 
 void SymbolLayer::setTextAnchor(const PropertyValue<SymbolAnchorType>& value) {
-    if (value == getTextAnchor())
-        return;
+    if (value == getTextAnchor()) return;
     auto impl_ = mutableImpl();
     impl_->layout.get<TextAnchor>() = value;
     baseImpl = std::move(impl_);
@@ -406,8 +385,7 @@ const PropertyValue<expression::Formatted>& SymbolLayer::getTextField() const {
 }
 
 void SymbolLayer::setTextField(const PropertyValue<expression::Formatted>& value) {
-    if (value == getTextField())
-        return;
+    if (value == getTextField()) return;
     auto impl_ = mutableImpl();
     impl_->layout.get<TextField>() = value;
     baseImpl = std::move(impl_);
@@ -422,8 +400,7 @@ const PropertyValue<std::vector<std::string>>& SymbolLayer::getTextFont() const
 }
 
 void SymbolLayer::setTextFont(const PropertyValue<std::vector<std::string>>& value) {
-    if (value == getTextFont())
-        return;
+    if (value == getTextFont()) return;
     auto impl_ = mutableImpl();
     impl_->layout.get<TextFont>() = value;
     baseImpl = std::move(impl_);
@@ -438,8 +415,7 @@ const PropertyValue<bool>& SymbolLayer::getTextIgnorePlacement() const {
 }
 
 void SymbolLayer::setTextIgnorePlacement(const PropertyValue<bool>& value) {
-    if (value == getTextIgnorePlacement())
-        return;
+    if (value == getTextIgnorePlacement()) return;
     auto impl_ = mutableImpl();
     impl_->layout.get<TextIgnorePlacement>() = value;
     baseImpl = std::move(impl_);
@@ -454,8 +430,7 @@ const PropertyValue<TextJustifyType>& SymbolLayer::getTextJustify() const {
 }
 
 void SymbolLayer::setTextJustify(const PropertyValue<TextJustifyType>& value) {
-    if (value == getTextJustify())
-        return;
+    if (value == getTextJustify()) return;
     auto impl_ = mutableImpl();
     impl_->layout.get<TextJustify>() = value;
     baseImpl = std::move(impl_);
@@ -470,8 +445,7 @@ const PropertyValue<bool>& SymbolLayer::getTextKeepUpright() const {
 }
 
 void SymbolLayer::setTextKeepUpright(const PropertyValue<bool>& value) {
-    if (value == getTextKeepUpright())
-        return;
+    if (value == getTextKeepUpright()) return;
     auto impl_ = mutableImpl();
     impl_->layout.get<TextKeepUpright>() = value;
     baseImpl = std::move(impl_);
@@ -486,8 +460,7 @@ const PropertyValue<float>& SymbolLayer::getTextLetterSpacing() const {
 }
 
 void SymbolLayer::setTextLetterSpacing(const PropertyValue<float>& value) {
-    if (value == getTextLetterSpacing())
-        return;
+    if (value == getTextLetterSpacing()) return;
     auto impl_ = mutableImpl();
     impl_->layout.get<TextLetterSpacing>() = value;
     baseImpl = std::move(impl_);
@@ -502,8 +475,7 @@ const PropertyValue<float>& SymbolLayer::getTextLineHeight() const {
 }
 
 void SymbolLayer::setTextLineHeight(const PropertyValue<float>& value) {
-    if (value == getTextLineHeight())
-        return;
+    if (value == getTextLineHeight()) return;
     auto impl_ = mutableImpl();
     impl_->layout.get<TextLineHeight>() = value;
     baseImpl = std::move(impl_);
@@ -518,8 +490,7 @@ const PropertyValue<float>& SymbolLayer::getTextMaxAngle() const {
 }
 
 void SymbolLayer::setTextMaxAngle(const PropertyValue<float>& value) {
-    if (value == getTextMaxAngle())
-        return;
+    if (value == getTextMaxAngle()) return;
     auto impl_ = mutableImpl();
     impl_->layout.get<TextMaxAngle>() = value;
     baseImpl = std::move(impl_);
@@ -534,8 +505,7 @@ const PropertyValue<float>& SymbolLayer::getTextMaxWidth() const {
 }
 
 void SymbolLayer::setTextMaxWidth(const PropertyValue<float>& value) {
-    if (value == getTextMaxWidth())
-        return;
+    if (value == getTextMaxWidth()) return;
     auto impl_ = mutableImpl();
     impl_->layout.get<TextMaxWidth>() = value;
     baseImpl = std::move(impl_);
@@ -550,8 +520,7 @@ const PropertyValue<std::array<float, 2>>& SymbolLayer::getTextOffset() const {
 }
 
 void SymbolLayer::setTextOffset(const PropertyValue<std::array<float, 2>>& value) {
-    if (value == getTextOffset())
-        return;
+    if (value == getTextOffset()) return;
     auto impl_ = mutableImpl();
     impl_->layout.get<TextOffset>() = value;
     baseImpl = std::move(impl_);
@@ -566,8 +535,7 @@ const PropertyValue<bool>& SymbolLayer::getTextOptional() const {
 }
 
 void SymbolLayer::setTextOptional(const PropertyValue<bool>& value) {
-    if (value == getTextOptional())
-        return;
+    if (value == getTextOptional()) return;
     auto impl_ = mutableImpl();
     impl_->layout.get<TextOptional>() = value;
     baseImpl = std::move(impl_);
@@ -582,8 +550,7 @@ const PropertyValue<float>& SymbolLayer::getTextPadding() const {
 }
 
 void SymbolLayer::setTextPadding(const PropertyValue<float>& value) {
-    if (value == getTextPadding())
-        return;
+    if (value == getTextPadding()) return;
     auto impl_ = mutableImpl();
     impl_->layout.get<TextPadding>() = value;
     baseImpl = std::move(impl_);
@@ -598,8 +565,7 @@ const PropertyValue<AlignmentType>& SymbolLayer::getTextPitchAlignment() const {
 }
 
 void SymbolLayer::setTextPitchAlignment(const PropertyValue<AlignmentType>& value) {
-    if (value == getTextPitchAlignment())
-        return;
+    if (value == getTextPitchAlignment()) return;
     auto impl_ = mutableImpl();
     impl_->layout.get<TextPitchAlignment>() = value;
     baseImpl = std::move(impl_);
@@ -614,8 +580,7 @@ const PropertyValue<float>& SymbolLayer::getTextRadialOffset() const {
 }
 
 void SymbolLayer::setTextRadialOffset(const PropertyValue<float>& value) {
-    if (value == getTextRadialOffset())
-        return;
+    if (value == getTextRadialOffset()) return;
     auto impl_ = mutableImpl();
     impl_->layout.get<TextRadialOffset>() = value;
     baseImpl = std::move(impl_);
@@ -630,8 +595,7 @@ const PropertyValue<float>& SymbolLayer::getTextRotate() const {
 }
 
 void SymbolLayer::setTextRotate(const PropertyValue<float>& value) {
-    if (value == getTextRotate())
-        return;
+    if (value == getTextRotate()) return;
     auto impl_ = mutableImpl();
     impl_->layout.get<TextRotate>() = value;
     baseImpl = std::move(impl_);
@@ -646,8 +610,7 @@ const PropertyValue<AlignmentType>& SymbolLayer::getTextRotationAlignment() cons
 }
 
 void SymbolLayer::setTextRotationAlignment(const PropertyValue<AlignmentType>& value) {
-    if (value == getTextRotationAlignment())
-        return;
+    if (value == getTextRotationAlignment()) return;
     auto impl_ = mutableImpl();
     impl_->layout.get<TextRotationAlignment>() = value;
     baseImpl = std::move(impl_);
@@ -662,8 +625,7 @@ const PropertyValue<float>& SymbolLayer::getTextSize() const {
 }
 
 void SymbolLayer::setTextSize(const PropertyValue<float>& value) {
-    if (value == getTextSize())
-        return;
+    if (value == getTextSize()) return;
     auto impl_ = mutableImpl();
     impl_->layout.get<TextSize>() = value;
     baseImpl = std::move(impl_);
@@ -678,8 +640,7 @@ const PropertyValue<TextTransformType>& SymbolLayer::getTextTransform() const {
 }
 
 void SymbolLayer::setTextTransform(const PropertyValue<TextTransformType>& value) {
-    if (value == getTextTransform())
-        return;
+    if (value == getTextTransform()) return;
     auto impl_ = mutableImpl();
     impl_->layout.get<TextTransform>() = value;
     baseImpl = std::move(impl_);
@@ -694,8 +655,7 @@ const PropertyValue<std::vector<TextVariableAnchorType>>& SymbolLayer::getTextVa
 }
 
 void SymbolLayer::setTextVariableAnchor(const PropertyValue<std::vector<TextVariableAnchorType>>& value) {
-    if (value == getTextVariableAnchor())
-        return;
+    if (value == getTextVariableAnchor()) return;
     auto impl_ = mutableImpl();
     impl_->layout.get<TextVariableAnchor>() = value;
     baseImpl = std::move(impl_);
@@ -710,8 +670,7 @@ const PropertyValue<std::vector<TextWritingModeType>>& SymbolLayer::getTextWriti
 }
 
 void SymbolLayer::setTextWritingMode(const PropertyValue<std::vector<TextWritingModeType>>& value) {
-    if (value == getTextWritingMode())
-        return;
+    if (value == getTextWritingMode()) return;
     auto impl_ = mutableImpl();
     impl_->layout.get<TextWritingMode>() = value;
     baseImpl = std::move(impl_);
diff --git a/test/gl/bucket.test.cpp b/test/gl/bucket.test.cpp
index 9f8af4e46..2a7455e17 100644
--- a/test/gl/bucket.test.cpp
+++ b/test/gl/bucket.test.cpp
@@ -67,7 +67,7 @@ TEST(Buckets, CircleBucket) {
 TEST(Buckets, FillBucket) {
     gl::HeadlessBackend backend({ 512, 256 });
     gfx::BackendScope scope { backend };
-    style::Properties<>::PossiblyEvaluated layout;
+    FillBucket::PossiblyEvaluatedLayoutProperties layout;
 
     gl::Context context{ backend };
     FillBucket bucket { layout, {}, 5.0f, 1};
@@ -89,7 +89,7 @@ TEST(Buckets, FillBucket) {
 TEST(Buckets, LineBucket) {
     gl::HeadlessBackend backend({ 512, 256 });
     gfx::BackendScope scope { backend };
-    style::LineLayoutProperties::PossiblyEvaluated layout;
+    LineBucket::PossiblyEvaluatedLayoutProperties layout;
 
     gl::Context context{ backend };
     LineBucket bucket { layout, {}, 10.0f, 1 };