From bf15332c5e3d1b411cc2a6a8bfa7b908b17a6479 Mon Sep 17 00:00:00 2001
From: Tim Sylvester <tim@wetdogweather.com>
Date: Tue, 30 Jan 2024 08:02:05 -0800
Subject: [PATCH] Combine multiple segments into a drawable when
 `sortFeaturesByKey` is not used (#2060)

---
 src/mbgl/mtl/drawable.cpp                     | 14 +++--
 .../renderer/layers/render_symbol_layer.cpp   | 52 ++++++++++++++-----
 2 files changed, 50 insertions(+), 16 deletions(-)

diff --git a/src/mbgl/mtl/drawable.cpp b/src/mbgl/mtl/drawable.cpp
index 1cae17c942f..c8f948a4f61 100644
--- a/src/mbgl/mtl/drawable.cpp
+++ b/src/mbgl/mtl/drawable.cpp
@@ -24,6 +24,9 @@
 #include <simd/simd.h>
 
 #include <cassert>
+#if !defined(NDEBUG)
+#include <sstream>
+#endif
 
 namespace mbgl {
 namespace mtl {
@@ -72,11 +75,16 @@ MTL::PrimitiveType getPrimitiveType(const gfx::DrawModeType type) noexcept {
 
 #if !defined(NDEBUG)
 std::string debugLabel(const gfx::Drawable& drawable) {
-    std::string result = drawable.getName();
+    std::ostringstream oss;
+    oss << drawable.getID().id() << "/" << drawable.getName() << "/tile=";
+
     if (const auto& tileID = drawable.getTileID()) {
-        result.append("/tile=").append(util::toString(*tileID));
+        oss << util::toString(*tileID);
+    } else {
+        oss << "(none)";
     }
-    return result;
+
+    return oss.str();
 }
 #endif // !defined(NDEBUG)
 
diff --git a/src/mbgl/renderer/layers/render_symbol_layer.cpp b/src/mbgl/renderer/layers/render_symbol_layer.cpp
index 063bcdfd2ab..a59e0082092 100644
--- a/src/mbgl/renderer/layers/render_symbol_layer.cpp
+++ b/src/mbgl/renderer/layers/render_symbol_layer.cpp
@@ -88,15 +88,14 @@ struct RenderableSegment {
                       const LayerRenderData& renderData_,
                       const SymbolBucket::PaintProperties& bucketPaintProperties_,
                       float sortKey_,
-                      const SymbolType type_,
-                      const uint8_t overscaledZ_ = 0)
+                      const SymbolType type_)
         : segment(segment_),
           tile(tile_),
           renderData(renderData_),
           bucketPaintProperties(bucketPaintProperties_),
           sortKey(sortKey_),
           type(type_),
-          overscaledZ(overscaledZ_) {}
+          overscaledZ(tile.getOverscaledTileID().overscaledZ) {}
 
     SegmentWrapper segment;
     const RenderTile& tile;
@@ -129,6 +128,19 @@ struct RenderableSegment {
     }
 };
 
+struct SegmentGroup {
+    // A reference to the first or only segment
+    RenderableSegment renderable;
+    // A reference to multiple segments, or none
+    SegmentVectorWrapper segments;
+
+    bool operator<(const SegmentGroup& other) const { return renderable < other.renderable; }
+};
+
+namespace {
+const SegmentVector<SymbolTextAttributes> emptySegmentVector;
+}
+
 #if MLN_LEGACY_RENDERER
 
 template <typename DrawFn>
@@ -1038,7 +1050,7 @@ void RenderSymbolLayer::update(gfx::ShaderRegistry& shaders,
     }
 
     const bool sortFeaturesByKey = !impl_cast(baseImpl).layout.get<SymbolSortKey>().isUndefined();
-    std::multiset<RenderableSegment> renderableSegments;
+    std::multiset<SegmentGroup> renderableSegments;
     std::unique_ptr<gfx::DrawableBuilder> builder;
 
     const auto currentZoom = static_cast<float>(state.getZoom());
@@ -1169,13 +1181,20 @@ void RenderSymbolLayer::update(gfx::ShaderRegistry& shaders,
         }
 
         float serialKey = 1.0f;
-        auto addRenderables = [&, it = renderableSegments.begin()](const SymbolBucket::Buffer& buffer,
-                                                                   const SymbolType type) mutable {
-            for (const auto& segment : buffer.segments) {
-                const auto key = sortFeaturesByKey ? segment.sortKey : (serialKey += 1.0);
-                assert(segment.vertexOffset + segment.vertexLength <= buffer.vertices().elements());
-                it = renderableSegments.emplace_hint(
-                    it, std::ref(segment), tile, renderData, bucketPaintProperties, key, type, tileID.overscaledZ);
+        auto addRenderables = [&](const SymbolBucket::Buffer& buffer, const SymbolType type) mutable {
+            if (sortFeaturesByKey) {
+                // Features need to be rendered in a specific order, so we add each segment individually
+                for (const auto& segment : buffer.segments) {
+                    assert(segment.vertexOffset + segment.vertexLength <= buffer.vertices().elements());
+                    renderableSegments.emplace(SegmentGroup{
+                        {segment, tile, renderData, bucketPaintProperties, segment.sortKey, type}, emptySegmentVector});
+                }
+            } else if (!buffer.segments.empty()) {
+                // Features can be rendered in the order produced, and as grouped by the bucket
+                const auto& firstSeg = buffer.segments.front();
+                renderableSegments.emplace(SegmentGroup{
+                    {firstSeg, tile, renderData, bucketPaintProperties, serialKey, type}, buffer.segments});
+                serialKey += 1.0;
             }
         };
 
@@ -1208,7 +1227,10 @@ void RenderSymbolLayer::update(gfx::ShaderRegistry& shaders,
     };
     std::unordered_map<UnwrappedTileID, TileInfo> tileCache;
 
-    for (auto& renderable : renderableSegments) {
+    for (auto& group : renderableSegments) {
+        const auto& renderable = group.renderable;
+        const auto& segments = group.segments.get();
+
         const auto isText = (renderable.type == SymbolType::Text);
         const auto sdfIcons = (renderable.type == SymbolType::IconSDF);
 
@@ -1320,7 +1342,11 @@ void RenderSymbolLayer::update(gfx::ShaderRegistry& shaders,
                 builder->setDrawableName(layerPrefix + std::string(suffix));
                 builder->setVertexAttributes(attribs);
 
-                builder->setSegments(gfx::Triangles(), buffer.sharedTriangles, &renderable.segment.get(), 1);
+                if (segments.empty()) {
+                    builder->setSegments(gfx::Triangles(), buffer.sharedTriangles, &renderable.segment.get(), 1);
+                } else {
+                    builder->setSegments(gfx::Triangles(), buffer.sharedTriangles, segments.data(), segments.size());
+                }
 
                 builder->flush(context);