Enable data-driven values for text-font

mapbox · Nov 22, 2017 · 199010d · 199010d
1 parent 1fb68f3
commit 199010d
Show file tree

Hide file tree

Showing 24 changed files with 291 additions and 15 deletions.
diff --git 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
@@ -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];

diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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<Value | void>;
 }
diff --git 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
@@ -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
@@ -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
@@ -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>,
@@ -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"]),

diff --git 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
diff --git a/test/integration/render-tests/text-font/data-expression/style.json 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"
+            ]
+          ]
+        ]
+      }
+    }
+  ]
+}