[core] Encapsulate placement code handling symbols sort ranges

8 files changed, 48 insertions, 53 deletions

diff --git a/src/mbgl/layout/symbol_instance.hpp b/src/mbgl/layout/symbol_instance.hpp
index 3303e5f7e3..fa05affc13 100644
--- a/src/mbgl/layout/symbol_instance.hpp
+++ b/src/mbgl/layout/symbol_instance.hpp
@@ -123,11 +123,6 @@ public:
     static constexpr uint32_t invalidCrossTileID() { return std::numeric_limits<uint32_t>::max(); }
 };
 
-class SortKeyRange {
-public:
-    float sortKey;
-    size_t symbolInstanceStart;
-    size_t symbolInstanceEnd;
-};
+using SymbolInstanceReferences = std::vector<std::reference_wrapper<const SymbolInstance>>;
 
 } // namespace mbgl
diff --git a/src/mbgl/layout/symbol_layout.cpp b/src/mbgl/layout/symbol_layout.cpp
index 63ce223574..093b11de1d 100644
--- a/src/mbgl/layout/symbol_layout.cpp
+++ b/src/mbgl/layout/symbol_layout.cpp
@@ -613,8 +613,8 @@ void SymbolLayout::addFeature(const std::size_t layoutFeatureIndex,
                                          iconType);
 
             if (sortFeaturesByKey) {
-                if (sortKeyRanges.size() && sortKeyRanges.back().sortKey == feature.sortKey) {
-                    sortKeyRanges.back().symbolInstanceEnd = symbolInstances.size();
+                if (!sortKeyRanges.empty() && sortKeyRanges.back().sortKey == feature.sortKey) {
+                    sortKeyRanges.back().end = symbolInstances.size();
                 } else {
                     sortKeyRanges.push_back({feature.sortKey, symbolInstances.size() - 1, symbolInstances.size()});
                 }
diff --git a/src/mbgl/renderer/buckets/symbol_bucket.cpp b/src/mbgl/renderer/buckets/symbol_bucket.cpp
index 7b8eec4471..af2a479c26 100644
--- a/src/mbgl/renderer/buckets/symbol_bucket.cpp
+++ b/src/mbgl/renderer/buckets/symbol_bucket.cpp
@@ -275,8 +275,8 @@ void SymbolBucket::sortFeatures(const float angle) {
     featureSortOrder = std::move(symbolsSortOrder);
 }
 
-std::vector<std::reference_wrapper<const SymbolInstance>> SymbolBucket::getSortedSymbols(const float angle) const {
-    std::vector<std::reference_wrapper<const SymbolInstance>> result(symbolInstances.begin(), symbolInstances.end());
+SymbolInstanceReferences SymbolBucket::getSortedSymbols(const float angle) const {
+    SymbolInstanceReferences result(symbolInstances.begin(), symbolInstances.end());
     const float sin = std::sin(angle);
     const float cos = std::cos(angle);
 
@@ -292,6 +292,15 @@ std::vector<std::reference_wrapper<const SymbolInstance>> SymbolBucket::getSorte
     return result;
 }
 
+SymbolInstanceReferences SymbolBucket::getSymbols(const optional<SortKeyRange>& range) const {
+    if (!range) return SymbolInstanceReferences(symbolInstances.begin(), symbolInstances.end());
+    assert(range->start < range->end);
+    assert(range->end <= symbolInstances.size());
+    auto begin = symbolInstances.begin() + range->start;
+    auto end = symbolInstances.begin() + range->end;
+    return SymbolInstanceReferences(begin, end);
+}
+
 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 2b41ac9295..3cdea1310c 100644
--- a/src/mbgl/renderer/buckets/symbol_bucket.hpp
+++ b/src/mbgl/renderer/buckets/symbol_bucket.hpp
@@ -1,16 +1,17 @@
 #pragma once
 
-#include <mbgl/renderer/bucket.hpp>
-#include <mbgl/map/mode.hpp>
-#include <mbgl/gfx/vertex_buffer.hpp>
 #include <mbgl/gfx/index_buffer.hpp>
+#include <mbgl/gfx/vertex_buffer.hpp>
+#include <mbgl/layout/symbol_feature.hpp>
+#include <mbgl/layout/symbol_instance.hpp>
+#include <mbgl/map/mode.hpp>
+#include <mbgl/programs/collision_box_program.hpp>
 #include <mbgl/programs/segment.hpp>
 #include <mbgl/programs/symbol_program.hpp>
-#include <mbgl/programs/collision_box_program.hpp>
-#include <mbgl/text/glyph_range.hpp>
+#include <mbgl/renderer/bucket.hpp>
 #include <mbgl/style/layers/symbol_layer_properties.hpp>
-#include <mbgl/layout/symbol_feature.hpp>
-#include <mbgl/layout/symbol_instance.hpp>
+#include <mbgl/text/glyph_range.hpp>
+#include <mbgl/text/placement.hpp>
 
 #include <vector>
 
@@ -97,8 +98,11 @@ public:
 
 
     void sortFeatures(const float angle);
-    // The result contains references to the `symbolInstances` items, sorted by viewport Y.
-    std::vector<std::reference_wrapper<const SymbolInstance>> getSortedSymbols(const float angle) const;
+    // Returns references to the `symbolInstances` items, sorted by viewport Y.
+    SymbolInstanceReferences getSortedSymbols(const float angle) const;
+    // Returns references to the `symbolInstances` items, which belong to the `sortKeyRange` range;
+    // returns references to all the symbols if |sortKeyRange| is `nullopt`.
+    SymbolInstanceReferences getSymbols(const optional<SortKeyRange>& sortKeyRange = nullopt) const;
 
     Immutable<style::SymbolLayoutProperties::PossiblyEvaluated> layout;
     const std::string bucketLeaderID;
@@ -118,7 +122,7 @@ public:
     bool hasUninitializedSymbols : 1;
 
     std::vector<SymbolInstance> symbolInstances;
-    std::vector<SortKeyRange> sortKeyRanges;
+    const std::vector<SortKeyRange> sortKeyRanges;
 
     struct PaintProperties {
         SymbolIconProgram::Binders iconBinders;
diff --git a/src/mbgl/renderer/layers/render_symbol_layer.cpp b/src/mbgl/renderer/layers/render_symbol_layer.cpp
index 2a94c452c7..416704f167 100644
--- a/src/mbgl/renderer/layers/render_symbol_layer.cpp
+++ b/src/mbgl/renderer/layers/render_symbol_layer.cpp
@@ -587,20 +587,16 @@ void RenderSymbolLayer::prepare(const LayerPrepareParameters& params) {
             auto featureIndex = static_cast<const GeometryTile*>(tile)->getFeatureIndex();
 
             if (bucket->sortKeyRanges.empty()) {
-                placementData.push_back(
-                    {*bucket, renderTile, featureIndex, firstInBucket, 0.0f, 0, bucket->symbolInstances.size()});
+                placementData.push_back({*bucket, renderTile, featureIndex, firstInBucket, nullopt});
             } else {
-                for (const SortKeyRange& sortKeyRange : bucket->sortKeyRanges) {
-                    LayerPlacementData layerData{*bucket,
-                                                 renderTile,
-                                                 featureIndex,
-                                                 firstInBucket,
-                                                 sortKeyRange.sortKey,
-                                                 sortKeyRange.symbolInstanceStart,
-                                                 sortKeyRange.symbolInstanceEnd};
-                    auto sortPosition = std::upper_bound(placementData.cbegin(), placementData.cend(), layerData);
+                for (const auto& sortKeyRange : bucket->sortKeyRanges) {
+                    LayerPlacementData layerData{*bucket, renderTile, featureIndex, firstInBucket, sortKeyRange};
+                    auto sortPosition = std::upper_bound(
+                        placementData.cbegin(), placementData.cend(), layerData, [](const auto& lhs, const auto& rhs) {
+                            assert(lhs.sortKeyRange && rhs.sortKeyRange);
+                            return lhs.sortKeyRange->sortKey < rhs.sortKeyRange->sortKey;
+                        });
                     placementData.insert(sortPosition, std::move(layerData));
-
                     firstInBucket = false;
                 }
             }
diff --git a/src/mbgl/renderer/render_layer.hpp b/src/mbgl/renderer/render_layer.hpp
index 4931893271..6a10898797 100644
--- a/src/mbgl/renderer/render_layer.hpp
+++ b/src/mbgl/renderer/render_layer.hpp
@@ -28,18 +28,20 @@ public:
     Immutable<style::LayerProperties> layerProperties;
 };
 
+class SortKeyRange {
+public:
+    float sortKey;
+    size_t start;
+    size_t end;
+};
+
 class LayerPlacementData {
 public:
-    friend bool operator<(const LayerPlacementData& lhs, const LayerPlacementData& rhs) {
-        return lhs.sortKey < rhs.sortKey;
-    }
     std::reference_wrapper<Bucket> bucket;
     std::reference_wrapper<const RenderTile> tile;
     std::shared_ptr<FeatureIndex> featureIndex;
     bool firstInBucket;
-    float sortKey;
-    size_t symbolInstanceStart;
-    size_t symbolInstanceEnd;
+    optional<SortKeyRange> sortKeyRange;
 };
 
 class LayerPrepareParameters {
diff --git a/src/mbgl/text/placement.cpp b/src/mbgl/text/placement.cpp
index 57cf1f0f1f..398e0f9e03 100644
--- a/src/mbgl/text/placement.cpp
+++ b/src/mbgl/text/placement.cpp
@@ -113,12 +113,7 @@ void Placement::placeLayer(const RenderLayer& layer) {
     std::set<uint32_t> seenCrossTileIDs;
     for (const auto& item : layer.getPlacementData()) {
         Bucket& bucket = item.bucket;
-        BucketPlacementParameters params{item.tile,
-                                         layer.baseImpl->source,
-                                         item.featureIndex,
-                                         item.sortKey,
-                                         item.symbolInstanceStart,
-                                         item.symbolInstanceEnd};
+        BucketPlacementParameters params{item.tile, layer.baseImpl->source, item.featureIndex, item.sortKeyRange};
         bucket.place(*this, params, seenCrossTileIDs);
     }
 }
@@ -661,12 +656,8 @@ void Placement::placeBucket(const SymbolBucket& bucket,
         }
 
     } else {
-        auto beginIt = bucket.symbolInstances.begin() + params.symbolInstanceStart;
-        auto endIt = bucket.symbolInstances.begin() + params.symbolInstanceEnd;
-        assert(params.symbolInstanceStart < params.symbolInstanceEnd);
-        assert(params.symbolInstanceEnd <= bucket.symbolInstances.size());
-        for (auto it = beginIt; it != endIt; ++it) {
-            placeSymbol(*it);
+        for (const SymbolInstance& symbol : bucket.getSymbols(params.sortKeyRange)) {
+            placeSymbol(symbol);
         }
     }
 
diff --git a/src/mbgl/text/placement.hpp b/src/mbgl/text/placement.hpp
index 9fbd6b9743..dd29a6e3b8 100644
--- a/src/mbgl/text/placement.hpp
+++ b/src/mbgl/text/placement.hpp
@@ -91,9 +91,7 @@ public:
     const RenderTile& tile;
     std::string sourceId;
     std::shared_ptr<FeatureIndex> featureIndex;
-    float sortKey;
-    size_t symbolInstanceStart;
-    size_t symbolInstanceEnd;
+    optional<SortKeyRange> sortKeyRange;
 };
 
 class Placement;