From 3cd19854fd16c51d0ffa44c3025c2e1c20a5685e Mon Sep 17 00:00:00 2001 From: Molly Lloyd Date: Fri, 17 Aug 2018 13:58:55 -0700 Subject: [PATCH] Don't wait for pattern images to layout layers no -pattern property set bonus: remove limitation on non-deterministic expression outputs for pattern properties and reliance on `possibleOutputs()` state --- src/data/bucket.js | 4 +- src/data/bucket/circle_bucket.js | 2 + src/data/bucket/fill_bucket.js | 71 +++++++++++++----- src/data/bucket/fill_extrusion_bucket.js | 74 +++++++++++++------ src/data/bucket/line_bucket.js | 48 ++++++++++-- src/data/bucket/symbol_bucket.js | 2 + src/data/program_configuration.js | 22 +++--- src/render/draw_fill_extrusion.js | 4 +- src/source/worker_tile.js | 7 +- src/style-spec/expression/index.js | 3 +- .../validate/validate_expression.js | 3 +- test/ignores.json | 1 - .../feature-expression/style.json | 6 +- 13 files changed, 172 insertions(+), 75 deletions(-) diff --git a/src/data/bucket.js b/src/data/bucket.js index fdec909c0d0..33fb1bb1b65 100644 --- a/src/data/bucket.js +++ b/src/data/bucket.js @@ -38,7 +38,8 @@ export type BucketFeature = {| geometry: Array>, properties: Object, type: 1 | 2 | 3, - id?: any + id?: any, + +patterns: {[string]: {"min": string, "mid": string, "max": string}} |}; /** @@ -66,6 +67,7 @@ export type BucketFeature = {| */ export interface Bucket { layerIds: Array; + hasPattern: boolean; +layers: Array; +stateDependentLayers: Array; diff --git a/src/data/bucket/circle_bucket.js b/src/data/bucket/circle_bucket.js index beb5758e8f2..107d8161539 100644 --- a/src/data/bucket/circle_bucket.js +++ b/src/data/bucket/circle_bucket.js @@ -55,6 +55,7 @@ class CircleBucket implements Bucke indexArray: TriangleIndexArray; indexBuffer: IndexBuffer; + hasPattern: boolean; programConfigurations: ProgramConfigurationSet; segments: SegmentVector; uploaded: boolean; @@ -65,6 +66,7 @@ class CircleBucket implements Bucke this.layers = options.layers; this.layerIds = this.layers.map(layer => layer.id); this.index = options.index; + this.hasPattern = false; this.layoutVertexArray = new CircleLayoutArray(); this.indexArray = new TriangleIndexArray(); diff --git a/src/data/bucket/fill_bucket.js b/src/data/bucket/fill_bucket.js index 07c32ed0778..b221637d934 100644 --- a/src/data/bucket/fill_bucket.js +++ b/src/data/bucket/fill_bucket.js @@ -46,6 +46,7 @@ class FillBucket implements Bucket { indexArray2: LineIndexArray; indexBuffer2: IndexBuffer; + hasPattern: boolean; programConfigurations: ProgramConfigurationSet; segments: SegmentVector; segments2: SegmentVector; @@ -58,6 +59,7 @@ class FillBucket implements Bucket { this.layers = options.layers; this.layerIds = this.layers.map(layer => layer.id); this.index = options.index; + this.hasPattern = false; this.layoutVertexArray = new FillLayoutArray(); this.indexArray = new TriangleIndexArray(); @@ -72,32 +74,61 @@ class FillBucket implements Bucket { this.features = []; for (const layer of this.layers) { - const fillPattern = layer.paint.get('fill-pattern'); - const images = fillPattern.property.getPossibleOutputs(); - for (const image of images) { - patterns[image] = true; + const patternProperty = layer.paint.get('fill-pattern'); + if (!patternProperty.isConstant()) { + this.hasPattern = true; + } + + const constantPattern = patternProperty.constantOr(null); + if (constantPattern) { + this.hasPattern = true; + patterns[constantPattern.to] = true; + patterns[constantPattern.from] = true; } } for (const {feature, index, sourceLayerIndex} of features) { - if (this.layers[0]._featureFilter(new EvaluationParameters(this.zoom), feature)) { - - const geometry = loadGeometry(feature); - const fillFeature: BucketFeature = { - sourceLayerIndex: sourceLayerIndex, - index: index, - geometry: geometry, - properties: feature.properties, - type: feature.type - }; - - if (typeof feature.id !== 'undefined') { - fillFeature.id = feature.id; - } + if (!this.layers[0]._featureFilter(new EvaluationParameters(this.zoom), feature)) continue; + + const geometry = loadGeometry(feature); + + const bucketFeature: BucketFeature = { + sourceLayerIndex: sourceLayerIndex, + index: index, + geometry: geometry, + properties: feature.properties, + type: feature.type, + patterns: {} + }; - this.features.push(fillFeature); - options.featureIndex.insert(feature, geometry, index, sourceLayerIndex, this.index); + if (typeof feature.id !== 'undefined') { + bucketFeature.id = feature.id; } + + if (this.hasPattern) { + for (const layer of this.layers) { + const patternProperty = layer.paint.get('fill-pattern'); + + const patternPropertyValue = patternProperty.value; + if (patternPropertyValue.kind !== "constant") { + const min = patternPropertyValue.evaluate({zoom: this.zoom - 1}, feature, {}); + const mid = patternPropertyValue.evaluate({zoom: this.zoom}, feature, {}); + const max = patternPropertyValue.evaluate({zoom: this.zoom + 1}, feature, {}); + // add to patternDependencies + patterns[min] = true; + patterns[mid] = true; + patterns[max] = true; + + // save for layout + bucketFeature.patterns[layer.id] = { min, mid, max }; + } + } + this.features.push(bucketFeature); + } else { + this.addFeature(bucketFeature, geometry, index, {}); + } + + options.featureIndex.insert(feature, geometry, index, sourceLayerIndex, this.index); } } diff --git a/src/data/bucket/fill_extrusion_bucket.js b/src/data/bucket/fill_extrusion_bucket.js index 62e6322066d..431a988780f 100644 --- a/src/data/bucket/fill_extrusion_bucket.js +++ b/src/data/bucket/fill_extrusion_bucket.js @@ -22,6 +22,7 @@ import type { IndexedFeature, PopulateParameters } from '../bucket'; + import type FillExtrusionStyleLayer from '../../style/style_layer/fill_extrusion_style_layer'; import type Context from '../../gl/context'; import type IndexBuffer from '../../gl/index_buffer'; @@ -62,6 +63,7 @@ class FillExtrusionBucket implements Bucket { indexArray: TriangleIndexArray; indexBuffer: IndexBuffer; + hasPattern: boolean; programConfigurations: ProgramConfigurationSet; segments: SegmentVector; uploaded: boolean; @@ -73,6 +75,7 @@ class FillExtrusionBucket implements Bucket { this.layers = options.layers; this.layerIds = this.layers.map(layer => layer.id); this.index = options.index; + this.hasPattern = false; this.layoutVertexArray = new FillExtrusionLayoutArray(); this.indexArray = new TriangleIndexArray(); @@ -84,33 +87,62 @@ class FillExtrusionBucket implements Bucket { const patterns = options.patternDependencies; this.features = []; - for (let i = 0; i < this.layers.length; i++) { - const layer = this.layers[i]; - const fillExtrusionPattern = layer.paint.get('fill-extrusion-pattern'); - const images = fillExtrusionPattern.property.getPossibleOutputs(); - for (const image of images) { - patterns[image] = true; + for (const layer of this.layers) { + const patternProperty = layer.paint.get('fill-extrusion-pattern'); + if (!patternProperty.isConstant()) { + this.hasPattern = true; } - } + const constantPattern = patternProperty.constantOr(null); + if (constantPattern) { + this.hasPattern = true; + patterns[constantPattern.to] = true; + patterns[constantPattern.from] = true; + } + } for (const {feature, index, sourceLayerIndex} of features) { - if (this.layers[0]._featureFilter(new EvaluationParameters(this.zoom), feature)) { - const geometry = loadGeometry(feature); - const fillExtrusionFeature: BucketFeature = { - sourceLayerIndex: sourceLayerIndex, - index: index, - geometry: geometry, - properties: feature.properties, - type: feature.type - }; - - if (typeof feature.id !== 'undefined') { - fillExtrusionFeature.id = feature.id; + if (!this.layers[0]._featureFilter(new EvaluationParameters(this.zoom), feature)) continue; + + const geometry = loadGeometry(feature); + + const bucketFeature: BucketFeature = { + sourceLayerIndex: sourceLayerIndex, + index: index, + geometry: geometry, + properties: feature.properties, + type: feature.type, + patterns: {} + }; + + if (typeof feature.id !== 'undefined') { + bucketFeature.id = feature.id; + } + + if (this.hasPattern) { + for (const layer of this.layers) { + const patternProperty = layer.paint.get('fill-extrusion-pattern'); + + const patternPropertyValue = patternProperty.value; + if (patternPropertyValue.kind !== "constant") { + const min = patternPropertyValue.evaluate({zoom: this.zoom - 1}, feature, {}); + const mid = patternPropertyValue.evaluate({zoom: this.zoom}, feature, {}); + const max = patternPropertyValue.evaluate({zoom: this.zoom + 1}, feature, {}); + // add to patternDependencies + patterns[min] = true; + patterns[mid] = true; + patterns[max] = true; + + // save for layout + bucketFeature.patterns[layer.id] = { min, mid, max }; + } } - this.features.push(fillExtrusionFeature); - options.featureIndex.insert(feature, geometry, index, sourceLayerIndex, this.index); + this.features.push(bucketFeature); + } else { + this.addFeature(bucketFeature, geometry, index, {}); } + + options.featureIndex.insert(feature, geometry, index, sourceLayerIndex, this.index); } } diff --git a/src/data/bucket/line_bucket.js b/src/data/bucket/line_bucket.js index 05ec069557f..63ef40933dd 100644 --- a/src/data/bucket/line_bucket.js +++ b/src/data/bucket/line_bucket.js @@ -106,6 +106,7 @@ class LineBucket implements Bucket { indexArray: TriangleIndexArray; indexBuffer: IndexBuffer; + hasPattern: boolean; programConfigurations: ProgramConfigurationSet; segments: SegmentVector; uploaded: boolean; @@ -117,6 +118,7 @@ class LineBucket implements Bucket { this.layerIds = this.layers.map(layer => layer.id); this.index = options.index; this.features = []; + this.hasPattern = false; this.layoutVertexArray = new LineLayoutArray(); this.indexArray = new TriangleIndexArray(); @@ -129,10 +131,16 @@ class LineBucket implements Bucket { this.features = []; for (const layer of this.layers) { - const linePattern = layer.paint.get('line-pattern'); - const images = linePattern.property.getPossibleOutputs(); - for (const image of images) { - patterns[image] = true; + const patternProperty = layer.paint.get('line-pattern'); + if (!patternProperty.isConstant()) { + this.hasPattern = true; + } + + const constantPattern = patternProperty.constantOr(null); + if (constantPattern) { + this.hasPattern = true; + patterns[constantPattern.to] = true; + patterns[constantPattern.from] = true; } } @@ -140,19 +148,43 @@ class LineBucket implements Bucket { if (!this.layers[0]._featureFilter(new EvaluationParameters(this.zoom), feature)) continue; const geometry = loadGeometry(feature); - const lineFeature: BucketFeature = { + + const bucketFeature: BucketFeature = { sourceLayerIndex: sourceLayerIndex, index: index, geometry: geometry, properties: feature.properties, - type: feature.type + type: feature.type, + patterns: {} }; if (typeof feature.id !== 'undefined') { - lineFeature.id = feature.id; + bucketFeature.id = feature.id; + } + + if (this.hasPattern) { + for (const layer of this.layers) { + const patternProperty = layer.paint.get('line-pattern'); + + const patternPropertyValue = patternProperty.value; + if (patternPropertyValue.kind !== "constant") { + const min = patternPropertyValue.evaluate({zoom: this.zoom - 1}, feature, {}); + const mid = patternPropertyValue.evaluate({zoom: this.zoom}, feature, {}); + const max = patternPropertyValue.evaluate({zoom: this.zoom + 1}, feature, {}); + // add to patternDependencies + patterns[min] = true; + patterns[mid] = true; + patterns[max] = true; + + // save for layout + bucketFeature.patterns[layer.id] = { min, mid, max }; + } + } + this.features.push(bucketFeature); + } else { + this.addFeature(bucketFeature, geometry, index, {}); } - this.features.push(lineFeature); options.featureIndex.insert(feature, geometry, index, sourceLayerIndex, this.index); } } diff --git a/src/data/bucket/symbol_bucket.js b/src/data/bucket/symbol_bucket.js index 5de48d56a9b..31c36180dd2 100644 --- a/src/data/bucket/symbol_bucket.js +++ b/src/data/bucket/symbol_bucket.js @@ -269,6 +269,7 @@ class SymbolBucket implements Bucket { iconsNeedLinear: boolean; bucketInstanceId: number; justReloaded: boolean; + hasPattern: boolean; textSizeData: SizeData; iconSizeData: SizeData; @@ -302,6 +303,7 @@ class SymbolBucket implements Bucket { this.index = options.index; this.pixelRatio = options.pixelRatio; this.sourceLayerIndex = options.sourceLayerIndex; + this.hasPattern = false; const layer = this.layers[0]; const unevaluatedLayoutValues = layer._unevaluatedLayout._values; diff --git a/src/data/program_configuration.js b/src/data/program_configuration.js index ed38e908516..5a169ac9f62 100644 --- a/src/data/program_configuration.js +++ b/src/data/program_configuration.js @@ -406,6 +406,7 @@ class CrossFadedCompositeBinder implements Binder { useIntegerZoom: boolean; zoom: number; maxValue: number; + layerId: string; zoomInPaintVertexArray: StructArray; zoomOutPaintVertexArray: StructArray; @@ -413,7 +414,7 @@ class CrossFadedCompositeBinder implements Binder { zoomOutPaintVertexBuffer: ?VertexBuffer; paintVertexAttributes: Array; - constructor(expression: CompositeExpression, names: Array, type: string, useIntegerZoom: boolean, zoom: number, PaintVertexArray: Class) { + constructor(expression: CompositeExpression, names: Array, type: string, useIntegerZoom: boolean, zoom: number, PaintVertexArray: Class, layerId: string) { this.expression = expression; this.names = names; @@ -422,6 +423,7 @@ class CrossFadedCompositeBinder implements Binder { this.useIntegerZoom = useIntegerZoom; this.zoom = zoom; this.maxValue = -Infinity; + this.layerId = layerId; this.paintVertexAttributes = names.map((name) => ({ @@ -449,17 +451,15 @@ class CrossFadedCompositeBinder implements Binder { const zoomInArray = this.zoomInPaintVertexArray; const zoomOutArray = this.zoomOutPaintVertexArray; - + const { layerId } = this; const start = zoomInArray.length; zoomInArray.reserve(length); zoomOutArray.reserve(length); - const min = this.expression.evaluate({zoom: this.zoom - 1}, feature); - const mid = this.expression.evaluate({zoom: this.zoom }, feature); - const max = this.expression.evaluate({zoom: this.zoom + 1}, feature); + if (imagePositions && feature.patterns && feature.patterns[layerId]) { + const { min, mid, max } = feature.patterns[layerId]; - if (imagePositions) { const imageMin = imagePositions[min]; const imageMid = imagePositions[mid]; const imageMax = imagePositions[max]; @@ -487,12 +487,10 @@ class CrossFadedCompositeBinder implements Binder { const zoomInArray = this.zoomInPaintVertexArray; const zoomOutArray = this.zoomOutPaintVertexArray; + const { layerId } = this; - const min = this.expression.evaluate({zoom: this.zoom - 1}, feature, featureState); - const mid = this.expression.evaluate({zoom: this.zoom }, feature, featureState); - const max = this.expression.evaluate({zoom: this.zoom + 1}, feature, featureState); - - if (imagePositions) { + if (imagePositions && feature.patterns && feature.patterns[layerId]) { + const {min, mid, max} = feature.patterns[layerId]; const imageMin = imagePositions[min]; const imageMid = imagePositions[mid]; const imageMax = imagePositions[max]; @@ -594,7 +592,7 @@ export default class ProgramConfiguration { keys.push(`/u_${property}`); } else { const StructArrayLayout = layoutType(property, type, 'source'); - self.binders[property] = new CrossFadedCompositeBinder(value.value, names, type, useIntegerZoom, zoom, StructArrayLayout); + self.binders[property] = new CrossFadedCompositeBinder(value.value, names, type, useIntegerZoom, zoom, StructArrayLayout, layer.id); keys.push(`/a_${property}`); } } else if (value.value.kind === 'constant') { diff --git a/src/render/draw_fill_extrusion.js b/src/render/draw_fill_extrusion.js index 499614f0286..d607bc3dbf2 100644 --- a/src/render/draw_fill_extrusion.js +++ b/src/render/draw_fill_extrusion.js @@ -87,13 +87,11 @@ function drawExtrusionTiles(painter, source, layer, coords, depthMode, stencilMo const context = painter.context; const gl = context.gl; const patternProperty = layer.paint.get('fill-extrusion-pattern'); - const image = patternProperty.constantOr((1: any)); + const image = patternProperty.constantOr((1:any)); const crossfade = layer.getCrossfadeParameters(); for (const coord of coords) { const tile = source.getTile(coord); - if (image && !tile.patternsLoaded()) return; - const bucket: ?FillExtrusionBucket = (tile.getBucket(layer): any); if (!bucket) continue; diff --git a/src/source/worker_tile.js b/src/source/worker_tile.js index 828195f511d..e1035e3e8d7 100644 --- a/src/source/worker_tile.js +++ b/src/source/worker_tile.js @@ -116,7 +116,6 @@ class WorkerTile { sourceLayerIndex: sourceLayerIndex, sourceID: this.source }); - bucket.populate(features, options); featureIndex.bucketLayerIDs.push(family.map((l) => l.id)); } @@ -181,14 +180,16 @@ class WorkerTile { if (bucket instanceof SymbolBucket) { recalculateLayers(bucket.layers, this.zoom); performSymbolLayout(bucket, glyphMap, glyphAtlas.positions, iconMap, imageAtlas.iconPositions, this.showCollisionBoxes); - } else if (bucket instanceof LineBucket || bucket instanceof FillBucket || bucket instanceof FillExtrusionBucket) { + } else if (bucket.hasPattern && + (bucket instanceof LineBucket || + bucket instanceof FillBucket || + bucket instanceof FillExtrusionBucket)) { recalculateLayers(bucket.layers, this.zoom); bucket.addFeatures(options, imageAtlas.patternPositions); } } this.status = 'done'; - callback(null, { buckets: values(buckets).filter(b => !b.isEmpty()), featureIndex, diff --git a/src/style-spec/expression/index.js b/src/style-spec/expression/index.js index a360e88376e..248ea6e6295 100644 --- a/src/style-spec/expression/index.js +++ b/src/style-spec/expression/index.js @@ -28,7 +28,8 @@ import type {PropertyValueSpecification} from '../types'; export type Feature = { +type: 1 | 2 | 3 | 'Unknown' | 'Point' | 'MultiPoint' | 'LineString' | 'MultiLineString' | 'Polygon' | 'MultiPolygon', +id?: any, - +properties: {[string]: any} + +properties: {[string]: any}, + +patterns?: {[string]: {"min": string, "mid": string, "max": string}} }; export type FeatureState = {[string]: any}; diff --git a/src/style-spec/validate/validate_expression.js b/src/style-spec/validate/validate_expression.js index b831069e4e6..3e35a1055ce 100644 --- a/src/style-spec/validate/validate_expression.js +++ b/src/style-spec/validate/validate_expression.js @@ -14,8 +14,7 @@ export default function validateExpression(options: any) { }); } - if (options.expressionContext === 'property' && (options.propertyKey === 'text-font' || - options.valueSpec["property-type"] === 'cross-faded-data-driven') && + if (options.expressionContext === 'property' && (options.propertyKey === 'text-font') && (expression.value: any)._styleExpression.expression.possibleOutputs().indexOf(undefined) !== -1) { return [new ValidationError(options.key, options.value, `Invalid data expression for "${options.propertyKey}". Output values must be contained as literals within the expression.`)]; } diff --git a/test/ignores.json b/test/ignores.json index 03e08e140ff..b0d4c2ccd82 100644 --- a/test/ignores.json +++ b/test/ignores.json @@ -2,7 +2,6 @@ "query-tests/regressions/mapbox-gl-js#4494": "https://github.com/mapbox/mapbox-gl-js/issues/2716", "render-tests/combinations/fill-extrusion-translucent--background-opaque": "https://github.com/mapbox/mapbox-gl-js/issues/5831", "render-tests/combinations/fill-extrusion-translucent--fill-opaque": "https://github.com/mapbox/mapbox-gl-js/issues/5831", - "render-tests/fill-extrusion-pattern/function-2": "https://github.com/mapbox/mapbox-gl-js/issues/3327", "render-tests/geojson/inline-linestring-fill": "current behavior is arbitrary", "render-tests/map-mode/static": "https://github.com/mapbox/mapbox-gl-js/issues/5649", "render-tests/map-mode/tile": "skip - mapbox-gl-js does not need to render tiles", diff --git a/test/integration/render-tests/fill-extrusion-pattern/feature-expression/style.json b/test/integration/render-tests/fill-extrusion-pattern/feature-expression/style.json index f658cd69581..360934bf306 100644 --- a/test/integration/render-tests/fill-extrusion-pattern/feature-expression/style.json +++ b/test/integration/render-tests/fill-extrusion-pattern/feature-expression/style.json @@ -14,7 +14,7 @@ { "type": "Feature", "properties": { - "property": 10 + "property": "generic_icon" }, "geometry": { "type": "Polygon", @@ -47,7 +47,7 @@ { "type": "Feature", "properties": { - "property": 20 + "property": "generic_metro" }, "geometry": { "type": "Polygon", @@ -90,7 +90,7 @@ "type": "fill-extrusion", "source": "geojson", "paint": { - "fill-extrusion-pattern": ["case", ["==",["get", "property"], 10], "generic_icon", "generic_metro"], + "fill-extrusion-pattern": ["get", "property"], "fill-extrusion-height": 10 } }