[core] Fix issues for use-cases when icon-text-fit is combined with text-variable-anchor

platform/android/CHANGELOG.md

@@ -7,6 +7,7 @@ Mapbox welcomes participation and contributions from everyone.  If you'd like to
  - Fixed use of objects after moving, potentially causing crashes. [#15408](https://github.com/mapbox/mapbox-gl-native/pull/1540)
  - Fixed a possible crash that could be caused by invoking the wrong layer implementation casting function [#15398](https://github.com/mapbox/mapbox-gl-native/pull/15398).
  - Font lookup on pre lollipop devices failed, provide default font list instead [#15410](https://github.com/mapbox/mapbox-gl-native/pull/15410).
+ - 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).
 
 ## 8.3.0-alpha.3 - August 15, 2019
 [Changes](https://github.com/mapbox/mapbox-gl-native/compare/android-v8.3.0-alpha.2...android-v8.3.0-alpha.3) since [Mapbox Maps SDK for Android v8.3.0-alpha.2](https://github.com/mapbox/mapbox-gl-native/releases/tag/android-v8.3.0-alpha.2):

platform/ios/CHANGELOG.md

@@ -2,6 +2,11 @@
 
 Mapbox welcomes participation and contributions from everyone. Please read [CONTRIBUTING.md](../../CONTRIBUTING.md) to get started.
 
+## master
+
+### Styles and rendering
+* 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))
+
 ## 5.3.0
 
 ### Styles and rendering

platform/macos/CHANGELOG.md

@@ -11,6 +11,7 @@
 * 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)
+* 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))
 
 ### Styles and rendering
 

platform/node/CHANGELOG.md

@@ -1,5 +1,6 @@
 # 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)).
 
 # 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))

platform/node/test/ignores.json

@@ -100,17 +100,5 @@
   "query-tests/fill-extrusion/sort-rotated": "https://github.com/mapbox/mapbox-gl-native/issues/13139",
   "query-tests/fill-extrusion/sort": "https://github.com/mapbox/mapbox-gl-native/issues/13139",
   "query-tests/fill-extrusion/top-in": "https://github.com/mapbox/mapbox-gl-native/issues/13139",
-  "query-tests/regressions/mapbox-gl-js#7883": "https://github.com/mapbox/mapbox-gl-native/issues/14585",
-  "render-tests/icon-text-fit/both-padding": "https://github.com/mapbox/mapbox-gl-native/issues/15346",
-  "render-tests/icon-text-fit/both": "https://github.com/mapbox/mapbox-gl-native/issues/15346",
-  "render-tests/icon-text-fit/height-padding": "https://github.com/mapbox/mapbox-gl-native/issues/15346",
-  "render-tests/icon-text-fit/height": "https://github.com/mapbox/mapbox-gl-native/issues/15346",
-  "render-tests/icon-text-fit/placement-line": "https://github.com/mapbox/mapbox-gl-native/issues/15346",
-  "render-tests/icon-text-fit/width-padding": "https://github.com/mapbox/mapbox-gl-native/issues/15346",
-  "render-tests/icon-text-fit/width": "https://github.com/mapbox/mapbox-gl-native/issues/15346",
-  "render-tests/regressions/mapbox-gl-js#5631": "https://github.com/mapbox/mapbox-gl-native/issues/15346",
-  "render-tests/text-variable-anchor/all-anchors-icon-text-fit": "https://github.com/mapbox/mapbox-gl-native/issues/15346",
-  "render-tests/text-variable-anchor/icon-text-fit-collision-box": "https://github.com/mapbox/mapbox-gl-native/issues/15346",
-  "render-tests/text-writing-mode/point_label/cjk-variable-anchors-vertical-horizontal-mode-icon-text-fit": "https://github.com/mapbox/mapbox-gl-native/issues/15346",
-  "render-tests/text-writing-mode/point_label/mixed-multiline-vertical-horizontal-mode-icon-text-fit": "https://github.com/mapbox/mapbox-gl-native/issues/15346"
+  "query-tests/regressions/mapbox-gl-js#7883": "https://github.com/mapbox/mapbox-gl-native/issues/14585"
 }

src/mbgl/layout/symbol_instance.cpp

