[Core] Ensure queryRenderedFeatures accounts for icon-rotate (#13105)

8 files changed, 63 insertions, 32 deletions

diff --git a/platform/node/test/ignores.json b/platform/node/test/ignores.json
index 9632677d46..fa4f4f7cd1 100644
--- a/platform/node/test/ignores.json
+++ b/platform/node/test/ignores.json
@@ -93,7 +93,6 @@
   "render-tests/fill-pattern/update-feature-state": "skip - port https://github.com/mapbox/mapbox-gl-js/pull/6263 - needs issue",
   "render-tests/geojson/inline-linestring-fill": "current behavior is arbitrary",
   "render-tests/geojson/inline-polygon-symbol": "behavior needs reconciliation with gl-js",
-  "render-tests/icon-rotate/with-offset": "https://github.com/mapbox/mapbox-gl-native/issues/11872",
   "render-tests/mixed-zoom/z10-z11": "https://github.com/mapbox/mapbox-gl-native/issues/10397",
   "render-tests/raster-masking/overlapping-zoom": "https://github.com/mapbox/mapbox-gl-native/issues/10195",
   "render-tests/real-world/bangkok": "https://github.com/mapbox/mapbox-gl-native/issues/10412",
diff --git a/scripts/changelog_staging/queryrenderedfeatures-with-icon-rotate.json b/scripts/changelog_staging/queryrenderedfeatures-with-icon-rotate.json
new file mode 100644
index 0000000000..36f7bafe85
--- /dev/null
+++ b/scripts/changelog_staging/queryrenderedfeatures-with-icon-rotate.json
@@ -0,0 +1,4 @@
+{
+    "core": "Fix bug in which using `icon-rotate` and `icon-offset` together prevented `queryRenderedFeatures` from returning the symbol",
+    "issue": 12909
+}
diff --git a/src/mbgl/layout/symbol_instance.cpp b/src/mbgl/layout/symbol_instance.cpp
index 2d60020dd4..139a42113c 100644
--- a/src/mbgl/layout/symbol_instance.cpp
+++ b/src/mbgl/layout/symbol_instance.cpp
@@ -23,15 +23,16 @@ SymbolInstance::SymbolInstance(Anchor& anchor_,
                                const std::size_t layoutFeatureIndex_,
                                const std::size_t dataFeatureIndex_,
                                const std::u16string& key_,
-                               const float overscaling) :
+                               const float overscaling,
+                               const float rotate) :
     anchor(anchor_),
     line(line_),
     hasText(false),
     hasIcon(shapedIcon),
 
     // Create the collision features that will be used to check whether this symbol instance can be placed
-    textCollisionFeature(line_, anchor, shapedTextOrientations.first, textBoxScale, textPadding, textPlacement, indexedFeature, overscaling),
-    iconCollisionFeature(line_, anchor, shapedIcon, iconBoxScale, iconPadding, indexedFeature),
+    textCollisionFeature(line_, anchor, shapedTextOrientations.first, textBoxScale, textPadding, textPlacement, indexedFeature, overscaling, rotate),
+    iconCollisionFeature(line_, anchor, shapedIcon, iconBoxScale, iconPadding, indexedFeature, rotate),
     layoutFeatureIndex(layoutFeatureIndex_),
     dataFeatureIndex(dataFeatureIndex_),
     textOffset(textOffset_),
diff --git a/src/mbgl/layout/symbol_instance.hpp b/src/mbgl/layout/symbol_instance.hpp
index 6be19a0595..6148d7fe88 100644
--- a/src/mbgl/layout/symbol_instance.hpp
+++ b/src/mbgl/layout/symbol_instance.hpp
@@ -31,7 +31,8 @@ public:
                    const std::size_t layoutFeatureIndex,
                    const std::size_t dataFeatureIndex,
                    const std::u16string& key,
-                   const float overscaling);
+                   const float overscaling,
+                   const float rotate);
 
     Anchor anchor;
     GeometryCoordinates line;
