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

Commit

Permalink
[core] fix icon-text-fit
Browse files Browse the repository at this point in the history
This fixes rendering by account for the 1px texture padding around icons that were stretched with icon-text-fit. We've added the 1px padding before, but didn't scale it accordingly when we are resizing the icon when it is stretched to fit the text. Adjusts the code to match the logic in GL JS.
  • Loading branch information
kkaefer committed Oct 14, 2019
1 parent aa43802 commit afedfcc
Show file tree
Hide file tree
Showing 7 changed files with 215 additions and 236 deletions.
2 changes: 1 addition & 1 deletion mapbox-gl-js
Submodule mapbox-gl-js updated 280 files
3 changes: 3 additions & 0 deletions platform/node/test/ignores.json
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,9 @@
"render-tests/fill-sort-key/literal": "https://github.com/mapbox/mapbox-gl-native/issues/15008",
"render-tests/line-sort-key/literal": "https://github.com/mapbox/mapbox-gl-native/issues/15008",
"render-tests/regressions/mapbox-gl-js#8817": "skip - https://github.com/mapbox/mapbox-gl-native/issues/15737",
"render-tests/text-max-width/zero-width-point-placement": "https://github.com/mapbox/mapbox-gl-native/issues/15648",
"render-tests/icon-image/image-expression": "https://github.com/mapbox/mapbox-gl-native/issues/15800",
"render-tests/icon-text-fit/text-variable-anchor-overlap": "https://github.com/mapbox/mapbox-gl-native/issues/15809",
"query-tests/fill-extrusion/base-in": "https://github.com/mapbox/mapbox-gl-native/issues/13139",
"query-tests/fill-extrusion/box-in": "https://github.com/mapbox/mapbox-gl-native/issues/13139",
"query-tests/fill-extrusion/side-in": "https://github.com/mapbox/mapbox-gl-native/issues/13139",
Expand Down
19 changes: 10 additions & 9 deletions src/mbgl/layout/symbol_layout.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ SymbolLayout::SymbolLayout(const BucketParameters& parameters,
allowVerticalPlacement = allowVerticalPlacement || placementMode == style::TextWritingModeType::Vertical;
return !seen.insert(placementMode).second;
});
modes.erase(end, modes.end());
modes.erase(end, modes.end());
placementModes = std::move(modes);
}

Expand Down Expand Up @@ -525,21 +525,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;

