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

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

Merged
merged 3 commits into from
Aug 20, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions platform/android/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
5 changes: 5 additions & 0 deletions platform/ios/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions platform/macos/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
1 change: 1 addition & 0 deletions platform/node/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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))
Expand Down
14 changes: 1 addition & 13 deletions platform/node/test/ignores.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
21 changes: 19 additions & 2 deletions src/mbgl/layout/symbol_instance.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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();
}
Expand Down
7 changes: 6 additions & 1 deletion src/mbgl/layout/symbol_instance.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -39,6 +39,7 @@ struct SymbolInstanceSharedData {
SymbolQuads leftJustifiedGlyphQuads;
SymbolQuads verticalGlyphQuads;
optional<SymbolQuad> iconQuad;
optional<SymbolQuad> verticalIconQuad;
};

class SymbolInstance {
Expand All @@ -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,
Expand All @@ -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:
Expand All @@ -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
Expand All @@ -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;
Expand Down
73 changes: 52 additions & 21 deletions src/mbgl/layout/symbol_layout.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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;
Expand Down Expand Up @@ -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);
Expand All @@ -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,
Expand All @@ -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);
};

Expand Down Expand Up @@ -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) {
Expand All @@ -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();
}

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -837,6 +865,9 @@ void SymbolLayout::addToDebugBuffers(SymbolBucket& bucket) {
if (symbolInstance.verticalTextCollisionFeature) {
populateCollisionBox(*symbolInstance.verticalTextCollisionFeature);
}
if (symbolInstance.verticalIconCollisionFeature) {
populateCollisionBox(*symbolInstance.verticalIconCollisionFeature);
}
populateCollisionBox(symbolInstance.iconCollisionFeature);
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/mbgl/layout/symbol_layout.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
Loading