[core] fix mixed sdf + non-sdf icon rendering in one layer (#15456)

* [core] fix icon symbol rendring when sdf and non-sdf icon in the same symbol layer

* fix build error

* fix typo

* revert renderableSegment change

* simplify codes

* fix build error

* refine sdf icon flag

* [core] fix mixed sdf + non-sdf icon rendering in one layer

* remove iconstatus getter in stymbol bucket

* fix review findings

* provide bitwise operator for SymbolContent enum

* use MBGL_MBGL_CONSTEXPR

* add one missing update for sdfIcon

* make renderer symbol type as enum

4 files changed, 74 insertions, 37 deletions

diff --git a/src/mbgl/layout/symbol_instance.cpp b/src/mbgl/layout/symbol_instance.cpp
index 8be025afe1..ad36e6c38c 100644
--- a/src/mbgl/layout/symbol_instance.cpp
+++ b/src/mbgl/layout/symbol_instance.cpp
@@ -88,12 +88,11 @@ SymbolInstance::SymbolInstance(Anchor& anchor_,
                                const float iconRotation,
                                const float textRotation,
                                float radialTextOffset_,
-                               bool allowVerticalPlacement) :
+                               bool allowVerticalPlacement,
+                               const SymbolContent iconType) :
     sharedData(std::move(sharedData_)),
     anchor(anchor_),
-    // 'hasText' depends on finding at least one glyph in the shaping that's also in the GlyphPositionMap
-    hasText(!sharedData->empty()),
-    hasIcon(shapedIcon),
+    symbolContent(iconType),
     // Create the collision features that will be used to check whether this symbol instance can be placed
     // As a collision approximation, we can use either the vertical or any of the horizontal versions of the feature
     textCollisionFeature(sharedData->line, anchor, getAnyShaping(shapedTextOrientations), textBoxScale_, textPadding, textPlacement, indexedFeature, overscaling, textRotation),