const auto iconTextFit = evaluatedLayoutProperties.get<style::IconTextFit>();
// Adjust shaped icon size when icon-text-fit is used.
optional<PositionedIcon> verticallyShapedIcon;
if (shapedIcon && hasIconTextFit) {
if (shapedIcon && iconTextFit != IconTextFitType::None) {
// Create vertically shaped icon for vertical writing mode if needed.
if (allowVerticalPlacement && shapedTextOrientations.vertical) {
verticallyShapedIcon = shapedIcon;
verticallyShapedIcon->fitIconToText(evaluatedLayoutProperties,
shapedTextOrientations.vertical,
layoutTextSize);
verticallyShapedIcon->fitIconToText(
shapedTextOrientations.vertical, iconTextFit, layout->get<IconTextFitPadding>(), iconOffset, fontScale);
}
const auto shapedText = getDefaultHorizontalShaping(shapedTextOrientations);
if (shapedText) {
shapedIcon->fitIconToText(
shapedText, iconTextFit, layout->get<IconTextFitPadding>(), iconOffset, fontScale);
}
shapedIcon->fitIconToText(evaluatedLayoutProperties,
getDefaultHorizontalShaping(shapedTextOrientations),
layoutTextSize);
}

auto addSymbolInstance = [&] (Anchor& anchor, std::shared_ptr<SymbolInstanceSharedData> sharedData) {
Expand Down
28 changes: 22 additions & 6 deletions src/mbgl/text/quads.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,28 @@ SymbolQuad getIconQuad(const PositionedIcon& shapedIcon,
// If you have a 10px icon that isn't perfectly aligned to the pixel grid it will cover 11 actual
// pixels. The quad needs to be padded to account for this, otherwise they'll look slightly clipped
// on one edge in some cases.
const float border = 1.0;

float top = shapedIcon.top() - border / image.pixelRatio;
float left = shapedIcon.left() - border / image.pixelRatio;
float bottom = shapedIcon.bottom() + border / image.pixelRatio;
float right = shapedIcon.right() + border / image.pixelRatio;
constexpr const float border = 1.0f;

// Expand the box to respect the 1 pixel border in the atlas image. We're using `image.paddedRect - border`
// instead of image.displaySize because we only pad with one pixel for retina images as well, and the
// displaySize uses the logical dimensions, not the physical pixel dimensions.
// Unlike the JavaScript version, we're _not_ including the padding in the texture rect, so the
// logic "dimension * padded / non-padded - dimension" is swapped.
const float iconWidth = shapedIcon.right() - shapedIcon.left();
const float expandX = (iconWidth * (static_cast<float>(image.textureRect.w) + 2.0f * border) /
static_cast<float>(image.textureRect.w) -
iconWidth) /
2.0f;
const float left = shapedIcon.left() - expandX;
const float right = shapedIcon.right() + expandX;

const float iconHeight = shapedIcon.bottom() - shapedIcon.top();
const float expandY = (iconHeight * (static_cast<float>(image.textureRect.h) + 2.0f * border) /
static_cast<float>(image.textureRect.h) -
iconHeight) /
2.0f;
const float top = shapedIcon.top() - expandY;
const float bottom = shapedIcon.bottom() + expandY;

Point<float> tl{left, top};
Point<float> tr{right, top};
Expand Down
62 changes: 35 additions & 27 deletions src/mbgl/text/shaping.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -83,33 +83,41 @@ PositionedIcon PositionedIcon::shapeIcon(const ImagePosition& image,
return PositionedIcon { image, top, bottom, left, right, iconRotation };
}

void PositionedIcon::fitIconToText(const style::SymbolLayoutProperties::Evaluated& layout,
const Shaping& shapedText,
float layoutTextSize) {
using namespace style;
assert(layout.get<IconTextFit>() != IconTextFitType::None);
if (shapedText) {
auto iconWidth = _right - _left;
auto iconHeight = _bottom - _top;
auto size = layoutTextSize / 24.0f;
auto textLeft = shapedText.left * size;
auto textRight = shapedText.right * size;
auto textTop = shapedText.top * size;
auto textBottom = shapedText.bottom * size;
auto textWidth = textRight - textLeft;
auto textHeight = textBottom - textTop;
auto padT = layout.get<IconTextFitPadding>()[0];
auto padR = layout.get<IconTextFitPadding>()[1];
auto padB = layout.get<IconTextFitPadding>()[2];
auto padL = layout.get<IconTextFitPadding>()[3];
auto offsetY = layout.get<IconTextFit>() == IconTextFitType::Width ? (textHeight - iconHeight) * 0.5 : 0;
auto offsetX = layout.get<IconTextFit>() == IconTextFitType::Height ? (textWidth - iconWidth) * 0.5 : 0;
auto width = layout.get<IconTextFit>() == IconTextFitType::Width || layout.get<IconTextFit>() == IconTextFitType::Both ? textWidth : iconWidth;
auto height = layout.get<IconTextFit>() == IconTextFitType::Height || layout.get<IconTextFit>() == IconTextFitType::Both ? textHeight : iconHeight;
_left = textLeft + offsetX - padL;
_top = textTop + offsetY - padT;
_right = textLeft + offsetX + padR + width;
_bottom = textTop + offsetY + padB + height;
void PositionedIcon::fitIconToText(const Shaping& shapedText,
const style::IconTextFitType textFit,
const std::array<float, 4>& padding,
const std::array<float, 2>& iconOffset,
const float fontScale) {
assert(textFit != style::IconTextFitType::None);
assert(shapedText);

// We don't respect the icon-anchor, because icon-text-fit is set. Instead,
// the icon will be centered on the text, then stretched in the given
// dimensions.

const float textLeft = shapedText.left * fontScale;
const float textRight = shapedText.right * fontScale;

if (textFit == style::IconTextFitType::Width || textFit == style::IconTextFitType::Both) {
// Stretched horizontally to the text width
_left = iconOffset[0] + textLeft - padding[3];
_right = iconOffset[0] + textRight + padding[1];
} else {
// Centered on the text
_left = iconOffset[0] + (textLeft + textRight - image().displaySize()[0]) / 2.0f;
_right = _left + image().displaySize()[0];
}

const float textTop = shapedText.top * fontScale;
const float textBottom = shapedText.bottom * fontScale;
if (textFit == style::IconTextFitType::Height || textFit == style::IconTextFitType::Both) {
// Stretched vertically to the text height
_top = iconOffset[1] + textTop - padding[0];
_bottom = iconOffset[1] + textBottom + padding[2];
} else {
// Centered on the text
_top = iconOffset[1] + (textTop + textBottom - image().displaySize()[1]) / 2.0f;
_bottom = _top + image().displaySize()[1];
}
}

Expand Down
8 changes: 5 additions & 3 deletions src/mbgl/text/shaping.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,11 @@ class PositionedIcon {

// Updates shaped icon's bounds based on shaped text's bounds and provided
// layout properties.
void fitIconToText(const style::SymbolLayoutProperties::Evaluated& layout,
const Shaping& shapedText,
float layoutTextSize);
void fitIconToText(const Shaping& shapedText,
const style::IconTextFitType textFit,
const std::array<float, 4>& padding,
const std::array<float, 2>& iconOffset,
const float fontScale);

const ImagePosition& image() const { return _image; }
float top() const { return _top; }
Expand Down
Loading

0 comments on commit afedfcc

Please sign in to comment.