From 30ac8804229156ac3fba9cf5d6f7cc4c5d59d123 Mon Sep 17 00:00:00 2001 From: ryanhamley Date: Wed, 18 Sep 2019 21:06:26 -0700 Subject: [PATCH 01/15] Partial implementation of styleimagemissing --- src/data/bucket/symbol_bucket.js | 2 +- src/style/style_layer/symbol_style_layer.js | 17 +++++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/src/data/bucket/symbol_bucket.js b/src/data/bucket/symbol_bucket.js index 6a1e4e54258..3170b57ba45 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.toString().length > 0; + const hasIcon = iconImage.value.kind !== 'constant' || iconImage.property.specification.type === 'image' || iconImage.value.value && iconImage.value.value.toString().length > 0; const symbolSortKey = layout.get('symbol-sort-key'); this.features = []; diff --git a/src/style/style_layer/symbol_style_layer.js b/src/style/style_layer/symbol_style_layer.js index a7b13975869..b81da5bf6d6 100644 --- a/src/style/style_layer/symbol_style_layer.js +++ b/src/style/style_layer/symbol_style_layer.js @@ -17,6 +17,7 @@ import { } from '../properties'; import { + createExpression, isExpression, StyleExpression, ZoomConstantExpression, @@ -95,10 +96,26 @@ class SymbolStyleLayer extends StyleLayer { getValueAndResolveTokens(name: *, feature: Feature, availableImages: Array) { const value = this.layout.get(name).evaluate(feature, {}, availableImages); const unevaluated = this._unevaluatedLayout._values[name]; + // debugger; if (!unevaluated.isDataDriven() && !isExpression(unevaluated.value)) { return resolveTokens(feature.properties, value); } + if (!value && unevaluated.property.specification.type === 'image') { + const unevaluatedValue = unevaluated.value; + if (Array.isArray(unevaluatedValue)) { + for (let i = 0; i < unevaluatedValue.length; i++) { + if (unevaluatedValue[i] === 'image') { + return unevaluatedValue[1]; + } else if (Array.isArray(unevaluatedValue[i]) && unevaluatedValue[i][0] === 'image') { + return unevaluatedValue[i][1]; + } + } + } + + return unevaluated.value; + } + return value; } From 273b146a7b9a451f5c0f491bb145872b1af57898 Mon Sep 17 00:00:00 2001 From: ryanhamley Date: Wed, 18 Sep 2019 21:14:29 -0700 Subject: [PATCH 02/15] Cleanup --- src/style/style_layer/symbol_style_layer.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/style/style_layer/symbol_style_layer.js b/src/style/style_layer/symbol_style_layer.js index b81da5bf6d6..b5690d617c6 100644 --- a/src/style/style_layer/symbol_style_layer.js +++ b/src/style/style_layer/symbol_style_layer.js @@ -17,7 +17,6 @@ import { } from '../properties'; import { - createExpression, isExpression, StyleExpression, ZoomConstantExpression, @@ -96,7 +95,6 @@ class SymbolStyleLayer extends StyleLayer { getValueAndResolveTokens(name: *, feature: Feature, availableImages: Array) { const value = this.layout.get(name).evaluate(feature, {}, availableImages); const unevaluated = this._unevaluatedLayout._values[name]; - // debugger; if (!unevaluated.isDataDriven() && !isExpression(unevaluated.value)) { return resolveTokens(feature.properties, value); } From b46932f6b7c0f825cb62c538f3edd0c8bec33b1c Mon Sep 17 00:00:00 2001 From: ryanhamley Date: Thu, 19 Sep 2019 16:45:20 -0700 Subject: [PATCH 03/15] Return object from Image operator --- src/data/bucket/symbol_bucket.js | 9 ++++-- .../expression/definitions/coalesce.js | 5 +++ .../expression/definitions/image.js | 5 +-- .../expression/types/resolved_image.js | 15 ++++++--- src/style/style_layer/symbol_style_layer.js | 31 ++++++++++--------- src/symbol/symbol_layout.js | 1 + 6 files changed, 44 insertions(+), 22 deletions(-) diff --git a/src/data/bucket/symbol_bucket.js b/src/data/bucket/symbol_bucket.js index 3170b57ba45..08a01ef13e0 100644 --- a/src/data/bucket/symbol_bucket.js +++ b/src/data/bucket/symbol_bucket.js @@ -378,7 +378,10 @@ 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.property.specification.type === 'image' || iconImage.value.value && iconImage.value.value.toString().length > 0; + // debugger; + console.log('iconImage', iconImage); + const hasIcon = iconImage.value.kind !== 'constant' || !!iconImage.value.value; + console.log('hasIcon', hasIcon); const symbolSortKey = layout.get('symbol-sort-key'); this.features = []; @@ -415,9 +418,11 @@ class SymbolBucket implements Bucket { // but plain string token evaluation skips that pathway so do the // conversion here. const resolvedTokens = layer.getValueAndResolveTokens('icon-image', feature, availableImages); + console.log('resolveTokens', resolvedTokens instanceof ResolvedImage, resolvedTokens); icon = resolvedTokens instanceof ResolvedImage ? resolvedTokens : - ResolvedImage.fromString(resolvedTokens); + typeof resolvedTokens === 'string'? ResolvedImage.fromString({name: resolvedTokens, available: false}) : ResolvedImage.fromString(resolvedTokens); + console.log('icon', icon); } if (!text && !icon) { diff --git a/src/style-spec/expression/definitions/coalesce.js b/src/style-spec/expression/definitions/coalesce.js index 5d0c82cf3dd..72a1dc5a93a 100644 --- a/src/style-spec/expression/definitions/coalesce.js +++ b/src/style-spec/expression/definitions/coalesce.js @@ -55,6 +55,11 @@ class Coalesce implements Expression { let result = null; for (const arg of this.args) { result = arg.evaluate(ctx); + + if (arg.type.kind === 'image' && !result.available) { + result = null; + } + if (result !== null) break; } return result; diff --git a/src/style-spec/expression/definitions/image.js b/src/style-spec/expression/definitions/image.js index 125d5d2f5ef..f02e8ff78ff 100644 --- a/src/style-spec/expression/definitions/image.js +++ b/src/style-spec/expression/definitions/image.js @@ -29,12 +29,13 @@ export default class ImageExpression implements Expression { evaluate(ctx: EvaluationContext) { const evaluatedImageName = this.input.evaluate(ctx); + let available = false; if (ctx.availableImages && ctx.availableImages.indexOf(evaluatedImageName) > -1) { - return evaluatedImageName; + available = true; } - return null; + return {name: evaluatedImageName, available}; } eachChild(fn: (Expression) => void) { diff --git a/src/style-spec/expression/types/resolved_image.js b/src/style-spec/expression/types/resolved_image.js index ec850b971f3..1da2107f569 100644 --- a/src/style-spec/expression/types/resolved_image.js +++ b/src/style-spec/expression/types/resolved_image.js @@ -1,18 +1,25 @@ // @flow +export type ResolvedImageOptions = { + name: string, + available: boolean +}; + export default class ResolvedImage { name: string; + available: boolean; - constructor(name: string) { - this.name = name; + constructor(options: ResolvedImageOptions) { + this.name = options.name; + this.available = options.available; } toString(): string { return this.name; } - static fromString(imageName: string): ResolvedImage { - return new ResolvedImage(imageName); + static fromString(options: ResolvedImageOptions): ResolvedImage { + return new ResolvedImage(options); } serialize(): Array { diff --git a/src/style/style_layer/symbol_style_layer.js b/src/style/style_layer/symbol_style_layer.js index b5690d617c6..e1463c3f22b 100644 --- a/src/style/style_layer/symbol_style_layer.js +++ b/src/style/style_layer/symbol_style_layer.js @@ -94,25 +94,28 @@ class SymbolStyleLayer extends StyleLayer { getValueAndResolveTokens(name: *, feature: Feature, availableImages: Array) { const value = this.layout.get(name).evaluate(feature, {}, availableImages); + console.log('value', value); const unevaluated = this._unevaluatedLayout._values[name]; if (!unevaluated.isDataDriven() && !isExpression(unevaluated.value)) { return resolveTokens(feature.properties, value); } - if (!value && unevaluated.property.specification.type === 'image') { - const unevaluatedValue = unevaluated.value; - if (Array.isArray(unevaluatedValue)) { - for (let i = 0; i < unevaluatedValue.length; i++) { - if (unevaluatedValue[i] === 'image') { - return unevaluatedValue[1]; - } else if (Array.isArray(unevaluatedValue[i]) && unevaluatedValue[i][0] === 'image') { - return unevaluatedValue[i][1]; - } - } - } - - return unevaluated.value; - } + // debugger; + // if (unevaluated.property.specification.type === 'image' && !value.available) { + + // const unevaluatedValue = unevaluated.value; + // if (Array.isArray(unevaluatedValue)) { + // for (let i = 0; i < unevaluatedValue.length; i++) { + // if (unevaluatedValue[i] === 'image') { + // return unevaluatedValue[1]; + // } else if (Array.isArray(unevaluatedValue[i]) && unevaluatedValue[i][0] === 'image') { + // return unevaluatedValue[i][1]; + // } + // } + // } + // + // return unevaluated.value; + // } return value; } diff --git a/src/symbol/symbol_layout.js b/src/symbol/symbol_layout.js index 468681fe43b..b304307d01d 100644 --- a/src/symbol/symbol_layout.js +++ b/src/symbol/symbol_layout.js @@ -289,6 +289,7 @@ export function performSymbolLayout(bucket: SymbolBucket, } let shapedIcon; + console.log('feature', feature); if (feature.icon && feature.icon.name) { const image = imageMap[feature.icon.name]; if (image) { From 86bcb926f51741c4eede4db519a69e5e4459e5e2 Mon Sep 17 00:00:00 2001 From: ryanhamley Date: Thu, 19 Sep 2019 17:48:25 -0700 Subject: [PATCH 04/15] Allow coalesce to work with Image object --- src/data/bucket/symbol_bucket.js | 7 +++++-- src/style-spec/expression/definitions/coalesce.js | 8 ++++++++ 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/data/bucket/symbol_bucket.js b/src/data/bucket/symbol_bucket.js index 08a01ef13e0..2b1f1e9aac2 100644 --- a/src/data/bucket/symbol_bucket.js +++ b/src/data/bucket/symbol_bucket.js @@ -380,7 +380,7 @@ class SymbolBucket implements Bucket { (textFont.value.kind !== 'constant' || textFont.value.value.length > 0); // debugger; console.log('iconImage', iconImage); - const hasIcon = iconImage.value.kind !== 'constant' || !!iconImage.value.value; + const hasIcon = iconImage.value.kind !== 'constant' || !!iconImage.value.value || !!iconImage.parameters; console.log('hasIcon', hasIcon); const symbolSortKey = layout.get('symbol-sort-key'); @@ -418,10 +418,13 @@ class SymbolBucket implements Bucket { // but plain string token evaluation skips that pathway so do the // conversion here. const resolvedTokens = layer.getValueAndResolveTokens('icon-image', feature, availableImages); + if (!resolvedTokens) { + console.log('layout', layout); + } console.log('resolveTokens', resolvedTokens instanceof ResolvedImage, resolvedTokens); icon = resolvedTokens instanceof ResolvedImage ? resolvedTokens : - typeof resolvedTokens === 'string'? ResolvedImage.fromString({name: resolvedTokens, available: false}) : ResolvedImage.fromString(resolvedTokens); + !resolvedTokens || typeof resolvedTokens === 'string'? ResolvedImage.fromString({name: resolvedTokens, available: false}) : ResolvedImage.fromString(resolvedTokens); console.log('icon', icon); } diff --git a/src/style-spec/expression/definitions/coalesce.js b/src/style-spec/expression/definitions/coalesce.js index 72a1dc5a93a..3280a681911 100644 --- a/src/style-spec/expression/definitions/coalesce.js +++ b/src/style-spec/expression/definitions/coalesce.js @@ -53,11 +53,19 @@ class Coalesce implements Expression { evaluate(ctx: EvaluationContext) { let result = null; + let argCount = 0; + let requestedImageName; + // debugger; for (const arg of this.args) { + argCount++; result = arg.evaluate(ctx); if (arg.type.kind === 'image' && !result.available) { + if (!requestedImageName) requestedImageName = arg.input.value; result = null; + if (argCount === this.args.length) { + result = requestedImageName; + } } if (result !== null) break; From 63d2ea4935441ae64d62a05526064f92eac999ab Mon Sep 17 00:00:00 2001 From: ryanhamley Date: Thu, 19 Sep 2019 17:57:23 -0700 Subject: [PATCH 05/15] Fix lint --- src/data/bucket/symbol_bucket.js | 2 +- src/style/style_layer/symbol_style_layer.js | 24 ++++++++++----------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/data/bucket/symbol_bucket.js b/src/data/bucket/symbol_bucket.js index 2b1f1e9aac2..a02c0922997 100644 --- a/src/data/bucket/symbol_bucket.js +++ b/src/data/bucket/symbol_bucket.js @@ -424,7 +424,7 @@ class SymbolBucket implements Bucket { console.log('resolveTokens', resolvedTokens instanceof ResolvedImage, resolvedTokens); icon = resolvedTokens instanceof ResolvedImage ? resolvedTokens : - !resolvedTokens || typeof resolvedTokens === 'string'? ResolvedImage.fromString({name: resolvedTokens, available: false}) : ResolvedImage.fromString(resolvedTokens); + !resolvedTokens || typeof resolvedTokens === 'string' ? ResolvedImage.fromString({name: resolvedTokens, available: false}) : ResolvedImage.fromString(resolvedTokens); console.log('icon', icon); } diff --git a/src/style/style_layer/symbol_style_layer.js b/src/style/style_layer/symbol_style_layer.js index e1463c3f22b..957103db85c 100644 --- a/src/style/style_layer/symbol_style_layer.js +++ b/src/style/style_layer/symbol_style_layer.js @@ -103,18 +103,18 @@ class SymbolStyleLayer extends StyleLayer { // debugger; // if (unevaluated.property.specification.type === 'image' && !value.available) { - // const unevaluatedValue = unevaluated.value; - // if (Array.isArray(unevaluatedValue)) { - // for (let i = 0; i < unevaluatedValue.length; i++) { - // if (unevaluatedValue[i] === 'image') { - // return unevaluatedValue[1]; - // } else if (Array.isArray(unevaluatedValue[i]) && unevaluatedValue[i][0] === 'image') { - // return unevaluatedValue[i][1]; - // } - // } - // } - // - // return unevaluated.value; + // const unevaluatedValue = unevaluated.value; + // if (Array.isArray(unevaluatedValue)) { + // for (let i = 0; i < unevaluatedValue.length; i++) { + // if (unevaluatedValue[i] === 'image') { + // return unevaluatedValue[1]; + // } else if (Array.isArray(unevaluatedValue[i]) && unevaluatedValue[i][0] === 'image') { + // return unevaluatedValue[i][1]; + // } + // } + // } + // + // return unevaluated.value; // } return value; From 9ae1733130efcab7523195812fe4851899ba4377 Mon Sep 17 00:00:00 2001 From: ryanhamley Date: Thu, 19 Sep 2019 18:18:29 -0700 Subject: [PATCH 06/15] Clean up code --- src/data/bucket/symbol_bucket.js | 8 -------- src/source/geojson_worker_source.js | 1 + .../expression/definitions/coalesce.js | 1 - src/style/style_layer/symbol_style_layer.js | 20 +------------------ src/symbol/symbol_layout.js | 1 - 5 files changed, 2 insertions(+), 29 deletions(-) diff --git a/src/data/bucket/symbol_bucket.js b/src/data/bucket/symbol_bucket.js index a02c0922997..0fded54a993 100644 --- a/src/data/bucket/symbol_bucket.js +++ b/src/data/bucket/symbol_bucket.js @@ -378,10 +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); - // debugger; - console.log('iconImage', iconImage); const hasIcon = iconImage.value.kind !== 'constant' || !!iconImage.value.value || !!iconImage.parameters; - console.log('hasIcon', hasIcon); const symbolSortKey = layout.get('symbol-sort-key'); this.features = []; @@ -418,14 +415,9 @@ class SymbolBucket implements Bucket { // but plain string token evaluation skips that pathway so do the // conversion here. const resolvedTokens = layer.getValueAndResolveTokens('icon-image', feature, availableImages); - if (!resolvedTokens) { - console.log('layout', layout); - } - console.log('resolveTokens', resolvedTokens instanceof ResolvedImage, resolvedTokens); icon = resolvedTokens instanceof ResolvedImage ? resolvedTokens : !resolvedTokens || typeof resolvedTokens === 'string' ? ResolvedImage.fromString({name: resolvedTokens, available: false}) : ResolvedImage.fromString(resolvedTokens); - console.log('icon', icon); } if (!text && !icon) { diff --git a/src/source/geojson_worker_source.js b/src/source/geojson_worker_source.js index be29d9a2c61..be1e1f33956 100644 --- a/src/source/geojson_worker_source.js +++ b/src/source/geojson_worker_source.js @@ -264,6 +264,7 @@ class GeoJSONWorkerSource extends VectorTileWorkerSource { if (params.request) { getJSON(params.request, callback); } else if (typeof params.data === 'string') { + console.log('params.data', JSON.parse(params.data)); try { return callback(null, JSON.parse(params.data)); } catch (e) { diff --git a/src/style-spec/expression/definitions/coalesce.js b/src/style-spec/expression/definitions/coalesce.js index 3280a681911..e14761ca715 100644 --- a/src/style-spec/expression/definitions/coalesce.js +++ b/src/style-spec/expression/definitions/coalesce.js @@ -55,7 +55,6 @@ class Coalesce implements Expression { let result = null; let argCount = 0; let requestedImageName; - // debugger; for (const arg of this.args) { argCount++; result = arg.evaluate(ctx); diff --git a/src/style/style_layer/symbol_style_layer.js b/src/style/style_layer/symbol_style_layer.js index 957103db85c..9c67acab500 100644 --- a/src/style/style_layer/symbol_style_layer.js +++ b/src/style/style_layer/symbol_style_layer.js @@ -94,29 +94,11 @@ class SymbolStyleLayer extends StyleLayer { getValueAndResolveTokens(name: *, feature: Feature, availableImages: Array) { const value = this.layout.get(name).evaluate(feature, {}, availableImages); - console.log('value', value); const unevaluated = this._unevaluatedLayout._values[name]; - if (!unevaluated.isDataDriven() && !isExpression(unevaluated.value)) { + if (!unevaluated.isDataDriven() && !isExpression(unevaluated.value) && value) { return resolveTokens(feature.properties, value); } - // debugger; - // if (unevaluated.property.specification.type === 'image' && !value.available) { - - // const unevaluatedValue = unevaluated.value; - // if (Array.isArray(unevaluatedValue)) { - // for (let i = 0; i < unevaluatedValue.length; i++) { - // if (unevaluatedValue[i] === 'image') { - // return unevaluatedValue[1]; - // } else if (Array.isArray(unevaluatedValue[i]) && unevaluatedValue[i][0] === 'image') { - // return unevaluatedValue[i][1]; - // } - // } - // } - // - // return unevaluated.value; - // } - return value; } diff --git a/src/symbol/symbol_layout.js b/src/symbol/symbol_layout.js index b304307d01d..468681fe43b 100644 --- a/src/symbol/symbol_layout.js +++ b/src/symbol/symbol_layout.js @@ -289,7 +289,6 @@ export function performSymbolLayout(bucket: SymbolBucket, } let shapedIcon; - console.log('feature', feature); if (feature.icon && feature.icon.name) { const image = imageMap[feature.icon.name]; if (image) { From 2f6721c4e4ece5bfeb2c28eaedcdf12de530e91a Mon Sep 17 00:00:00 2001 From: ryanhamley Date: Thu, 19 Sep 2019 21:40:37 -0700 Subject: [PATCH 07/15] mostly fix the tests --- src/source/geojson_worker_source.js | 1 - src/style-spec/expression/definitions/coercion.js | 3 ++- src/style-spec/function/index.js | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/source/geojson_worker_source.js b/src/source/geojson_worker_source.js index be1e1f33956..be29d9a2c61 100644 --- a/src/source/geojson_worker_source.js +++ b/src/source/geojson_worker_source.js @@ -264,7 +264,6 @@ class GeoJSONWorkerSource extends VectorTileWorkerSource { if (params.request) { getJSON(params.request, callback); } else if (typeof params.data === 'string') { - console.log('params.data', JSON.parse(params.data)); try { return callback(null, JSON.parse(params.data)); } catch (e) { diff --git a/src/style-spec/expression/definitions/coercion.js b/src/style-spec/expression/definitions/coercion.js index 67bcf649405..e50c1eb6ddd 100644 --- a/src/style-spec/expression/definitions/coercion.js +++ b/src/style-spec/expression/definitions/coercion.js @@ -102,7 +102,8 @@ 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 ResolvedImage.fromString(valueToString(this.args[0].evaluate(ctx))); + const name = valueToString(this.args[0].evaluate(ctx)); + return ResolvedImage.fromString({name, available: false}); } else { return valueToString(this.args[0].evaluate(ctx)); } diff --git a/src/style-spec/function/index.js b/src/style-spec/function/index.js index 476e3297b10..3d2da186117 100644 --- a/src/style-spec/function/index.js +++ b/src/style-spec/function/index.js @@ -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 = ResolvedImage.fromString(input.toString()); + input = ResolvedImage.fromString({name: input.toString(), available: false}); } else if (getType(input) !== propertySpec.type && (propertySpec.type !== 'enum' || !propertySpec.values[input])) { input = undefined; } From 181fea9ab541e30e48112daaeb0936260a30c938 Mon Sep 17 00:00:00 2001 From: ryanhamley Date: Thu, 19 Sep 2019 21:43:50 -0700 Subject: [PATCH 08/15] Fix expression tests --- test/integration/expression-tests/image/basic/test.json | 2 +- test/integration/expression-tests/image/coalesce/test.json | 2 +- test/integration/expression-tests/image/compound/test.json | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/test/integration/expression-tests/image/basic/test.json b/test/integration/expression-tests/image/basic/test.json index 377de766382..b0b0bded6c3 100644 --- a/test/integration/expression-tests/image/basic/test.json +++ b/test/integration/expression-tests/image/basic/test.json @@ -17,7 +17,7 @@ "type": "image" }, "outputs": [ - "monument-15" + {"name":"monument-15","available":true} ], "serialized": [ "image", diff --git a/test/integration/expression-tests/image/coalesce/test.json b/test/integration/expression-tests/image/coalesce/test.json index e6b7347a731..9e58bdcdea4 100644 --- a/test/integration/expression-tests/image/coalesce/test.json +++ b/test/integration/expression-tests/image/coalesce/test.json @@ -14,7 +14,7 @@ "type": "image" }, "outputs": [ - "monument-15" + {"name":"monument-15","available":true} ], "serialized": [ "coalesce", diff --git a/test/integration/expression-tests/image/compound/test.json b/test/integration/expression-tests/image/compound/test.json index 1d9f4591f5e..ad7ec7311df 100644 --- a/test/integration/expression-tests/image/compound/test.json +++ b/test/integration/expression-tests/image/compound/test.json @@ -12,7 +12,7 @@ "type": "image" }, "outputs": [ - "monument-15" + {"name":"monument-15","available":true} ], "serialized": [ "image", From 2d48c39085e0b89ebfae9f4dab9d952b5b2defd7 Mon Sep 17 00:00:00 2001 From: ryanhamley Date: Fri, 20 Sep 2019 11:19:18 -0700 Subject: [PATCH 09/15] Fix Flow issue --- src/style-spec/expression/definitions/coalesce.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/style-spec/expression/definitions/coalesce.js b/src/style-spec/expression/definitions/coalesce.js index e14761ca715..25a256b231d 100644 --- a/src/style-spec/expression/definitions/coalesce.js +++ b/src/style-spec/expression/definitions/coalesce.js @@ -9,6 +9,7 @@ import type ParsingContext from '../parsing_context'; import type EvaluationContext from '../evaluation_context'; import type {Value} from '../values'; import type {Type} from '../types'; +import type ImageExpression from './image'; class Coalesce implements Expression { type: Type; @@ -60,7 +61,7 @@ class Coalesce implements Expression { result = arg.evaluate(ctx); if (arg.type.kind === 'image' && !result.available) { - if (!requestedImageName) requestedImageName = arg.input.value; + if (!requestedImageName) requestedImageName = arg.evaluate(ctx).name; result = null; if (argCount === this.args.length) { result = requestedImageName; From 1a5b2ab250e0e735d3c793c43dc231ac1b9110e1 Mon Sep 17 00:00:00 2001 From: ryanhamley Date: Fri, 20 Sep 2019 11:22:42 -0700 Subject: [PATCH 10/15] Fix lint issue --- src/style-spec/expression/definitions/coalesce.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/style-spec/expression/definitions/coalesce.js b/src/style-spec/expression/definitions/coalesce.js index 25a256b231d..20a6c4a1955 100644 --- a/src/style-spec/expression/definitions/coalesce.js +++ b/src/style-spec/expression/definitions/coalesce.js @@ -9,7 +9,6 @@ import type ParsingContext from '../parsing_context'; import type EvaluationContext from '../evaluation_context'; import type {Value} from '../values'; import type {Type} from '../types'; -import type ImageExpression from './image'; class Coalesce implements Expression { type: Type; From 13cfbf7e95a28e12725b68dbf57468bc2a2ea8b8 Mon Sep 17 00:00:00 2001 From: ryanhamley Date: Fri, 20 Sep 2019 12:01:33 -0700 Subject: [PATCH 11/15] Mock actor in worker source tests --- test/unit/source/geojson_worker_source.test.js | 12 +++++++----- .../source/vector_tile_worker_source.test.js | 18 ++++++++++-------- 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/test/unit/source/geojson_worker_source.test.js b/test/unit/source/geojson_worker_source.test.js index 125e377b1d5..a53088e5e92 100644 --- a/test/unit/source/geojson_worker_source.test.js +++ b/test/unit/source/geojson_worker_source.test.js @@ -4,6 +4,8 @@ import StyleLayerIndex from '../../../src/style/style_layer_index'; import {OverscaledTileID} from '../../../src/source/tile_id'; import perf from '../../../src/util/performance'; +const actor = {send: () => {}}; + test('reloadTile', (t) => { t.test('does not rebuild vector data unless data has changed', (t) => { const layers = [ @@ -14,7 +16,7 @@ test('reloadTile', (t) => { } ]; const layerIndex = new StyleLayerIndex(layers); - const source = new GeoJSONWorkerSource(null, layerIndex, []); + const source = new GeoJSONWorkerSource(actor, layerIndex, []); const originalLoadVectorData = source.loadVectorData; let loadVectorCallCount = 0; source.loadVectorData = function(params, callback) { @@ -126,7 +128,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(actor, 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 +160,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(actor, 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 +171,7 @@ test('resourceTiming', (t) => { t.test('loadData - data', (t) => { const layerIndex = new StyleLayerIndex(layers); - const source = new GeoJSONWorkerSource(null, layerIndex, []); + const source = new GeoJSONWorkerSource(actor, layerIndex, []); source.loadData({source: 'testSource', data: JSON.stringify(geoJson)}, (err, result) => { t.equal(err, null); @@ -205,7 +207,7 @@ test('loadData', (t) => { const layerIndex = new StyleLayerIndex(layers); function createWorker() { - const worker = new GeoJSONWorkerSource(null, layerIndex, []); + const worker = new GeoJSONWorkerSource(actor, 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 c72199204e9..df90a9e4632 100644 --- a/test/unit/source/vector_tile_worker_source.test.js +++ b/test/unit/source/vector_tile_worker_source.test.js @@ -7,8 +7,10 @@ import VectorTileWorkerSource from '../../../src/source/vector_tile_worker_sourc import StyleLayerIndex from '../../../src/style/style_layer_index'; import perf from '../../../src/util/performance'; +const actor = {send: () => {}}; + test('VectorTileWorkerSource#abortTile aborts pending request', (t) => { - const source = new VectorTileWorkerSource(null, new StyleLayerIndex(), []); + const source = new VectorTileWorkerSource(actor, new StyleLayerIndex(), []); source.loadTile({ source: 'source', @@ -33,7 +35,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(actor, new StyleLayerIndex(), []); source.loaded = { '0': {} @@ -52,7 +54,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(actor, new StyleLayerIndex(), []); const parse = t.spy(); source.loaded = { @@ -74,7 +76,7 @@ test('VectorTileWorkerSource#reloadTile reloads a previously-loaded tile', (t) = }); test('VectorTileWorkerSource#reloadTile queues a reload when parsing is in progress', (t) => { - const source = new VectorTileWorkerSource(null, new StyleLayerIndex(), []); + const source = new VectorTileWorkerSource(actor, new StyleLayerIndex(), []); const parse = t.spy(); source.loaded = { @@ -108,7 +110,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(actor, new StyleLayerIndex(), []); const parse = t.spy(); source.loaded = { @@ -156,7 +158,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(actor, new StyleLayerIndex(), []); const parse = t.spy(); source.loaded = { @@ -215,7 +217,7 @@ test('VectorTileWorkerSource provides resource timing information', (t) => { type: 'fill' }]); - const source = new VectorTileWorkerSource(null, layerIndex, [], loadVectorData); + const source = new VectorTileWorkerSource(actor, layerIndex, [], loadVectorData); t.stub(perf, 'getEntriesByName').callsFake(() => { return [ exampleResourceTiming ]; }); @@ -250,7 +252,7 @@ test('VectorTileWorkerSource provides resource timing information (fallback meth type: 'fill' }]); - const source = new VectorTileWorkerSource(null, layerIndex, [], loadVectorData); + const source = new VectorTileWorkerSource(actor, layerIndex, [], loadVectorData); const sampleMarks = [100, 350]; const marks = {}; From cd63a7846bb10efd198e4635ef0a217c2d7fdc9c Mon Sep 17 00:00:00 2001 From: ryanhamley Date: Fri, 20 Sep 2019 14:30:25 -0700 Subject: [PATCH 12/15] Fix check for undefined icon-image property --- src/data/bucket/symbol_bucket.js | 6 +++++- src/style-spec/expression/definitions/coalesce.js | 2 ++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/data/bucket/symbol_bucket.js b/src/data/bucket/symbol_bucket.js index 0fded54a993..f092f8dfa8f 100644 --- a/src/data/bucket/symbol_bucket.js +++ b/src/data/bucket/symbol_bucket.js @@ -378,7 +378,11 @@ 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.parameters; + // we should always resolve the icon-image value if the property was defined in the style + // this allows us to fire the styleimagemissing event if image evaluation returns null + // the only way to distinguish between null returned from a coalesce statement with no valid images + // and null returned because icon-image wasn't defined is to check whether or not iconImage.parameters is an empty object + const hasIcon = iconImage.value.kind !== 'constant' || !!iconImage.value.value || Object.keys(iconImage.parameters).length > 0; const symbolSortKey = layout.get('symbol-sort-key'); this.features = []; diff --git a/src/style-spec/expression/definitions/coalesce.js b/src/style-spec/expression/definitions/coalesce.js index 20a6c4a1955..889959cb351 100644 --- a/src/style-spec/expression/definitions/coalesce.js +++ b/src/style-spec/expression/definitions/coalesce.js @@ -59,6 +59,8 @@ class Coalesce implements Expression { argCount++; result = arg.evaluate(ctx); + // we need to keep track of the first requested image in a coalesce statement + // if coalesce can't find a valid image, we return the first image name so styleimagemissing can fire if (arg.type.kind === 'image' && !result.available) { if (!requestedImageName) requestedImageName = arg.evaluate(ctx).name; result = null; From bf9b06e5d5e0c4b437dc9b08de45db7d90209ddf Mon Sep 17 00:00:00 2001 From: ryanhamley Date: Fri, 20 Sep 2019 16:08:51 -0700 Subject: [PATCH 13/15] Enable styleimagemissing with pattern properties --- src/data/bucket/pattern_bucket_features.js | 9 ++++++--- src/style-spec/expression/definitions/coalesce.js | 1 - src/style/properties.js | 4 +++- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/data/bucket/pattern_bucket_features.js b/src/data/bucket/pattern_bucket_features.js index ca50a2f8fd1..b85970271f0 100644 --- a/src/data/bucket/pattern_bucket_features.js +++ b/src/data/bucket/pattern_bucket_features.js @@ -41,9 +41,12 @@ export function addPatternDependencies(type: string, layers: PatternStyleLayers, const patternPropertyValue = patternProperty.value; if (patternPropertyValue.kind !== "constant") { - 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); + let min = patternPropertyValue.evaluate({zoom: zoom - 1}, patternFeature, {}, options.availableImages); + let mid = patternPropertyValue.evaluate({zoom}, patternFeature, {}, options.availableImages); + let max = patternPropertyValue.evaluate({zoom: zoom + 1}, patternFeature, {}, options.availableImages); + min = min && min.name ? min.name : min; + mid = mid && mid.name ? mid.name : mid; + max = max && max.name ? max.name : max; // add to patternDependencies patterns[min] = true; patterns[mid] = true; diff --git a/src/style-spec/expression/definitions/coalesce.js b/src/style-spec/expression/definitions/coalesce.js index 889959cb351..788b8a3e4d1 100644 --- a/src/style-spec/expression/definitions/coalesce.js +++ b/src/style-spec/expression/definitions/coalesce.js @@ -58,7 +58,6 @@ class Coalesce implements Expression { for (const arg of this.args) { argCount++; result = arg.evaluate(ctx); - // we need to keep track of the first requested image in a coalesce statement // if coalesce can't find a valid image, we return the first image name so styleimagemissing can fire if (arg.type.kind === 'image' && !result.available) { diff --git a/src/style/properties.js b/src/style/properties.js index 29d1d45255a..f9411ee002d 100644 --- a/src/style/properties.js +++ b/src/style/properties.js @@ -599,7 +599,9 @@ export class CrossFadedDataDrivenProperty extends DataDrivenProperty Date: Mon, 23 Sep 2019 13:19:41 -0700 Subject: [PATCH 14/15] Add debug page --- debug/default-image.html | 85 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 85 insertions(+) create mode 100644 debug/default-image.html diff --git a/debug/default-image.html b/debug/default-image.html new file mode 100644 index 00000000000..ac4350d9e56 --- /dev/null +++ b/debug/default-image.html @@ -0,0 +1,85 @@ + + + + Mapbox GL JS debug page + + + + + + + +
+ + + + + + From a08d25659634aef1ca69b4606f81d685f448a71e Mon Sep 17 00:00:00 2001 From: ryanhamley Date: Mon, 23 Sep 2019 20:46:07 -0700 Subject: [PATCH 15/15] Make if statement more legible --- src/data/bucket/symbol_bucket.js | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/data/bucket/symbol_bucket.js b/src/data/bucket/symbol_bucket.js index f092f8dfa8f..8a6960189c5 100644 --- a/src/data/bucket/symbol_bucket.js +++ b/src/data/bucket/symbol_bucket.js @@ -419,9 +419,13 @@ class SymbolBucket implements Bucket { // but plain string token evaluation skips that pathway so do the // conversion here. const resolvedTokens = layer.getValueAndResolveTokens('icon-image', feature, availableImages); - icon = resolvedTokens instanceof ResolvedImage ? - resolvedTokens : - !resolvedTokens || typeof resolvedTokens === 'string' ? ResolvedImage.fromString({name: resolvedTokens, available: false}) : ResolvedImage.fromString(resolvedTokens); + if (resolvedTokens instanceof ResolvedImage) { + icon = resolvedTokens; + } else if (!resolvedTokens || typeof resolvedTokens === 'string') { + icon = ResolvedImage.fromString({name: resolvedTokens, available: false}); + } else { + icon = ResolvedImage.fromString(resolvedTokens); + } } if (!text && !icon) {