@@ -107,7 +106,8 @@ SymbolInstance::SymbolInstance(Anchor& anchor_,
     textBoxScale(textBoxScale_),
     radialTextOffset(radialTextOffset_),
     singleLine(shapedTextOrientations.singleLine) {
-
+    // 'hasText' depends on finding at least one glyph in the shaping that's also in the GlyphPositionMap
+    if(!sharedData->empty()) symbolContent |= SymbolContent::Text;
     if (allowVerticalPlacement && shapedTextOrientations.vertical) {
         const float verticalPointLabelAngle = 90.0f;
         verticalTextCollisionFeature = CollisionFeature(line(), anchor, shapedTextOrientations.vertical, textBoxScale_, textPadding, textPlacement, indexedFeature, overscaling, textRotation + verticalPointLabelAngle);
@@ -164,6 +164,18 @@ const optional<SymbolQuad>& SymbolInstance::iconQuad() const {
     assert(sharedData);
     return sharedData->iconQuad;
 }
+    
+bool SymbolInstance::hasText() const {
+    return static_cast<bool>(symbolContent & SymbolContent::Text);
+}
+    
+bool SymbolInstance::hasIcon() const {
+    return static_cast<bool>(symbolContent & SymbolContent::IconRGBA) || hasSdfIcon();
+}
+    
+bool SymbolInstance::hasSdfIcon() const {
+    return static_cast<bool>(symbolContent & SymbolContent::IconSDF);
+}
 
 const optional<SymbolQuad>& SymbolInstance::verticalIconQuad() const {
     assert(sharedData);
diff --git a/src/mbgl/layout/symbol_instance.hpp b/src/mbgl/layout/symbol_instance.hpp
index 60883c12db..7d002d2285 100644
--- a/src/mbgl/layout/symbol_instance.hpp
+++ b/src/mbgl/layout/symbol_instance.hpp
@@ -4,7 +4,8 @@
 #include <mbgl/text/glyph_atlas.hpp>
 #include <mbgl/text/collision_feature.hpp>
 #include <mbgl/style/layers/symbol_layer_properties.hpp>
-
+#include <mbgl/util/traits.hpp>
+#include <mbgl/util/util.hpp>
 
 namespace mbgl {
 
@@ -42,6 +43,25 @@ struct SymbolInstanceSharedData {
     optional<SymbolQuad> verticalIconQuad;
 };
 
+enum class SymbolContent : uint8_t {
+    None = 0,
+    Text = 1 << 0,
+    IconRGBA = 1 << 1,
+    IconSDF = 1 << 2
+};
+
+MBGL_CONSTEXPR SymbolContent operator|(SymbolContent a, SymbolContent b) {
+    return SymbolContent(mbgl::underlying_type(a) | mbgl::underlying_type(b));
+}
+
+MBGL_CONSTEXPR SymbolContent& operator|=(SymbolContent& a, SymbolContent b) {
+    return (a = a | b);
+}
+
+MBGL_CONSTEXPR SymbolContent operator&(SymbolContent a, SymbolContent b) {
+    return SymbolContent(mbgl::underlying_type(a) & mbgl::underlying_type(b));
+}
+
 class SymbolInstance {
 public:
     SymbolInstance(Anchor& anchor_,
@@ -64,7 +84,8 @@ public:
                    const float iconRotation,
                    const float textRotation,
                    float radialTextOffset,
-                   bool allowVerticalPlacement);
+                   bool allowVerticalPlacement,
+                   const SymbolContent iconType = SymbolContent::None);
 
     optional<size_t> getDefaultHorizontalPlacedTextIndex() const;
     const GeometryCoordinates& line() const;
@@ -72,6 +93,9 @@ public:
     const SymbolQuads& leftJustifiedGlyphQuads() const;
     const SymbolQuads& centerJustifiedGlyphQuads() const;
     const SymbolQuads& verticalGlyphQuads() const;
+    bool hasText() const;
+    bool hasIcon() const;
+    bool hasSdfIcon() const;
     const optional<SymbolQuad>& iconQuad() const;
     const optional<SymbolQuad>& verticalIconQuad() const;
     void releaseSharedData();
@@ -81,8 +105,7 @@ private:
 
 public:
     Anchor anchor;
-    bool hasText;
-    bool hasIcon;
+    SymbolContent symbolContent;
 
     std::size_t rightJustifiedGlyphQuadsSize;
     std::size_t centerJustifiedGlyphQuadsSize;
diff --git a/src/mbgl/layout/symbol_layout.cpp b/src/mbgl/layout/symbol_layout.cpp
index 6a13bb22ae..fe169785ce 100644
--- a/src/mbgl/layout/symbol_layout.cpp
+++ b/src/mbgl/layout/symbol_layout.cpp
@@ -398,16 +398,18 @@ void SymbolLayout::prepareSymbols(const GlyphMap& glyphMap, const GlyphPositions
         }
 
         // if feature has icon, get sprite atlas position
+        SymbolContent iconType{SymbolContent::None};
         if (feature.icon) {
             auto image = imageMap.find(*feature.icon);
             if (image != imageMap.end()) {
+                iconType = SymbolContent::IconRGBA;
                 shapedIcon = PositionedIcon::shapeIcon(
                     imagePositions.at(*feature.icon),
                     layout->evaluate<IconOffset>(zoom, feature),
                     layout->evaluate<IconAnchor>(zoom, feature),
                     layout->evaluate<IconRotate>(zoom, feature) * util::DEG2RAD);
                 if (image->second->sdf) {
-                    sdfIcons = true;
+                    iconType = SymbolContent::IconSDF;
                 }
                 if (image->second->pixelRatio != pixelRatio) {
                     iconsNeedLinear = true;
@@ -419,7 +421,7 @@ void SymbolLayout::prepareSymbols(const GlyphMap& glyphMap, const GlyphPositions
 
         // if either shapedText or icon position is present, add the feature
         if (getDefaultHorizontalShaping(shapedTextOrientations) || shapedIcon) {
-            addFeature(std::distance(features.begin(), it), feature, shapedTextOrientations, std::move(shapedIcon), glyphPositions, textOffset);
+            addFeature(std::distance(features.begin(), it), feature, shapedTextOrientations, std::move(shapedIcon), glyphPositions, textOffset, iconType);
         }
 
         feature.geometry.clear();
@@ -433,7 +435,8 @@ void SymbolLayout::addFeature(const std::size_t layoutFeatureIndex,
                               const ShapedTextOrientations& shapedTextOrientations,
                               optional<PositionedIcon> shapedIcon,
                               const GlyphPositions& glyphPositions,
-                              Point<float> offset) {
+                              Point<float> offset,
+                              const SymbolContent iconType) {
     const float minScale = 0.5f;
     const float glyphSize = 24.0f;
 
@@ -500,7 +503,7 @@ void SymbolLayout::addFeature(const std::size_t layoutFeatureIndex,
                     iconBoxScale, iconPadding, iconOffset, indexedFeature,
                     layoutFeatureIndex, feature.index,
                     feature.formattedText ? feature.formattedText->rawText() : std::u16string(),
-                    overscaling, iconRotation, textRotation, radialTextOffset, allowVerticalPlacement);
+                    overscaling, iconRotation, textRotation, radialTextOffset, allowVerticalPlacement, iconType);
         }
     };
 
@@ -620,14 +623,14 @@ std::vector<float> CalculateTileDistances(const GeometryCoordinates& line, const
 }
 
 void SymbolLayout::createBucket(const ImagePositions&, std::unique_ptr<FeatureIndex>&, std::unordered_map<std::string, LayerRenderData>& renderData, const bool firstLoad, const bool showCollisionBoxes) {
-    auto bucket = std::make_shared<SymbolBucket>(layout, layerPaintProperties, textSize, iconSize, zoom, sdfIcons, iconsNeedLinear,
+    auto bucket = std::make_shared<SymbolBucket>(layout, layerPaintProperties, textSize, iconSize, zoom, iconsNeedLinear,
                                                  sortFeaturesByY, bucketLeaderID, std::move(symbolInstances), tilePixelRatio,
                                                  allowVerticalPlacement,
                                                  std::move(placementModes));
 
     for (SymbolInstance &symbolInstance : bucket->symbolInstances) {
-        const bool hasText = symbolInstance.hasText;
-        const bool hasIcon = symbolInstance.hasIcon;
+        const bool hasText = symbolInstance.hasText();
+        const bool hasIcon = symbolInstance.hasIcon();
         const bool singleLine = symbolInstance.singleLine;
 
         const auto& feature = features.at(symbolInstance.layoutFeatureIndex);
@@ -637,26 +640,25 @@ void SymbolLayout::createBucket(const ImagePositions&, std::unique_ptr<FeatureIn
         // Process icon first, so that text symbols would have reference to iconIndex which
         // is used when dynamic vertices for icon-text-fit image have to be updated.
         if (hasIcon) {
-            if (symbolInstance.hasIcon) {
-                const Range<float> sizeData = bucket->iconSizeBinder->getVertexSizeData(feature);
-                const auto placeIcon = [&] (const SymbolQuad& iconQuad, auto& index, const WritingModeType writingMode) {
-                    bucket->icon.placedSymbols.emplace_back(symbolInstance.anchor.point, symbolInstance.anchor.segment, sizeData.min, sizeData.max,
-                            symbolInstance.iconOffset, writingMode, symbolInstance.line(), std::vector<float>());
-                    index = bucket->icon.placedSymbols.size() - 1;
-                    PlacedSymbol& iconSymbol = bucket->icon.placedSymbols.back();
-                    iconSymbol.angle = (allowVerticalPlacement && writingMode == WritingModeType::Vertical) ? M_PI_2 : 0;
-                    iconSymbol.vertexStartIndex = addSymbol(bucket->icon, sizeData, iconQuad,
-                                                            symbolInstance.anchor, iconSymbol, feature.sortKey);
-                };
-
-                placeIcon(*symbolInstance.iconQuad(), symbolInstance.placedIconIndex, WritingModeType::None);
-                if (symbolInstance.verticalIconQuad()) {
-                    placeIcon(*symbolInstance.verticalIconQuad(), symbolInstance.placedVerticalIconIndex, WritingModeType::Vertical);
-                }
+            const Range<float> sizeData = bucket->iconSizeBinder->getVertexSizeData(feature);
+            auto& iconBuffer = symbolInstance.hasSdfIcon() ? bucket->sdfIcon : bucket->icon;
+            const auto placeIcon = [&] (const SymbolQuad& iconQuad, auto& index, const WritingModeType writingMode) {
+                iconBuffer.placedSymbols.emplace_back(symbolInstance.anchor.point, symbolInstance.anchor.segment, sizeData.min, sizeData.max,
+                        symbolInstance.iconOffset, writingMode, symbolInstance.line(), std::vector<float>());
+                index = iconBuffer.placedSymbols.size() - 1;
+                PlacedSymbol& iconSymbol = iconBuffer.placedSymbols.back();
+                iconSymbol.angle = (allowVerticalPlacement && writingMode == WritingModeType::Vertical) ? M_PI_2 : 0;
+                iconSymbol.vertexStartIndex = addSymbol(iconBuffer, sizeData, iconQuad,
+                                                        symbolInstance.anchor, iconSymbol, feature.sortKey);
+            };
 
-                for (auto& pair : bucket->paintProperties) {
-                    pair.second.iconBinders.populateVertexVectors(feature, bucket->icon.vertices.elements(), {}, {});
-                }
+            placeIcon(*symbolInstance.iconQuad(), symbolInstance.placedIconIndex, WritingModeType::None);
+            if (symbolInstance.verticalIconQuad()) {
+                placeIcon(*symbolInstance.verticalIconQuad(), symbolInstance.placedVerticalIconIndex, WritingModeType::Vertical);
+            }
+
+            for (auto& pair : bucket->paintProperties) {
+                pair.second.iconBinders.populateVertexVectors(feature, iconBuffer.vertices.elements(), {}, {});
             }
         }
 
diff --git a/src/mbgl/layout/symbol_layout.hpp b/src/mbgl/layout/symbol_layout.hpp
index 6cc21c6d91..8d4c51148a 100644
--- a/src/mbgl/layout/symbol_layout.hpp
+++ b/src/mbgl/layout/symbol_layout.hpp
@@ -53,7 +53,8 @@ private:
                     const ShapedTextOrientations& shapedTextOrientations,
                     optional<PositionedIcon> shapedIcon,
                     const GlyphPositions&,
-                    Point<float> textOffset);
+                    Point<float> textOffset,
+                    const SymbolContent iconType);
 
     bool anchorIsTooClose(const std::u16string& text, const float repeatDistance, const Anchor&);
     std::map<std::u16string, std::vector<Anchor>> compareText;
@@ -93,7 +94,6 @@ private:
     const uint32_t tileSize;
     const float tilePixelRatio;
 
-    bool sdfIcons = false;
     bool iconsNeedLinear = false;
     bool sortFeaturesByY = false;
     bool allowVerticalPlacement = false;