replace placedTextIndices vector with two members

4 files changed, 32 insertions, 33 deletions

diff --git a/src/mbgl/layout/symbol_instance.hpp b/src/mbgl/layout/symbol_instance.hpp
index 95a09fe3e9..c6c02c2e38 100644
--- a/src/mbgl/layout/symbol_instance.hpp
+++ b/src/mbgl/layout/symbol_instance.hpp
@@ -50,8 +50,9 @@ public:
     std::array<float, 2> iconOffset;
     std::u16string key;
     bool isDuplicate;
-    std::vector<size_t> placedTextIndices; // TODO clean this up 
-    std::vector<size_t> placedIconIndices;
+    optional<size_t> placedTextIndex;
+    optional<size_t> placedVerticalTextIndex;
+    optional<size_t> placedIconIndex;
     uint32_t crossTileID = 0;
 };
 
diff --git a/src/mbgl/layout/symbol_layout.cpp b/src/mbgl/layout/symbol_layout.cpp
index 1a8b55a51b..ba7aab1de5 100644
--- a/src/mbgl/layout/symbol_layout.cpp
+++ b/src/mbgl/layout/symbol_layout.cpp
@@ -438,7 +438,7 @@ std::unique_ptr<SymbolBucket> SymbolLayout::place(const bool showCollisionBoxes)
             const Range<float> sizeData = bucket->textSizeBinder->getVertexSizeData(feature);
             bucket->text.placedSymbols.emplace_back(symbolInstance.anchor.point, symbolInstance.anchor.segment, sizeData.min, sizeData.max,
                     symbolInstance.textOffset, symbolInstance.writingModes, symbolInstance.line, CalculateTileDistances(symbolInstance.line, symbolInstance.anchor));
-            symbolInstance.placedTextIndices.push_back(bucket->text.placedSymbols.size() - 1);
+            symbolInstance.placedTextIndex = bucket->text.placedSymbols.size() - 1;
             PlacedSymbol& horizontalSymbol = bucket->text.placedSymbols.back();
 
             bool firstHorizontal = true;
