Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Commit

Permalink
[Core] Ensure queryRenderedFeatures accounts for icon-rotate (#13105)
Browse files Browse the repository at this point in the history
  • Loading branch information
@ryanhamley
ryanhamley authored Oct 18, 2018
1 parent 201db67 commit 0c6b162
Show file tree
Hide file tree
Showing 8 changed files with 63 additions and 32 deletions.
1 change: 0 additions & 1 deletion platform/node/test/ignores.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
4 changes: 4 additions & 0 deletions scripts/changelog_staging/queryrenderedfeatures-with-icon-rotate.json
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"core": "Fix bug in which using `icon-rotate` and `icon-offset` together prevented `queryRenderedFeatures` from returning the symbol",
"issue": 12909
}
7 changes: 4 additions & 3 deletions src/mbgl/layout/symbol_instance.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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_),
Expand Down
3 changes: 2 additions & 1 deletion src/mbgl/layout/symbol_instance.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ class SymbolInstance {
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;
Expand Down
39 changes: 20 additions & 19 deletions src/mbgl/layout/symbol_layout.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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];
Expand All @@ -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);
Expand All @@ -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;
Expand Down Expand Up @@ -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();
}

Expand All @@ -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;
Expand All @@ -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>();
Expand All @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
Expand All @@ -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(), {}, {});
Expand Down Expand Up @@ -520,15 +521,15 @@ 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);
buffer.dynamicVertices.emplace_back(dynamicVertex);
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);
Expand All @@ -543,7 +544,7 @@ size_t SymbolLayout::addSymbol(Buffer& buffer,
segment.indexLength += 6;

placedSymbol.glyphOffsets.push_back(symbol.glyphOffset.x);

return index;
}

Expand Down
26 changes: 24 additions & 2 deletions src/mbgl/text/collision_feature.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
}
}
}

Expand Down
13 changes: 8 additions & 5 deletions src/mbgl/text/collision_feature.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,9 @@ class CollisionFeature {
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
Expand All @@ -66,7 +67,8 @@ class CollisionFeature {
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),
Expand All @@ -75,7 +77,7 @@ class CollisionFeature {
boxScale,
padding,
style::SymbolPlacementType::Point,
indexedFeature_, 1) {}
indexedFeature_, 1, rotate) {}

CollisionFeature(const GeometryCoordinates& line,
const Anchor&,
Expand All @@ -87,7 +89,8 @@ class CollisionFeature {
const float padding,
const style::SymbolPlacementType,
IndexedSubfeature,
const float overscaling);
const float overscaling,
const float rotate);

std::vector<CollisionBox> boxes;
IndexedSubfeature indexedFeature;
Expand Down
2 changes: 1 addition & 1 deletion test/text/cross_tile_symbol_index.test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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};
}


Expand Down

0 comments on commit 0c6b162

Please sign in to comment.