diff --git a/src/mbgl/layout/symbol_layout.cpp b/src/mbgl/layout/symbol_layout.cpp
index f4a55a1f91..f7b6d949bd 100644
--- a/src/mbgl/layout/symbol_layout.cpp
+++ b/src/mbgl/layout/symbol_layout.cpp
@@ -102,7 +102,7 @@ SymbolLayout::SymbolLayout(const BucketParameters& parameters,
         auto feature = sourceLayer->getFeature(i);
         if (!leader.filter(expression::EvaluationContext { this->zoom, feature.get() }))
             continue;
-        
+
         SymbolFeature ft(std::move(feature));
 
         ft.index = i;
@@ -112,7 +112,7 @@ SymbolLayout::SymbolLayout(const BucketParameters& parameters,
             auto textTransform = layout.evaluate<TextTransform>(zoom, ft);
             FontStack baseFontStack = layout.evaluate<TextFont>(zoom, ft);
             FontStackHash baseFontStackHash = FontStackHasher()(baseFontStack);
-            
+
             ft.formattedText = TaggedString();
             for (std::size_t j = 0; j < formatted.sections.size(); j++) {
                 const auto& section = formatted.sections[j];
@@ -122,7 +122,7 @@ SymbolLayout::SymbolLayout(const BucketParameters& parameters,
                 } else if (textTransform == TextTransformType::Lowercase) {
                     u8string = platform::lowercase(u8string);
                 }
-                
+
                 ft.formattedText->addSection(applyArabicShaping(util::utf8_to_utf16::convert(u8string)),
                                              section.fontScale ? *section.fontScale : 1.0,
                                              section.fontStack ? FontStackHasher()(*section.fontStack) : baseFontStackHash);
@@ -133,7 +133,7 @@ SymbolLayout::SymbolLayout(const BucketParameters& parameters,
             const bool canVerticalizeText = layout.get<TextRotationAlignment>() == AlignmentType::Map
                                          && layout.get<SymbolPlacement>() != SymbolPlacementType::Point
                                          && util::i18n::allowsVerticalWritingMode(ft.formattedText->rawText());
-            
+
             // Loop through all characters of this text and collect unique codepoints.
             for (std::size_t j = 0; j < ft.formattedText->length(); j++) {
                 const auto& sectionFontStack = formatted.sections[ft.formattedText->getSectionIndex(j)].fontStack;
@@ -236,7 +236,7 @@ void SymbolLayout::prepareSymbols(const GlyphMap& glyphMap, const GlyphPositions
         if (shapedTextOrientations.first || shapedIcon) {
             addFeature(std::distance(features.begin(), it), feature, shapedTextOrientations, shapedIcon, glyphPositions);
         }
-        
+
         feature.geometry.clear();
     }
 
@@ -250,18 +250,18 @@ void SymbolLayout::addFeature(const std::size_t layoutFeatureIndex,
                               const GlyphPositions& glyphPositions) {
     const float minScale = 0.5f;
     const float glyphSize = 24.0f;
-    
+
     const float layoutTextSize = layout.evaluate<TextSize>(zoom + 1, feature);
     const float layoutIconSize = layout.evaluate<IconSize>(zoom + 1, feature);
     const std::array<float, 2> textOffset = layout.evaluate<TextOffset>(zoom, feature);
     const std::array<float, 2> iconOffset = layout.evaluate<IconOffset>(zoom, feature);
-    
+
     // To reduce the number of labels that jump around when zooming we need
     // to use a text-size value that is the same for all zoom levels.
     // This calculates text-size at a high zoom level so that all tiles can
     // use the same value when calculating anchor positions.
     const float textMaxSize = layout.evaluate<TextSize>(18, feature);
-    
+
     const float fontScale = layoutTextSize / glyphSize;
     const float textBoxScale = tilePixelRatio * fontScale;
     const float textMaxBoxScale = tilePixelRatio * textMaxSize / glyphSize;
@@ -270,6 +270,7 @@ void SymbolLayout::addFeature(const std::size_t layoutFeatureIndex,
     const float textPadding = layout.get<TextPadding>() * tilePixelRatio;
     const float iconPadding = layout.get<IconPadding>() * tilePixelRatio;
     const float textMaxAngle = layout.get<TextMaxAngle>() * util::DEG2RAD;
+    const float rotation = layout.evaluate<IconRotate>(zoom, feature);
     const SymbolPlacementType textPlacement = layout.get<TextRotationAlignment>() != AlignmentType::Map
                                                   ? SymbolPlacementType::Point
                                                   : layout.get<SymbolPlacement>();
@@ -291,10 +292,10 @@ void SymbolLayout::addFeature(const std::size_t layoutFeatureIndex,
                     textBoxScale, textPadding, textPlacement, textOffset,
                     iconBoxScale, iconPadding, iconOffset,
                     glyphPositions, indexedFeature, layoutFeatureIndex, feature.index,
-                    feature.formattedText ? feature.formattedText->rawText() : std::u16string(), overscaling);
+                    feature.formattedText ? feature.formattedText->rawText() : std::u16string(), overscaling, rotation);
         }
     };
-    
+
     const auto& type = feature.getType();
 
     if (layout.get<SymbolPlacement>() == SymbolPlacementType::Line) {
@@ -408,7 +409,7 @@ void SymbolLayout::createBucket(const ImagePositions&, std::unique_ptr<FeatureIn
     const bool zOrderByViewport = layout.get<SymbolZOrder>() == SymbolZOrderType::ViewportY;
     const bool sortFeaturesByY = zOrderByViewport && (layout.get<TextAllowOverlap>() || layout.get<IconAllowOverlap>() ||
         layout.get<TextIgnorePlacement>() || layout.get<IconIgnorePlacement>());
-    
+
     auto bucket = std::make_shared<SymbolBucket>(layout, layerPaintProperties, textSize, iconSize, zoom, sdfIcons, iconsNeedLinear, sortFeaturesByY, bucketLeaderID, std::move(symbolInstances));
 
     for (SymbolInstance &symbolInstance : bucket->symbolInstances) {
@@ -437,20 +438,20 @@ void SymbolLayout::createBucket(const ImagePositions&, std::unique_ptr<FeatureIn
                     firstHorizontal = false;
                 }
             }
-            
+
             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.placedVerticalTextIndex = bucket->text.placedSymbols.size() - 1;
-                
+
                 PlacedSymbol& verticalSymbol = bucket->text.placedSymbols.back();
                 bool firstVertical = true;
-                
+
                 for (const auto& symbol : symbolInstance.verticalGlyphQuads) {
                     size_t index = addSymbol(
                         bucket->text, sizeData, symbol,
                         symbolInstance.anchor, verticalSymbol);
-                    
+
                     if (firstVertical) {
                         verticalSymbol.vertexStartIndex = index;
                         firstVertical = false;
@@ -470,7 +471,7 @@ void SymbolLayout::createBucket(const ImagePositions&, std::unique_ptr<FeatureIn
                                                         symbolInstance.anchor, iconSymbol);
             }
         }
-        
+
         for (auto& pair : bucket->paintPropertyBinders) {
             pair.second.first.populateVertexVectors(feature, bucket->icon.vertices.vertexSize(), {}, {});
             pair.second.second.populateVertexVectors(feature, bucket->text.vertices.vertexSize(), {}, {});
@@ -520,7 +521,7 @@ size_t SymbolLayout::addSymbol(Buffer& buffer,
     buffer.vertices.emplace_back(SymbolLayoutAttributes::vertex(labelAnchor.point, tr, symbol.glyphOffset.y, tex.x + tex.w, tex.y, sizeData));
     buffer.vertices.emplace_back(SymbolLayoutAttributes::vertex(labelAnchor.point, bl, symbol.glyphOffset.y, tex.x, tex.y + tex.h, sizeData));
     buffer.vertices.emplace_back(SymbolLayoutAttributes::vertex(labelAnchor.point, br, symbol.glyphOffset.y, tex.x + tex.w, tex.y + tex.h, sizeData));
-    
+
     // Dynamic/Opacity vertices are initialized so that the vertex count always agrees with
     // the layout vertex buffer, but they will always be updated before rendering happens
     auto dynamicVertex = SymbolDynamicLayoutAttributes::vertex(labelAnchor.point, 0);
@@ -528,7 +529,7 @@ size_t SymbolLayout::addSymbol(Buffer& buffer,
     buffer.dynamicVertices.emplace_back(dynamicVertex);
     buffer.dynamicVertices.emplace_back(dynamicVertex);
     buffer.dynamicVertices.emplace_back(dynamicVertex);
-    
+
     auto opacityVertex = SymbolOpacityAttributes::vertex(1.0, 1.0);
     buffer.opacityVertices.emplace_back(opacityVertex);
     buffer.opacityVertices.emplace_back(opacityVertex);
@@ -543,7 +544,7 @@ size_t SymbolLayout::addSymbol(Buffer& buffer,
     segment.indexLength += 6;
 
     placedSymbol.glyphOffsets.push_back(symbol.glyphOffset.x);
-    
+
     return index;
 }
 
diff --git a/src/mbgl/text/collision_feature.cpp b/src/mbgl/text/collision_feature.cpp
index ee7322e692..c708f834da 100644
--- a/src/mbgl/text/collision_feature.cpp
+++ b/src/mbgl/text/collision_feature.cpp
@@ -14,7 +14,8 @@ CollisionFeature::CollisionFeature(const GeometryCoordinates& line,
                                    const float padding,
                                    const style::SymbolPlacementType placement,
                                    IndexedSubfeature indexedFeature_,
-                                   const float overscaling)
+                                   const float overscaling,
+                                   const float rotate)
         : indexedFeature(std::move(indexedFeature_))
         , alongLine(placement != style::SymbolPlacementType::Point) {
     if (top == 0 && bottom == 0 && left == 0 && right == 0) return;
@@ -35,7 +36,28 @@ CollisionFeature::CollisionFeature(const GeometryCoordinates& line,
         GeometryCoordinate anchorPoint = convertPoint<int16_t>(anchor.point);
         bboxifyLabel(line, anchorPoint, anchor.segment, length, height, overscaling);
     } else {
-        boxes.emplace_back(anchor.point, Point<float>{ 0, 0 }, x1, y1, x2, y2);
+        if (rotate) {
+            // Account for *-rotate in point collision boxes
+            // Doesn't account for icon-text-fit
+            const float rotateRadians = rotate * M_PI / 180.0;
+
+            const Point<float> tl = util::rotate(Point<float>(x1, y1), rotateRadians);
+            const Point<float> tr = util::rotate(Point<float>(x2, y1), rotateRadians);
+            const Point<float> bl = util::rotate(Point<float>(x1, y2), rotateRadians);
+            const Point<float> br = util::rotate(Point<float>(x2, y2), rotateRadians);
+            
+            // Collision features require an "on-axis" geometry,
+            // so take the envelope of the rotated geometry
+            // (may be quite large for wide labels rotated 45 degrees)
+            const float xMin = std::min({tl.x, tr.x, bl.x, br.x});
+            const float xMax = std::max({tl.x, tr.x, bl.x, br.x});
+            const float yMin = std::min({tl.y, tr.y, bl.y, br.y});
+            const float yMax = std::max({tl.y, tr.y, bl.y, br.y});
+            
+            boxes.emplace_back(anchor.point, Point<float>{ 0, 0 }, xMin, yMin, xMax, yMax);
+        } else {
+            boxes.emplace_back(anchor.point, Point<float>{ 0, 0 }, x1, y1, x2, y2);
+        }
     }
 }
 
diff --git a/src/mbgl/text/collision_feature.hpp b/src/mbgl/text/collision_feature.hpp
index df1b12819c..5fbd8a5e4b 100644
--- a/src/mbgl/text/collision_feature.hpp
+++ b/src/mbgl/text/collision_feature.hpp
@@ -52,8 +52,9 @@ public:
                      const float padding,
                      const style::SymbolPlacementType placement,
                      const IndexedSubfeature& indexedFeature_,
-                     const float overscaling)
-        : CollisionFeature(line, anchor, shapedText.top, shapedText.bottom, shapedText.left, shapedText.right, boxScale, padding, placement, indexedFeature_, overscaling) {}
+                     const float overscaling,
+                     const float rotate)
+        : CollisionFeature(line, anchor, shapedText.top, shapedText.bottom, shapedText.left, shapedText.right, boxScale, padding, placement, indexedFeature_, overscaling, rotate) {}
 
     // for icons
     // Icons collision features are always SymbolPlacementType::Point, which means the collision feature
@@ -66,7 +67,8 @@ public:
                      optional<PositionedIcon> shapedIcon,
                      const float boxScale,
                      const float padding,
-                     const IndexedSubfeature& indexedFeature_)
+                     const IndexedSubfeature& indexedFeature_,
+                     const float rotate)
         : CollisionFeature(line, anchor,
                            (shapedIcon ? shapedIcon->top() : 0),
                            (shapedIcon ? shapedIcon->bottom() : 0),
@@ -75,7 +77,7 @@ public:
                            boxScale,
                            padding,
                            style::SymbolPlacementType::Point,
-                           indexedFeature_, 1) {}
+                           indexedFeature_, 1, rotate) {}
 
     CollisionFeature(const GeometryCoordinates& line,
                      const Anchor&,
@@ -87,7 +89,8 @@ public:
                      const float padding,
                      const style::SymbolPlacementType,
                      IndexedSubfeature,
-                     const float overscaling);
+                     const float overscaling,
+                     const float rotate);
 
     std::vector<CollisionBox> boxes;
     IndexedSubfeature indexedFeature;
diff --git a/test/text/cross_tile_symbol_index.test.cpp b/test/text/cross_tile_symbol_index.test.cpp
index 866e79729c..67f82413b2 100644
--- a/test/text/cross_tile_symbol_index.test.cpp
+++ b/test/text/cross_tile_symbol_index.test.cpp
@@ -11,7 +11,7 @@ SymbolInstance makeSymbolInstance(float x, float y, std::u16string key) {
     style::SymbolLayoutProperties::Evaluated layout_;
     IndexedSubfeature subfeature(0, "", "", 0);
     Anchor anchor(x, y, 0, 0);
-    return {anchor, line, shaping, {}, layout_, 0, 0, 0, style::SymbolPlacementType::Point, {{0, 0}}, 0, 0, {{0, 0}}, positions, subfeature, 0, 0, key, 0 };
+    return {anchor, line, shaping, {}, layout_, 0, 0, 0, style::SymbolPlacementType::Point, {{0, 0}}, 0, 0, {{0, 0}}, positions, subfeature, 0, 0, key, 0, 0};
 }