diff options
-rw-r--r-- | platform/node/test/ignores.json | 1 | ||||
-rw-r--r-- | scripts/changelog_staging/queryrenderedfeatures-with-icon-rotate.json | 4 | ||||
-rw-r--r-- | src/mbgl/layout/symbol_instance.cpp | 7 | ||||
-rw-r--r-- | src/mbgl/layout/symbol_instance.hpp | 3 | ||||
-rw-r--r-- | src/mbgl/layout/symbol_layout.cpp | 39 | ||||
-rw-r--r-- | src/mbgl/text/collision_feature.cpp | 26 | ||||
-rw-r--r-- | src/mbgl/text/collision_feature.hpp | 13 | ||||
-rw-r--r-- | test/text/cross_tile_symbol_index.test.cpp | 2 |
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}; } |