Cherry-pick #15456 [core] fix mixed sdf + non-sdf icon rendering in o…

…ne layer (#15494)

* [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

* Add changelog for fix of mixed SDF+non-SDF icon rendering in one layer (#15492)

* Add changelog for fix of mixed SDF+non-SDF icon rendering in one layer

* Add bracket for ios changelog number

* Add more brackets

platform/android/CHANGELOG.md

@@ -4,6 +4,9 @@ Mapbox welcomes participation and contributions from everyone.  If you'd like to
 
 ## master
 
+### Bug fixes
+- Fixed a rendering issue that non-SDF icon would be treated as SDF icon if they are in the same layer. [#15456](https://github.com/mapbox/mapbox-gl-native/pull/15456)
+
 ## 8.3.0
 This release changes how offline tile requests are billed — they are now billed on a pay-as-you-go basis and all developers are able raise the offline tile limit for their users. Offline requests were previously exempt from monthly active user (MAU) billing and increasing the offline per-user tile limit to more than 6,000 tiles required the purchase of an enterprise license. By upgrading to this release, you are opting into the changes outlined in [this blog post](https://blog.mapbox.com/offline-maps-for-all-bb0fc51827be) and [#15380](https://github.com/mapbox/mapbox-gl-native/pull/15380).
 

platform/ios/CHANGELOG.md

@@ -2,6 +2,10 @@
 
 Mapbox welcomes participation and contributions from everyone. Please read [CONTRIBUTING.md](../../CONTRIBUTING.md) to get started.
 
+## master
+
+* Fixed a rendering issue that non-SDF icon would be treated as SDF icon if they are in the same layer. ([#15456](https://github.com/mapbox/mapbox-gl-native/pull/15456))
+
 ## 5.3.0
 
 This release changes how offline tile requests are billed — they are now billed on a pay-as-you-go basis and all developers are able raise the offline tile limit for their users. Offline requests were previously exempt from monthly active user (MAU) billing and increasing the offline per-user tile limit to more than 6,000 tiles required the purchase of an enterprise license. By upgrading to this release, you are opting into the changes outlined in [this blog post](https://blog.mapbox.com/offline-maps-for-all-bb0fc51827be) and [#15380](https://github.com/mapbox/mapbox-gl-native/pull/15380).

platform/macos/CHANGELOG.md

@@ -9,10 +9,11 @@
 * The `MGLIdeographicFontFamilyName` Info.plist key now also accepts an array of font family names, to customize font fallback behavior. It can also be set to a Boolean value of `NO` to force the SDK to typeset CJK characters in a remote font specified by `MGLSymbolStyleLayer.textFontNames`. ([#14862](https://github.com/mapbox/mapbox-gl-native/pull/14862)) 
 * Performance improvements for queryRenderedFeatures API and optimization that allocates containers based on a number of rendered layers. ([#14930](https://github.com/mapbox/mapbox-gl-native/pull/14930))
 * Fixed rendering layers after fill-extrusion regression caused by optimization of fill-extrusion rendering. ([#15065](https://github.com/mapbox/mapbox-gl-native/pull/15065))
-* `MGLLoggingLevel` has been updated for better matching core log levels. Now can use `[MGLLoggingConfiguration sharedConfiguration].loggingLevel` to filter logs from core . [#15120](https://github.com/mapbox/mapbox-gl-native/pull/15120)
-* Fixed an issue where animated camera transitions zoomed in or out too dramatically. [#15281](https://github.com/mapbox/mapbox-gl-native/pull/15281)
+* `MGLLoggingLevel` has been updated for better matching core log levels. Now can use `[MGLLoggingConfiguration sharedConfiguration].loggingLevel` to filter logs from core . ([#15120](https://github.com/mapbox/mapbox-gl-native/pull/15120))
+* Fixed an issue where animated camera transitions zoomed in or out too dramatically. ([#15281](https://github.com/mapbox/mapbox-gl-native/pull/15281))
 * Fixed rendering and collision detection issues with using `text-variable-anchor` and `icon-text-fit` properties on the same layer. ([#15367](https://github.com/mapbox/mapbox-gl-native/pull/15367))
-- Fixed symbol overlap when zooming out quickly. ([15416](https://github.com/mapbox/mapbox-gl-native/pull/15416))
+* Fixed symbol overlap when zooming out quickly. ([15416](https://github.com/mapbox/mapbox-gl-native/pull/15416))
+* Fixed a rendering issue that non-SDF icon would be treated as SDF icon if they are in the same layer. ([#15456](https://github.com/mapbox/mapbox-gl-native/pull/15456))
 
 ### Styles and rendering
 

platform/node/CHANGELOG.md

@@ -1,15 +1,16 @@
 # master
 * Introduce `text-writing-mode` layout property for symbol layer ([#14932](https://github.com/mapbox/mapbox-gl-native/pull/14932)). The `text-writing-mode` layout property allows control over symbol's preferred writing mode. The new property value is an array, whose values are enumeration values from a ( `horizontal` | `vertical` ) set.
 * Fixed rendering and collision detection issues with using `text-variable-anchor` and `icon-text-fit` properties on the same layer ([#15367](https://github.com/mapbox/mapbox-gl-native/pull/15367)).
+* Fixed a rendering issue that non-SDF icon would be treated as SDF icon if they are in the same layer. ([#15456](https://github.com/mapbox/mapbox-gl-native/pull/15456))
 
 # 4.2.0
 - Add an option to set whether or not an image should be treated as a SDF ([#15054](https://github.com/mapbox/mapbox-gl-native/issues/15054))
-- Fix problems associated with node 10 and NAN [#14847](https://github.com/mapbox/mapbox-gl-native/pull/14847)
+- Fix problems associated with node 10 and NAN ([#14847](https://github.com/mapbox/mapbox-gl-native/pull/14847))
 
 # 4.1.0
-- Add `symbol-z-order` symbol layout property to style spec [#12783](https://github.com/mapbox/mapbox-gl-native/pull/12783)
-- Add `crossSourceCollisions` map option, with default of `true`. When set to `false`, cross-source collision detection is disabled. ([#12820] (https://github.com/mapbox/mapbox-gl-native/issues/12820))
-- Fixed bugs in coercion expression operators ("to-array" applied to empty arrays, "to-color" applied to colors, and "to-number" applied to null) [#12864](https://github.com/mapbox/mapbox-gl-native/pull/12864)
+- Add `symbol-z-order` symbol layout property to style spec ([#12783](https://github.com/mapbox/mapbox-gl-native/pull/12783))
+- Add `crossSourceCollisions` map option, with default of `true`. When set to `false`, cross-source collision detection is disabled. ([#12820](https://github.com/mapbox/mapbox-gl-native/issues/12820))
+- Fixed bugs in coercion expression operators ("to-array" applied to empty arrays, "to-color" applied to colors, and "to-number" applied to null) ([#12864](https://github.com/mapbox/mapbox-gl-native/pull/12864))
 - Fixed an issue where fill and line layers would occasionally flicker on zoom ([#12982](https://github.com/mapbox/mapbox-gl-native/pull/12982))
 
 # 4.0.0

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,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,14 +84,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 +105,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;