@@ -21,15 +21,18 @@ const Shaping& getAnyShaping(const ShapedTextOrientations& shapedTextOrientation
 SymbolInstanceSharedData::SymbolInstanceSharedData(GeometryCoordinates line_,
                                                    const ShapedTextOrientations& shapedTextOrientations,
                                                    const optional<PositionedIcon>& shapedIcon,
+                                                   const optional<PositionedIcon>& verticallyShapedIcon,
                                                    const style::SymbolLayoutProperties::Evaluated& layout,
-                                                   const float layoutTextSize,
                                                    const style::SymbolPlacementType textPlacement,
                                                    const std::array<float, 2>& textOffset,
                                                    const GlyphPositions& positions,
 						   bool allowVerticalPlacement) : line(std::move(line_)) {
     // Create the quads used for rendering the icon and glyphs.
     if (shapedIcon) {
-        iconQuad = getIconQuad(*shapedIcon, layout, layoutTextSize, shapedTextOrientations.horizontal);
+        iconQuad = getIconQuad(*shapedIcon, getAnyShaping(shapedTextOrientations).writingMode);
+        if (verticallyShapedIcon) {
+            verticalIconQuad = getIconQuad(*verticallyShapedIcon, shapedTextOrientations.vertical.writingMode);
+        }
     }
 
     bool singleLineInitialized = false;
@@ -69,6 +72,7 @@ SymbolInstance::SymbolInstance(Anchor& anchor_,
                                std::shared_ptr<SymbolInstanceSharedData> sharedData_,
                                const ShapedTextOrientations& shapedTextOrientations,
                                const optional<PositionedIcon>& shapedIcon,
+                               const optional<PositionedIcon>& verticallyShapedIcon,
                                const float textBoxScale_,
                                const float textPadding,
                                const SymbolPlacementType textPlacement,
@@ -107,6 +111,14 @@ SymbolInstance::SymbolInstance(Anchor& anchor_,
     if (allowVerticalPlacement && shapedTextOrientations.vertical) {
         const float verticalPointLabelAngle = 90.0f;
         verticalTextCollisionFeature = CollisionFeature(line(), anchor, shapedTextOrientations.vertical, textBoxScale_, textPadding, textPlacement, indexedFeature, overscaling, textRotation + verticalPointLabelAngle);
+        if (verticallyShapedIcon) {
+            verticalIconCollisionFeature = CollisionFeature(sharedData->line,
+                                                            anchor,
+                                                            verticallyShapedIcon,
+                                                            iconBoxScale, iconPadding,
+                                                            indexedFeature,
+                                                            iconRotation + verticalPointLabelAngle);
+        }
     }
 
     rightJustifiedGlyphQuadsSize = sharedData->rightJustifiedGlyphQuads.size();
@@ -153,6 +165,11 @@ const optional<SymbolQuad>& SymbolInstance::iconQuad() const {
     return sharedData->iconQuad;
 }
 
+const optional<SymbolQuad>& SymbolInstance::verticalIconQuad() const {
+    assert(sharedData);
+    return sharedData->verticalIconQuad;
+}
+
 void SymbolInstance::releaseSharedData() {
     sharedData.reset();
 }

src/mbgl/layout/symbol_instance.hpp

@@ -25,8 +25,8 @@ struct SymbolInstanceSharedData {
     SymbolInstanceSharedData(GeometryCoordinates line,
                             const ShapedTextOrientations& shapedTextOrientations,
                             const optional<PositionedIcon>& shapedIcon,
+                            const optional<PositionedIcon>& verticallyShapedIcon,
                             const style::SymbolLayoutProperties::Evaluated& layout,
-                            const float layoutTextSize,
                             const style::SymbolPlacementType textPlacement,
                             const std::array<float, 2>& textOffset,
                             const GlyphPositions& positions,
@@ -39,6 +39,7 @@ struct SymbolInstanceSharedData {
     SymbolQuads leftJustifiedGlyphQuads;
     SymbolQuads verticalGlyphQuads;
     optional<SymbolQuad> iconQuad;
+    optional<SymbolQuad> verticalIconQuad;
 };
 
 class SymbolInstance {
@@ -47,6 +48,7 @@ class SymbolInstance {
                    std::shared_ptr<SymbolInstanceSharedData> sharedData,
                    const ShapedTextOrientations& shapedTextOrientations,
                    const optional<PositionedIcon>& shapedIcon,
+                   const optional<PositionedIcon>& verticallyShapedIcon,
                    const float textBoxScale,
                    const float textPadding,
                    const style::SymbolPlacementType textPlacement,
@@ -71,6 +73,7 @@ class SymbolInstance {
     const SymbolQuads& centerJustifiedGlyphQuads() const;
     const SymbolQuads& verticalGlyphQuads() const;
     const optional<SymbolQuad>& iconQuad() const;
+    const optional<SymbolQuad>& verticalIconQuad() const;
     void releaseSharedData();
 
 private:
@@ -89,6 +92,7 @@ class SymbolInstance {
     CollisionFeature textCollisionFeature;
     CollisionFeature iconCollisionFeature;
     optional<CollisionFeature> verticalTextCollisionFeature = nullopt;
+    optional<CollisionFeature> verticalIconCollisionFeature = nullopt;
     WritingModeType writingModes;
     std::size_t layoutFeatureIndex; // Index into the set of features included at layout time
     std::size_t dataFeatureIndex;   // Index into the underlying tile data feature set
@@ -101,6 +105,7 @@ class SymbolInstance {
     optional<size_t> placedLeftTextIndex;
     optional<size_t> placedVerticalTextIndex;
     optional<size_t> placedIconIndex;
+    optional<size_t> placedVerticalIconIndex;
     float textBoxScale;
     float radialTextOffset;
     bool singleLine;

src/mbgl/layout/symbol_layout.cpp

@@ -419,7 +419,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, shapedIcon, glyphPositions, textOffset);
+            addFeature(std::distance(features.begin(), it), feature, shapedTextOrientations, std::move(shapedIcon), glyphPositions, textOffset);
         }
 
         feature.geometry.clear();
@@ -431,7 +431,7 @@ void SymbolLayout::prepareSymbols(const GlyphMap& glyphMap, const GlyphPositions
 void SymbolLayout::addFeature(const std::size_t layoutFeatureIndex,
                               const SymbolFeature& feature,
                               const ShapedTextOrientations& shapedTextOrientations,
-                              const optional<PositionedIcon>& shapedIcon,
+                              optional<PositionedIcon> shapedIcon,
                               const GlyphPositions& glyphPositions,
                               Point<float> offset) {
     const float minScale = 0.5f;
@@ -467,6 +467,22 @@ void SymbolLayout::addFeature(const std::size_t layoutFeatureIndex,
     const float textRepeatDistance = symbolSpacing / 2;
     const auto evaluatedLayoutProperties = layout->evaluate(zoom, feature);
     IndexedSubfeature indexedFeature(feature.index, sourceLayer->getName(), bucketLeaderID, symbolInstances.size());
+    const bool hasIconTextFit = evaluatedLayoutProperties.get<style::IconTextFit>() != IconTextFitType::None;
+
+    // Adjust shaped icon size when icon-text-fit is used.
+    optional<PositionedIcon> verticallyShapedIcon;
+    if (shapedIcon && hasIconTextFit) {
+        // Create vertically shaped icon for vertical writing mode if needed.
+        if (allowVerticalPlacement && shapedTextOrientations.vertical) {
+            verticallyShapedIcon = shapedIcon;
+            verticallyShapedIcon->fitIconToText(evaluatedLayoutProperties,
+                                                shapedTextOrientations.vertical,
+                                                layoutTextSize);
+        }
+        shapedIcon->fitIconToText(evaluatedLayoutProperties,
+                                  getDefaultHorizontalShaping(shapedTextOrientations),
+                                  layoutTextSize);
+    }
 
     auto addSymbolInstance = [&] (Anchor& anchor, std::shared_ptr<SymbolInstanceSharedData> sharedData) {
         assert(sharedData);
@@ -478,7 +494,8 @@ void SymbolLayout::addFeature(const std::size_t layoutFeatureIndex,
             // In tiled rendering mode, add all symbols in the buffers so that we can:
             //  (1) render symbols that overlap into this tile
             //  (2) approximate collision detection effects from neighboring symbols
-            symbolInstances.emplace_back(anchor, std::move(sharedData), shapedTextOrientations, shapedIcon,
+            symbolInstances.emplace_back(anchor, std::move(sharedData), shapedTextOrientations,
+                    shapedIcon, verticallyShapedIcon,
                     textBoxScale, textPadding, textPlacement, textOffset,
                     iconBoxScale, iconPadding, iconOffset, indexedFeature,
                     layoutFeatureIndex, feature.index,
@@ -489,7 +506,7 @@ void SymbolLayout::addFeature(const std::size_t layoutFeatureIndex,
 
     const auto createSymbolInstanceSharedData = [&] (GeometryCoordinates line) {
         return std::make_shared<SymbolInstanceSharedData>(std::move(line),
-            shapedTextOrientations, shapedIcon, evaluatedLayoutProperties, layoutTextSize,
+            shapedTextOrientations, shapedIcon, verticallyShapedIcon, evaluatedLayoutProperties,
             textPlacement, textOffset, glyphPositions, allowVerticalPlacement);
     };
 
@@ -617,6 +634,32 @@ void SymbolLayout::createBucket(const ImagePositions&, std::unique_ptr<FeatureIn
 
         // Insert final placement into collision tree and add glyphs/icons to buffers
 
+        // 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);
+                }
+
+                for (auto& pair : bucket->paintProperties) {
+                    pair.second.iconBinders.populateVertexVectors(feature, bucket->icon.vertices.elements(), {}, {});
+                }
+            }
+        }
+
         if (hasText && feature.formattedText) {
             optional<std::size_t> lastAddedSection;
             if (singleLine) {
@@ -643,21 +686,6 @@ void SymbolLayout::createBucket(const ImagePositions&, std::unique_ptr<FeatureIn
             updatePaintPropertiesForSection(*bucket, feature, *lastAddedSection);
         }
 
-        if (hasIcon) {
-            if (symbolInstance.hasIcon) {
-                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.placedIconIndex = bucket->icon.placedSymbols.size() - 1;
-                PlacedSymbol& iconSymbol = bucket->icon.placedSymbols.back();
-                iconSymbol.vertexStartIndex = addSymbol(bucket->icon, sizeData, *symbolInstance.iconQuad(),
-                                                        symbolInstance.anchor, iconSymbol, feature.sortKey);
-
-                for (auto& pair : bucket->paintProperties) {
-                    pair.second.iconBinders.populateVertexVectors(feature, bucket->icon.vertices.elements(), {}, {});
-                }
-            }
-        }
         symbolInstance.releaseSharedData();
     }
 
@@ -693,9 +721,9 @@ std::size_t SymbolLayout::addSymbolGlyphQuads(SymbolBucket& bucket,
                                               optional<std::size_t> lastAddedSection) {
     const Range<float> sizeData = bucket.textSizeBinder->getVertexSizeData(feature);
     const bool hasFormatSectionOverrides = bucket.hasFormatSectionOverrides();
-
+    const auto& placedIconIndex = writingMode == WritingModeType::Vertical ? symbolInstance.placedVerticalIconIndex : symbolInstance.placedIconIndex;
     bucket.text.placedSymbols.emplace_back(symbolInstance.anchor.point, symbolInstance.anchor.segment, sizeData.min, sizeData.max,
-            symbolInstance.textOffset, writingMode, symbolInstance.line(), CalculateTileDistances(symbolInstance.line(), symbolInstance.anchor));
+            symbolInstance.textOffset, writingMode, symbolInstance.line(), CalculateTileDistances(symbolInstance.line(), symbolInstance.anchor), placedIconIndex);
     placedIndex = bucket.text.placedSymbols.size() - 1;
     PlacedSymbol& placedSymbol = bucket.text.placedSymbols.back();
     placedSymbol.angle = (allowVerticalPlacement && writingMode == WritingModeType::Vertical) ? M_PI_2 : 0;
@@ -837,6 +865,9 @@ void SymbolLayout::addToDebugBuffers(SymbolBucket& bucket) {
         if (symbolInstance.verticalTextCollisionFeature) {
             populateCollisionBox(*symbolInstance.verticalTextCollisionFeature);
         }
+        if (symbolInstance.verticalIconCollisionFeature) {
+            populateCollisionBox(*symbolInstance.verticalIconCollisionFeature);
+        }
         populateCollisionBox(symbolInstance.iconCollisionFeature);
     }
 }

src/mbgl/layout/symbol_layout.hpp

@@ -51,7 +51,7 @@ class SymbolLayout final : public Layout {
     void addFeature(const size_t,
                     const SymbolFeature&,
                     const ShapedTextOrientations& shapedTextOrientations,
-                    const optional<PositionedIcon>& shapedIcon,
+                    optional<PositionedIcon> shapedIcon,
                     const GlyphPositions&,
                     Point<float> textOffset);