@@ -455,7 +455,7 @@ std::unique_ptr<SymbolBucket> SymbolLayout::place(const bool showCollisionBoxes)
             if (symbolInstance.writingModes & WritingModeType::Vertical) {
                 bucket->text.placedSymbols.emplace_back(symbolInstance.anchor.point, symbolInstance.anchor.segment, sizeData.min, sizeData.max,
                         symbolInstance.textOffset, WritingModeType::Vertical, symbolInstance.line, CalculateTileDistances(symbolInstance.line, symbolInstance.anchor));
-                symbolInstance.placedTextIndices.push_back(bucket->text.placedSymbols.size() - 1);
+                symbolInstance.placedVerticalTextIndex = bucket->text.placedSymbols.size() - 1;
                 
                 PlacedSymbol& verticalSymbol = bucket->text.placedSymbols.back();
                 bool firstVertical = true;
@@ -478,7 +478,7 @@ std::unique_ptr<SymbolBucket> SymbolLayout::place(const bool showCollisionBoxes)
                 const Range<float> sizeData = bucket->iconSizeBinder->getVertexSizeData(feature);
                 bucket->icon.placedSymbols.emplace_back(symbolInstance.anchor.point, symbolInstance.anchor.segment, sizeData.min, sizeData.max,
                         symbolInstance.iconOffset, WritingModeType::None, symbolInstance.line, std::vector<float>());
-                symbolInstance.placedIconIndices.push_back(bucket->icon.placedSymbols.size() - 1);
+                symbolInstance.placedIconIndex = bucket->icon.placedSymbols.size() - 1;
                 PlacedSymbol& iconSymbol = bucket->icon.placedSymbols.back();
                 iconSymbol.vertexStartIndex = addSymbol(
                                                         bucket->icon, sizeData, *symbolInstance.iconQuad,
diff --git a/src/mbgl/renderer/buckets/symbol_bucket.cpp b/src/mbgl/renderer/buckets/symbol_bucket.cpp
index d1b3c3469b..6799b38d7e 100644
--- a/src/mbgl/renderer/buckets/symbol_bucket.cpp
+++ b/src/mbgl/renderer/buckets/symbol_bucket.cpp
@@ -145,6 +145,14 @@ void SymbolBucket::updateOpacity() {
     uploaded = false;
 }
 
+void addPlacedSymbol(gl::IndexVector<gl::Triangles>& triangles, const PlacedSymbol& placedSymbol) {
+    auto endIndex = placedSymbol.vertexStartIndex + placedSymbol.glyphOffsets.size() * 4;
+    for (auto vertexIndex = placedSymbol.vertexStartIndex; vertexIndex < endIndex; vertexIndex += 4) {
+        triangles.emplace_back(vertexIndex + 0, vertexIndex + 1, vertexIndex + 2);
+        triangles.emplace_back(vertexIndex + 1, vertexIndex + 2, vertexIndex + 3);
+    }
+}
+
 void SymbolBucket::sortFeatures(const float angle) {
     if (!sortFeaturesByY) {
         return;
@@ -194,22 +202,14 @@ void SymbolBucket::sortFeatures(const float angle) {
     for (auto i : symbolInstanceIndexes) {
         const SymbolInstance& symbolInstance = symbolInstances[i];
 
-        for (auto& placedTextSymbolIndex : symbolInstance.placedTextIndices) {
-            const PlacedSymbol& placedSymbol = text.placedSymbols[placedTextSymbolIndex];
-
-            auto endIndex = placedSymbol.vertexStartIndex + placedSymbol.glyphOffsets.size() * 4;
-            for (auto vertexIndex = placedSymbol.vertexStartIndex; vertexIndex < endIndex; vertexIndex += 4) {
-                text.triangles.emplace_back(vertexIndex + 0, vertexIndex + 1, vertexIndex + 2);
-                text.triangles.emplace_back(vertexIndex + 1, vertexIndex + 2, vertexIndex + 3);
-            }
+        if (symbolInstance.placedTextIndex) {
+            addPlacedSymbol(text.triangles, text.placedSymbols[*symbolInstance.placedTextIndex]);
         }
-
-        for (auto& placedIconSymbolIndex : symbolInstance.placedIconIndices) { // TODO: This iteration is an awkward way to say "if the symbol has an icon"
-            const PlacedSymbol& placedIcon = icon.placedSymbols[placedIconSymbolIndex];
-            
-            const auto vertexIndex = placedIcon.vertexStartIndex;
-            icon.triangles.emplace_back(vertexIndex + 0, vertexIndex + 1, vertexIndex + 2);
-            icon.triangles.emplace_back(vertexIndex + 1, vertexIndex + 2, vertexIndex + 3);
+        if (symbolInstance.placedVerticalTextIndex) {
+            addPlacedSymbol(text.triangles, text.placedSymbols[*symbolInstance.placedVerticalTextIndex]);
+        }
+        if (symbolInstance.placedIconIndex) {
+            addPlacedSymbol(icon.triangles, icon.placedSymbols[*symbolInstance.placedIconIndex]);
         }
     }
 }
diff --git a/src/mbgl/text/placement.cpp b/src/mbgl/text/placement.cpp
index 2df946cb67..e5ccd26b7c 100644
--- a/src/mbgl/text/placement.cpp
+++ b/src/mbgl/text/placement.cpp
@@ -102,10 +102,8 @@ void Placement::placeLayerBucket(
             bool placeText = false;
             bool placeIcon = false;
 
-            if (symbolInstance.placedTextIndices.size()) {
-                assert(symbolInstance.placedTextIndices.size() != 0);
-
-                PlacedSymbol& placedSymbol = bucket.text.placedSymbols.at(symbolInstance.placedTextIndices.at(0));
+            if (symbolInstance.placedTextIndex) {
+                PlacedSymbol& placedSymbol = bucket.text.placedSymbols.at(*symbolInstance.placedTextIndex);
                 const float fontSize = evaluateSizeForFeature(partiallyEvaluatedTextSize, placedSymbol);
 
                 placeText = collisionIndex.placeFeature(symbolInstance.textCollisionFeature,
@@ -116,9 +114,8 @@ void Placement::placeLayerBucket(
                         showCollisionBoxes);
             }
 
-            if (symbolInstance.placedIconIndices.size()) {
-
-                PlacedSymbol& placedSymbol = bucket.icon.placedSymbols.at(symbolInstance.placedIconIndices.at(0));
+            if (symbolInstance.placedIconIndex) {
+                PlacedSymbol& placedSymbol = bucket.icon.placedSymbols.at(*symbolInstance.placedIconIndex);
                 const float fontSize = evaluateSizeForFeature(partiallyEvaluatedIconSize, placedSymbol);
 
                 placeIcon = collisionIndex.placeFeature(symbolInstance.iconCollisionFeature,
@@ -227,9 +224,11 @@ void Placement::updateBucketOpacities(SymbolBucket& bucket, std::unordered_set<u
             for (size_t i = 0; i < symbolInstance.verticalGlyphQuads.size() * 4; i++) {
                 bucket.text.opacityVertices.emplace_back(opacityVertex);
             }
-            // TODO On JS we avoid setting this if it hasn't chnaged, but it may be cheap enough here we don't care
-            for (auto index : symbolInstance.placedTextIndices) {
-                bucket.text.placedSymbols[index].hidden = opacityState.isHidden();
+            if (symbolInstance.placedTextIndex) {
+                bucket.text.placedSymbols[*symbolInstance.placedTextIndex].hidden = opacityState.isHidden();
+            }
+            if (symbolInstance.placedVerticalTextIndex) {
+                bucket.text.placedSymbols[*symbolInstance.placedVerticalTextIndex].hidden = opacityState.isHidden();
             }
         }
         if (symbolInstance.hasIcon) {
@@ -240,9 +239,8 @@ void Placement::updateBucketOpacities(SymbolBucket& bucket, std::unordered_set<u
                 bucket.icon.opacityVertices.emplace_back(opacityVertex);
                 bucket.icon.opacityVertices.emplace_back(opacityVertex);
             }
-            // TODO On JS we avoid setting this if it hasn't chnaged, but it may be cheap enough here we don't care
-            for (auto index : symbolInstance.placedIconIndices) {
-                bucket.icon.placedSymbols[index].hidden = opacityState.isHidden();
+            if (symbolInstance.placedIconIndex) {
+                bucket.icon.placedSymbols[*symbolInstance.placedIconIndex].hidden = opacityState.isHidden();
             }
         }