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

Fix icon-text-fit #15634

Merged
merged 3 commits into from
Oct 14, 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
3 changes: 2 additions & 1 deletion expression-test/expression_test_parser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,8 @@ optional<Value> toValue(const JSValue& jsvalue) {

style::expression::type::Type stringToType(const std::string& type) {
using namespace style::expression;
if (type == "string"s || type == "number-format"s) {
if (type == "string"s || type == "number-format"s ||
type == "image"s) { // TODO: replace once we implement image expressions
return type::String;
} else if (type == "number"s) {
return type::Number;
Expand Down
2 changes: 1 addition & 1 deletion mapbox-gl-js
Submodule mapbox-gl-js updated 280 files
6 changes: 6 additions & 0 deletions platform/android/scripts/generate-style-code.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ global.propertyType = function propertyType(property) {
case 'formatted':
return 'Formatted';
case 'string':
case 'image': // TODO: replace once we implement image expressions
return 'String';
case 'enum':
return 'String';
Expand All @@ -93,6 +94,7 @@ global.propertyJavaType = function propertyType(property) {
case 'formatted':
return 'Formatted';
case 'string':
case 'image': // TODO: replace once we implement image expressions
return 'String';
case 'enum':
return 'String';
Expand Down Expand Up @@ -141,6 +143,7 @@ global.propertyNativeType = function (property) {
return 'float';
case 'formatted':
case 'string':
case 'image': // TODO: replace once we implement image expressions
return 'std::string';
case 'enum':
if(property['light-property']){
Expand Down Expand Up @@ -177,6 +180,7 @@ global.defaultExpressionJava = function(property) {
case 'formatted':
return 'format';
case 'string':
case 'image': // TODO: replace once we implement image expressions
return "string";
case 'enum':
return "string";
Expand All @@ -203,6 +207,7 @@ global.defaultValueJava = function(property) {
case 'formatted':
return 'new Formatted(new FormattedSection("default"))'
case 'string':
case 'image': // TODO: replace once we implement image expressions
return '"' + property['default'] + '"';
case 'enum':
return snakeCaseUpper(property.name) + "_" + snakeCaseUpper(Object.keys(property.values)[0]);
Expand Down Expand Up @@ -335,6 +340,7 @@ global.evaluatedType = function (property) {
return 'float';
case 'formatted':
case 'string':
case 'image': // TODO: replace once we implement image expressions
return 'std::string';
case 'enum':
return (isLightProperty(property) ? 'Light' : '') + `${camelize(property.name)}Type`;
Expand Down
9 changes: 9 additions & 0 deletions platform/darwin/scripts/generate-style-code.js
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,7 @@ global.objCTestValue = function (property, layerType, arraysAsStructs, indent) {
`@"'${_.startCase(propertyName)}'"` :
`@"${_.startCase(propertyName)}"`;
case 'string':
case 'image': // TODO: replace once we implement image expressions
return `@"'${_.startCase(propertyName)}'"`;
case 'enum':
return `@"'${_.last(_.keys(property.values))}'"`;
Expand Down Expand Up @@ -208,6 +209,7 @@ global.mbglTestValue = function (property, layerType) {
return '1.0';
case 'formatted':
case 'string':
case 'image': // TODO: replace once we implement image expressions
return `"${_.startCase(propertyName)}"`;
case 'enum': {
let type = camelize(originalPropertyName(property));
Expand Down Expand Up @@ -294,6 +296,7 @@ global.testHelperMessage = function (property, layerType, isFunction) {
return 'testNumber' + fnSuffix;
case 'formatted':
case 'string':
case 'image': // TODO: replace once we implement image expressions
return 'testString' + fnSuffix;
case 'enum':
let objCType = global.objCType(layerType, property.name);
Expand Down Expand Up @@ -474,6 +477,7 @@ global.describeType = function (property) {
return 'numeric';
case 'formatted':
case 'string':
case 'image': // TODO: replace once we implement image expressions
return 'string';
case 'enum':
return '`MGL' + camelize(property.name) + '`';
Expand Down Expand Up @@ -522,6 +526,7 @@ global.describeValue = function (value, property, layerType) {
return 'the float ' + '`' + formatNumber(value) + '`';
case 'formatted':
case 'string':
case 'image': // TODO: replace once we implement image expressions
if (value === '') {
return 'the empty string';
}
Expand Down Expand Up @@ -608,6 +613,7 @@ global.propertyType = function (property) {
return 'NSNumber *';
case 'formatted':
case 'string':
case 'image': // TODO: replace once we implement image expressions
return 'NSString *';
case 'enum':
return 'NSValue *';
Expand Down Expand Up @@ -640,6 +646,7 @@ global.isInterpolatable = function (property) {
return type !== 'boolean' &&
type !== 'enum' &&
type !== 'string' &&
type !== 'image' &&
type !== 'formatted';
};

Expand All @@ -653,6 +660,7 @@ global.valueTransformerArguments = function (property) {
case 'formatted':
return ['mbgl::style::expression::Formatted', objCType];
case 'string':
case 'image': // TODO: replace once we implement image expressions
return ['std::string', objCType];
case 'enum':
return [mbglType(property), 'NSValue *', mbglType(property), `MGL${camelize(property.name)}`];
Expand Down Expand Up @@ -692,6 +700,7 @@ global.mbglType = function(property) {
case 'formatted':
return 'mbgl::style::expression::Formatted';
case 'string':
case 'image': // TODO: replace once we implement image expressions
return 'std::string';
case 'enum': {
let type = camelize(originalPropertyName(property));
Expand Down
7 changes: 7 additions & 0 deletions platform/node/test/ignores.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@
"expression-tests/legacy/interval/composite": "https://github.com/mapbox/mapbox-gl-native/issues/12747",
"expression-tests/legacy/interval/composite-default": "https://github.com/mapbox/mapbox-gl-native/issues/12747",
"expression-tests/legacy/interval/tokens-zoom": "https://github.com/mapbox/mapbox-gl-native/issues/12747",
"expression-tests/image/basic": "https://github.com/mapbox/mapbox-gl-native/issues/15800",
"expression-tests/image/compound": "https://github.com/mapbox/mapbox-gl-native/issues/15800",
"expression-tests/image/coalesce": "https://github.com/mapbox/mapbox-gl-native/issues/15800",
"expression-tests/image/implicit-assert": "https://github.com/mapbox/mapbox-gl-native/issues/15800",
"query-tests/geometry/multilinestring": "needs investigation",
"query-tests/geometry/multipolygon": "needs investigation",
"query-tests/geometry/polygon": "needs investigation",
Expand Down Expand Up @@ -72,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
3 changes: 3 additions & 0 deletions scripts/generate-style-code.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ global.expressionType = function (property) {
case 'enum':
return 'NumberType';
case 'string':
case 'image': // TODO: replace once we implement image expressions
return 'StringType';
case 'color':
return `ColorType`;
Expand Down Expand Up @@ -68,6 +69,7 @@ global.evaluatedType = function (property) {
case 'formatted':
return 'expression::Formatted';
case 'string':
case 'image': // TODO: replace once we implement image expressions
return 'std::string';
case 'enum':
return (isLightProperty(property) ? 'Light' : '') + `${camelize(property.name)}Type`;
Expand Down Expand Up @@ -166,6 +168,7 @@ global.defaultValue = function (property) {
}
case 'formatted':
case 'string':
case 'image': // TODO: replace once we implement image expressions
return JSON.stringify(property.default || "");
case 'enum':
if (property.default === undefined) {
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
6 changes: 3 additions & 3 deletions src/mbgl/style/expression/assertion.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,12 @@ Assertion::Assertion(type::Type type_, std::vector<std::unique_ptr<Expression>>
}

ParseResult Assertion::parse(const Convertible& value, ParsingContext& ctx) {
static std::unordered_map<std::string, type::Type> types {
static std::unordered_map<std::string, type::Type> types{
{"string", type::String},
{"image", type::String}, // TODO: replace once we implement image expressions
{"number", type::Number},
{"boolean", type::Boolean},
{"object", type::Object}
};
{"object", type::Object}};

std::size_t length = arrayLength(value);

Expand Down
66 changes: 34 additions & 32 deletions src/mbgl/style/expression/parsing_context.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -100,38 +100,40 @@ ParseResult ParsingContext::parse(const Convertible& value, std::size_t index_,
}

using ParseFunction = ParseResult (*)(const conversion::Convertible&, ParsingContext&);
MAPBOX_ETERNAL_CONSTEXPR const auto expressionRegistry = mapbox::eternal::hash_map<mapbox::eternal::string, ParseFunction>({
{"==", parseComparison},
{"!=", parseComparison},
{">", parseComparison},
{"<", parseComparison},
{">=", parseComparison},
{"<=", parseComparison},
{"all", All::parse},
{"any", Any::parse},
{"array", Assertion::parse},
{"at", At::parse},
{"boolean", Assertion::parse},
{"case", Case::parse},
{"coalesce", Coalesce::parse},
{"collator", CollatorExpression::parse},
{"format", FormatExpression::parse},
{"interpolate", parseInterpolate},
{"length", Length::parse},
{"let", Let::parse},
{"literal", Literal::parse},
{"match", parseMatch},
{"number", Assertion::parse},
{"number-format", NumberFormat::parse},
{"object", Assertion::parse},
{"step", Step::parse},
{"string", Assertion::parse},
{"to-boolean", Coercion::parse},
{"to-color", Coercion::parse},
{"to-number", Coercion::parse},
{"to-string", Coercion::parse},
{"var", Var::parse},
});
MAPBOX_ETERNAL_CONSTEXPR const auto expressionRegistry =
mapbox::eternal::hash_map<mapbox::eternal::string, ParseFunction>({
{"==", parseComparison},
{"!=", parseComparison},
{">", parseComparison},
{"<", parseComparison},
{">=", parseComparison},
{"<=", parseComparison},
{"all", All::parse},
{"any", Any::parse},
{"array", Assertion::parse},
{"at", At::parse},
{"boolean", Assertion::parse},
{"case", Case::parse},
{"coalesce", Coalesce::parse},
{"collator", CollatorExpression::parse},
{"format", FormatExpression::parse},
{"image", Assertion::parse}, // TODO: replace once we implement image expressions
{"interpolate", parseInterpolate},
{"length", Length::parse},
{"let", Let::parse},
{"literal", Literal::parse},
{"match", parseMatch},
{"number", Assertion::parse},
{"number-format", NumberFormat::parse},
{"object", Assertion::parse},
{"step", Step::parse},
{"string", Assertion::parse},
{"to-boolean", Coercion::parse},
{"to-color", Coercion::parse},
{"to-number", Coercion::parse},
{"to-string", Coercion::parse},
{"var", Var::parse},
});

bool isExpression(const std::string& name) {
return expressionRegistry.contains(name.c_str());
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
Loading