Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable data-driven values for text-font #5698

Merged
merged 2 commits into from
Nov 22, 2017
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
9 changes: 6 additions & 3 deletions src/data/bucket/symbol_bucket.js
Original file line number Diff line number Diff line change
Expand Up @@ -486,10 +486,12 @@ class SymbolBucket implements Bucket {
const layer = this.layers[0];
const layout = layer.layout;

const textFont = layout.get('text-font').join(',');
const textFont = layout.get('text-font');
const textField = layout.get('text-field');
const iconImage = layout.get('icon-image');
const hasText = textField.value.kind !== 'constant' || textField.value.value.length > 0 && textFont.length > 0;
const hasText =
(textField.value.kind !== 'constant' || textField.value.value.length > 0) &&
(textFont.value.kind !== 'constant' || textFont.value.value.length > 0);
const hasIcon = iconImage.value.kind !== 'constant' || iconImage.value.value && iconImage.value.value.length > 0;

this.features = [];
Expand All @@ -500,7 +502,6 @@ class SymbolBucket implements Bucket {

const icons = options.iconDependencies;
const stacks = options.glyphDependencies;
const stack = stacks[textFont] = stacks[textFont] || {};
const globalProperties = {zoom: this.zoom};

for (const {feature, index, sourceLayerIndex} of features) {
Expand Down Expand Up @@ -542,6 +543,8 @@ class SymbolBucket implements Bucket {
}

if (text) {
const fontStack = textFont.evaluate(feature).join(',');
const stack = stacks[fontStack] = stacks[fontStack] || {};
const textAlongLine = layout.get('text-rotation-alignment') === 'map' && layout.get('symbol-placement') === 'line';
const allowsVerticalWritingMode = scriptDetection.allowsVerticalWritingMode(text);
for (let i = 0; i < text.length; i++) {
Expand Down
4 changes: 4 additions & 0 deletions src/style-spec/expression/compound_expression.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@ class CompoundExpression implements Expression {
this.args.forEach(fn);
}

possibleOutputs() {
return [undefined];
}

static parse(args: Array<mixed>, context: ParsingContext): ?Expression {
const op: string = (args[0]: any);
const definition = CompoundExpression.definitions[op];
Expand Down
4 changes: 4 additions & 0 deletions src/style-spec/expression/definitions/array.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,10 @@ class ArrayAssertion implements Expression {
eachChild(fn: (Expression) => void) {
fn(this.input);
}

possibleOutputs() {
return this.input.possibleOutputs();
}
}

module.exports = ArrayAssertion;
4 changes: 4 additions & 0 deletions src/style-spec/expression/definitions/assertion.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,10 @@ class Assertion implements Expression {
eachChild(fn: (Expression) => void) {
this.args.forEach(fn);
}

possibleOutputs() {
return [].concat(...this.args.map((arg) => arg.possibleOutputs()));
}
}

module.exports = Assertion;
4 changes: 4 additions & 0 deletions src/style-spec/expression/definitions/at.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,10 @@ class At implements Expression {
fn(this.index);
fn(this.input);
}

possibleOutputs() {
return [undefined];
}
}

module.exports = At;
6 changes: 6 additions & 0 deletions src/style-spec/expression/definitions/case.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,12 @@ class Case implements Expression {
}
fn(this.otherwise);
}

possibleOutputs() {
return []
.concat(...this.branches.map(([_, out]) => out.possibleOutputs()))
.concat(this.otherwise.possibleOutputs());
}
}

module.exports = Case;
4 changes: 4 additions & 0 deletions src/style-spec/expression/definitions/coalesce.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,10 @@ class Coalesce implements Expression {
eachChild(fn: (Expression) => void) {
this.args.forEach(fn);
}

possibleOutputs() {
return [].concat(...this.args.map((arg) => arg.possibleOutputs()));
}
}

module.exports = Coalesce;
4 changes: 4 additions & 0 deletions src/style-spec/expression/definitions/coercion.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,10 @@ class Coercion implements Expression {
eachChild(fn: (Expression) => void) {
this.args.forEach(fn);
}

possibleOutputs() {
return [].concat(...this.args.map((arg) => arg.possibleOutputs()));
}
}

module.exports = Coercion;
4 changes: 4 additions & 0 deletions src/style-spec/expression/definitions/interpolate.js
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,10 @@ class Interpolate implements Expression {
fn(expression);
}
}

possibleOutputs() {
return [].concat(...this.outputs.map((output) => output.possibleOutputs()));
}
}

/**
Expand Down
4 changes: 4 additions & 0 deletions src/style-spec/expression/definitions/let.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,10 @@ class Let implements Expression {

return new Let(bindings, result);
}

possibleOutputs() {
return this.result.possibleOutputs();
}
}

module.exports = Let;
4 changes: 4 additions & 0 deletions src/style-spec/expression/definitions/literal.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,10 @@ class Literal implements Expression {
}

eachChild() {}

possibleOutputs() {
return [this.value];
}
}

module.exports = Literal;
6 changes: 6 additions & 0 deletions src/style-spec/expression/definitions/match.js
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,12 @@ class Match implements Expression {
this.outputs.forEach(fn);
fn(this.otherwise);
}

possibleOutputs() {
return []
.concat(...this.outputs.map((out) => out.possibleOutputs()))
.concat(this.otherwise.possibleOutputs());
}
}

module.exports = Match;
4 changes: 4 additions & 0 deletions src/style-spec/expression/definitions/step.js
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,10 @@ class Step implements Expression {
fn(expression);
}
}

possibleOutputs() {
return [].concat(...this.outputs.map((output) => output.possibleOutputs()));
}
}

module.exports = Step;
4 changes: 4 additions & 0 deletions src/style-spec/expression/definitions/var.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@ class Var implements Expression {
}

eachChild() {}

possibleOutputs() {
return [undefined];
}
}

module.exports = Var;
8 changes: 8 additions & 0 deletions src/style-spec/expression/expression.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// @flow

import type {Type} from './types';
import type {Value} from './values';
import type ParsingContext from './parsing_context';
import type EvaluationContext from './evaluation_context';

Expand All @@ -11,4 +12,11 @@ export interface Expression {
evaluate(ctx: EvaluationContext): any;

eachChild(fn: Expression => void): void;

/**
* Statically analyze the expression, attempting to enumerate possible outputs. Returns
* an array of values plus the sentinel value `undefined`, used to indicate that the
* complete set of outputs is statically undecidable.
*/
possibleOutputs(): Array<Value | void>;
}
1 change: 1 addition & 0 deletions src/style-spec/reference/v8.json
Original file line number Diff line number Diff line change
Expand Up @@ -1311,6 +1311,7 @@
"value": "string",
"function": "piecewise-constant",
"zoom-function": true,
"property-function": true,
"default": ["Open Sans Regular", "Arial Unicode MS Regular"],
"doc": "Font stack to use for displaying text.",
"requires": [
Expand Down
15 changes: 10 additions & 5 deletions src/style-spec/validate/validate_expression.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,16 @@ const unbundle = require('../util/unbundle_jsonlint');

module.exports = function validateExpression(options: any) {
const expression = (options.expressionContext === 'property' ? createPropertyExpression : createExpression)(unbundle.deep(options.value), options.valueSpec);
if (expression.result !== 'error') {
return [];
if (expression.result === 'error') {
return expression.value.map((error) => {
return new ValidationError(`${options.key}${error.key}`, options.value, error.message);
});
}

return expression.value.map((error) => {
return new ValidationError(`${options.key}${error.key}`, options.value, error.message);
});
if (options.expressionContext === 'property' && options.propertyKey === 'text-font' &&
(expression.value: any).parsed.possibleOutputs().indexOf(undefined) !== -1) {
return [new ValidationError(options.key, options.value, 'Invalid data expression for "text-font". Output values must be contained as literals within the expression.')];
}

return [];
};
8 changes: 7 additions & 1 deletion src/style-spec/validate/validate_property.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
const validate = require('./validate');
const ValidationError = require('../error/validation_error');
const getType = require('../util/get_type');
const {isFunction} = require('../function');
const unbundle = require('../util/unbundle_jsonlint');

module.exports = function validateProperty(options, propertyType) {
const key = options.key;
Expand Down Expand Up @@ -45,6 +47,9 @@ module.exports = function validateProperty(options, propertyType) {
if (propertyKey === 'text-field' && style && !style.glyphs) {
errors.push(new ValidationError(key, value, 'use of "text-field" requires a style "glyphs" property'));
}
if (propertyKey === 'text-font' && isFunction(unbundle.deep(value)) && unbundle(value.type) === 'identity') {
errors.push(new ValidationError(key, value, '"text-font" does not support identity functions'));
}
}

return errors.concat(validate({
Expand All @@ -53,6 +58,7 @@ module.exports = function validateProperty(options, propertyType) {
valueSpec: valueSpec,
style: style,
styleSpec: styleSpec,
expressionContext: 'property'
expressionContext: 'property',
propertyKey
}));
};
4 changes: 2 additions & 2 deletions src/style/style_layer/symbol_style_layer_properties.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export type LayoutProps = {|
"text-pitch-alignment": DataConstantProperty<"map" | "viewport" | "auto">,
"text-rotation-alignment": DataConstantProperty<"map" | "viewport" | "auto">,
"text-field": DataDrivenProperty<string>,
"text-font": DataConstantProperty<Array<string>>,
"text-font": DataDrivenProperty<Array<string>>,
"text-size": DataDrivenProperty<number>,
"text-max-width": DataDrivenProperty<number>,
"text-line-height": DataConstantProperty<number>,
Expand Down Expand Up @@ -74,7 +74,7 @@ const layout: Properties<LayoutProps> = new Properties({
"text-pitch-alignment": new DataConstantProperty(styleSpec["layout_symbol"]["text-pitch-alignment"]),
"text-rotation-alignment": new DataConstantProperty(styleSpec["layout_symbol"]["text-rotation-alignment"]),
"text-field": new DataDrivenProperty(styleSpec["layout_symbol"]["text-field"]),
"text-font": new DataConstantProperty(styleSpec["layout_symbol"]["text-font"]),
"text-font": new DataDrivenProperty(styleSpec["layout_symbol"]["text-font"]),
"text-size": new DataDrivenProperty(styleSpec["layout_symbol"]["text-size"]),
"text-max-width": new DataDrivenProperty(styleSpec["layout_symbol"]["text-max-width"]),
"text-line-height": new DataConstantProperty(styleSpec["layout_symbol"]["text-line-height"]),
Expand Down
6 changes: 3 additions & 3 deletions src/symbol/symbol_layout.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,14 +46,14 @@ function performSymbolLayout(bucket: SymbolBucket,

const oneEm = 24;
const lineHeight = layout.get('text-line-height') * oneEm;
const fontstack = layout.get('text-font').join(',');
const textAlongLine = layout.get('text-rotation-alignment') === 'map' && layout.get('symbol-placement') === 'line';
const keepUpright = layout.get('text-keep-upright');

const glyphs = glyphMap[fontstack] || {};
const glyphPositionMap = glyphPositions[fontstack] || {};

for (const feature of bucket.features) {
const fontstack = layout.get('text-font').evaluate(feature).join(',');
const glyphs = glyphMap[fontstack] || {};
const glyphPositionMap = glyphPositions[fontstack] || {};

const shapedTextOrientations = {};
const text = feature.text;
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
73 changes: 73 additions & 0 deletions test/integration/render-tests/text-font/data-expression/style.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
{
"version": 8,
"metadata": {
"test": {
"height": 64,
"width": 64
}
},
"center": [ 0, 0 ],
"zoom": 0,
"sources": {
"point": {
"type": "geojson",
"data": {
"type": "FeatureCollection",
"features": [
{
"type": "Feature",
"properties": {
"text-font": "default"
},
"geometry": {
"type": "Point",
"coordinates": [ 0, -10 ]
}
},
{
"type": "Feature",
"properties": {
"text-font": "noto"
},
"geometry": {
"type": "Point",
"coordinates": [ 0, 10 ]
}
}
]
}
}
},
"glyphs": "local://glyphs/{fontstack}/{range}.pbf",
"layers": [
{
"id": "text",
"type": "symbol",
"source": "point",
"layout": {
"text-field": "A",
"text-font": [
"match",
[
"get",
"text-font"
],
"noto",
[
"literal",
[
"NotoCJK"
]
],
[
"literal",
[
"Open Sans Semibold",
"Arial Unicode MS Bold"
]
]
]
}
}
]
}
Loading