From 0dec7648492fd77320623b3345d0afb48d08957c Mon Sep 17 00:00:00 2001 From: ryanhamley Date: Fri, 23 Aug 2019 13:08:04 -0700 Subject: [PATCH 01/23] Implement Image operator and type --- .../expression/definitions/coercion.js | 2 ++ src/style-spec/expression/types.js | 28 +++++++++++-------- src/style-spec/expression/types/image.js | 17 +++++++++++ src/style-spec/expression/values.js | 10 +++++-- 4 files changed, 42 insertions(+), 15 deletions(-) create mode 100644 src/style-spec/expression/types/image.js diff --git a/src/style-spec/expression/definitions/coercion.js b/src/style-spec/expression/definitions/coercion.js index e6c2e9f81e7..dfea855061c 100644 --- a/src/style-spec/expression/definitions/coercion.js +++ b/src/style-spec/expression/definitions/coercion.js @@ -99,6 +99,8 @@ class Coercion implements Expression { // There is no explicit 'to-formatted' but this coercion can be implicitly // created by properties that expect the 'formatted' type. return Formatted.fromString(valueToString(this.args[0].evaluate(ctx))); + } else if (this.type.kind === 'image') { + console.log('image type!', this.args); } else { return valueToString(this.args[0].evaluate(ctx)); } diff --git a/src/style-spec/expression/types.js b/src/style-spec/expression/types.js index 1e209cbfb4f..c178f85f06d 100644 --- a/src/style-spec/expression/types.js +++ b/src/style-spec/expression/types.js @@ -10,6 +10,7 @@ export type ValueTypeT = { kind: 'value' }; export type ErrorTypeT = { kind: 'error' }; export type CollatorTypeT = { kind: 'collator' }; export type FormattedTypeT = { kind: 'formatted' }; +export type ImageTypeT = { kind: 'image' }; export type EvaluationKind = 'constant' | 'source' | 'camera' | 'composite'; @@ -24,7 +25,8 @@ export type Type = ArrayType | // eslint-disable-line no-use-before-define ErrorTypeT | CollatorTypeT | - FormattedTypeT + FormattedTypeT | + ImageTypeT export type ArrayType = { kind: 'array', @@ -32,16 +34,17 @@ export type ArrayType = { N: ?number } -export const NullType = {kind: 'null'}; -export const NumberType = {kind: 'number'}; -export const StringType = {kind: 'string'}; -export const BooleanType = {kind: 'boolean'}; -export const ColorType = {kind: 'color'}; -export const ObjectType = {kind: 'object'}; -export const ValueType = {kind: 'value'}; -export const ErrorType = {kind: 'error'}; -export const CollatorType = {kind: 'collator'}; -export const FormattedType = {kind: 'formatted'}; +export const NullType = { kind: 'null' }; +export const NumberType = { kind: 'number' }; +export const StringType = { kind: 'string' }; +export const BooleanType = { kind: 'boolean' }; +export const ColorType = { kind: 'color' }; +export const ObjectType = { kind: 'object' }; +export const ValueType = { kind: 'value' }; +export const ErrorType = { kind: 'error' }; +export const CollatorType = { kind: 'collator' }; +export const FormattedType = { kind: 'formatted' }; +export const ImageType = { kind: 'image' }; export function array(itemType: Type, N: ?number): ArrayType { return { @@ -70,7 +73,8 @@ const valueMemberTypes = [ ColorType, FormattedType, ObjectType, - array(ValueType) + array(ValueType), + ImageType ]; /** diff --git a/src/style-spec/expression/types/image.js b/src/style-spec/expression/types/image.js new file mode 100644 index 00000000000..c2bcf98931e --- /dev/null +++ b/src/style-spec/expression/types/image.js @@ -0,0 +1,17 @@ +// @flow + +export default class Image { + name: string; + + constructor(name: string) { + this.name = name; + } + + static fromString(imageName: string): Image { + return new Image(imageName); + } + + serialize(): Array { + return ["image", this.name]; + } +} diff --git a/src/style-spec/expression/values.js b/src/style-spec/expression/values.js index f3ccc080bbc..3bf6ea3ea8e 100644 --- a/src/style-spec/expression/values.js +++ b/src/style-spec/expression/values.js @@ -5,7 +5,7 @@ import assert from 'assert'; import Color from '../util/color'; import Collator from './types/collator'; import Formatted from './types/formatted'; -import {NullType, NumberType, StringType, BooleanType, ColorType, ObjectType, ValueType, CollatorType, FormattedType, array} from './types'; +import { NullType, NumberType, StringType, BooleanType, ColorType, ObjectType, ValueType, CollatorType, FormattedType, ImageType, array } from './types'; import type {Type} from './types'; @@ -28,7 +28,7 @@ export function validateRGBA(r: mixed, g: mixed, b: mixed, a?: mixed): ?string { return null; } -export type Value = null | string | boolean | number | Color | Collator | Formatted | $ReadOnlyArray | { +[string]: Value } +export type Value = null | string | boolean | number | Color | Collator | Formatted | Image | $ReadOnlyArray | { +[string]: Value } export function isValue(mixed: mixed): boolean { if (mixed === null) { @@ -39,6 +39,8 @@ export function isValue(mixed: mixed): boolean { return true; } else if (typeof mixed === 'number') { return true; + } else if (typeof mixed === 'image') { + return true; } else if (mixed instanceof Color) { return true; } else if (mixed instanceof Collator) { @@ -73,6 +75,8 @@ export function typeOf(value: Value): Type { return BooleanType; } else if (typeof value === 'number') { return NumberType; + } else if (typeof value === 'image') { + return ImageType; } else if (value instanceof Color) { return ColorType; } else if (value instanceof Collator) { @@ -108,7 +112,7 @@ export function toString(value: Value) { return ''; } else if (type === 'string' || type === 'number' || type === 'boolean') { return String(value); - } else if (value instanceof Color || value instanceof Formatted) { + } else if (value instanceof Color || value instanceof Formatted || value instanceof Image) { return value.toString(); } else { return JSON.stringify(value); From b80b9c9124cfe678c4b80fb4fdb310e6e2ea439a Mon Sep 17 00:00:00 2001 From: ryanhamley Date: Fri, 23 Aug 2019 13:09:21 -0700 Subject: [PATCH 02/23] Add image validation --- src/style-spec/validate/validate.js | 4 +++- src/style-spec/validate/validate_image.js | 11 +++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) create mode 100644 src/style-spec/validate/validate_image.js diff --git a/src/style-spec/validate/validate.js b/src/style-spec/validate/validate.js index f9468cc458f..de8c6d16954 100644 --- a/src/style-spec/validate/validate.js +++ b/src/style-spec/validate/validate.js @@ -19,6 +19,7 @@ import validateSource from './validate_source'; import validateLight from './validate_light'; import validateString from './validate_string'; import validateFormatted from './validate_formatted'; +import validateImage from './validate_image'; const VALIDATORS = { '*'() { @@ -37,7 +38,8 @@ const VALIDATORS = { 'source': validateSource, 'light': validateLight, 'string': validateString, - 'formatted': validateFormatted + 'formatted': validateFormatted, + 'image': validateImage }; // Main recursive validation function. Tracks: diff --git a/src/style-spec/validate/validate_image.js b/src/style-spec/validate/validate_image.js new file mode 100644 index 00000000000..b02763ffd92 --- /dev/null +++ b/src/style-spec/validate/validate_image.js @@ -0,0 +1,11 @@ +// @flow +import validateExpression from './validate_expression'; +import validateString from './validate_string'; + +export default function validateImage(options: any) { + if (validateString(options).length === 0) { + return []; + } + + return validateExpression(options); +} From 3420f47d8720a985bacde91db733294f7aa4879f Mon Sep 17 00:00:00 2001 From: ryanhamley Date: Fri, 23 Aug 2019 13:10:10 -0700 Subject: [PATCH 03/23] Update *-pattern and icon-image to accept type Image --- src/style-spec/reference/v8.json | 10 +++++----- .../style_layer/background_style_layer_properties.js | 3 ++- .../fill_extrusion_style_layer_properties.js | 3 ++- src/style/style_layer/fill_style_layer_properties.js | 4 +++- src/style/style_layer/line_style_layer_properties.js | 4 +++- src/style/style_layer/symbol_style_layer_properties.js | 4 +++- 6 files changed, 18 insertions(+), 10 deletions(-) diff --git a/src/style-spec/reference/v8.json b/src/style-spec/reference/v8.json index 6e998fd095a..7da8f81e3f3 100644 --- a/src/style-spec/reference/v8.json +++ b/src/style-spec/reference/v8.json @@ -1291,7 +1291,7 @@ "property-type": "data-constant" }, "icon-image": { - "type": "string", + "type": "image", "doc": "Name of image in sprite to use for drawing an image background.", "tokens": true, "sdk-support": { @@ -3788,7 +3788,7 @@ "property-type": "data-constant" }, "fill-pattern": { - "type": "string", + "type": "image", "transition": true, "doc": "Name of image in sprite to use for drawing image fills. For seamless patterns, image width and height must be a factor of two (2, 4, 8, ..., 512). Note that zoom-dependent expressions will be evaluated only at integer zoom levels.", "sdk-support": { @@ -3934,7 +3934,7 @@ "property-type": "data-constant" }, "fill-extrusion-pattern": { - "type": "string", + "type": "image", "transition": true, "doc": "Name of image in sprite to use for drawing images on extruded fills. For seamless patterns, image width and height must be a factor of two (2, 4, 8, ..., 512). Note that zoom-dependent expressions will be evaluated only at integer zoom levels.", "sdk-support": { @@ -4323,7 +4323,7 @@ "property-type": "cross-faded" }, "line-pattern": { - "type": "string", + "type": "image", "transition": true, "doc": "Name of image in sprite to use for drawing image lines. For seamless patterns, image width must be a factor of two (2, 4, 8, ..., 512). Note that zoom-dependent expressions will be evaluated only at integer zoom levels.", "sdk-support": { @@ -5703,7 +5703,7 @@ "property-type": "data-constant" }, "background-pattern": { - "type": "string", + "type": "image", "transition": true, "doc": "Name of image in sprite to use for drawing an image background. For seamless patterns, image width and height must be a factor of two (2, 4, 8, ..., 512). Note that zoom-dependent expressions will be evaluated only at integer zoom levels.", "sdk-support": { diff --git a/src/style/style_layer/background_style_layer_properties.js b/src/style/style_layer/background_style_layer_properties.js index c4c9b36ab37..594d80ff299 100644 --- a/src/style/style_layer/background_style_layer_properties.js +++ b/src/style/style_layer/background_style_layer_properties.js @@ -17,10 +17,11 @@ import type Color from '../../style-spec/util/color'; import type Formatted from '../../style-spec/expression/types/formatted'; +import type Image from '../../style-spec/expression/types/image'; export type PaintProps = {| "background-color": DataConstantProperty, - "background-pattern": CrossFadedProperty, + "background-pattern": CrossFadedProperty, "background-opacity": DataConstantProperty, |}; diff --git a/src/style/style_layer/fill_extrusion_style_layer_properties.js b/src/style/style_layer/fill_extrusion_style_layer_properties.js index 4aa0360fa30..2312be4ac87 100644 --- a/src/style/style_layer/fill_extrusion_style_layer_properties.js +++ b/src/style/style_layer/fill_extrusion_style_layer_properties.js @@ -17,13 +17,14 @@ import type Color from '../../style-spec/util/color'; import type Formatted from '../../style-spec/expression/types/formatted'; +import type Image from '../../style-spec/expression/types/image'; export type PaintProps = {| "fill-extrusion-opacity": DataConstantProperty, "fill-extrusion-color": DataDrivenProperty, "fill-extrusion-translate": DataConstantProperty<[number, number]>, "fill-extrusion-translate-anchor": DataConstantProperty<"map" | "viewport">, - "fill-extrusion-pattern": CrossFadedDataDrivenProperty, + "fill-extrusion-pattern": CrossFadedDataDrivenProperty, "fill-extrusion-height": DataDrivenProperty, "fill-extrusion-base": DataDrivenProperty, "fill-extrusion-vertical-gradient": DataConstantProperty, diff --git a/src/style/style_layer/fill_style_layer_properties.js b/src/style/style_layer/fill_style_layer_properties.js index 48ed8d3572a..afff377f113 100644 --- a/src/style/style_layer/fill_style_layer_properties.js +++ b/src/style/style_layer/fill_style_layer_properties.js @@ -17,6 +17,8 @@ import type Color from '../../style-spec/util/color'; import type Formatted from '../../style-spec/expression/types/formatted'; +import type Image from '../../style-spec/expression/types/image'; + export type LayoutProps = {| "fill-sort-key": DataDrivenProperty, |}; @@ -32,7 +34,7 @@ export type PaintProps = {| "fill-outline-color": DataDrivenProperty, "fill-translate": DataConstantProperty<[number, number]>, "fill-translate-anchor": DataConstantProperty<"map" | "viewport">, - "fill-pattern": CrossFadedDataDrivenProperty, + "fill-pattern": CrossFadedDataDrivenProperty, |}; const paint: Properties = new Properties({ diff --git a/src/style/style_layer/line_style_layer_properties.js b/src/style/style_layer/line_style_layer_properties.js index 4f8fcf8c61c..2c3b5f101b1 100644 --- a/src/style/style_layer/line_style_layer_properties.js +++ b/src/style/style_layer/line_style_layer_properties.js @@ -17,6 +17,8 @@ import type Color from '../../style-spec/util/color'; import type Formatted from '../../style-spec/expression/types/formatted'; +import type Image from '../../style-spec/expression/types/image'; + export type LayoutProps = {| "line-cap": DataConstantProperty<"butt" | "round" | "square">, "line-join": DataDrivenProperty<"bevel" | "round" | "miter">, @@ -43,7 +45,7 @@ export type PaintProps = {| "line-offset": DataDrivenProperty, "line-blur": DataDrivenProperty, "line-dasharray": CrossFadedProperty>, - "line-pattern": CrossFadedDataDrivenProperty, + "line-pattern": CrossFadedDataDrivenProperty, "line-gradient": ColorRampProperty, |}; diff --git a/src/style/style_layer/symbol_style_layer_properties.js b/src/style/style_layer/symbol_style_layer_properties.js index 858410c7fe4..89b8d8bb0d1 100644 --- a/src/style/style_layer/symbol_style_layer_properties.js +++ b/src/style/style_layer/symbol_style_layer_properties.js @@ -17,6 +17,8 @@ import type Color from '../../style-spec/util/color'; import type Formatted from '../../style-spec/expression/types/formatted'; +import type Image from '../../style-spec/expression/types/image'; + import { ColorType } from '../../style-spec/expression/types'; @@ -34,7 +36,7 @@ export type LayoutProps = {| "icon-size": DataDrivenProperty, "icon-text-fit": DataConstantProperty<"none" | "width" | "height" | "both">, "icon-text-fit-padding": DataConstantProperty<[number, number, number, number]>, - "icon-image": DataDrivenProperty, + "icon-image": DataDrivenProperty, "icon-rotate": DataDrivenProperty, "icon-padding": DataConstantProperty, "icon-keep-upright": DataConstantProperty, From de26cb5044c798d354c25253ad9b477382399465 Mon Sep 17 00:00:00 2001 From: ryanhamley Date: Fri, 23 Aug 2019 16:15:55 -0700 Subject: [PATCH 04/23] Coerce strings to Image type --- src/data/bucket/symbol_bucket.js | 12 ++++++++++-- src/style-spec/expression/definitions/coercion.js | 2 ++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/data/bucket/symbol_bucket.js b/src/data/bucket/symbol_bucket.js index 91a7b0aff06..b48dc7d564f 100644 --- a/src/data/bucket/symbol_bucket.js +++ b/src/data/bucket/symbol_bucket.js @@ -36,6 +36,7 @@ import {getSizeData} from '../../symbol/symbol_size'; import {register} from '../../util/web_worker_transfer'; import EvaluationParameters from '../../style/evaluation_parameters'; import Formatted from '../../style-spec/expression/types/formatted'; +import Image from '../../style-spec/expression/types/image'; import type { Bucket, @@ -76,7 +77,7 @@ export type CollisionArrays = { export type SymbolFeature = {| sortKey: number | void, text: Formatted | void, - icon: string | void, + icon: Image | void, index: number, sourceLayerIndex: number, geometry: Array>, @@ -409,7 +410,14 @@ class SymbolBucket implements Bucket { let icon; if (hasIcon) { - icon = layer.getValueAndResolveTokens('icon-image', feature); + // Expression evaluation will automatically coerce to Image + // but plain string token evaluation skips that pathway so do the + // conversion here. + const resolvedTokens = layer.getValueAndResolveTokens('icon-image', feature); + icon = resolvedTokens instanceof Image ? + resolvedTokens : + Image.fromString(resolvedTokens); + console.log('icon', icon); } if (!text && !icon) { diff --git a/src/style-spec/expression/definitions/coercion.js b/src/style-spec/expression/definitions/coercion.js index dfea855061c..7799ebab22c 100644 --- a/src/style-spec/expression/definitions/coercion.js +++ b/src/style-spec/expression/definitions/coercion.js @@ -7,6 +7,7 @@ import {Color, toString as valueToString, validateRGBA} from '../values'; import RuntimeError from '../runtime_error'; import Formatted from '../types/formatted'; import FormatExpression from '../definitions/format'; +import Image from '../types/image'; import type {Expression} from '../expression'; import type ParsingContext from '../parsing_context'; @@ -101,6 +102,7 @@ class Coercion implements Expression { return Formatted.fromString(valueToString(this.args[0].evaluate(ctx))); } else if (this.type.kind === 'image') { console.log('image type!', this.args); + return Image.fromString(valueToString(this.args[0].evaluate(ctx))); } else { return valueToString(this.args[0].evaluate(ctx)); } From d1157d2c64dcb9e11261ae6199079f5759d47e6e Mon Sep 17 00:00:00 2001 From: ryanhamley Date: Fri, 23 Aug 2019 16:25:28 -0700 Subject: [PATCH 05/23] Add skeleton of image operator --- .../expression/definitions/image.js | 42 +++++++++++++++++++ .../expression/definitions/index.js | 2 + 2 files changed, 44 insertions(+) create mode 100644 src/style-spec/expression/definitions/image.js diff --git a/src/style-spec/expression/definitions/image.js b/src/style-spec/expression/definitions/image.js new file mode 100644 index 00000000000..47fce40ab5e --- /dev/null +++ b/src/style-spec/expression/definitions/image.js @@ -0,0 +1,42 @@ +// @flow + +import { ValueType, ImageType, StringType } from '../types'; +import Image from '../types/image'; +import { toString } from '../values'; + +import type { Expression } from '../expression'; +import type EvaluationContext from '../evaluation_context'; +import type ParsingContext from '../parsing_context'; +import type { Type } from '../types'; + +export default class ImageExpression implements Expression { + type: Type; + name: string; + + constructor(name: string) { + this.type = ImageType; + this.name = name; + } + + static parse(args: $ReadOnlyArray, context: ParsingContext): ?Expression { + + } + + evaluate(ctx: EvaluationContext) { + + } + + eachChild(fn: (Expression) => void) { + + } + + possibleOutputs() { + // Technically the combinatoric set of all children + // Usually, this.text will be undefined anyway + return [undefined]; + } + + serialize() { + return ["image", this.name]; + } +} diff --git a/src/style-spec/expression/definitions/index.js b/src/style-spec/expression/definitions/index.js index 455d00d55f9..e4e53171b24 100644 --- a/src/style-spec/expression/definitions/index.js +++ b/src/style-spec/expression/definitions/index.js @@ -39,6 +39,7 @@ import { import CollatorExpression from './collator'; import NumberFormat from './number_format'; import FormatExpression from './format'; +import Image from './image'; import Length from './length'; import type {Varargs} from '../compound_expression'; @@ -59,6 +60,7 @@ const expressions: ExpressionRegistry = { 'coalesce': Coalesce, 'collator': CollatorExpression, 'format': FormatExpression, + 'image': Image, 'interpolate': Interpolate, 'interpolate-hcl': Interpolate, 'interpolate-lab': Interpolate, From e08c77174ab52f7845565f666a113f65e0144e95 Mon Sep 17 00:00:00 2001 From: ryanhamley Date: Wed, 28 Aug 2019 14:00:07 -0700 Subject: [PATCH 06/23] Working on implementing operator --- src/style-spec/expression/definitions/image.js | 13 +++++++++++-- src/style-spec/expression/index.js | 3 +++ src/style-spec/expression/is_constant.js | 2 ++ src/style-spec/validate/validate_expression.js | 1 + 4 files changed, 17 insertions(+), 2 deletions(-) diff --git a/src/style-spec/expression/definitions/image.js b/src/style-spec/expression/definitions/image.js index 47fce40ab5e..617e98e1ff0 100644 --- a/src/style-spec/expression/definitions/image.js +++ b/src/style-spec/expression/definitions/image.js @@ -19,15 +19,24 @@ export default class ImageExpression implements Expression { } static parse(args: $ReadOnlyArray, context: ParsingContext): ?Expression { + if (args.length !== 2) { + return context.error(`Expected two arguments.`); + } + const imageName = args[1]; + + console.log('parse', args, context); + console.log('new ImageExpression(imageName)', new ImageExpression(imageName)); + return new ImageExpression(imageName); } evaluate(ctx: EvaluationContext) { - + console.log('evaluate', ctx); } eachChild(fn: (Expression) => void) { - + console.log('each child', fn); + fn(this.name); } possibleOutputs() { diff --git a/src/style-spec/expression/index.js b/src/style-spec/expression/index.js index 9c203d22cc1..c74ca7779dd 100644 --- a/src/style-spec/expression/index.js +++ b/src/style-spec/expression/index.js @@ -213,6 +213,7 @@ export type StylePropertyExpression = export function createPropertyExpression(expression: mixed, propertySpec: StylePropertySpecification): Result> { expression = createExpression(expression, propertySpec); + console.log('expression', expression); if (expression.result === 'error') { return expression; } @@ -220,6 +221,7 @@ export function createPropertyExpression(expression: mixed, propertySpec: StyleP const parsed = expression.value.expression; const isFeatureConstant = isConstant.isFeatureConstant(parsed); + console.log('isFeatureConstant', isFeatureConstant); if (!isFeatureConstant && !supportsPropertyExpression(propertySpec)) { return error([new ParsingError('', 'data expressions not supported')]); } @@ -351,6 +353,7 @@ function findZoomCurve(expression: Expression): Step | Interpolate | ParsingErro import {ColorType, StringType, NumberType, BooleanType, ValueType, FormattedType, array} from './types'; function getExpectedType(spec: StylePropertySpecification): Type { + console.log('spec', spec); const types = { color: ColorType, string: StringType, diff --git a/src/style-spec/expression/is_constant.js b/src/style-spec/expression/is_constant.js index 92e7ab24be2..5c4b8c9fad1 100644 --- a/src/style-spec/expression/is_constant.js +++ b/src/style-spec/expression/is_constant.js @@ -5,6 +5,7 @@ import CompoundExpression from './compound_expression'; import type {Expression} from './expression.js'; function isFeatureConstant(e: Expression) { + console.log('e', e); if (e instanceof CompoundExpression) { if (e.name === 'get' && e.args.length === 1) { return false; @@ -25,6 +26,7 @@ function isFeatureConstant(e: Expression) { let result = true; e.eachChild(arg => { + console.log('arg', arg); if (result && !isFeatureConstant(arg)) { result = false; } }); return result; diff --git a/src/style-spec/validate/validate_expression.js b/src/style-spec/validate/validate_expression.js index 573a9ac3d02..b6307bcf40d 100644 --- a/src/style-spec/validate/validate_expression.js +++ b/src/style-spec/validate/validate_expression.js @@ -7,6 +7,7 @@ import {deepUnbundle} from '../util/unbundle_jsonlint'; import {isStateConstant, isGlobalPropertyConstant, isFeatureConstant} from '../expression/is_constant'; export default function validateExpression(options: any): Array { + console.log('validate', options); const expression = (options.expressionContext === 'property' ? createPropertyExpression : createExpression)(deepUnbundle(options.value), options.valueSpec); if (expression.result === 'error') { return expression.value.map((error) => { From 13763ca738f679d02bd87d12a58299a3e1b588aa Mon Sep 17 00:00:00 2001 From: ryanhamley Date: Fri, 30 Aug 2019 15:51:17 -0700 Subject: [PATCH 07/23] Implement evaluation logic --- src/data/bucket/symbol_bucket.js | 1 - .../expression/definitions/coercion.js | 7 ++++++- .../expression/definitions/image.js | 21 +++++++++---------- .../expression/definitions/literal.js | 1 + src/style-spec/expression/index.js | 3 --- src/style-spec/expression/is_constant.js | 2 -- src/style-spec/expression/parsing_context.js | 2 +- src/style-spec/expression/types/image.js | 4 ++++ src/style-spec/function/index.js | 7 +++++-- .../validate/validate_expression.js | 1 - 10 files changed, 27 insertions(+), 22 deletions(-) diff --git a/src/data/bucket/symbol_bucket.js b/src/data/bucket/symbol_bucket.js index b48dc7d564f..b0b541a9cc9 100644 --- a/src/data/bucket/symbol_bucket.js +++ b/src/data/bucket/symbol_bucket.js @@ -417,7 +417,6 @@ class SymbolBucket implements Bucket { icon = resolvedTokens instanceof Image ? resolvedTokens : Image.fromString(resolvedTokens); - console.log('icon', icon); } if (!text && !icon) { diff --git a/src/style-spec/expression/definitions/coercion.js b/src/style-spec/expression/definitions/coercion.js index 7799ebab22c..f15aee6ee79 100644 --- a/src/style-spec/expression/definitions/coercion.js +++ b/src/style-spec/expression/definitions/coercion.js @@ -101,7 +101,6 @@ class Coercion implements Expression { // created by properties that expect the 'formatted' type. return Formatted.fromString(valueToString(this.args[0].evaluate(ctx))); } else if (this.type.kind === 'image') { - console.log('image type!', this.args); return Image.fromString(valueToString(this.args[0].evaluate(ctx))); } else { return valueToString(this.args[0].evaluate(ctx)); @@ -117,9 +116,15 @@ class Coercion implements Expression { } serialize() { + console.log('serialize'); if (this.type.kind === 'formatted') { return new FormatExpression([{text: this.args[0], scale: null, font: null, textColor: null}]).serialize(); } + + if (this.type.kind === 'image') { + return new ImageExpression(this.args[0]).serialize(); + } + const serialized = [`to-${this.type.kind}`]; this.eachChild(child => { serialized.push(child.serialize()); }); return serialized; diff --git a/src/style-spec/expression/definitions/image.js b/src/style-spec/expression/definitions/image.js index 617e98e1ff0..32eaad4c3c8 100644 --- a/src/style-spec/expression/definitions/image.js +++ b/src/style-spec/expression/definitions/image.js @@ -11,11 +11,11 @@ import type { Type } from '../types'; export default class ImageExpression implements Expression { type: Type; - name: string; + input: string; - constructor(name: string) { + constructor(input: Expression) { this.type = ImageType; - this.name = name; + this.input = input; } static parse(args: $ReadOnlyArray, context: ParsingContext): ?Expression { @@ -23,20 +23,19 @@ export default class ImageExpression implements Expression { return context.error(`Expected two arguments.`); } - const imageName = args[1]; + const name = context.parse(args[1], 1, StringType); + if (!name) return null; - console.log('parse', args, context); - console.log('new ImageExpression(imageName)', new ImageExpression(imageName)); - return new ImageExpression(imageName); + return new ImageExpression(name); } evaluate(ctx: EvaluationContext) { - console.log('evaluate', ctx); + console.log('evaluate', this.input.evaluate(ctx)); + return this.input.evaluate(ctx); } eachChild(fn: (Expression) => void) { - console.log('each child', fn); - fn(this.name); + fn(this.input); } possibleOutputs() { @@ -46,6 +45,6 @@ export default class ImageExpression implements Expression { } serialize() { - return ["image", this.name]; + return ["image", this.input]; } } diff --git a/src/style-spec/expression/definitions/literal.js b/src/style-spec/expression/definitions/literal.js index 5bc9578aee5..2482cd82c09 100644 --- a/src/style-spec/expression/definitions/literal.js +++ b/src/style-spec/expression/definitions/literal.js @@ -54,6 +54,7 @@ class Literal implements Expression { } serialize(): Array { + console.log('literal serialize', this); if (this.type.kind === 'array' || this.type.kind === 'object') { return ["literal", this.value]; } else if (this.value instanceof Color) { diff --git a/src/style-spec/expression/index.js b/src/style-spec/expression/index.js index c74ca7779dd..9c203d22cc1 100644 --- a/src/style-spec/expression/index.js +++ b/src/style-spec/expression/index.js @@ -213,7 +213,6 @@ export type StylePropertyExpression = export function createPropertyExpression(expression: mixed, propertySpec: StylePropertySpecification): Result> { expression = createExpression(expression, propertySpec); - console.log('expression', expression); if (expression.result === 'error') { return expression; } @@ -221,7 +220,6 @@ export function createPropertyExpression(expression: mixed, propertySpec: StyleP const parsed = expression.value.expression; const isFeatureConstant = isConstant.isFeatureConstant(parsed); - console.log('isFeatureConstant', isFeatureConstant); if (!isFeatureConstant && !supportsPropertyExpression(propertySpec)) { return error([new ParsingError('', 'data expressions not supported')]); } @@ -353,7 +351,6 @@ function findZoomCurve(expression: Expression): Step | Interpolate | ParsingErro import {ColorType, StringType, NumberType, BooleanType, ValueType, FormattedType, array} from './types'; function getExpectedType(spec: StylePropertySpecification): Type { - console.log('spec', spec); const types = { color: ColorType, string: StringType, diff --git a/src/style-spec/expression/is_constant.js b/src/style-spec/expression/is_constant.js index 5c4b8c9fad1..92e7ab24be2 100644 --- a/src/style-spec/expression/is_constant.js +++ b/src/style-spec/expression/is_constant.js @@ -5,7 +5,6 @@ import CompoundExpression from './compound_expression'; import type {Expression} from './expression.js'; function isFeatureConstant(e: Expression) { - console.log('e', e); if (e instanceof CompoundExpression) { if (e.name === 'get' && e.args.length === 1) { return false; @@ -26,7 +25,6 @@ function isFeatureConstant(e: Expression) { let result = true; e.eachChild(arg => { - console.log('arg', arg); if (result && !isFeatureConstant(arg)) { result = false; } }); return result; diff --git a/src/style-spec/expression/parsing_context.js b/src/style-spec/expression/parsing_context.js index 2c7623e11e1..13a9d831848 100644 --- a/src/style-spec/expression/parsing_context.js +++ b/src/style-spec/expression/parsing_context.js @@ -112,7 +112,7 @@ class ParsingContext { // if ((expected.kind === 'string' || expected.kind === 'number' || expected.kind === 'boolean' || expected.kind === 'object' || expected.kind === 'array') && actual.kind === 'value') { parsed = annotate(parsed, expected, options.typeAnnotation || 'assert'); - } else if ((expected.kind === 'color' || expected.kind === 'formatted') && (actual.kind === 'value' || actual.kind === 'string')) { + } else if ((expected.kind === 'color' || expected.kind === 'formatted' || expected.kind === 'image') && (actual.kind === 'value' || actual.kind === 'string')) { parsed = annotate(parsed, expected, options.typeAnnotation || 'coerce'); } else if (this.checkSubtype(expected, actual)) { return null; diff --git a/src/style-spec/expression/types/image.js b/src/style-spec/expression/types/image.js index c2bcf98931e..cf3f7c647cc 100644 --- a/src/style-spec/expression/types/image.js +++ b/src/style-spec/expression/types/image.js @@ -7,6 +7,10 @@ export default class Image { this.name = name; } + toString(): string { + return this.name; + } + static fromString(imageName: string): Image { return new Image(imageName); } diff --git a/src/style-spec/function/index.js b/src/style-spec/function/index.js index 6ef3bc32b3d..8d70dc0803b 100644 --- a/src/style-spec/function/index.js +++ b/src/style-spec/function/index.js @@ -6,8 +6,9 @@ import getType from '../util/get_type'; import * as interpolate from '../util/interpolate'; import Interpolate from '../expression/definitions/interpolate'; import Formatted from '../expression/types/formatted'; -import {supportsInterpolation} from '../util/properties'; -import {findStopLessThanOrEqualTo} from '../expression/stops'; +import Image from '../expression/types/image'; +import { supportsInterpolation } from '../util/properties'; +import { findStopLessThanOrEqualTo } from '../expression/stops'; export function isFunction(value) { return typeof value === 'object' && value !== null && !Array.isArray(value); @@ -201,6 +202,8 @@ function evaluateIdentityFunction(parameters, propertySpec, input) { input = Color.parse(input); } else if (propertySpec.type === 'formatted') { input = Formatted.fromString(input.toString()); + } else if (propertySpec.type === 'image') { + input = Image.fromString(input.toString()); } else if (getType(input) !== propertySpec.type && (propertySpec.type !== 'enum' || !propertySpec.values[input])) { input = undefined; } diff --git a/src/style-spec/validate/validate_expression.js b/src/style-spec/validate/validate_expression.js index b6307bcf40d..573a9ac3d02 100644 --- a/src/style-spec/validate/validate_expression.js +++ b/src/style-spec/validate/validate_expression.js @@ -7,7 +7,6 @@ import {deepUnbundle} from '../util/unbundle_jsonlint'; import {isStateConstant, isGlobalPropertyConstant, isFeatureConstant} from '../expression/is_constant'; export default function validateExpression(options: any): Array { - console.log('validate', options); const expression = (options.expressionContext === 'property' ? createPropertyExpression : createExpression)(deepUnbundle(options.value), options.valueSpec); if (expression.result === 'error') { return expression.value.map((error) => { From 39d85c13dec4a5c25e0467322033dfd4722ffdb8 Mon Sep 17 00:00:00 2001 From: ryanhamley Date: Wed, 11 Sep 2019 12:53:24 -0700 Subject: [PATCH 08/23] Get available images into the worker --- src/data/bucket.js | 3 ++- src/data/bucket/symbol_bucket.js | 3 ++- src/render/image_manager.js | 2 +- src/source/geojson_worker_source.js | 4 ++-- src/source/vector_tile_worker_source.js | 6 ++++-- src/source/worker.js | 20 +++++++++++++++++-- src/source/worker_tile.js | 5 +++-- .../expression/definitions/coercion.js | 1 - .../expression/definitions/image.js | 10 ++++++++-- .../expression/definitions/literal.js | 1 - .../expression/evaluation_context.js | 2 ++ src/style-spec/expression/index.js | 14 +++++++------ src/style/properties.js | 8 ++++---- src/style/style.js | 2 +- src/style/style_layer/symbol_style_layer.js | 4 ++-- 15 files changed, 57 insertions(+), 28 deletions(-) diff --git a/src/data/bucket.js b/src/data/bucket.js index 9c5fbec5533..f0ecc4ef7c2 100644 --- a/src/data/bucket.js +++ b/src/data/bucket.js @@ -23,7 +23,8 @@ export type PopulateParameters = { featureIndex: FeatureIndex, iconDependencies: {}, patternDependencies: {}, - glyphDependencies: {} + glyphDependencies: {}, + availableImages: Array } export type IndexedFeature = { diff --git a/src/data/bucket/symbol_bucket.js b/src/data/bucket/symbol_bucket.js index b0b541a9cc9..d2b5240bb0e 100644 --- a/src/data/bucket/symbol_bucket.js +++ b/src/data/bucket/symbol_bucket.js @@ -389,6 +389,7 @@ class SymbolBucket implements Bucket { const icons = options.iconDependencies; const stacks = options.glyphDependencies; + const availableImages = options.availableImages; const globalProperties = new EvaluationParameters(this.zoom); for (const {feature, index, sourceLayerIndex} of features) { @@ -413,7 +414,7 @@ class SymbolBucket implements Bucket { // Expression evaluation will automatically coerce to Image // but plain string token evaluation skips that pathway so do the // conversion here. - const resolvedTokens = layer.getValueAndResolveTokens('icon-image', feature); + const resolvedTokens = layer.getValueAndResolveTokens('icon-image', feature, availableImages); icon = resolvedTokens instanceof Image ? resolvedTokens : Image.fromString(resolvedTokens); diff --git a/src/render/image_manager.js b/src/render/image_manager.js index eb2e1e9e10d..e3b53bfa911 100644 --- a/src/render/image_manager.js +++ b/src/render/image_manager.js @@ -117,7 +117,7 @@ class ImageManager extends Evented { getImages(ids: Array, callback: Callback<{[string]: StyleImage}>) { // If the sprite has been loaded, or if all the icon dependencies are already present - // (i.e. if they've been addeded via runtime styling), then notify the requestor immediately. + // (i.e. if they've been added via runtime styling), then notify the requestor immediately. // Otherwise, delay notification until the sprite is loaded. At that point, if any of the // dependencies are still unavailable, we'll just assume they are permanently missing. let hasAllDependencies = true; diff --git a/src/source/geojson_worker_source.js b/src/source/geojson_worker_source.js index dfa681f9ede..be29d9a2c61 100644 --- a/src/source/geojson_worker_source.js +++ b/src/source/geojson_worker_source.js @@ -104,8 +104,8 @@ class GeoJSONWorkerSource extends VectorTileWorkerSource { * GeoJSON based on parameters passed from the main-thread Source. * See {@link GeoJSONWorkerSource#loadGeoJSON}. */ - constructor(actor: Actor, layerIndex: StyleLayerIndex, loadGeoJSON: ?LoadGeoJSON) { - super(actor, layerIndex, loadGeoJSONTile); + constructor(actor: Actor, layerIndex: StyleLayerIndex, availableImages: Array, loadGeoJSON: ?LoadGeoJSON) { + super(actor, layerIndex, availableImages, loadGeoJSONTile); if (loadGeoJSON) { this.loadGeoJSON = loadGeoJSON; } diff --git a/src/source/vector_tile_worker_source.js b/src/source/vector_tile_worker_source.js index dacec5b78c8..d732b8ddb7a 100644 --- a/src/source/vector_tile_worker_source.js +++ b/src/source/vector_tile_worker_source.js @@ -72,6 +72,7 @@ function loadVectorTile(params: WorkerTileParameters, callback: LoadVectorDataCa class VectorTileWorkerSource implements WorkerSource { actor: Actor; layerIndex: StyleLayerIndex; + availableImages: Array; loadVectorData: LoadVectorData; loading: { [string]: WorkerTile }; loaded: { [string]: WorkerTile }; @@ -82,9 +83,10 @@ class VectorTileWorkerSource implements WorkerSource { * {@link VectorTileWorkerSource#loadTile}. The default implementation simply * loads the pbf at `params.url`. */ - constructor(actor: Actor, layerIndex: StyleLayerIndex, loadVectorData: ?LoadVectorData) { + constructor(actor: Actor, layerIndex: StyleLayerIndex, availableImages: Array, loadVectorData: ?LoadVectorData) { this.actor = actor; this.layerIndex = layerIndex; + this.availableImages = availableImages; this.loadVectorData = loadVectorData || loadVectorTile; this.loading = {}; this.loaded = {}; @@ -129,7 +131,7 @@ class VectorTileWorkerSource implements WorkerSource { } workerTile.vectorTile = response.vectorTile; - workerTile.parse(response.vectorTile, this.layerIndex, this.actor, (err, result) => { + workerTile.parse(response.vectorTile, this.layerIndex, this.availableImages, this.actor, (err, result) => { if (err || !result) return callback(err); // Transferring a copy of rawTileData because the worker needs to retain its copy. diff --git a/src/source/worker.js b/src/source/worker.js index 310a30f6546..7d6f494b58c 100644 --- a/src/source/worker.js +++ b/src/source/worker.js @@ -30,6 +30,7 @@ export default class Worker { self: WorkerGlobalScopeInterface; actor: Actor; layerIndexes: { [string]: StyleLayerIndex }; + availableImages: { [string]: Array }; workerSourceTypes: { [string]: Class }; workerSources: { [string]: { [string]: { [string]: WorkerSource } } }; demWorkerSources: { [string]: { [string]: RasterDEMTileWorkerSource } }; @@ -40,6 +41,7 @@ export default class Worker { this.actor = new Actor(self, this); this.layerIndexes = {}; + this.availableImages = {}; this.workerSourceTypes = { vector: VectorTileWorkerSource, @@ -71,6 +73,11 @@ export default class Worker { this.referrer = referrer; } + setImages(mapId: string, images: Array, callback: WorkerTileCallback) { + this.availableImages[mapId] = images; + callback(); + } + setLayers(mapId: string, layers: Array, callback: WorkerTileCallback) { this.getLayerIndex(mapId).replace(layers); callback(); @@ -157,6 +164,16 @@ export default class Worker { } } + getAvailableImages(mapId: string) { + let availableImages = this.availableImages[mapId]; + + if (!availableImages) { + availableImages = []; + } + + return availableImages; + } + getLayerIndex(mapId: string) { let layerIndexes = this.layerIndexes[mapId]; if (!layerIndexes) { @@ -179,8 +196,7 @@ export default class Worker { this.actor.send(type, data, callback, mapId); } }; - - this.workerSources[mapId][type][source] = new (this.workerSourceTypes[type]: any)((actor: any), this.getLayerIndex(mapId)); + this.workerSources[mapId][type][source] = new (this.workerSourceTypes[type]: any)((actor: any), this.getLayerIndex(mapId), this.getAvailableImages(mapId)); } return this.workerSources[mapId][type][source]; diff --git a/src/source/worker_tile.js b/src/source/worker_tile.js index c06fef5062b..46e16f3425b 100644 --- a/src/source/worker_tile.js +++ b/src/source/worker_tile.js @@ -60,7 +60,7 @@ class WorkerTile { this.returnDependencies = !!params.returnDependencies; } - parse(data: VectorTile, layerIndex: StyleLayerIndex, actor: Actor, callback: WorkerTileCallback) { + parse(data: VectorTile, layerIndex: StyleLayerIndex, availableImages: Array, actor: Actor, callback: WorkerTileCallback) { this.status = 'parsing'; this.data = data; @@ -76,7 +76,8 @@ class WorkerTile { featureIndex, iconDependencies: {}, patternDependencies: {}, - glyphDependencies: {} + glyphDependencies: {}, + availableImages }; const layerFamilies = layerIndex.familiesBySource[this.source]; diff --git a/src/style-spec/expression/definitions/coercion.js b/src/style-spec/expression/definitions/coercion.js index f15aee6ee79..5e12c9fdb69 100644 --- a/src/style-spec/expression/definitions/coercion.js +++ b/src/style-spec/expression/definitions/coercion.js @@ -116,7 +116,6 @@ class Coercion implements Expression { } serialize() { - console.log('serialize'); if (this.type.kind === 'formatted') { return new FormatExpression([{text: this.args[0], scale: null, font: null, textColor: null}]).serialize(); } diff --git a/src/style-spec/expression/definitions/image.js b/src/style-spec/expression/definitions/image.js index 32eaad4c3c8..025382b733c 100644 --- a/src/style-spec/expression/definitions/image.js +++ b/src/style-spec/expression/definitions/image.js @@ -30,8 +30,14 @@ export default class ImageExpression implements Expression { } evaluate(ctx: EvaluationContext) { - console.log('evaluate', this.input.evaluate(ctx)); - return this.input.evaluate(ctx); + const evaluatedImageName = this.input.evaluate(ctx); + console.log('evaluate', this.input, ctx, evaluatedImageName); + console.log('is it available?', ctx.availableImages.indexOf(evaluatedImageName)); + if (ctx.availableImages.indexOf(evaluatedImageName) > -1) { + return evaluatedImageName; + } + + return null; } eachChild(fn: (Expression) => void) { diff --git a/src/style-spec/expression/definitions/literal.js b/src/style-spec/expression/definitions/literal.js index 2482cd82c09..5bc9578aee5 100644 --- a/src/style-spec/expression/definitions/literal.js +++ b/src/style-spec/expression/definitions/literal.js @@ -54,7 +54,6 @@ class Literal implements Expression { } serialize(): Array { - console.log('literal serialize', this); if (this.type.kind === 'array' || this.type.kind === 'object') { return ["literal", this.value]; } else if (this.value instanceof Color) { diff --git a/src/style-spec/expression/evaluation_context.js b/src/style-spec/expression/evaluation_context.js index a37478c71c0..a3223a4bd29 100644 --- a/src/style-spec/expression/evaluation_context.js +++ b/src/style-spec/expression/evaluation_context.js @@ -11,6 +11,7 @@ class EvaluationContext { feature: ?Feature; featureState: ?FeatureState; formattedSection: ?FormattedSection; + availableImages: ?Array; _parseColorCache: {[string]: ?Color}; @@ -20,6 +21,7 @@ class EvaluationContext { this.featureState = null; this.formattedSection = null; this._parseColorCache = {}; + this.availableImages = null; } id() { diff --git a/src/style-spec/expression/index.js b/src/style-spec/expression/index.js index 9c203d22cc1..fbb35167e29 100644 --- a/src/style-spec/expression/index.js +++ b/src/style-spec/expression/index.js @@ -59,19 +59,21 @@ export class StyleExpression { this._enumValues = propertySpec && propertySpec.type === 'enum' ? propertySpec.values : null; } - evaluateWithoutErrorHandling(globals: GlobalProperties, feature?: Feature, featureState?: FeatureState, formattedSection?: FormattedSection): any { + evaluateWithoutErrorHandling(globals: GlobalProperties, feature?: Feature, featureState?: FeatureState, availableImages?: Array, formattedSection?: FormattedSection): any { this._evaluator.globals = globals; this._evaluator.feature = feature; this._evaluator.featureState = featureState; + this._evaluator.availableImages = availableImages || null; this._evaluator.formattedSection = formattedSection; return this.expression.evaluate(this._evaluator); } - evaluate(globals: GlobalProperties, feature?: Feature, featureState?: FeatureState, formattedSection?: FormattedSection): any { + evaluate(globals: GlobalProperties, feature?: Feature, featureState?: FeatureState, availableImages?: Array, formattedSection?: FormattedSection): any { this._evaluator.globals = globals; this._evaluator.feature = feature || null; this._evaluator.featureState = featureState || null; + this._evaluator.availableImages = availableImages || null; this._evaluator.formattedSection = formattedSection || null; try { @@ -135,12 +137,12 @@ export class ZoomConstantExpression { this.isStateDependent = kind !== ('constant': EvaluationKind) && !isConstant.isStateConstant(expression.expression); } - evaluateWithoutErrorHandling(globals: GlobalProperties, feature?: Feature, featureState?: FeatureState, formattedSection?: FormattedSection): any { - return this._styleExpression.evaluateWithoutErrorHandling(globals, feature, featureState, formattedSection); + evaluateWithoutErrorHandling(globals: GlobalProperties, feature?: Feature, featureState?: FeatureState, availableImages: Array, formattedSection?: FormattedSection): any { + return this._styleExpression.evaluateWithoutErrorHandling(globals, feature, featureState, availableImages, formattedSection); } - evaluate(globals: GlobalProperties, feature?: Feature, featureState?: FeatureState, formattedSection?: FormattedSection): any { - return this._styleExpression.evaluate(globals, feature, featureState, formattedSection); + evaluate(globals: GlobalProperties, feature?: Feature, featureState?: FeatureState, availableImages: Array, formattedSection?: FormattedSection): any { + return this._styleExpression.evaluate(globals, feature, featureState, availableImages, formattedSection); } } diff --git a/src/style/properties.js b/src/style/properties.js index 8ee9a9319f3..b89f2bee96e 100644 --- a/src/style/properties.js +++ b/src/style/properties.js @@ -453,8 +453,8 @@ export class PossiblyEvaluatedPropertyValue { } } - evaluate(feature: Feature, featureState: FeatureState): T { - return this.property.evaluate(this.value, this.parameters, feature, featureState); + evaluate(feature: Feature, featureState: FeatureState, availableImages: Array): T { + return this.property.evaluate(this.value, this.parameters, feature, featureState, availableImages); } } @@ -577,11 +577,11 @@ export class DataDrivenProperty implements Property, parameters: EvaluationParameters, feature: Feature, featureState: FeatureState): T { + evaluate(value: PossiblyEvaluatedValue, parameters: EvaluationParameters, feature: Feature, featureState: FeatureState, availableImages: Array): T { if (value.kind === 'constant') { return value.value; } else { - return value.evaluate(parameters, feature, featureState); + return value.evaluate(parameters, feature, featureState, availableImages); } } } diff --git a/src/style/style.js b/src/style/style.js index 1e364e5904b..6ef4f2d1765 100644 --- a/src/style/style.js +++ b/src/style/style.js @@ -238,6 +238,7 @@ class Style extends Evented { } this.imageManager.setLoaded(true); + this.dispatcher.broadcast('setImages', this.imageManager.listImages()); this.fire(new Event('data', {dataType: 'style'})); }); } else { @@ -256,7 +257,6 @@ class Style extends Evented { layer.setEventedParent(this, {layer: {id: layer.id}}); this._layers[layer.id] = layer; } - this.dispatcher.broadcast('setLayers', this._serializeLayers(this._order)); this.light = new Light(this.stylesheet.light); diff --git a/src/style/style_layer/symbol_style_layer.js b/src/style/style_layer/symbol_style_layer.js index 153d6edbf0d..f5e06a6e539 100644 --- a/src/style/style_layer/symbol_style_layer.js +++ b/src/style/style_layer/symbol_style_layer.js @@ -92,8 +92,8 @@ class SymbolStyleLayer extends StyleLayer { this._setPaintOverrides(); } - getValueAndResolveTokens(name: *, feature: Feature) { - const value = this.layout.get(name).evaluate(feature, {}); + getValueAndResolveTokens(name: *, feature: Feature, availableImages: Array) { + const value = this.layout.get(name).evaluate(feature, {}, availableImages); const unevaluated = this._unevaluatedLayout._values[name]; if (!unevaluated.isDataDriven() && !isExpression(unevaluated.value)) { return resolveTokens(feature.properties, value); From 8c35357850bf186191518d8bbcce494920bc5962 Mon Sep 17 00:00:00 2001 From: ryanhamley Date: Thu, 12 Sep 2019 16:34:13 -0700 Subject: [PATCH 09/23] Make image list available for constant expression values --- src/source/worker_tile.js | 6 +++--- src/style-spec/expression/definitions/image.js | 2 -- src/style-spec/expression/index.js | 5 +++-- src/style-spec/expression/parsing_context.js | 5 +++-- src/style/properties.js | 12 ++++++------ src/style/style.js | 2 +- src/style/style_layer.js | 4 ++-- src/style/style_layer/symbol_style_layer.js | 4 ++-- 8 files changed, 20 insertions(+), 20 deletions(-) diff --git a/src/source/worker_tile.js b/src/source/worker_tile.js index 46e16f3425b..9b6a3b414a0 100644 --- a/src/source/worker_tile.js +++ b/src/source/worker_tile.js @@ -107,7 +107,7 @@ class WorkerTile { if (layer.maxzoom && this.zoom >= layer.maxzoom) continue; if (layer.visibility === 'none') continue; - recalculateLayers(family, this.zoom); + recalculateLayers(family, this.zoom, availableImages); const bucket = buckets[layer.id] = layer.createBucket({ index: featureIndex.bucketLayerIDs.length, @@ -209,11 +209,11 @@ class WorkerTile { } } -function recalculateLayers(layers: $ReadOnlyArray, zoom: number) { +function recalculateLayers(layers: $ReadOnlyArray, zoom: number, availableImages: Array) { // Layers are shared and may have been used by a WorkerTile with a different zoom. const parameters = new EvaluationParameters(zoom); for (const layer of layers) { - layer.recalculate(parameters); + layer.recalculate(parameters, availableImages); } } diff --git a/src/style-spec/expression/definitions/image.js b/src/style-spec/expression/definitions/image.js index 025382b733c..b3cca91898f 100644 --- a/src/style-spec/expression/definitions/image.js +++ b/src/style-spec/expression/definitions/image.js @@ -31,8 +31,6 @@ export default class ImageExpression implements Expression { evaluate(ctx: EvaluationContext) { const evaluatedImageName = this.input.evaluate(ctx); - console.log('evaluate', this.input, ctx, evaluatedImageName); - console.log('is it available?', ctx.availableImages.indexOf(evaluatedImageName)); if (ctx.availableImages.indexOf(evaluatedImageName) > -1) { return evaluatedImageName; } diff --git a/src/style-spec/expression/index.js b/src/style-spec/expression/index.js index fbb35167e29..9654fe0c8b8 100644 --- a/src/style-spec/expression/index.js +++ b/src/style-spec/expression/index.js @@ -350,7 +350,7 @@ function findZoomCurve(expression: Expression): Step | Interpolate | ParsingErro return result; } -import {ColorType, StringType, NumberType, BooleanType, ValueType, FormattedType, array} from './types'; +import { ColorType, StringType, NumberType, BooleanType, ValueType, FormattedType, ImageType, array } from './types'; function getExpectedType(spec: StylePropertySpecification): Type { const types = { @@ -359,7 +359,8 @@ function getExpectedType(spec: StylePropertySpecification): Type { number: NumberType, enum: StringType, boolean: BooleanType, - formatted: FormattedType + formatted: FormattedType, + image: ImageType }; if (spec.type === 'array') { diff --git a/src/style-spec/expression/parsing_context.js b/src/style-spec/expression/parsing_context.js index 13a9d831848..cb2a1e7ee05 100644 --- a/src/style-spec/expression/parsing_context.js +++ b/src/style-spec/expression/parsing_context.js @@ -121,8 +121,9 @@ class ParsingContext { // If an expression's arguments are all literals, we can evaluate // it immediately and replace it with a literal value in the - // parsed/compiled result. - if (!(parsed instanceof Literal) && isConstant(parsed)) { + // parsed/compiled result. Expressions that expect an image should + // not be resolved here so we can later get the available images. + if (!(parsed instanceof Literal) && (parsed.type.kind !== 'image') && isConstant(parsed)) { const ec = new EvaluationContext(); try { parsed = new Literal(parsed.type, parsed.evaluate(ec)); diff --git a/src/style/properties.js b/src/style/properties.js index b89f2bee96e..b5f723649c4 100644 --- a/src/style/properties.js +++ b/src/style/properties.js @@ -105,8 +105,8 @@ export class PropertyValue { return this.expression.kind === 'source' || this.expression.kind === 'composite'; } - possiblyEvaluate(parameters: EvaluationParameters): R { - return this.property.possiblyEvaluate(this, parameters); + possiblyEvaluate(parameters: EvaluationParameters, availableImages: Array): R { + return this.property.possiblyEvaluate(this, parameters, availableImages); } } @@ -385,10 +385,10 @@ export class Layout { return result; } - possiblyEvaluate(parameters: EvaluationParameters): PossiblyEvaluated { + possiblyEvaluate(parameters: EvaluationParameters, availableImages: Array): PossiblyEvaluated { const result = new PossiblyEvaluated(this._properties); // eslint-disable-line no-use-before-define for (const property of Object.keys(this._values)) { - result._values[property] = this._values[property].possiblyEvaluate(parameters); + result._values[property] = this._values[property].possiblyEvaluate(parameters, availableImages); } return result; } @@ -542,9 +542,9 @@ export class DataDrivenProperty implements Property>, parameters: EvaluationParameters): PossiblyEvaluatedPropertyValue { + possiblyEvaluate(value: PropertyValue>, parameters: EvaluationParameters, availableImages: Array): PossiblyEvaluatedPropertyValue { if (value.expression.kind === 'constant' || value.expression.kind === 'camera') { - return new PossiblyEvaluatedPropertyValue(this, {kind: 'constant', value: value.expression.evaluate(parameters)}, parameters); + return new PossiblyEvaluatedPropertyValue(this, {kind: 'constant', value: value.expression.evaluate(parameters, null, null, availableImages)}, parameters); } else { return new PossiblyEvaluatedPropertyValue(this, value.expression, parameters); } diff --git a/src/style/style.js b/src/style/style.js index 6ef4f2d1765..a6cb976f7ec 100644 --- a/src/style/style.js +++ b/src/style/style.js @@ -382,7 +382,7 @@ class Style extends Evented { for (const layerId of this._order) { const layer = this._layers[layerId]; - layer.recalculate(parameters); + layer.recalculate(parameters, this.imageManager.listImages()); if (!layer.isHidden(parameters.zoom) && layer.source) { this.sourceCaches[layer.source].used = true; } diff --git a/src/style/style_layer.js b/src/style/style_layer.js index 5df0f8d09e6..3f6f91db382 100644 --- a/src/style/style_layer.js +++ b/src/style/style_layer.js @@ -193,13 +193,13 @@ class StyleLayer extends Evented { return this._transitioningPaint.hasTransition(); } - recalculate(parameters: EvaluationParameters) { + recalculate(parameters: EvaluationParameters, availableImages: Array) { if (parameters.getCrossfadeParameters) { this._crossfadeParameters = parameters.getCrossfadeParameters(); } if (this._unevaluatedLayout) { - (this: any).layout = this._unevaluatedLayout.possiblyEvaluate(parameters); + (this: any).layout = this._unevaluatedLayout.possiblyEvaluate(parameters, availableImages); } (this: any).paint = this._transitioningPaint.possiblyEvaluate(parameters); diff --git a/src/style/style_layer/symbol_style_layer.js b/src/style/style_layer/symbol_style_layer.js index f5e06a6e539..a7b13975869 100644 --- a/src/style/style_layer/symbol_style_layer.js +++ b/src/style/style_layer/symbol_style_layer.js @@ -48,8 +48,8 @@ class SymbolStyleLayer extends StyleLayer { super(layer, properties); } - recalculate(parameters: EvaluationParameters) { - super.recalculate(parameters); + recalculate(parameters: EvaluationParameters, availableImages: Array) { + super.recalculate(parameters, availableImages); if (this.layout.get('icon-rotation-alignment') === 'auto') { if (this.layout.get('symbol-placement') !== 'point') { From 8f0c6bd78905999fdf380c2ba1dc0f6f04270d5f Mon Sep 17 00:00:00 2001 From: ryanhamley Date: Fri, 13 Sep 2019 14:04:37 -0700 Subject: [PATCH 10/23] Fix lint issues --- bench/lib/tile_parser.js | 2 +- src/data/bucket/symbol_bucket.js | 2 +- src/source/vector_tile_worker_source.js | 2 +- src/source/worker_tile.js | 4 ++-- src/style-spec/expression/definitions/coercion.js | 1 + src/style-spec/expression/definitions/image.js | 4 +--- src/style-spec/expression/index.js | 4 ++-- src/style-spec/expression/values.js | 8 ++++---- src/style/properties.js | 10 +++++----- 9 files changed, 18 insertions(+), 19 deletions(-) diff --git a/bench/lib/tile_parser.js b/bench/lib/tile_parser.js index a30fbbb27d2..1bba3e00182 100644 --- a/bench/lib/tile_parser.js +++ b/bench/lib/tile_parser.js @@ -135,7 +135,7 @@ export default class TileParser { const vectorTile = new VT.VectorTile(new Protobuf(tile.buffer)); return new Promise((resolve, reject) => { - workerTile.parse(vectorTile, this.layerIndex, (this.actor: any), (err, result) => { + workerTile.parse(vectorTile, this.layerIndex, [], (this.actor: any), (err, result) => { if (err) { reject(err); } else { diff --git a/src/data/bucket/symbol_bucket.js b/src/data/bucket/symbol_bucket.js index d2b5240bb0e..fd690cf56f9 100644 --- a/src/data/bucket/symbol_bucket.js +++ b/src/data/bucket/symbol_bucket.js @@ -402,7 +402,7 @@ class SymbolBucket implements Bucket { // Expression evaluation will automatically coerce to Formatted // but plain string token evaluation skips that pathway so do the // conversion here. - const resolvedTokens = layer.getValueAndResolveTokens('text-field', feature); + const resolvedTokens = layer.getValueAndResolveTokens('text-field', feature, availableImages); text = transformText(resolvedTokens instanceof Formatted ? resolvedTokens : Formatted.fromString(resolvedTokens), diff --git a/src/source/vector_tile_worker_source.js b/src/source/vector_tile_worker_source.js index d732b8ddb7a..ecb29a23019 100644 --- a/src/source/vector_tile_worker_source.js +++ b/src/source/vector_tile_worker_source.js @@ -168,7 +168,7 @@ class VectorTileWorkerSource implements WorkerSource { } else if (workerTile.status === 'done') { // if there was no vector tile data on the initial load, don't try and re-parse tile if (workerTile.vectorTile) { - workerTile.parse(workerTile.vectorTile, this.layerIndex, this.actor, done); + workerTile.parse(workerTile.vectorTile, this.layerIndex, this.availableImages, this.actor, done); } else { done(); } diff --git a/src/source/worker_tile.js b/src/source/worker_tile.js index 9b6a3b414a0..17cd7f11b43 100644 --- a/src/source/worker_tile.js +++ b/src/source/worker_tile.js @@ -181,13 +181,13 @@ class WorkerTile { for (const key in buckets) { const bucket = buckets[key]; if (bucket instanceof SymbolBucket) { - recalculateLayers(bucket.layers, this.zoom); + recalculateLayers(bucket.layers, this.zoom, availableImages); performSymbolLayout(bucket, glyphMap, glyphAtlas.positions, iconMap, imageAtlas.iconPositions, this.showCollisionBoxes); } else if (bucket.hasPattern && (bucket instanceof LineBucket || bucket instanceof FillBucket || bucket instanceof FillExtrusionBucket)) { - recalculateLayers(bucket.layers, this.zoom); + recalculateLayers(bucket.layers, this.zoom, availableImages); bucket.addFeatures(options, imageAtlas.patternPositions); } } diff --git a/src/style-spec/expression/definitions/coercion.js b/src/style-spec/expression/definitions/coercion.js index 5e12c9fdb69..f712ecb9ee4 100644 --- a/src/style-spec/expression/definitions/coercion.js +++ b/src/style-spec/expression/definitions/coercion.js @@ -7,6 +7,7 @@ import {Color, toString as valueToString, validateRGBA} from '../values'; import RuntimeError from '../runtime_error'; import Formatted from '../types/formatted'; import FormatExpression from '../definitions/format'; +import ImageExpression from '../definitions/image'; import Image from '../types/image'; import type {Expression} from '../expression'; diff --git a/src/style-spec/expression/definitions/image.js b/src/style-spec/expression/definitions/image.js index b3cca91898f..c812d408360 100644 --- a/src/style-spec/expression/definitions/image.js +++ b/src/style-spec/expression/definitions/image.js @@ -1,8 +1,6 @@ // @flow -import { ValueType, ImageType, StringType } from '../types'; -import Image from '../types/image'; -import { toString } from '../values'; +import { ImageType, StringType } from '../types'; import type { Expression } from '../expression'; import type EvaluationContext from '../evaluation_context'; diff --git a/src/style-spec/expression/index.js b/src/style-spec/expression/index.js index 9654fe0c8b8..bb0b2d529b8 100644 --- a/src/style-spec/expression/index.js +++ b/src/style-spec/expression/index.js @@ -137,11 +137,11 @@ export class ZoomConstantExpression { this.isStateDependent = kind !== ('constant': EvaluationKind) && !isConstant.isStateConstant(expression.expression); } - evaluateWithoutErrorHandling(globals: GlobalProperties, feature?: Feature, featureState?: FeatureState, availableImages: Array, formattedSection?: FormattedSection): any { + evaluateWithoutErrorHandling(globals: GlobalProperties, feature?: Feature, featureState?: FeatureState, availableImages?: Array, formattedSection?: FormattedSection): any { return this._styleExpression.evaluateWithoutErrorHandling(globals, feature, featureState, availableImages, formattedSection); } - evaluate(globals: GlobalProperties, feature?: Feature, featureState?: FeatureState, availableImages: Array, formattedSection?: FormattedSection): any { + evaluate(globals: GlobalProperties, feature?: Feature, featureState?: FeatureState, availableImages?: Array, formattedSection?: FormattedSection): any { return this._styleExpression.evaluate(globals, feature, featureState, availableImages, formattedSection); } } diff --git a/src/style-spec/expression/values.js b/src/style-spec/expression/values.js index 3bf6ea3ea8e..06fa3c5388c 100644 --- a/src/style-spec/expression/values.js +++ b/src/style-spec/expression/values.js @@ -39,14 +39,14 @@ export function isValue(mixed: mixed): boolean { return true; } else if (typeof mixed === 'number') { return true; - } else if (typeof mixed === 'image') { - return true; } else if (mixed instanceof Color) { return true; } else if (mixed instanceof Collator) { return true; } else if (mixed instanceof Formatted) { return true; + } else if (mixed instanceof Image) { + return true; } else if (Array.isArray(mixed)) { for (const item of mixed) { if (!isValue(item)) { @@ -75,14 +75,14 @@ export function typeOf(value: Value): Type { return BooleanType; } else if (typeof value === 'number') { return NumberType; - } else if (typeof value === 'image') { - return ImageType; } else if (value instanceof Color) { return ColorType; } else if (value instanceof Collator) { return CollatorType; } else if (value instanceof Formatted) { return FormattedType; + } else if (value instanceof Image) { + return ImageType; } else if (Array.isArray(value)) { const length = value.length; let itemType: ?Type; diff --git a/src/style/properties.js b/src/style/properties.js index b5f723649c4..35b7d06d4ca 100644 --- a/src/style/properties.js +++ b/src/style/properties.js @@ -105,7 +105,7 @@ export class PropertyValue { return this.expression.kind === 'source' || this.expression.kind === 'composite'; } - possiblyEvaluate(parameters: EvaluationParameters, availableImages: Array): R { + possiblyEvaluate(parameters: EvaluationParameters, availableImages?: Array): R { return this.property.possiblyEvaluate(this, parameters, availableImages); } } @@ -385,7 +385,7 @@ export class Layout { return result; } - possiblyEvaluate(parameters: EvaluationParameters, availableImages: Array): PossiblyEvaluated { + possiblyEvaluate(parameters: EvaluationParameters, availableImages?: Array): PossiblyEvaluated { const result = new PossiblyEvaluated(this._properties); // eslint-disable-line no-use-before-define for (const property of Object.keys(this._values)) { result._values[property] = this._values[property].possiblyEvaluate(parameters, availableImages); @@ -453,7 +453,7 @@ export class PossiblyEvaluatedPropertyValue { } } - evaluate(feature: Feature, featureState: FeatureState, availableImages: Array): T { + evaluate(feature: Feature, featureState: FeatureState, availableImages?: Array): T { return this.property.evaluate(this.value, this.parameters, feature, featureState, availableImages); } } @@ -542,7 +542,7 @@ export class DataDrivenProperty implements Property>, parameters: EvaluationParameters, availableImages: Array): PossiblyEvaluatedPropertyValue { + possiblyEvaluate(value: PropertyValue>, parameters: EvaluationParameters, availableImages?: Array): PossiblyEvaluatedPropertyValue { if (value.expression.kind === 'constant' || value.expression.kind === 'camera') { return new PossiblyEvaluatedPropertyValue(this, {kind: 'constant', value: value.expression.evaluate(parameters, null, null, availableImages)}, parameters); } else { @@ -577,7 +577,7 @@ export class DataDrivenProperty implements Property, parameters: EvaluationParameters, feature: Feature, featureState: FeatureState, availableImages: Array): T { + evaluate(value: PossiblyEvaluatedValue, parameters: EvaluationParameters, feature: Feature, featureState: FeatureState, availableImages?: Array): T { if (value.kind === 'constant') { return value.value; } else { From 0c0eb94b179d7129a1b2c4c82e3422a4c29324c5 Mon Sep 17 00:00:00 2001 From: ryanhamley Date: Mon, 16 Sep 2019 16:11:02 -0700 Subject: [PATCH 11/23] Add Image type to build code --- build/generate-flow-typed-style-spec.js | 2 ++ build/generate-style-code.js | 4 ++++ src/data/bucket/symbol_bucket.js | 6 +++--- src/data/program_configuration.js | 6 +++--- src/source/vector_tile_worker_source.js | 2 +- src/style-spec/expression/definitions/image.js | 4 ++-- src/style-spec/expression/index.js | 16 ++++++++-------- src/style/properties.js | 4 ++-- src/style/style_layer/fill_style_layer.js | 4 ++-- src/style/style_layer/line_style_layer.js | 4 ++-- src/symbol/symbol_layout.js | 6 +++--- 11 files changed, 32 insertions(+), 26 deletions(-) diff --git a/build/generate-flow-typed-style-spec.js b/build/generate-flow-typed-style-spec.js index 13a60878cca..6f2e71cd93b 100644 --- a/build/generate-flow-typed-style-spec.js +++ b/build/generate-flow-typed-style-spec.js @@ -120,6 +120,8 @@ export type ColorSpecification = string; export type FormattedSpecification = string; +export type ImageSpecification = string; + export type FilterSpecification = | ['has', string] | ['!has', string] diff --git a/build/generate-style-code.js b/build/generate-style-code.js index 1cad05c9048..2e816288071 100644 --- a/build/generate-style-code.js +++ b/build/generate-style-code.js @@ -32,6 +32,8 @@ global.flowType = function (property) { return `Color`; case 'formatted': return `Formatted`; + case 'image': + return `Image`; case 'array': if (property.length) { return `[${new Array(property.length).fill(flowType({type: property.value})).join(', ')}]`; @@ -73,6 +75,8 @@ global.runtimeType = function (property) { return `ColorType`; case 'formatted': return `FormattedType`; + case 'Image': + return `ImageType`; case 'array': if (property.length) { return `array(${runtimeType({type: property.value})}, ${property.length})`; diff --git a/src/data/bucket/symbol_bucket.js b/src/data/bucket/symbol_bucket.js index fd690cf56f9..c422d8f4fdf 100644 --- a/src/data/bucket/symbol_bucket.js +++ b/src/data/bucket/symbol_bucket.js @@ -378,7 +378,7 @@ class SymbolBucket implements Bucket { const hasText = (textField.value.kind !== 'constant' || textField.value.value.toString().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; + const hasIcon = iconImage.value.kind !== 'constant' || iconImage.value.value && iconImage.value.value.toString().length > 0; const symbolSortKey = layout.get('symbol-sort-key'); this.features = []; @@ -409,7 +409,7 @@ class SymbolBucket implements Bucket { layer, feature); } - let icon; + let icon: Image | void; if (hasIcon) { // Expression evaluation will automatically coerce to Image // but plain string token evaluation skips that pathway so do the @@ -444,7 +444,7 @@ class SymbolBucket implements Bucket { this.features.push(symbolFeature); if (icon) { - icons[icon] = true; + icons[icon.name] = true; } if (text) { diff --git a/src/data/program_configuration.js b/src/data/program_configuration.js index a056a8bb95f..95cbd501db0 100644 --- a/src/data/program_configuration.js +++ b/src/data/program_configuration.js @@ -222,7 +222,7 @@ class SourceExpressionBinder implements Binder { const start = paintArray.length; paintArray.reserve(newLength); - const value = this.expression.evaluate(new EvaluationParameters(0), feature, {}, formattedSection); + const value = this.expression.evaluate(new EvaluationParameters(0), feature, {}, [], formattedSection); if (this.type === 'color') { const color = packColor(value); @@ -326,8 +326,8 @@ class CompositeExpressionBinder implements Binder { const start = paintArray.length; paintArray.reserve(newLength); - const min = this.expression.evaluate(new EvaluationParameters(this.zoom), feature, {}, formattedSection); - const max = this.expression.evaluate(new EvaluationParameters(this.zoom + 1), feature, {}, formattedSection); + const min = this.expression.evaluate(new EvaluationParameters(this.zoom), feature, {}, [], formattedSection); + const max = this.expression.evaluate(new EvaluationParameters(this.zoom + 1), feature, {}, [], formattedSection); if (this.type === 'color') { const minColor = packColor(min); diff --git a/src/source/vector_tile_worker_source.js b/src/source/vector_tile_worker_source.js index ecb29a23019..cfd597fa0aa 100644 --- a/src/source/vector_tile_worker_source.js +++ b/src/source/vector_tile_worker_source.js @@ -158,7 +158,7 @@ class VectorTileWorkerSource implements WorkerSource { const reloadCallback = workerTile.reloadCallback; if (reloadCallback) { delete workerTile.reloadCallback; - workerTile.parse(workerTile.vectorTile, vtSource.layerIndex, vtSource.actor, reloadCallback); + workerTile.parse(workerTile.vectorTile, vtSource.layerIndex, this.availableImages, vtSource.actor, reloadCallback); } callback(err, data); }; diff --git a/src/style-spec/expression/definitions/image.js b/src/style-spec/expression/definitions/image.js index c812d408360..745c58c0ed2 100644 --- a/src/style-spec/expression/definitions/image.js +++ b/src/style-spec/expression/definitions/image.js @@ -9,7 +9,7 @@ import type { Type } from '../types'; export default class ImageExpression implements Expression { type: Type; - input: string; + input: Expression; constructor(input: Expression) { this.type = ImageType; @@ -29,7 +29,7 @@ export default class ImageExpression implements Expression { evaluate(ctx: EvaluationContext) { const evaluatedImageName = this.input.evaluate(ctx); - if (ctx.availableImages.indexOf(evaluatedImageName) > -1) { + if (ctx.availableImages && ctx.availableImages.indexOf(evaluatedImageName) > -1) { return evaluatedImageName; } diff --git a/src/style-spec/expression/index.js b/src/style-spec/expression/index.js index bb0b2d529b8..d24146d6d91 100644 --- a/src/style-spec/expression/index.js +++ b/src/style-spec/expression/index.js @@ -162,12 +162,12 @@ export class ZoomDependentExpression { this.interpolationType = interpolationType; } - evaluateWithoutErrorHandling(globals: GlobalProperties, feature?: Feature, featureState?: FeatureState, formattedSection?: FormattedSection): any { - return this._styleExpression.evaluateWithoutErrorHandling(globals, feature, featureState, formattedSection); + evaluateWithoutErrorHandling(globals: GlobalProperties, feature?: Feature, featureState?: FeatureState, availableImages?: Array, formattedSection?: FormattedSection): any { + return this._styleExpression.evaluateWithoutErrorHandling(globals, feature, featureState, availableImages, formattedSection); } - evaluate(globals: GlobalProperties, feature?: Feature, featureState?: FeatureState, formattedSection?: FormattedSection): any { - return this._styleExpression.evaluate(globals, feature, featureState, formattedSection); + evaluate(globals: GlobalProperties, feature?: Feature, featureState?: FeatureState, availableImages?: Array, formattedSection?: FormattedSection): any { + return this._styleExpression.evaluate(globals, feature, featureState, availableImages, formattedSection); } interpolationFactor(input: number, lower: number, upper: number): number { @@ -181,18 +181,18 @@ export class ZoomDependentExpression { export type ConstantExpression = { kind: 'constant', - +evaluate: (globals: GlobalProperties, feature?: Feature) => any, + +evaluate: (globals: GlobalProperties, feature?: Feature, featureState?: FeatureState, availableImages?: Array) => any, } export type SourceExpression = { kind: 'source', isStateDependent: boolean, - +evaluate: (globals: GlobalProperties, feature?: Feature, featureState?: FeatureState, formattedSection?: FormattedSection) => any, + +evaluate: (globals: GlobalProperties, feature?: Feature, featureState?: FeatureState, availableImages?: Array, formattedSection?: FormattedSection) => any, }; export type CameraExpression = { kind: 'camera', - +evaluate: (globals: GlobalProperties, feature?: Feature, featureState?: FeatureState) => any, + +evaluate: (globals: GlobalProperties, feature?: Feature, featureState?: FeatureState, availableImages?: Array) => any, +interpolationFactor: (input: number, lower: number, upper: number) => number, zoomStops: Array, interpolationType: ?InterpolationType @@ -201,7 +201,7 @@ export type CameraExpression = { export type CompositeExpression = { kind: 'composite', isStateDependent: boolean, - +evaluate: (globals: GlobalProperties, feature?: Feature, featureState?: FeatureState, formattedSection?: FormattedSection) => any, + +evaluate: (globals: GlobalProperties, feature?: Feature, featureState?: FeatureState, availableImages?: Array, formattedSection?: FormattedSection) => any, +interpolationFactor: (input: number, lower: number, upper: number) => number, zoomStops: Array, interpolationType: ?InterpolationType diff --git a/src/style/properties.js b/src/style/properties.js index 35b7d06d4ca..7ef4557b50d 100644 --- a/src/style/properties.js +++ b/src/style/properties.js @@ -67,7 +67,7 @@ export type CrossFaded = { */ export interface Property { specification: StylePropertySpecification; - possiblyEvaluate(value: PropertyValue, parameters: EvaluationParameters): R; + possiblyEvaluate(value: PropertyValue, parameters: EvaluationParameters, availableImages?: Array): R; interpolate(a: R, b: R, t: number): R; } @@ -544,7 +544,7 @@ export class DataDrivenProperty implements Property>, parameters: EvaluationParameters, availableImages?: Array): PossiblyEvaluatedPropertyValue { if (value.expression.kind === 'constant' || value.expression.kind === 'camera') { - return new PossiblyEvaluatedPropertyValue(this, {kind: 'constant', value: value.expression.evaluate(parameters, null, null, availableImages)}, parameters); + return new PossiblyEvaluatedPropertyValue(this, {kind: 'constant', value: value.expression.evaluate(parameters, (null: any), {}, availableImages)}, parameters); } else { return new PossiblyEvaluatedPropertyValue(this, value.expression, parameters); } diff --git a/src/style/style_layer/fill_style_layer.js b/src/style/style_layer/fill_style_layer.js index d27790e7571..7afb6bca33d 100644 --- a/src/style/style_layer/fill_style_layer.js +++ b/src/style/style_layer/fill_style_layer.js @@ -28,8 +28,8 @@ class FillStyleLayer extends StyleLayer { super(layer, properties); } - recalculate(parameters: EvaluationParameters) { - super.recalculate(parameters); + recalculate(parameters: EvaluationParameters, availableImages: Array) { + super.recalculate(parameters, availableImages); const outlineColor = this.paint._values['fill-outline-color']; if (outlineColor.value.kind === 'constant' && outlineColor.value.value === undefined) { diff --git a/src/style/style_layer/line_style_layer.js b/src/style/style_layer/line_style_layer.js index 037a5fb7779..336e60d45ec 100644 --- a/src/style/style_layer/line_style_layer.js +++ b/src/style/style_layer/line_style_layer.js @@ -69,8 +69,8 @@ class LineStyleLayer extends StyleLayer { this.gradientTexture = null; } - recalculate(parameters: EvaluationParameters) { - super.recalculate(parameters); + recalculate(parameters: EvaluationParameters, availableImages: Array) { + super.recalculate(parameters, availableImages); (this.paint._values: any)['line-floorwidth'] = lineFloorwidthProperty.possiblyEvaluate(this._transitioningPaint._values['line-width'].value, parameters); diff --git a/src/symbol/symbol_layout.js b/src/symbol/symbol_layout.js index fee27b74f1c..468681fe43b 100644 --- a/src/symbol/symbol_layout.js +++ b/src/symbol/symbol_layout.js @@ -289,11 +289,11 @@ export function performSymbolLayout(bucket: SymbolBucket, } let shapedIcon; - if (feature.icon) { - const image = imageMap[feature.icon]; + if (feature.icon && feature.icon.name) { + const image = imageMap[feature.icon.name]; if (image) { shapedIcon = shapeIcon( - imagePositions[feature.icon], + imagePositions[feature.icon.name], layout.get('icon-offset').evaluate(feature, {}), layout.get('icon-anchor').evaluate(feature, {})); if (bucket.sdfIcons === undefined) { From a6519cb93ffb1c37eaabb8a1bf83a51121a22f13 Mon Sep 17 00:00:00 2001 From: ryanhamley Date: Tue, 17 Sep 2019 10:54:54 -0700 Subject: [PATCH 12/23] Ensure pattern properties are working --- src/data/bucket/fill_bucket.js | 2 +- src/data/bucket/pattern_bucket_features.js | 6 ++-- .../expression/definitions/image.js | 1 + src/style-spec/expression/values.js | 1 + src/style/properties.js | 28 +++++++++---------- src/style/style_layer.js | 2 +- 6 files changed, 21 insertions(+), 19 deletions(-) diff --git a/src/data/bucket/fill_bucket.js b/src/data/bucket/fill_bucket.js index f2fa38fa149..39d9901d013 100644 --- a/src/data/bucket/fill_bucket.js +++ b/src/data/bucket/fill_bucket.js @@ -83,7 +83,7 @@ class FillBucket implements Bucket { const geometry = loadGeometry(feature); const sortKey = fillSortKey ? - fillSortKey.evaluate(feature, {}) : + fillSortKey.evaluate(feature, {}, options.availableImages) : undefined; const bucketFeature: BucketFeature = { diff --git a/src/data/bucket/pattern_bucket_features.js b/src/data/bucket/pattern_bucket_features.js index ada1a7aeec5..ca50a2f8fd1 100644 --- a/src/data/bucket/pattern_bucket_features.js +++ b/src/data/bucket/pattern_bucket_features.js @@ -41,9 +41,9 @@ export function addPatternDependencies(type: string, layers: PatternStyleLayers, const patternPropertyValue = patternProperty.value; if (patternPropertyValue.kind !== "constant") { - const min = patternPropertyValue.evaluate({zoom: zoom - 1}, patternFeature, {}); - const mid = patternPropertyValue.evaluate({zoom}, patternFeature, {}); - const max = patternPropertyValue.evaluate({zoom: zoom + 1}, patternFeature, {}); + const min = patternPropertyValue.evaluate({zoom: zoom - 1}, patternFeature, {}, options.availableImages); + const mid = patternPropertyValue.evaluate({zoom}, patternFeature, {}, options.availableImages); + const max = patternPropertyValue.evaluate({zoom: zoom + 1}, patternFeature, {}, options.availableImages); // add to patternDependencies patterns[min] = true; patterns[mid] = true; diff --git a/src/style-spec/expression/definitions/image.js b/src/style-spec/expression/definitions/image.js index 745c58c0ed2..29d2b73f9b9 100644 --- a/src/style-spec/expression/definitions/image.js +++ b/src/style-spec/expression/definitions/image.js @@ -29,6 +29,7 @@ export default class ImageExpression implements Expression { evaluate(ctx: EvaluationContext) { const evaluatedImageName = this.input.evaluate(ctx); + if (ctx.availableImages && ctx.availableImages.indexOf(evaluatedImageName) > -1) { return evaluatedImageName; } diff --git a/src/style-spec/expression/values.js b/src/style-spec/expression/values.js index 06fa3c5388c..405fa6b2144 100644 --- a/src/style-spec/expression/values.js +++ b/src/style-spec/expression/values.js @@ -5,6 +5,7 @@ import assert from 'assert'; import Color from '../util/color'; import Collator from './types/collator'; import Formatted from './types/formatted'; +import Image from './types/image'; import { NullType, NumberType, StringType, BooleanType, ColorType, ObjectType, ValueType, CollatorType, FormattedType, ImageType, array } from './types'; import type {Type} from './types'; diff --git a/src/style/properties.js b/src/style/properties.js index 7ef4557b50d..29d1d45255a 100644 --- a/src/style/properties.js +++ b/src/style/properties.js @@ -264,9 +264,9 @@ class TransitioningPropertyValue { } } - possiblyEvaluate(parameters: EvaluationParameters): R { + possiblyEvaluate(parameters: EvaluationParameters, availableImages: Array): R { const now = parameters.now || 0; - const finalValue = this.value.possiblyEvaluate(parameters); + const finalValue = this.value.possiblyEvaluate(parameters, availableImages); const prior = this.prior; if (!prior) { // No prior value. @@ -283,11 +283,11 @@ class TransitioningPropertyValue { return finalValue; } else if (now < this.begin) { // Transition hasn't started yet. - return prior.possiblyEvaluate(parameters); + return prior.possiblyEvaluate(parameters, availableImages); } else { // Interpolate between recursively-calculated prior value and final. const t = (now - this.begin) / (this.end - this.begin); - return this.property.interpolate(prior.possiblyEvaluate(parameters), finalValue, easeCubicInOut(t)); + return this.property.interpolate(prior.possiblyEvaluate(parameters, availableImages), finalValue, easeCubicInOut(t)); } } } @@ -317,10 +317,10 @@ export class Transitioning { this._values = (Object.create(properties.defaultTransitioningPropertyValues): any); } - possiblyEvaluate(parameters: EvaluationParameters): PossiblyEvaluated { + possiblyEvaluate(parameters: EvaluationParameters, availableImages?: Array): PossiblyEvaluated { const result = new PossiblyEvaluated(this._properties); // eslint-disable-line no-use-before-define for (const property of Object.keys(this._values)) { - result._values[property] = this._values[property].possiblyEvaluate(parameters); + result._values[property] = this._values[property].possiblyEvaluate(parameters, availableImages); } return result; } @@ -595,11 +595,11 @@ export class DataDrivenProperty implements Property extends DataDrivenProperty> { - possiblyEvaluate(value: PropertyValue, PossiblyEvaluatedPropertyValue>>, parameters: EvaluationParameters): PossiblyEvaluatedPropertyValue> { + possiblyEvaluate(value: PropertyValue, PossiblyEvaluatedPropertyValue>>, parameters: EvaluationParameters, availableImages?: Array): PossiblyEvaluatedPropertyValue> { if (value.value === undefined) { return new PossiblyEvaluatedPropertyValue(this, {kind: 'constant', value: undefined}, parameters); } else if (value.expression.kind === 'constant') { - const constantValue = value.expression.evaluate(parameters); + const constantValue = value.expression.evaluate(parameters, (null: any), {}, availableImages); const constant = this._calculate(constantValue, constantValue, constantValue, parameters); return new PossiblyEvaluatedPropertyValue(this, {kind: 'constant', value: constant}, parameters); } else if (value.expression.kind === 'camera') { @@ -615,9 +615,9 @@ export class CrossFadedDataDrivenProperty extends DataDrivenProperty>, globals: EvaluationParameters, feature: Feature, featureState: FeatureState): ?CrossFaded { + evaluate(value: PossiblyEvaluatedValue>, globals: EvaluationParameters, feature: Feature, featureState: FeatureState, availableImages?: Array): ?CrossFaded { if (value.kind === 'source') { - const constant = value.evaluate(globals, feature, featureState); + const constant = value.evaluate(globals, feature, featureState, availableImages); return this._calculate(constant, constant, constant, globals); } else if (value.kind === 'composite') { return this._calculate( @@ -652,11 +652,11 @@ export class CrossFadedProperty implements Property> { this.specification = specification; } - possiblyEvaluate(value: PropertyValue>, parameters: EvaluationParameters): ?CrossFaded { + possiblyEvaluate(value: PropertyValue>, parameters: EvaluationParameters, availableImages?: Array): ?CrossFaded { if (value.value === undefined) { return undefined; } else if (value.expression.kind === 'constant') { - const constant = value.expression.evaluate(parameters); + const constant = value.expression.evaluate(parameters, (null: any), {}, availableImages); return this._calculate(constant, constant, constant, parameters); } else { assert(!value.isDataDriven()); @@ -693,8 +693,8 @@ export class ColorRampProperty implements Property { this.specification = specification; } - possiblyEvaluate(value: PropertyValue, parameters: EvaluationParameters): boolean { - return !!value.expression.evaluate(parameters); + possiblyEvaluate(value: PropertyValue, parameters: EvaluationParameters, availableImages?: Array): boolean { + return !!value.expression.evaluate(parameters, (null: any), {}, availableImages); } interpolate(): boolean { return false; } diff --git a/src/style/style_layer.js b/src/style/style_layer.js index 3f6f91db382..37ce4c510d4 100644 --- a/src/style/style_layer.js +++ b/src/style/style_layer.js @@ -202,7 +202,7 @@ class StyleLayer extends Evented { (this: any).layout = this._unevaluatedLayout.possiblyEvaluate(parameters, availableImages); } - (this: any).paint = this._transitioningPaint.possiblyEvaluate(parameters); + (this: any).paint = this._transitioningPaint.possiblyEvaluate(parameters, availableImages); } serialize() { From 514840066d36962673c3230bdcafc7cb15a6425e Mon Sep 17 00:00:00 2001 From: ryanhamley Date: Tue, 17 Sep 2019 13:23:29 -0700 Subject: [PATCH 13/23] Fix existing tests --- src/style-spec/reference/v8.json | 9 ++++++ .../unit/source/geojson_worker_source.test.js | 10 +++---- .../source/vector_tile_worker_source.test.js | 28 +++++++++---------- test/unit/source/worker_tile.test.js | 8 +++--- test/unit/style-spec/spec.test.js | 3 +- 5 files changed, 34 insertions(+), 24 deletions(-) diff --git a/src/style-spec/reference/v8.json b/src/style-spec/reference/v8.json index 7da8f81e3f3..aa0068873fc 100644 --- a/src/style-spec/reference/v8.json +++ b/src/style-spec/reference/v8.json @@ -2804,6 +2804,15 @@ } } }, + "image": { + "doc": "Returns `image` type for use in `icon-image` and `*-pattern` entries. If set, the `image` argument will check that the requested image exists in the style and will return either the resolved image name or `null`, depending on whether or not the image is currently in the style. This validation process is synchronous and requires the image to have been added to the style before requesting it in the `image` argument.", + "group": "Types", + "sdk-support": { + "basic functionality": { + "js": "1.4.0" + } + } + }, "number-format": { "doc": "Converts the input number into a string representation using the providing formatting rules. If set, the `locale` argument specifies the locale to use, as a BCP 47 language tag. If set, the `currency` argument specifies an ISO 4217 code to use for currency-style formatting. If set, the `min-fraction-digits` and `max-fraction-digits` arguments specify the minimum and maximum number of fractional digits to include.", "group": "Types", diff --git a/test/unit/source/geojson_worker_source.test.js b/test/unit/source/geojson_worker_source.test.js index 94d853020d3..125e377b1d5 100644 --- a/test/unit/source/geojson_worker_source.test.js +++ b/test/unit/source/geojson_worker_source.test.js @@ -14,7 +14,7 @@ test('reloadTile', (t) => { } ]; const layerIndex = new StyleLayerIndex(layers); - const source = new GeoJSONWorkerSource(null, layerIndex); + const source = new GeoJSONWorkerSource(null, layerIndex, []); const originalLoadVectorData = source.loadVectorData; let loadVectorCallCount = 0; source.loadVectorData = function(params, callback) { @@ -126,7 +126,7 @@ test('resourceTiming', (t) => { t.stub(perf, 'getEntriesByName').callsFake(() => { return [ exampleResourceTiming ]; }); const layerIndex = new StyleLayerIndex(layers); - const source = new GeoJSONWorkerSource(null, layerIndex, (params, callback) => { return callback(null, geoJson); }); + const source = new GeoJSONWorkerSource(null, layerIndex, [], (params, callback) => { return callback(null, geoJson); }); source.loadData({source: 'testSource', request: {url: 'http://localhost/nonexistent', collectResourceTiming: true}}, (err, result) => { t.equal(err, null); @@ -158,7 +158,7 @@ test('resourceTiming', (t) => { t.stub(perf, 'clearMeasures').callsFake(() => { return null; }); const layerIndex = new StyleLayerIndex(layers); - const source = new GeoJSONWorkerSource(null, layerIndex, (params, callback) => { return callback(null, geoJson); }); + const source = new GeoJSONWorkerSource(null, layerIndex, [], (params, callback) => { return callback(null, geoJson); }); source.loadData({source: 'testSource', request: {url: 'http://localhost/nonexistent', collectResourceTiming: true}}, (err, result) => { t.equal(err, null); @@ -169,7 +169,7 @@ test('resourceTiming', (t) => { t.test('loadData - data', (t) => { const layerIndex = new StyleLayerIndex(layers); - const source = new GeoJSONWorkerSource(null, layerIndex); + const source = new GeoJSONWorkerSource(null, layerIndex, []); source.loadData({source: 'testSource', data: JSON.stringify(geoJson)}, (err, result) => { t.equal(err, null); @@ -205,7 +205,7 @@ test('loadData', (t) => { const layerIndex = new StyleLayerIndex(layers); function createWorker() { - const worker = new GeoJSONWorkerSource(null, layerIndex); + const worker = new GeoJSONWorkerSource(null, layerIndex, []); // Making the call to loadGeoJSON asynchronous // allows these tests to mimic a message queue building up diff --git a/test/unit/source/vector_tile_worker_source.test.js b/test/unit/source/vector_tile_worker_source.test.js index d2c59d2e17c..c72199204e9 100644 --- a/test/unit/source/vector_tile_worker_source.test.js +++ b/test/unit/source/vector_tile_worker_source.test.js @@ -8,7 +8,7 @@ import StyleLayerIndex from '../../../src/style/style_layer_index'; import perf from '../../../src/util/performance'; test('VectorTileWorkerSource#abortTile aborts pending request', (t) => { - const source = new VectorTileWorkerSource(null, new StyleLayerIndex()); + const source = new VectorTileWorkerSource(null, new StyleLayerIndex(), []); source.loadTile({ source: 'source', @@ -33,7 +33,7 @@ test('VectorTileWorkerSource#abortTile aborts pending request', (t) => { }); test('VectorTileWorkerSource#removeTile removes loaded tile', (t) => { - const source = new VectorTileWorkerSource(null, new StyleLayerIndex()); + const source = new VectorTileWorkerSource(null, new StyleLayerIndex(), []); source.loaded = { '0': {} @@ -52,7 +52,7 @@ test('VectorTileWorkerSource#removeTile removes loaded tile', (t) => { }); test('VectorTileWorkerSource#reloadTile reloads a previously-loaded tile', (t) => { - const source = new VectorTileWorkerSource(null, new StyleLayerIndex()); + const source = new VectorTileWorkerSource(null, new StyleLayerIndex(), []); const parse = t.spy(); source.loaded = { @@ -67,14 +67,14 @@ test('VectorTileWorkerSource#reloadTile reloads a previously-loaded tile', (t) = source.reloadTile({uid: 0}, callback); t.equal(parse.callCount, 1); - parse.firstCall.args[3](); + parse.firstCall.args[4](); t.equal(callback.callCount, 1); t.end(); }); test('VectorTileWorkerSource#reloadTile queues a reload when parsing is in progress', (t) => { - const source = new VectorTileWorkerSource(null, new StyleLayerIndex()); + const source = new VectorTileWorkerSource(null, new StyleLayerIndex(), []); const parse = t.spy(); source.loaded = { @@ -94,12 +94,12 @@ test('VectorTileWorkerSource#reloadTile queues a reload when parsing is in progr source.reloadTile({uid: 0}, callback2); t.equal(parse.callCount, 1); - parse.firstCall.args[3](); + parse.firstCall.args[4](); t.equal(parse.callCount, 2); t.equal(callback1.callCount, 1); t.equal(callback2.callCount, 0); - parse.secondCall.args[3](); + parse.secondCall.args[4](); t.equal(callback1.callCount, 1); t.equal(callback2.callCount, 1); @@ -108,7 +108,7 @@ test('VectorTileWorkerSource#reloadTile queues a reload when parsing is in progr test('VectorTileWorkerSource#reloadTile handles multiple pending reloads', (t) => { // https://github.com/mapbox/mapbox-gl-js/issues/6308 - const source = new VectorTileWorkerSource(null, new StyleLayerIndex()); + const source = new VectorTileWorkerSource(null, new StyleLayerIndex(), []); const parse = t.spy(); source.loaded = { @@ -129,7 +129,7 @@ test('VectorTileWorkerSource#reloadTile handles multiple pending reloads', (t) = source.reloadTile({uid: 0}, callback2); t.equal(parse.callCount, 1); - parse.firstCall.args[3](); + parse.firstCall.args[4](); t.equal(parse.callCount, 2); t.equal(callback1.callCount, 1); t.equal(callback2.callCount, 0); @@ -141,13 +141,13 @@ test('VectorTileWorkerSource#reloadTile handles multiple pending reloads', (t) = t.equal(callback2.callCount, 0); t.equal(callback3.callCount, 0); - parse.secondCall.args[3](); + parse.secondCall.args[4](); t.equal(parse.callCount, 3); t.equal(callback1.callCount, 1); t.equal(callback2.callCount, 1); t.equal(callback3.callCount, 0); - parse.thirdCall.args[3](); + parse.thirdCall.args[4](); t.equal(callback1.callCount, 1); t.equal(callback2.callCount, 1); t.equal(callback3.callCount, 1); @@ -156,7 +156,7 @@ test('VectorTileWorkerSource#reloadTile handles multiple pending reloads', (t) = }); test('VectorTileWorkerSource#reloadTile does not reparse tiles with no vectorTile data but does call callback', (t) => { - const source = new VectorTileWorkerSource(null, new StyleLayerIndex()); + const source = new VectorTileWorkerSource(null, new StyleLayerIndex(), []); const parse = t.spy(); source.loaded = { @@ -215,7 +215,7 @@ test('VectorTileWorkerSource provides resource timing information', (t) => { type: 'fill' }]); - const source = new VectorTileWorkerSource(null, layerIndex, loadVectorData); + const source = new VectorTileWorkerSource(null, layerIndex, [], loadVectorData); t.stub(perf, 'getEntriesByName').callsFake(() => { return [ exampleResourceTiming ]; }); @@ -250,7 +250,7 @@ test('VectorTileWorkerSource provides resource timing information (fallback meth type: 'fill' }]); - const source = new VectorTileWorkerSource(null, layerIndex, loadVectorData); + const source = new VectorTileWorkerSource(null, layerIndex, [], loadVectorData); const sampleMarks = [100, 350]; const marks = {}; diff --git a/test/unit/source/worker_tile.test.js b/test/unit/source/worker_tile.test.js index 4f44da3871e..cd0927e1ef3 100644 --- a/test/unit/source/worker_tile.test.js +++ b/test/unit/source/worker_tile.test.js @@ -32,7 +32,7 @@ test('WorkerTile#parse', (t) => { }]); const tile = createWorkerTile(); - tile.parse(createWrapper(), layerIndex, {}, (err, result) => { + tile.parse(createWrapper(), layerIndex, [], {}, (err, result) => { t.ifError(err); t.ok(result.buckets[0]); t.end(); @@ -48,7 +48,7 @@ test('WorkerTile#parse skips hidden layers', (t) => { }]); const tile = createWorkerTile(); - tile.parse(createWrapper(), layerIndex, {}, (err, result) => { + tile.parse(createWrapper(), layerIndex, [], {}, (err, result) => { t.ifError(err); t.equal(result.buckets.length, 0); t.end(); @@ -64,7 +64,7 @@ test('WorkerTile#parse skips layers without a corresponding source layer', (t) = }]); const tile = createWorkerTile(); - tile.parse({layers: {}}, layerIndex, {}, (err, result) => { + tile.parse({layers: {}}, layerIndex, [], {}, (err, result) => { t.ifError(err); t.equal(result.buckets.length, 0); t.end(); @@ -90,7 +90,7 @@ test('WorkerTile#parse warns once when encountering a v1 vector tile layer', (t) t.stub(console, 'warn'); const tile = createWorkerTile(); - tile.parse(data, layerIndex, {}, (err) => { + tile.parse(data, layerIndex, [], {}, (err) => { t.ifError(err); t.ok(console.warn.calledWithMatch(/does not use vector tile spec v2/)); t.end(); diff --git a/test/unit/style-spec/spec.test.js b/test/unit/style-spec/spec.test.js index e0ec6fc0cc1..16f07cf0342 100644 --- a/test/unit/style-spec/spec.test.js +++ b/test/unit/style-spec/spec.test.js @@ -36,7 +36,8 @@ function validSchema(k, t, obj, ref, version, kind) { 'text-transform-enum', 'visibility-enum', 'property-type', - 'formatted' + 'formatted', + 'image' ]); const keys = [ 'default', From 0a7746338eaffa7e4ba8660d8e1a7c18325e14fb Mon Sep 17 00:00:00 2001 From: ryanhamley Date: Tue, 17 Sep 2019 15:38:23 -0700 Subject: [PATCH 14/23] Add expression tests --- .../expression/definitions/image.js | 2 +- src/style-spec/types.js | 12 +++++---- test/expression.test.js | 5 ++-- .../expression-tests/image/basic/test.json | 27 +++++++++++++++++++ .../expression-tests/image/coalesce/test.json | 26 ++++++++++++++++++ .../expression-tests/image/compound/test.json | 22 +++++++++++++++ .../image/implicit-assert/test.json | 14 ++++++++++ 7 files changed, 100 insertions(+), 8 deletions(-) create mode 100644 test/integration/expression-tests/image/basic/test.json create mode 100644 test/integration/expression-tests/image/coalesce/test.json create mode 100644 test/integration/expression-tests/image/compound/test.json create mode 100644 test/integration/expression-tests/image/implicit-assert/test.json diff --git a/src/style-spec/expression/definitions/image.js b/src/style-spec/expression/definitions/image.js index 29d2b73f9b9..1a60bdb17fb 100644 --- a/src/style-spec/expression/definitions/image.js +++ b/src/style-spec/expression/definitions/image.js @@ -48,6 +48,6 @@ export default class ImageExpression implements Expression { } serialize() { - return ["image", this.input]; + return ["image", this.input.serialize()]; } } diff --git a/src/style-spec/types.js b/src/style-spec/types.js index efddf753d72..3416f7c9fd2 100644 --- a/src/style-spec/types.js +++ b/src/style-spec/types.js @@ -6,6 +6,8 @@ export type ColorSpecification = string; export type FormattedSpecification = string; +export type ImageSpecification = string; + export type FilterSpecification = | ['has', string] | ['!has', string] @@ -166,7 +168,7 @@ export type FillLayerSpecification = {| "fill-outline-color"?: DataDrivenPropertyValueSpecification, "fill-translate"?: PropertyValueSpecification<[number, number]>, "fill-translate-anchor"?: PropertyValueSpecification<"map" | "viewport">, - "fill-pattern"?: DataDrivenPropertyValueSpecification + "fill-pattern"?: DataDrivenPropertyValueSpecification |} |} @@ -197,7 +199,7 @@ export type LineLayerSpecification = {| "line-offset"?: DataDrivenPropertyValueSpecification, "line-blur"?: DataDrivenPropertyValueSpecification, "line-dasharray"?: PropertyValueSpecification>, - "line-pattern"?: DataDrivenPropertyValueSpecification, + "line-pattern"?: DataDrivenPropertyValueSpecification, "line-gradient"?: ExpressionSpecification |} |} @@ -224,7 +226,7 @@ export type SymbolLayerSpecification = {| "icon-size"?: DataDrivenPropertyValueSpecification, "icon-text-fit"?: PropertyValueSpecification<"none" | "width" | "height" | "both">, "icon-text-fit-padding"?: PropertyValueSpecification<[number, number, number, number]>, - "icon-image"?: DataDrivenPropertyValueSpecification, + "icon-image"?: DataDrivenPropertyValueSpecification, "icon-rotate"?: DataDrivenPropertyValueSpecification, "icon-padding"?: PropertyValueSpecification, "icon-keep-upright"?: PropertyValueSpecification, @@ -339,7 +341,7 @@ export type FillExtrusionLayerSpecification = {| "fill-extrusion-color"?: DataDrivenPropertyValueSpecification, "fill-extrusion-translate"?: PropertyValueSpecification<[number, number]>, "fill-extrusion-translate-anchor"?: PropertyValueSpecification<"map" | "viewport">, - "fill-extrusion-pattern"?: DataDrivenPropertyValueSpecification, + "fill-extrusion-pattern"?: DataDrivenPropertyValueSpecification, "fill-extrusion-height"?: DataDrivenPropertyValueSpecification, "fill-extrusion-base"?: DataDrivenPropertyValueSpecification, "fill-extrusion-vertical-gradient"?: PropertyValueSpecification @@ -403,7 +405,7 @@ export type BackgroundLayerSpecification = {| |}, "paint"?: {| "background-color"?: PropertyValueSpecification, - "background-pattern"?: PropertyValueSpecification, + "background-pattern"?: PropertyValueSpecification, "background-opacity"?: PropertyValueSpecification |} |} diff --git a/test/expression.test.js b/test/expression.test.js index 3fc046eb36e..883ea54c213 100644 --- a/test/expression.test.js +++ b/test/expression.test.js @@ -6,6 +6,7 @@ import {toString} from '../src/style-spec/expression/types'; import ignores from './ignores.json'; let tests; +const availableImages = ['monument-15']; if (process.argv[1] === __filename && process.argv.length > 2) { tests = process.argv.slice(2); @@ -54,7 +55,7 @@ run('js', {ignores, tests}, (fixture) => { if ('geometry' in input[1]) { feature.type = input[1].geometry.type; } - let value = expression.evaluateWithoutErrorHandling(input[0], feature); + let value = expression.evaluateWithoutErrorHandling(input[0], feature, {}, availableImages); if (type.kind === 'color') { value = [value.r, value.g, value.b, value.a]; } @@ -82,7 +83,7 @@ run('js', {ignores, tests}, (fixture) => { } })(); - result.outputs = evaluateExpression(expression, result.compiled); + result.outputs = evaluateExpression(expression, result.compiled, {}, availableImages); if (expression.result === 'success') { result.serialized = expression.value._styleExpression.expression.serialize(); result.roundTripOutputs = evaluateExpression( diff --git a/test/integration/expression-tests/image/basic/test.json b/test/integration/expression-tests/image/basic/test.json new file mode 100644 index 00000000000..377de766382 --- /dev/null +++ b/test/integration/expression-tests/image/basic/test.json @@ -0,0 +1,27 @@ +{ + "expression": [ + "image", + "monument-15" + ], + "inputs": [ + [ + {}, + {} + ] + ], + "expected": { + "compiled": { + "result": "success", + "isFeatureConstant": true, + "isZoomConstant": true, + "type": "image" + }, + "outputs": [ + "monument-15" + ], + "serialized": [ + "image", + "monument-15" + ] + } +} diff --git a/test/integration/expression-tests/image/coalesce/test.json b/test/integration/expression-tests/image/coalesce/test.json new file mode 100644 index 00000000000..e6b7347a731 --- /dev/null +++ b/test/integration/expression-tests/image/coalesce/test.json @@ -0,0 +1,26 @@ +{ + "expression": ["coalesce", ["image", "foo"], ["image", "bar"], ["image", "monument-15"]], + "propertySpec": { + "type": "image" + }, + "inputs": [ + [{}, {}] + ], + "expected": { + "compiled": { + "result": "success", + "isFeatureConstant": true, + "isZoomConstant": true, + "type": "image" + }, + "outputs": [ + "monument-15" + ], + "serialized": [ + "coalesce", + ["image", "foo"], + ["image", "bar"], + ["image", "monument-15"] + ] + } +} diff --git a/test/integration/expression-tests/image/compound/test.json b/test/integration/expression-tests/image/compound/test.json new file mode 100644 index 00000000000..1d9f4591f5e --- /dev/null +++ b/test/integration/expression-tests/image/compound/test.json @@ -0,0 +1,22 @@ +{ + "expression": [ + "image", + ["get", "icon"] + ], + "inputs": [[{}, {"properties": {"icon": "monument-15"}}]], + "expected": { + "compiled": { + "result": "success", + "isFeatureConstant": false, + "isZoomConstant": true, + "type": "image" + }, + "outputs": [ + "monument-15" + ], + "serialized": [ + "image", + ["string", ["get", "icon"]] + ] + } +} diff --git a/test/integration/expression-tests/image/implicit-assert/test.json b/test/integration/expression-tests/image/implicit-assert/test.json new file mode 100644 index 00000000000..731a678cc19 --- /dev/null +++ b/test/integration/expression-tests/image/implicit-assert/test.json @@ -0,0 +1,14 @@ +{ + "expression": ["number", ["get", "icon"]], + "propertySpec": { + "type": "image" + }, + "expected": { + "compiled": { + "result": "error", + "errors": [ + {"key": "", "error": "Expected image but found number instead."} + ] + } + } +} From 57b598905b8e0fcf198ab270c07a3182c7a87402 Mon Sep 17 00:00:00 2001 From: ryanhamley Date: Tue, 17 Sep 2019 17:40:10 -0700 Subject: [PATCH 15/23] Fix flow errors --- src/render/draw_background.js | 5 ++--- src/render/draw_fill.js | 4 ++-- src/render/draw_fill_extrusion.js | 4 ++-- src/render/draw_line.js | 4 ++-- src/render/image_atlas.js | 1 - 5 files changed, 8 insertions(+), 10 deletions(-) diff --git a/src/render/draw_background.js b/src/render/draw_background.js index 006f8138e0a..49bee662bc3 100644 --- a/src/render/draw_background.js +++ b/src/render/draw_background.js @@ -25,7 +25,7 @@ function drawBackground(painter: Painter, sourceCache: SourceCache, layer: Backg const transform = painter.transform; const tileSize = transform.tileSize; const image = layer.paint.get('background-pattern'); - if (painter.isPatternMissing(image)) return; + if (painter.isPatternMissing((image: any))) return; const pass = (!image && color.a === 1 && opacity === 1 && painter.opaquePassEnabledForLayer()) ? 'opaque' : 'translucent'; if (painter.renderPass !== pass) return; @@ -46,9 +46,8 @@ function drawBackground(painter: Painter, sourceCache: SourceCache, layer: Backg const crossfade = layer.getCrossfadeParameters(); for (const tileID of tileIDs) { const matrix = painter.transform.calculatePosMatrix(tileID.toUnwrapped()); - const uniformValues = image ? - backgroundPatternUniformValues(matrix, opacity, painter, image, {tileID, tileSize}, crossfade) : + backgroundPatternUniformValues(matrix, opacity, painter, (image: any), {tileID, tileSize}, crossfade) : backgroundUniformValues(matrix, opacity, color); program.draw(context, gl.TRIANGLES, depthMode, stencilMode, colorMode, CullFaceMode.disabled, diff --git a/src/render/draw_fill.js b/src/render/draw_fill.js index 16707d272f9..9bb3ea9cb2b 100644 --- a/src/render/draw_fill.js +++ b/src/render/draw_fill.js @@ -92,8 +92,8 @@ function drawFillTiles(painter, sourceCache, layer, coords, depthMode, colorMode const constantPattern = patternProperty.constantOr(null); if (constantPattern && tile.imageAtlas) { - const posTo = tile.imageAtlas.patternPositions[constantPattern.to]; - const posFrom = tile.imageAtlas.patternPositions[constantPattern.from]; + const posTo = tile.imageAtlas.patternPositions[(constantPattern: any).to]; + const posFrom = tile.imageAtlas.patternPositions[(constantPattern: any).from]; if (posTo && posFrom) programConfiguration.setConstantPatternPositions(posTo, posFrom); } diff --git a/src/render/draw_fill_extrusion.js b/src/render/draw_fill_extrusion.js index 0000edf4665..a9a75016668 100644 --- a/src/render/draw_fill_extrusion.js +++ b/src/render/draw_fill_extrusion.js @@ -71,8 +71,8 @@ function drawExtrusionTiles(painter, source, layer, coords, depthMode, stencilMo const constantPattern = patternProperty.constantOr(null); if (constantPattern && tile.imageAtlas) { - const posTo = tile.imageAtlas.patternPositions[constantPattern.to]; - const posFrom = tile.imageAtlas.patternPositions[constantPattern.from]; + const posTo = tile.imageAtlas.patternPositions[(constantPattern: any).to]; + const posFrom = tile.imageAtlas.patternPositions[(constantPattern: any).from]; if (posTo && posFrom) programConfiguration.setConstantPatternPositions(posTo, posFrom); } diff --git a/src/render/draw_line.js b/src/render/draw_line.js index 3d12a3a5fc9..7ab29abf7be 100644 --- a/src/render/draw_line.js +++ b/src/render/draw_line.js @@ -67,8 +67,8 @@ export default function drawLine(painter: Painter, sourceCache: SourceCache, lay const constantPattern = patternProperty.constantOr(null); if (constantPattern && tile.imageAtlas) { - const posTo = tile.imageAtlas.patternPositions[constantPattern.to]; - const posFrom = tile.imageAtlas.patternPositions[constantPattern.from]; + const posTo = tile.imageAtlas.patternPositions[(constantPattern: any).to]; + const posFrom = tile.imageAtlas.patternPositions[(constantPattern: any).from]; if (posTo && posFrom) programConfiguration.setConstantPatternPositions(posTo, posFrom); } diff --git a/src/render/image_atlas.js b/src/render/image_atlas.js index 18a32928da0..270be1191ee 100644 --- a/src/render/image_atlas.js +++ b/src/render/image_atlas.js @@ -140,4 +140,3 @@ export default class ImageAtlas { register('ImagePosition', ImagePosition); register('ImageAtlas', ImageAtlas); - From 0b5daae37be0310dc626cf96b5b892df20a09db3 Mon Sep 17 00:00:00 2001 From: ryanhamley Date: Tue, 17 Sep 2019 17:59:56 -0700 Subject: [PATCH 16/23] Resolve conflicts --- .../expression/definitions/image.js | 6 ++--- src/style-spec/expression/index.js | 2 +- src/style-spec/expression/types.js | 22 +++++++++---------- src/style-spec/expression/values.js | 2 +- src/style-spec/function/index.js | 4 ++-- 5 files changed, 18 insertions(+), 18 deletions(-) diff --git a/src/style-spec/expression/definitions/image.js b/src/style-spec/expression/definitions/image.js index 1a60bdb17fb..6d19e23330a 100644 --- a/src/style-spec/expression/definitions/image.js +++ b/src/style-spec/expression/definitions/image.js @@ -1,11 +1,11 @@ // @flow -import { ImageType, StringType } from '../types'; +import {ImageType, StringType} from '../types'; -import type { Expression } from '../expression'; +import type {Expression} from '../expression'; import type EvaluationContext from '../evaluation_context'; import type ParsingContext from '../parsing_context'; -import type { Type } from '../types'; +import type {Type} from '../types'; export default class ImageExpression implements Expression { type: Type; diff --git a/src/style-spec/expression/index.js b/src/style-spec/expression/index.js index d24146d6d91..7f6ae543f7c 100644 --- a/src/style-spec/expression/index.js +++ b/src/style-spec/expression/index.js @@ -350,7 +350,7 @@ function findZoomCurve(expression: Expression): Step | Interpolate | ParsingErro return result; } -import { ColorType, StringType, NumberType, BooleanType, ValueType, FormattedType, ImageType, array } from './types'; +import {ColorType, StringType, NumberType, BooleanType, ValueType, FormattedType, ImageType, array} from './types'; function getExpectedType(spec: StylePropertySpecification): Type { const types = { diff --git a/src/style-spec/expression/types.js b/src/style-spec/expression/types.js index c178f85f06d..6e89a87dc3d 100644 --- a/src/style-spec/expression/types.js +++ b/src/style-spec/expression/types.js @@ -34,17 +34,17 @@ export type ArrayType = { N: ?number } -export const NullType = { kind: 'null' }; -export const NumberType = { kind: 'number' }; -export const StringType = { kind: 'string' }; -export const BooleanType = { kind: 'boolean' }; -export const ColorType = { kind: 'color' }; -export const ObjectType = { kind: 'object' }; -export const ValueType = { kind: 'value' }; -export const ErrorType = { kind: 'error' }; -export const CollatorType = { kind: 'collator' }; -export const FormattedType = { kind: 'formatted' }; -export const ImageType = { kind: 'image' }; +export const NullType = {kind: 'null'}; +export const NumberType = {kind: 'number'}; +export const StringType = {kind: 'string'}; +export const BooleanType = {kind: 'boolean'}; +export const ColorType = {kind: 'color'}; +export const ObjectType = {kind: 'object'}; +export const ValueType = {kind: 'value'}; +export const ErrorType = {kind: 'error'}; +export const CollatorType = {kind: 'collator'}; +export const FormattedType = {kind: 'formatted'}; +export const ImageType = {kind: 'image'}; export function array(itemType: Type, N: ?number): ArrayType { return { diff --git a/src/style-spec/expression/values.js b/src/style-spec/expression/values.js index 405fa6b2144..5ba9004a329 100644 --- a/src/style-spec/expression/values.js +++ b/src/style-spec/expression/values.js @@ -6,7 +6,7 @@ import Color from '../util/color'; import Collator from './types/collator'; import Formatted from './types/formatted'; import Image from './types/image'; -import { NullType, NumberType, StringType, BooleanType, ColorType, ObjectType, ValueType, CollatorType, FormattedType, ImageType, array } from './types'; +import {NullType, NumberType, StringType, BooleanType, ColorType, ObjectType, ValueType, CollatorType, FormattedType, ImageType, array} from './types'; import type {Type} from './types'; diff --git a/src/style-spec/function/index.js b/src/style-spec/function/index.js index 8d70dc0803b..13f944288ba 100644 --- a/src/style-spec/function/index.js +++ b/src/style-spec/function/index.js @@ -7,8 +7,8 @@ import * as interpolate from '../util/interpolate'; import Interpolate from '../expression/definitions/interpolate'; import Formatted from '../expression/types/formatted'; import Image from '../expression/types/image'; -import { supportsInterpolation } from '../util/properties'; -import { findStopLessThanOrEqualTo } from '../expression/stops'; +import {supportsInterpolation} from '../util/properties'; +import {findStopLessThanOrEqualTo} from '../expression/stops'; export function isFunction(value) { return typeof value === 'object' && value !== null && !Array.isArray(value); From e83da4341c19ffee9a05c9a0afde9e95e5eea078 Mon Sep 17 00:00:00 2001 From: ryanhamley Date: Tue, 17 Sep 2019 18:02:48 -0700 Subject: [PATCH 17/23] Address feedback --- src/style-spec/expression/definitions/image.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/style-spec/expression/definitions/image.js b/src/style-spec/expression/definitions/image.js index 6d19e23330a..9ba0c4966c4 100644 --- a/src/style-spec/expression/definitions/image.js +++ b/src/style-spec/expression/definitions/image.js @@ -22,7 +22,7 @@ export default class ImageExpression implements Expression { } const name = context.parse(args[1], 1, StringType); - if (!name) return null; + if (!name) return context.error(`No icon name provided.`); return new ImageExpression(name); } @@ -42,8 +42,7 @@ export default class ImageExpression implements Expression { } possibleOutputs() { - // Technically the combinatoric set of all children - // Usually, this.text will be undefined anyway + // The output of image is determined by the list of available images in the evaluation context return [undefined]; } From 6106710d910260c1aa78767721062782af8d9796 Mon Sep 17 00:00:00 2001 From: ryanhamley Date: Wed, 18 Sep 2019 13:28:52 -0700 Subject: [PATCH 18/23] Address feedback --- src/style-spec/expression/definitions/image.js | 2 +- src/style-spec/reference/v8.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/style-spec/expression/definitions/image.js b/src/style-spec/expression/definitions/image.js index 9ba0c4966c4..125d5d2f5ef 100644 --- a/src/style-spec/expression/definitions/image.js +++ b/src/style-spec/expression/definitions/image.js @@ -22,7 +22,7 @@ export default class ImageExpression implements Expression { } const name = context.parse(args[1], 1, StringType); - if (!name) return context.error(`No icon name provided.`); + if (!name) return context.error(`No image name provided.`); return new ImageExpression(name); } diff --git a/src/style-spec/reference/v8.json b/src/style-spec/reference/v8.json index aa0068873fc..c9102eed2b1 100644 --- a/src/style-spec/reference/v8.json +++ b/src/style-spec/reference/v8.json @@ -2805,7 +2805,7 @@ } }, "image": { - "doc": "Returns `image` type for use in `icon-image` and `*-pattern` entries. If set, the `image` argument will check that the requested image exists in the style and will return either the resolved image name or `null`, depending on whether or not the image is currently in the style. This validation process is synchronous and requires the image to have been added to the style before requesting it in the `image` argument.", + "doc": "Returns an `image` type for use in `icon-image` and `*-pattern` entries. If set, the `image` argument will check that the requested image exists in the style and will return either the resolved image name or `null`, depending on whether or not the image is currently in the style. This validation process is synchronous and requires the image to have been added to the style before requesting it in the `image` argument.", "group": "Types", "sdk-support": { "basic functionality": { From 1c07025684800030d60d2fef7b4b3b5b87aecfce Mon Sep 17 00:00:00 2001 From: ryanhamley Date: Wed, 18 Sep 2019 13:53:19 -0700 Subject: [PATCH 19/23] Avoid casting to any type --- src/render/draw_fill.js | 5 +++-- src/render/draw_fill_extrusion.js | 5 +++-- src/render/draw_line.js | 5 +++-- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/render/draw_fill.js b/src/render/draw_fill.js index 9bb3ea9cb2b..3f96530448b 100644 --- a/src/render/draw_fill.js +++ b/src/render/draw_fill.js @@ -92,8 +92,9 @@ function drawFillTiles(painter, sourceCache, layer, coords, depthMode, colorMode const constantPattern = patternProperty.constantOr(null); if (constantPattern && tile.imageAtlas) { - const posTo = tile.imageAtlas.patternPositions[(constantPattern: any).to]; - const posFrom = tile.imageAtlas.patternPositions[(constantPattern: any).from]; + const atlas = tile.imageAtlas; + const posTo = atlas.patternPositions[constantPattern.to.toString()]; + const posFrom = atlas.patternPositions[constantPattern.from.toString()]; if (posTo && posFrom) programConfiguration.setConstantPatternPositions(posTo, posFrom); } diff --git a/src/render/draw_fill_extrusion.js b/src/render/draw_fill_extrusion.js index a9a75016668..07076bc3977 100644 --- a/src/render/draw_fill_extrusion.js +++ b/src/render/draw_fill_extrusion.js @@ -71,8 +71,9 @@ function drawExtrusionTiles(painter, source, layer, coords, depthMode, stencilMo const constantPattern = patternProperty.constantOr(null); if (constantPattern && tile.imageAtlas) { - const posTo = tile.imageAtlas.patternPositions[(constantPattern: any).to]; - const posFrom = tile.imageAtlas.patternPositions[(constantPattern: any).from]; + const atlas = tile.imageAtlas; + const posTo = atlas.patternPositions[constantPattern.to.toString()]; + const posFrom = atlas.patternPositions[constantPattern.from.toString()]; if (posTo && posFrom) programConfiguration.setConstantPatternPositions(posTo, posFrom); } diff --git a/src/render/draw_line.js b/src/render/draw_line.js index 7ab29abf7be..474bbb2f40f 100644 --- a/src/render/draw_line.js +++ b/src/render/draw_line.js @@ -67,8 +67,9 @@ export default function drawLine(painter: Painter, sourceCache: SourceCache, lay const constantPattern = patternProperty.constantOr(null); if (constantPattern && tile.imageAtlas) { - const posTo = tile.imageAtlas.patternPositions[(constantPattern: any).to]; - const posFrom = tile.imageAtlas.patternPositions[(constantPattern: any).from]; + const atlas = tile.imageAtlas + const posTo = atlas.patternPositions[constantPattern.to.toString()]; + const posFrom = atlas.patternPositions[constantPattern.from.toString()]; if (posTo && posFrom) programConfiguration.setConstantPatternPositions(posTo, posFrom); } From ea1d4d01cfe9bc88f12edfe3fea81bf4d4347ce9 Mon Sep 17 00:00:00 2001 From: ryanhamley Date: Wed, 18 Sep 2019 16:05:02 -0700 Subject: [PATCH 20/23] Rename Image type to ResolvedImage --- src/data/bucket/symbol_bucket.js | 10 +++++----- src/style-spec/expression/definitions/coercion.js | 4 ++-- .../expression/types/{image.js => resolved_image.js} | 6 +++--- src/style-spec/expression/values.js | 10 +++++----- src/style-spec/function/index.js | 4 ++-- .../style_layer/background_style_layer_properties.js | 4 ++-- .../fill_extrusion_style_layer_properties.js | 4 ++-- src/style/style_layer/fill_style_layer_properties.js | 4 ++-- src/style/style_layer/line_style_layer_properties.js | 4 ++-- src/style/style_layer/symbol_style_layer_properties.js | 4 ++-- 10 files changed, 27 insertions(+), 27 deletions(-) rename src/style-spec/expression/types/{image.js => resolved_image.js} (62%) diff --git a/src/data/bucket/symbol_bucket.js b/src/data/bucket/symbol_bucket.js index c422d8f4fdf..6a1e4e54258 100644 --- a/src/data/bucket/symbol_bucket.js +++ b/src/data/bucket/symbol_bucket.js @@ -36,7 +36,7 @@ import {getSizeData} from '../../symbol/symbol_size'; import {register} from '../../util/web_worker_transfer'; import EvaluationParameters from '../../style/evaluation_parameters'; import Formatted from '../../style-spec/expression/types/formatted'; -import Image from '../../style-spec/expression/types/image'; +import ResolvedImage from '../../style-spec/expression/types/resolved_image'; import type { Bucket, @@ -77,7 +77,7 @@ export type CollisionArrays = { export type SymbolFeature = {| sortKey: number | void, text: Formatted | void, - icon: Image | void, + icon: ResolvedImage | void, index: number, sourceLayerIndex: number, geometry: Array>, @@ -409,15 +409,15 @@ class SymbolBucket implements Bucket { layer, feature); } - let icon: Image | void; + let icon: ResolvedImage | void; if (hasIcon) { // Expression evaluation will automatically coerce to Image // but plain string token evaluation skips that pathway so do the // conversion here. const resolvedTokens = layer.getValueAndResolveTokens('icon-image', feature, availableImages); - icon = resolvedTokens instanceof Image ? + icon = resolvedTokens instanceof ResolvedImage ? resolvedTokens : - Image.fromString(resolvedTokens); + ResolvedImage.fromString(resolvedTokens); } if (!text && !icon) { diff --git a/src/style-spec/expression/definitions/coercion.js b/src/style-spec/expression/definitions/coercion.js index f712ecb9ee4..67bcf649405 100644 --- a/src/style-spec/expression/definitions/coercion.js +++ b/src/style-spec/expression/definitions/coercion.js @@ -8,7 +8,7 @@ import RuntimeError from '../runtime_error'; import Formatted from '../types/formatted'; import FormatExpression from '../definitions/format'; import ImageExpression from '../definitions/image'; -import Image from '../types/image'; +import ResolvedImage from '../types/resolved_image'; import type {Expression} from '../expression'; import type ParsingContext from '../parsing_context'; @@ -102,7 +102,7 @@ class Coercion implements Expression { // created by properties that expect the 'formatted' type. return Formatted.fromString(valueToString(this.args[0].evaluate(ctx))); } else if (this.type.kind === 'image') { - return Image.fromString(valueToString(this.args[0].evaluate(ctx))); + return ResolvedImage.fromString(valueToString(this.args[0].evaluate(ctx))); } else { return valueToString(this.args[0].evaluate(ctx)); } diff --git a/src/style-spec/expression/types/image.js b/src/style-spec/expression/types/resolved_image.js similarity index 62% rename from src/style-spec/expression/types/image.js rename to src/style-spec/expression/types/resolved_image.js index cf3f7c647cc..ec850b971f3 100644 --- a/src/style-spec/expression/types/image.js +++ b/src/style-spec/expression/types/resolved_image.js @@ -1,6 +1,6 @@ // @flow -export default class Image { +export default class ResolvedImage { name: string; constructor(name: string) { @@ -11,8 +11,8 @@ export default class Image { return this.name; } - static fromString(imageName: string): Image { - return new Image(imageName); + static fromString(imageName: string): ResolvedImage { + return new ResolvedImage(imageName); } serialize(): Array { diff --git a/src/style-spec/expression/values.js b/src/style-spec/expression/values.js index 5ba9004a329..56aa68b5cae 100644 --- a/src/style-spec/expression/values.js +++ b/src/style-spec/expression/values.js @@ -5,7 +5,7 @@ import assert from 'assert'; import Color from '../util/color'; import Collator from './types/collator'; import Formatted from './types/formatted'; -import Image from './types/image'; +import ResolvedImage from './types/resolved_image'; import {NullType, NumberType, StringType, BooleanType, ColorType, ObjectType, ValueType, CollatorType, FormattedType, ImageType, array} from './types'; import type {Type} from './types'; @@ -29,7 +29,7 @@ export function validateRGBA(r: mixed, g: mixed, b: mixed, a?: mixed): ?string { return null; } -export type Value = null | string | boolean | number | Color | Collator | Formatted | Image | $ReadOnlyArray | { +[string]: Value } +export type Value = null | string | boolean | number | Color | Collator | Formatted | ResolvedImage | $ReadOnlyArray | { +[string]: Value } export function isValue(mixed: mixed): boolean { if (mixed === null) { @@ -46,7 +46,7 @@ export function isValue(mixed: mixed): boolean { return true; } else if (mixed instanceof Formatted) { return true; - } else if (mixed instanceof Image) { + } else if (mixed instanceof ResolvedImage) { return true; } else if (Array.isArray(mixed)) { for (const item of mixed) { @@ -82,7 +82,7 @@ export function typeOf(value: Value): Type { return CollatorType; } else if (value instanceof Formatted) { return FormattedType; - } else if (value instanceof Image) { + } else if (value instanceof ResolvedImage) { return ImageType; } else if (Array.isArray(value)) { const length = value.length; @@ -113,7 +113,7 @@ export function toString(value: Value) { return ''; } else if (type === 'string' || type === 'number' || type === 'boolean') { return String(value); - } else if (value instanceof Color || value instanceof Formatted || value instanceof Image) { + } else if (value instanceof Color || value instanceof Formatted || value instanceof ResolvedImage) { return value.toString(); } else { return JSON.stringify(value); diff --git a/src/style-spec/function/index.js b/src/style-spec/function/index.js index 13f944288ba..476e3297b10 100644 --- a/src/style-spec/function/index.js +++ b/src/style-spec/function/index.js @@ -6,7 +6,7 @@ import getType from '../util/get_type'; import * as interpolate from '../util/interpolate'; import Interpolate from '../expression/definitions/interpolate'; import Formatted from '../expression/types/formatted'; -import Image from '../expression/types/image'; +import ResolvedImage from '../expression/types/resolved_image'; import {supportsInterpolation} from '../util/properties'; import {findStopLessThanOrEqualTo} from '../expression/stops'; @@ -203,7 +203,7 @@ function evaluateIdentityFunction(parameters, propertySpec, input) { } else if (propertySpec.type === 'formatted') { input = Formatted.fromString(input.toString()); } else if (propertySpec.type === 'image') { - input = Image.fromString(input.toString()); + input = ResolvedImage.fromString(input.toString()); } else if (getType(input) !== propertySpec.type && (propertySpec.type !== 'enum' || !propertySpec.values[input])) { input = undefined; } diff --git a/src/style/style_layer/background_style_layer_properties.js b/src/style/style_layer/background_style_layer_properties.js index 594d80ff299..3d7e45e4ab9 100644 --- a/src/style/style_layer/background_style_layer_properties.js +++ b/src/style/style_layer/background_style_layer_properties.js @@ -17,11 +17,11 @@ import type Color from '../../style-spec/util/color'; import type Formatted from '../../style-spec/expression/types/formatted'; -import type Image from '../../style-spec/expression/types/image'; +import type ResolvedImage from '../../style-spec/expression/types/resolved_image'; export type PaintProps = {| "background-color": DataConstantProperty, - "background-pattern": CrossFadedProperty, + "background-pattern": CrossFadedProperty, "background-opacity": DataConstantProperty, |}; diff --git a/src/style/style_layer/fill_extrusion_style_layer_properties.js b/src/style/style_layer/fill_extrusion_style_layer_properties.js index 2312be4ac87..6d13b8639a8 100644 --- a/src/style/style_layer/fill_extrusion_style_layer_properties.js +++ b/src/style/style_layer/fill_extrusion_style_layer_properties.js @@ -17,14 +17,14 @@ import type Color from '../../style-spec/util/color'; import type Formatted from '../../style-spec/expression/types/formatted'; -import type Image from '../../style-spec/expression/types/image'; +import type ResolvedImage from '../../style-spec/expression/types/resolved_image'; export type PaintProps = {| "fill-extrusion-opacity": DataConstantProperty, "fill-extrusion-color": DataDrivenProperty, "fill-extrusion-translate": DataConstantProperty<[number, number]>, "fill-extrusion-translate-anchor": DataConstantProperty<"map" | "viewport">, - "fill-extrusion-pattern": CrossFadedDataDrivenProperty, + "fill-extrusion-pattern": CrossFadedDataDrivenProperty, "fill-extrusion-height": DataDrivenProperty, "fill-extrusion-base": DataDrivenProperty, "fill-extrusion-vertical-gradient": DataConstantProperty, diff --git a/src/style/style_layer/fill_style_layer_properties.js b/src/style/style_layer/fill_style_layer_properties.js index afff377f113..55ff413208d 100644 --- a/src/style/style_layer/fill_style_layer_properties.js +++ b/src/style/style_layer/fill_style_layer_properties.js @@ -17,7 +17,7 @@ import type Color from '../../style-spec/util/color'; import type Formatted from '../../style-spec/expression/types/formatted'; -import type Image from '../../style-spec/expression/types/image'; +import type ResolvedImage from '../../style-spec/expression/types/resolved_image'; export type LayoutProps = {| "fill-sort-key": DataDrivenProperty, @@ -34,7 +34,7 @@ export type PaintProps = {| "fill-outline-color": DataDrivenProperty, "fill-translate": DataConstantProperty<[number, number]>, "fill-translate-anchor": DataConstantProperty<"map" | "viewport">, - "fill-pattern": CrossFadedDataDrivenProperty, + "fill-pattern": CrossFadedDataDrivenProperty, |}; const paint: Properties = new Properties({ diff --git a/src/style/style_layer/line_style_layer_properties.js b/src/style/style_layer/line_style_layer_properties.js index 2c3b5f101b1..55c635f71e4 100644 --- a/src/style/style_layer/line_style_layer_properties.js +++ b/src/style/style_layer/line_style_layer_properties.js @@ -17,7 +17,7 @@ import type Color from '../../style-spec/util/color'; import type Formatted from '../../style-spec/expression/types/formatted'; -import type Image from '../../style-spec/expression/types/image'; +import type ResolvedImage from '../../style-spec/expression/types/resolved_image'; export type LayoutProps = {| "line-cap": DataConstantProperty<"butt" | "round" | "square">, @@ -45,7 +45,7 @@ export type PaintProps = {| "line-offset": DataDrivenProperty, "line-blur": DataDrivenProperty, "line-dasharray": CrossFadedProperty>, - "line-pattern": CrossFadedDataDrivenProperty, + "line-pattern": CrossFadedDataDrivenProperty, "line-gradient": ColorRampProperty, |}; diff --git a/src/style/style_layer/symbol_style_layer_properties.js b/src/style/style_layer/symbol_style_layer_properties.js index 89b8d8bb0d1..d6df3d9974b 100644 --- a/src/style/style_layer/symbol_style_layer_properties.js +++ b/src/style/style_layer/symbol_style_layer_properties.js @@ -17,7 +17,7 @@ import type Color from '../../style-spec/util/color'; import type Formatted from '../../style-spec/expression/types/formatted'; -import type Image from '../../style-spec/expression/types/image'; +import type ResolvedImage from '../../style-spec/expression/types/resolved_image'; import { ColorType @@ -36,7 +36,7 @@ export type LayoutProps = {| "icon-size": DataDrivenProperty, "icon-text-fit": DataConstantProperty<"none" | "width" | "height" | "both">, "icon-text-fit-padding": DataConstantProperty<[number, number, number, number]>, - "icon-image": DataDrivenProperty, + "icon-image": DataDrivenProperty, "icon-rotate": DataDrivenProperty, "icon-padding": DataConstantProperty, "icon-keep-upright": DataConstantProperty, From 41c783ace52b3c28f13da6453465e9eb3f25b50b Mon Sep 17 00:00:00 2001 From: ryanhamley Date: Wed, 18 Sep 2019 16:05:11 -0700 Subject: [PATCH 21/23] Remove cast to any --- src/render/draw_background.js | 2 +- src/render/painter.js | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/render/draw_background.js b/src/render/draw_background.js index 49bee662bc3..1b445e62e66 100644 --- a/src/render/draw_background.js +++ b/src/render/draw_background.js @@ -25,7 +25,7 @@ function drawBackground(painter: Painter, sourceCache: SourceCache, layer: Backg const transform = painter.transform; const tileSize = transform.tileSize; const image = layer.paint.get('background-pattern'); - if (painter.isPatternMissing((image: any))) return; + if (painter.isPatternMissing(image)) return; const pass = (!image && color.a === 1 && opacity === 1 && painter.opaquePassEnabledForLayer()) ? 'opaque' : 'translucent'; if (painter.renderPass !== pass) return; diff --git a/src/render/painter.js b/src/render/painter.js index 056fbccfac8..cd6d894280d 100644 --- a/src/render/painter.js +++ b/src/render/painter.js @@ -62,6 +62,7 @@ import type GlyphManager from './glyph_manager'; import type VertexBuffer from '../gl/vertex_buffer'; import type IndexBuffer from '../gl/index_buffer'; import type {DepthRangeType, DepthMaskType, DepthFuncType} from '../gl/types'; +import type ResolvedImage from '../style-spec/expression/types/resolved_image'; export type RenderPass = 'offscreen' | 'opaque' | 'translucent'; @@ -508,10 +509,10 @@ class Painter { * * @returns true if a needed image is missing and rendering needs to be skipped. */ - isPatternMissing(image: ?CrossFaded): boolean { + isPatternMissing(image: ?CrossFaded): boolean { if (!image) return false; - const imagePosA = this.imageManager.getPattern(image.from); - const imagePosB = this.imageManager.getPattern(image.to); + const imagePosA = this.imageManager.getPattern(image.from.toString()); + const imagePosB = this.imageManager.getPattern(image.to.toString()); return !imagePosA || !imagePosB; } From adf6a95ae0440acf7694986fa57b9cad4479f27b Mon Sep 17 00:00:00 2001 From: ryanhamley Date: Wed, 18 Sep 2019 16:05:41 -0700 Subject: [PATCH 22/23] Lint fix --- src/render/draw_line.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/render/draw_line.js b/src/render/draw_line.js index 474bbb2f40f..9848dd19b19 100644 --- a/src/render/draw_line.js +++ b/src/render/draw_line.js @@ -67,7 +67,7 @@ export default function drawLine(painter: Painter, sourceCache: SourceCache, lay const constantPattern = patternProperty.constantOr(null); if (constantPattern && tile.imageAtlas) { - const atlas = tile.imageAtlas + const atlas = tile.imageAtlas; const posTo = atlas.patternPositions[constantPattern.to.toString()]; const posFrom = atlas.patternPositions[constantPattern.from.toString()]; if (posTo && posFrom) programConfiguration.setConstantPatternPositions(posTo, posFrom); From 844ea8dc0e62930f748c91e2930444edcb75eb4d Mon Sep 17 00:00:00 2001 From: ryanhamley Date: Wed, 18 Sep 2019 16:27:23 -0700 Subject: [PATCH 23/23] Remove last cast to any --- src/render/draw_background.js | 2 +- src/render/program/background_program.js | 3 ++- src/render/program/pattern.js | 7 ++++--- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/render/draw_background.js b/src/render/draw_background.js index 1b445e62e66..3855b519dab 100644 --- a/src/render/draw_background.js +++ b/src/render/draw_background.js @@ -47,7 +47,7 @@ function drawBackground(painter: Painter, sourceCache: SourceCache, layer: Backg for (const tileID of tileIDs) { const matrix = painter.transform.calculatePosMatrix(tileID.toUnwrapped()); const uniformValues = image ? - backgroundPatternUniformValues(matrix, opacity, painter, (image: any), {tileID, tileSize}, crossfade) : + backgroundPatternUniformValues(matrix, opacity, painter, image, {tileID, tileSize}, crossfade) : backgroundUniformValues(matrix, opacity, color); program.draw(context, gl.TRIANGLES, depthMode, stencilMode, colorMode, CullFaceMode.disabled, diff --git a/src/render/program/background_program.js b/src/render/program/background_program.js index 4399ac58f8d..1ae1cf9943c 100644 --- a/src/render/program/background_program.js +++ b/src/render/program/background_program.js @@ -17,6 +17,7 @@ import type Color from '../../style-spec/util/color'; import type {CrossFaded} from '../../style/properties'; import type {CrossfadeParameters} from '../../style/evaluation_parameters'; import type {OverscaledTileID} from '../../source/tile_id'; +import type ResolvedImage from '../../style-spec/expression/types/resolved_image'; export type BackgroundUniformsType = {| 'u_matrix': UniformMatrix4f, @@ -83,7 +84,7 @@ const backgroundPatternUniformValues = ( matrix: Float32Array, opacity: number, painter: Painter, - image: CrossFaded, + image: CrossFaded, tile: {tileID: OverscaledTileID, tileSize: number}, crossfade: CrossfadeParameters ): UniformValues => extend( diff --git a/src/render/program/pattern.js b/src/render/program/pattern.js index 33f731155a9..d33718abc9f 100644 --- a/src/render/program/pattern.js +++ b/src/render/program/pattern.js @@ -16,6 +16,7 @@ import type {CrossFaded} from '../../style/properties'; import type {CrossfadeParameters} from '../../style/evaluation_parameters'; import type {UniformValues} from '../uniform_binding'; import type Tile from '../../source/tile'; +import type ResolvedImage from '../../style-spec/expression/types/resolved_image'; type BackgroundPatternUniformsType = {| 'u_image': Uniform1i, @@ -68,11 +69,11 @@ function patternUniformValues(crossfade: CrossfadeParameters, painter: Painter, }; } -function bgPatternUniformValues(image: CrossFaded, crossfade: CrossfadeParameters, painter: Painter, +function bgPatternUniformValues(image: CrossFaded, crossfade: CrossfadeParameters, painter: Painter, tile: {tileID: OverscaledTileID, tileSize: number} ): UniformValues { - const imagePosA = painter.imageManager.getPattern(image.from); - const imagePosB = painter.imageManager.getPattern(image.to); + const imagePosA = painter.imageManager.getPattern(image.from.toString()); + const imagePosB = painter.imageManager.getPattern(image.to.toString()); assert(imagePosA && imagePosB); const {width, height} = painter.imageManager.getPixelSize();