diff --git a/src/data/bucket/symbol_bucket.js b/src/data/bucket/symbol_bucket.js index 7a11f4ecea5..620b250a290 100644 --- a/src/data/bucket/symbol_bucket.js +++ b/src/data/bucket/symbol_bucket.js @@ -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 = []; @@ -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) { @@ -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++) { diff --git a/src/style-spec/expression/compound_expression.js b/src/style-spec/expression/compound_expression.js index c5854aac4bf..d72ee5ee217 100644 --- a/src/style-spec/expression/compound_expression.js +++ b/src/style-spec/expression/compound_expression.js @@ -38,6 +38,10 @@ class CompoundExpression implements Expression { this.args.forEach(fn); } + possibleOutputs() { + return [undefined]; + } + static parse(args: Array, context: ParsingContext): ?Expression { const op: string = (args[0]: any); const definition = CompoundExpression.definitions[op]; diff --git a/src/style-spec/expression/definitions/array.js b/src/style-spec/expression/definitions/array.js index d67e238fc54..85b7ff683da 100644 --- a/src/style-spec/expression/definitions/array.js +++ b/src/style-spec/expression/definitions/array.js @@ -79,6 +79,10 @@ class ArrayAssertion implements Expression { eachChild(fn: (Expression) => void) { fn(this.input); } + + possibleOutputs() { + return this.input.possibleOutputs(); + } } module.exports = ArrayAssertion; diff --git a/src/style-spec/expression/definitions/assertion.js b/src/style-spec/expression/definitions/assertion.js index d476953fe2c..0ed23b13604 100644 --- a/src/style-spec/expression/definitions/assertion.js +++ b/src/style-spec/expression/definitions/assertion.js @@ -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; diff --git a/src/style-spec/expression/definitions/at.js b/src/style-spec/expression/definitions/at.js index e4462d3f7fd..306eaee83bf 100644 --- a/src/style-spec/expression/definitions/at.js +++ b/src/style-spec/expression/definitions/at.js @@ -57,6 +57,10 @@ class At implements Expression { fn(this.index); fn(this.input); } + + possibleOutputs() { + return [undefined]; + } } module.exports = At; diff --git a/src/style-spec/expression/definitions/case.js b/src/style-spec/expression/definitions/case.js index 941d6367642..aef77cd6a53 100644 --- a/src/style-spec/expression/definitions/case.js +++ b/src/style-spec/expression/definitions/case.js @@ -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; diff --git a/src/style-spec/expression/definitions/coalesce.js b/src/style-spec/expression/definitions/coalesce.js index d19ea79d918..5d21091175c 100644 --- a/src/style-spec/expression/definitions/coalesce.js +++ b/src/style-spec/expression/definitions/coalesce.js @@ -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; diff --git a/src/style-spec/expression/definitions/coercion.js b/src/style-spec/expression/definitions/coercion.js index 3f570e83e0b..d1d072f2458 100644 --- a/src/style-spec/expression/definitions/coercion.js +++ b/src/style-spec/expression/definitions/coercion.js @@ -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; diff --git a/src/style-spec/expression/definitions/interpolate.js b/src/style-spec/expression/definitions/interpolate.js index 5e10d5c9ab2..62912ff3a40 100644 --- a/src/style-spec/expression/definitions/interpolate.js +++ b/src/style-spec/expression/definitions/interpolate.js @@ -173,6 +173,10 @@ class Interpolate implements Expression { fn(expression); } } + + possibleOutputs() { + return [].concat(...this.outputs.map((output) => output.possibleOutputs())); + } } /** diff --git a/src/style-spec/expression/definitions/let.js b/src/style-spec/expression/definitions/let.js index 834098f41d5..b5cf9048007 100644 --- a/src/style-spec/expression/definitions/let.js +++ b/src/style-spec/expression/definitions/let.js @@ -57,6 +57,10 @@ class Let implements Expression { return new Let(bindings, result); } + + possibleOutputs() { + return this.result.possibleOutputs(); + } } module.exports = Let; diff --git a/src/style-spec/expression/definitions/literal.js b/src/style-spec/expression/definitions/literal.js index a3d2b6dc805..4dfa62a0b4f 100644 --- a/src/style-spec/expression/definitions/literal.js +++ b/src/style-spec/expression/definitions/literal.js @@ -46,6 +46,10 @@ class Literal implements Expression { } eachChild() {} + + possibleOutputs() { + return [this.value]; + } } module.exports = Literal; diff --git a/src/style-spec/expression/definitions/match.js b/src/style-spec/expression/definitions/match.js index 0a195188888..13e2adbf2f8 100644 --- a/src/style-spec/expression/definitions/match.js +++ b/src/style-spec/expression/definitions/match.js @@ -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; diff --git a/src/style-spec/expression/definitions/step.js b/src/style-spec/expression/definitions/step.js index 60699a581dd..bc7cd36cbaa 100644 --- a/src/style-spec/expression/definitions/step.js +++ b/src/style-spec/expression/definitions/step.js @@ -103,6 +103,10 @@ class Step implements Expression { fn(expression); } } + + possibleOutputs() { + return [].concat(...this.outputs.map((output) => output.possibleOutputs())); + } } module.exports = Step; diff --git a/src/style-spec/expression/definitions/var.js b/src/style-spec/expression/definitions/var.js index e52b335aae8..cf4e1c70395 100644 --- a/src/style-spec/expression/definitions/var.js +++ b/src/style-spec/expression/definitions/var.js @@ -31,6 +31,10 @@ class Var implements Expression { } eachChild() {} + + possibleOutputs() { + return [undefined]; + } } module.exports = Var; diff --git a/src/style-spec/expression/expression.js b/src/style-spec/expression/expression.js index ae96e293f2b..6a466cff9fd 100644 --- a/src/style-spec/expression/expression.js +++ b/src/style-spec/expression/expression.js @@ -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'; @@ -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; } diff --git a/src/style-spec/reference/v8.json b/src/style-spec/reference/v8.json index 8d1171048fe..c480565dba6 100644 --- a/src/style-spec/reference/v8.json +++ b/src/style-spec/reference/v8.json @@ -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": [ diff --git a/src/style-spec/validate/validate_expression.js b/src/style-spec/validate/validate_expression.js index 47b27d90430..37a766bfbae 100644 --- a/src/style-spec/validate/validate_expression.js +++ b/src/style-spec/validate/validate_expression.js @@ -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 []; }; diff --git a/src/style-spec/validate/validate_property.js b/src/style-spec/validate/validate_property.js index 89d2ab65e41..f1825686d7f 100644 --- a/src/style-spec/validate/validate_property.js +++ b/src/style-spec/validate/validate_property.js @@ -53,6 +53,7 @@ module.exports = function validateProperty(options, propertyType) { valueSpec: valueSpec, style: style, styleSpec: styleSpec, - expressionContext: 'property' + expressionContext: 'property', + propertyKey })); }; diff --git a/src/style/style_layer/symbol_style_layer_properties.js b/src/style/style_layer/symbol_style_layer_properties.js index 26445d387be..98957edd994 100644 --- a/src/style/style_layer/symbol_style_layer_properties.js +++ b/src/style/style_layer/symbol_style_layer_properties.js @@ -35,7 +35,7 @@ export type LayoutProps = {| "text-pitch-alignment": DataConstantProperty<"map" | "viewport" | "auto">, "text-rotation-alignment": DataConstantProperty<"map" | "viewport" | "auto">, "text-field": DataDrivenProperty, - "text-font": DataConstantProperty>, + "text-font": DataDrivenProperty>, "text-size": DataDrivenProperty, "text-max-width": DataDrivenProperty, "text-line-height": DataConstantProperty, @@ -74,7 +74,7 @@ const layout: Properties = 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"]), diff --git a/src/symbol/symbol_layout.js b/src/symbol/symbol_layout.js index d17f9a503fd..e9e832b42b1 100644 --- a/src/symbol/symbol_layout.js +++ b/src/symbol/symbol_layout.js @@ -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; diff --git a/test/integration/render-tests/text-font/data-expression/expected.png b/test/integration/render-tests/text-font/data-expression/expected.png new file mode 100644 index 00000000000..ae92a7271d8 Binary files /dev/null and b/test/integration/render-tests/text-font/data-expression/expected.png differ diff --git a/test/integration/render-tests/text-font/data-expression/style.json b/test/integration/render-tests/text-font/data-expression/style.json new file mode 100644 index 00000000000..1e5ff8838d6 --- /dev/null +++ b/test/integration/render-tests/text-font/data-expression/style.json @@ -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" + ] + ] + ] + } + } + ] +} diff --git a/test/unit/style-spec/fixture/text-font.input.json b/test/unit/style-spec/fixture/text-font.input.json index b96edc44c94..dbac9e5347f 100644 --- a/test/unit/style-spec/fixture/text-font.input.json +++ b/test/unit/style-spec/fixture/text-font.input.json @@ -17,6 +17,106 @@ "text-font": ["Helvetica"], "text-field": "{foo}" } + }, + { + "id": "valid expression - case", + "type": "symbol", + "source": "vector", + "source-layer": "layer", + "layout": { + "text-font": ["case", ["==", "a", ["string", ["get", "text-font"]]], ["literal", ["Arial"]], ["literal", ["Helvetica"]]], + "text-field": "{foo}" + } + }, + { + "id": "valid expression - match", + "type": "symbol", + "source": "vector", + "source-layer": "layer", + "layout": { + "text-font": ["match", ["get", "text-font"], "a", ["literal", ["Arial"]], ["literal", ["Helvetica"]]], + "text-field": "{foo}" + } + }, + { + "id": "valid expression - step", + "type": "symbol", + "source": "vector", + "source-layer": "layer", + "layout": { + "text-font": ["array", "string", ["step", ["get", "text-font"], ["literal", ["Arial"]], 0, ["literal", ["Helvetica"]]]], + "text-field": "{foo}" + } + }, + { + "id": "invalid expression - get", + "type": "symbol", + "source": "vector", + "source-layer": "layer", + "layout": { + "text-font": ["array", "string", ["get", "text-font"]], + "text-field": "{foo}" + } + }, + { + "id": "invalid expression - case", + "type": "symbol", + "source": "vector", + "source-layer": "layer", + "layout": { + "text-font": ["case", ["==", "a", ["string", ["get", "text-font"]]], ["array", "string", ["get", "text-font-a"]], ["literal", ["Helvetica"]]], + "text-field": "{foo}" + } + }, + { + "id": "invalid expression - match", + "type": "symbol", + "source": "vector", + "source-layer": "layer", + "layout": { + "text-font": ["match", ["get", "text-font"], "a", ["array", "string", ["get", "text-font-a"]], ["literal", ["Helvetica"]]], + "text-field": "{foo}" + } + }, + { + "id": "invalid expression - at", + "type": "symbol", + "source": "vector", + "source-layer": "layer", + "layout": { + "text-font": ["array", "string", ["at", 0, ["array", ["get", "text-font"]]]], + "text-field": "{foo}" + } + }, + { + "id": "invalid expression - coalesce", + "type": "symbol", + "source": "vector", + "source-layer": "layer", + "layout": { + "text-font": ["array", "string", ["coalesce", ["get", "text-font"]]], + "text-field": "{foo}" + } + }, + { + "id": "invalid expression - step", + "type": "symbol", + "source": "vector", + "source-layer": "layer", + "layout": { + "text-font": ["step", ["zoom"], ["array", "string", ["get", "text-font-0"]], 0, ["array", "string", ["get", "text-font-1"]]], + "text-field": "{foo}" + } + }, + { + "id": "invalid expression - let/var", + "type": "symbol", + "source": "vector", + "source-layer": "layer", + "layout": { + "text-font": ["let", "p", ["array", "string", ["get", "text-font"]], ["var", "p"]], + "text-field": "{foo}" + } } ] } diff --git a/test/unit/style-spec/fixture/text-font.output.json b/test/unit/style-spec/fixture/text-font.output.json index 0637a088a01..57894ab83e4 100644 --- a/test/unit/style-spec/fixture/text-font.output.json +++ b/test/unit/style-spec/fixture/text-font.output.json @@ -1 +1,30 @@ -[] \ No newline at end of file +[ + { + "message": "layers[4].layout.text-font: Invalid data expression for \"text-font\". Output values must be contained as literals within the expression.", + "line": 57 + }, + { + "message": "layers[5].layout.text-font: Invalid data expression for \"text-font\". Output values must be contained as literals within the expression.", + "line": 67 + }, + { + "message": "layers[6].layout.text-font: Invalid data expression for \"text-font\". Output values must be contained as literals within the expression.", + "line": 77 + }, + { + "message": "layers[7].layout.text-font: Invalid data expression for \"text-font\". Output values must be contained as literals within the expression.", + "line": 87 + }, + { + "message": "layers[8].layout.text-font: Invalid data expression for \"text-font\". Output values must be contained as literals within the expression.", + "line": 97 + }, + { + "message": "layers[9].layout.text-font: Invalid data expression for \"text-font\". Output values must be contained as literals within the expression.", + "line": 107 + }, + { + "message": "layers[10].layout.text-font: Invalid data expression for \"text-font\". Output values must be contained as literals within the expression.", + "line": 117 + } +] \ No newline at end of file