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

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);

src/mbgl/layout/symbol_instance.hpp

@@ -4,7 +4,7 @@
 #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>
 
 namespace mbgl {
 
@@ -42,6 +42,25 @@ struct SymbolInstanceSharedData {
     optional<SymbolQuad> verticalIconQuad;
 };
 
+enum class SymbolContent : uint8_t {
+    None = 0,
+    Text = 1 << 0,
+    IconRGBA = 1 << 1,
+    IconSDF = 1 << 2
+};
+
+constexpr SymbolContent operator|(SymbolContent a, SymbolContent b) {
+    return SymbolContent(mbgl::underlying_type(a) | mbgl::underlying_type(b));
+}
+
+constexpr SymbolContent& operator|=(SymbolContent& a, SymbolContent b) {
+    return (a = a | b);
+}
+
+constexpr SymbolContent operator&(SymbolContent a, SymbolContent b) {
+    return SymbolContent(mbgl::underlying_type(a) & mbgl::underlying_type(b));
+}
+
 class SymbolInstance {
 public:
     SymbolInstance(Anchor& anchor_,
@@ -64,14 +83,18 @@ class SymbolInstance {
                    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;
     const SymbolQuads& rightJustifiedGlyphQuads() const;
     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 +104,7 @@ class SymbolInstance {
 
 public:
     Anchor anchor;
-    bool hasText;
-    bool hasIcon;
+    SymbolContent symbolContent;
 
     std::size_t rightJustifiedGlyphQuadsSize;
     std::size_t centerJustifiedGlyphQuadsSize;

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(), {}, {});
             }
         }
 

src/mbgl/layout/symbol_layout.hpp

@@ -53,7 +53,8 @@ class SymbolLayout final : public Layout {
                     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 @@ class SymbolLayout final : public Layout {
     const uint32_t tileSize;
     const float tilePixelRatio;
 
-    bool sdfIcons = false;
     bool iconsNeedLinear = false;
     bool sortFeaturesByY = false;
     bool allowVerticalPlacement = false;

src/mbgl/renderer/buckets/symbol_bucket.cpp

@@ -18,7 +18,6 @@ SymbolBucket::SymbolBucket(Immutable<style::SymbolLayoutProperties::PossiblyEval
                            const style::PropertyValue<float>& textSize,
                            const style::PropertyValue<float>& iconSize,
                            float zoom,
-                           bool sdfIcons_,
                            bool iconsNeedLinear_,
                            bool sortFeaturesByY_,
                            const std::string bucketName_,
@@ -28,7 +27,6 @@ SymbolBucket::SymbolBucket(Immutable<style::SymbolLayoutProperties::PossiblyEval
                            std::vector<style::TextWritingModeType> placementModes_)
     : layout(std::move(layout_)),
       bucketLeaderID(std::move(bucketName_)),
-      sdfIcons(sdfIcons_),
       iconsNeedLinear(iconsNeedLinear_ || iconSize.isDataDriven() || !iconSize.isZoomConstant()),
       sortFeaturesByY(sortFeaturesByY_),
       staticUploaded(false),
@@ -87,30 +85,37 @@ void SymbolBucket::upload(gfx::UploadPass& uploadPass) {
         }
     }
 
-    if (hasIconData()) {
+    auto updateIconBuffer = [&](Buffer& iconBuffer) {
         if (!staticUploaded) {
-            icon.indexBuffer = uploadPass.createIndexBuffer(std::move(icon.triangles), sortFeaturesByY ? gfx::BufferUsageType::StreamDraw : gfx::BufferUsageType::StaticDraw);
-            icon.vertexBuffer = uploadPass.createVertexBuffer(std::move(icon.vertices));
+            iconBuffer.indexBuffer = uploadPass.createIndexBuffer(std::move(iconBuffer.triangles), sortFeaturesByY ? gfx::BufferUsageType::StreamDraw : gfx::BufferUsageType::StaticDraw);
+            iconBuffer.vertexBuffer = uploadPass.createVertexBuffer(std::move(iconBuffer.vertices));
             for (auto& pair : paintProperties) {
                 pair.second.iconBinders.upload(uploadPass);
             }
         } else if (!sortUploaded) {
-            uploadPass.updateIndexBuffer(*icon.indexBuffer, std::move(icon.triangles));
+            uploadPass.updateIndexBuffer(*iconBuffer.indexBuffer, std::move(iconBuffer.triangles));
         }
         if (!dynamicUploaded) {
-            if (!icon.dynamicVertexBuffer) {
-                icon.dynamicVertexBuffer = uploadPass.createVertexBuffer(std::move(icon.dynamicVertices), gfx::BufferUsageType::StreamDraw);
+            if (!iconBuffer.dynamicVertexBuffer) {
+                iconBuffer.dynamicVertexBuffer = uploadPass.createVertexBuffer(std::move(iconBuffer.dynamicVertices), gfx::BufferUsageType::StreamDraw);
             } else {
-                uploadPass.updateVertexBuffer(*icon.dynamicVertexBuffer, std::move(icon.dynamicVertices));
+                uploadPass.updateVertexBuffer(*iconBuffer.dynamicVertexBuffer, std::move(iconBuffer.dynamicVertices));
             }
         }
         if (!placementChangesUploaded) {
-            if (!icon.opacityVertexBuffer) {
-                icon.opacityVertexBuffer = uploadPass.createVertexBuffer(std::move(icon.opacityVertices), gfx::BufferUsageType::StreamDraw);
+            if (!iconBuffer.opacityVertexBuffer) {
+                iconBuffer.opacityVertexBuffer = uploadPass.createVertexBuffer(std::move(iconBuffer.opacityVertices), gfx::BufferUsageType::StreamDraw);
             } else {
-                uploadPass.updateVertexBuffer(*icon.opacityVertexBuffer, std::move(icon.opacityVertices));
+                uploadPass.updateVertexBuffer(*iconBuffer.opacityVertexBuffer, std::move(iconBuffer.opacityVertices));
             }
         }
+    };
+    if (hasIconData()) {
+        updateIconBuffer(icon);
+    }
+
+    if (hasSdfIconData()) {
+        updateIconBuffer(sdfIcon);
     }
 
     if (hasCollisionBoxData()) {
@@ -149,7 +154,7 @@ void SymbolBucket::upload(gfx::UploadPass& uploadPass) {
 }
 
 bool SymbolBucket::hasData() const {
-    return hasTextData() || hasIconData() || hasCollisionBoxData();
+    return hasTextData() || hasIconData() || hasSdfIconData() || hasCollisionBoxData();
 }
 
 bool SymbolBucket::hasTextData() const {
@@ -160,6 +165,10 @@ bool SymbolBucket::hasIconData() const {
     return !icon.segments.empty();
 }
 
+bool SymbolBucket::hasSdfIconData() const {
+    return !sdfIcon.segments.empty();
+}
+
 bool SymbolBucket::hasCollisionBoxData() const {
     return collisionBox && !collisionBox->segments.empty();
 }
@@ -188,9 +197,9 @@ void SymbolBucket::sortFeatures(const float angle) {
 
     sortedAngle = angle;
 
-    // The current approach to sorting doesn't sort across segments so don't try.
+    // The current approach to sorting doesn't sort across text and icon segments so don't try.
     // Sorting within segments separately seemed not to be worth the complexity.
-    if (text.segments.size() > 1 || icon.segments.size() > 1) {
+    if (text.segments.size() > 1 || (icon.segments.size() > 1 || sdfIcon.segments.size() > 1)) {
         return;
     }
 
@@ -199,6 +208,7 @@ void SymbolBucket::sortFeatures(const float angle) {
 
     text.triangles.clear();
     icon.triangles.clear();
+    sdfIcon.triangles.clear();
 
     featureSortOrder = std::make_unique<std::vector<size_t>>();
     featureSortOrder->reserve(symbolInstances.size());
@@ -225,12 +235,13 @@ void SymbolBucket::sortFeatures(const float angle) {
             addPlacedSymbol(text.triangles, text.placedSymbols[*symbolInstance.placedVerticalTextIndex]);
         }
 
+        auto& iconBuffer = symbolInstance.hasSdfIcon() ? sdfIcon : icon;
         if (symbolInstance.placedIconIndex) {
-            addPlacedSymbol(icon.triangles, icon.placedSymbols[*symbolInstance.placedIconIndex]);
+            addPlacedSymbol(iconBuffer.triangles, iconBuffer.placedSymbols[*symbolInstance.placedIconIndex]);
         }
 
         if (symbolInstance.placedVerticalIconIndex) {
-            addPlacedSymbol(icon.triangles, icon.placedSymbols[*symbolInstance.placedVerticalIconIndex]);
+            addPlacedSymbol(iconBuffer.triangles, iconBuffer.placedSymbols[*symbolInstance.placedVerticalIconIndex]);
         }
     }
 }

src/mbgl/renderer/buckets/symbol_bucket.hpp

@@ -55,7 +55,6 @@ class SymbolBucket final : public Bucket {
                  const style::PropertyValue<float>& textSize,
                  const style::PropertyValue<float>& iconSize,
                  float zoom,
-                 bool sdfIcons,
                  bool iconsNeedLinear,
                  bool sortFeaturesByY,
                  const std::string bucketLeaderID,
@@ -72,6 +71,7 @@ class SymbolBucket final : public Bucket {
     void updateVertices(Placement&, bool updateOpacities, const TransformState&, const RenderTile&, std::set<uint32_t>&) override;
     bool hasTextData() const;
     bool hasIconData() const;
+    bool hasSdfIconData() const;
     bool hasCollisionBoxData() const;
     bool hasCollisionCircleData() const;
     bool hasFormatSectionOverrides() const;
@@ -86,7 +86,6 @@ class SymbolBucket final : public Bucket {
     float sortedAngle = std::numeric_limits<float>::max();
 
     // Flags
-    const bool sdfIcons : 1;
     const bool iconsNeedLinear : 1;
     const bool sortFeaturesByY : 1;
     bool staticUploaded : 1;
@@ -124,7 +123,8 @@ class SymbolBucket final : public Bucket {
     std::unique_ptr<SymbolSizeBinder> iconSizeBinder;
 
     Buffer icon;
-
+    Buffer sdfIcon;
+
     struct CollisionBuffer {
         gfx::VertexVector<gfx::Vertex<CollisionBoxLayoutAttributes>> vertices;
         gfx::VertexVector<gfx::Vertex<CollisionBoxDynamicAttributes>> dynamicVertices;