Mapbox GL JS debug page

From cc13e2d4d753c8ad3e157f23e7776539439902f5 Mon Sep 17 00:00:00 2001 From: Vladimir Agafonkin Date: Mon, 18 Nov 2019 17:29:03 +0200 Subject: [PATCH 01/12] add ability to id by property for feature state --- debug/featurestate.html | 82 ++++++++++++++++++++++++ src/data/bucket.js | 3 +- src/data/bucket/circle_bucket.js | 2 +- src/data/bucket/fill_bucket.js | 2 +- src/data/bucket/fill_extrusion_bucket.js | 2 +- src/data/bucket/line_bucket.js | 2 +- src/data/bucket/symbol_bucket.js | 5 +- src/data/feature_position_map.js | 21 ++++-- src/data/program_configuration.js | 11 ++-- src/source/vector_tile_source.js | 3 +- src/source/worker_tile.js | 4 +- src/style/style.js | 34 +++------- 12 files changed, 128 insertions(+), 43 deletions(-) create mode 100644 debug/featurestate.html diff --git a/debug/featurestate.html b/debug/featurestate.html new file mode 100644 index 00000000000..fb58e2c3058 --- /dev/null +++ b/debug/featurestate.html @@ -0,0 +1,82 @@ + + + + Mapbox GL JS debug page + + + + + + + +

+ + + + + + diff --git a/src/data/bucket.js b/src/data/bucket.js index f0ecc4ef7c2..d6675f55e7c 100644 --- a/src/data/bucket.js +++ b/src/data/bucket.js @@ -16,7 +16,8 @@ export type BucketParameters = { overscaling: number, collisionBoxArray: CollisionBoxArray, sourceLayerIndex: number, - sourceID: string + sourceID: string, + featureStateID?: string } export type PopulateParameters = { diff --git a/src/data/bucket/circle_bucket.js b/src/data/bucket/circle_bucket.js index 2ee8ade034b..d652d1c7b03 100644 --- a/src/data/bucket/circle_bucket.js +++ b/src/data/bucket/circle_bucket.js @@ -71,7 +71,7 @@ class CircleBucket implements Bucke this.layoutVertexArray = new CircleLayoutArray(); this.indexArray = new TriangleIndexArray(); this.segments = new SegmentVector(); - this.programConfigurations = new ProgramConfigurationSet(layoutAttributes, options.layers, options.zoom); + this.programConfigurations = new ProgramConfigurationSet(layoutAttributes, options.layers, options.zoom, undefined, options.featureStateID); this.stateDependentLayerIds = this.layers.filter((l) => l.isStateDependent()).map((l) => l.id); } diff --git a/src/data/bucket/fill_bucket.js b/src/data/bucket/fill_bucket.js index 39d9901d013..681af69b25f 100644 --- a/src/data/bucket/fill_bucket.js +++ b/src/data/bucket/fill_bucket.js @@ -67,7 +67,7 @@ class FillBucket implements Bucket { this.layoutVertexArray = new FillLayoutArray(); this.indexArray = new TriangleIndexArray(); this.indexArray2 = new LineIndexArray(); - this.programConfigurations = new ProgramConfigurationSet(layoutAttributes, options.layers, options.zoom); + this.programConfigurations = new ProgramConfigurationSet(layoutAttributes, options.layers, options.zoom, undefined, options.featureStateID); this.segments = new SegmentVector(); this.segments2 = new SegmentVector(); this.stateDependentLayerIds = this.layers.filter((l) => l.isStateDependent()).map((l) => l.id); diff --git a/src/data/bucket/fill_extrusion_bucket.js b/src/data/bucket/fill_extrusion_bucket.js index 4f8632dc384..04e0a512305 100644 --- a/src/data/bucket/fill_extrusion_bucket.js +++ b/src/data/bucket/fill_extrusion_bucket.js @@ -81,7 +81,7 @@ class FillExtrusionBucket implements Bucket { this.layoutVertexArray = new FillExtrusionLayoutArray(); this.indexArray = new TriangleIndexArray(); - this.programConfigurations = new ProgramConfigurationSet(layoutAttributes, options.layers, options.zoom); + this.programConfigurations = new ProgramConfigurationSet(layoutAttributes, options.layers, options.zoom, undefined, options.featureStateID); this.segments = new SegmentVector(); this.stateDependentLayerIds = this.layers.filter((l) => l.isStateDependent()).map((l) => l.id); diff --git a/src/data/bucket/line_bucket.js b/src/data/bucket/line_bucket.js index 4e1f31c65d7..6e318eda877 100644 --- a/src/data/bucket/line_bucket.js +++ b/src/data/bucket/line_bucket.js @@ -110,7 +110,7 @@ class LineBucket implements Bucket { this.layoutVertexArray = new LineLayoutArray(); this.indexArray = new TriangleIndexArray(); - this.programConfigurations = new ProgramConfigurationSet(layoutAttributes, options.layers, options.zoom); + this.programConfigurations = new ProgramConfigurationSet(layoutAttributes, options.layers, options.zoom, undefined, options.featureStateID); this.segments = new SegmentVector(); this.stateDependentLayerIds = this.layers.filter((l) => l.isStateDependent()).map((l) => l.id); diff --git a/src/data/bucket/symbol_bucket.js b/src/data/bucket/symbol_bucket.js index 4746935a53c..74ecaa0e9bc 100644 --- a/src/data/bucket/symbol_bucket.js +++ b/src/data/bucket/symbol_bucket.js @@ -317,6 +317,7 @@ class SymbolBucket implements Bucket { this.index = options.index; this.pixelRatio = options.pixelRatio; this.sourceLayerIndex = options.sourceLayerIndex; + this.featureStateID = options.featureStateID; this.hasPattern = false; this.hasPaintOverrides = false; this.hasRTLText = false; @@ -348,8 +349,8 @@ class SymbolBucket implements Bucket { const layout = this.layers[0].layout; this.hasPaintOverrides = SymbolStyleLayer.hasPaintOverrides(layout); - this.text = new SymbolBuffers(new ProgramConfigurationSet(symbolLayoutAttributes.members, this.layers, this.zoom, property => /^text/.test(property))); - this.icon = new SymbolBuffers(new ProgramConfigurationSet(symbolLayoutAttributes.members, this.layers, this.zoom, property => /^icon/.test(property))); + this.text = new SymbolBuffers(new ProgramConfigurationSet(symbolLayoutAttributes.members, this.layers, this.zoom, property => /^text/.test(property), this.featureStateID)); + this.icon = new SymbolBuffers(new ProgramConfigurationSet(symbolLayoutAttributes.members, this.layers, this.zoom, property => /^icon/.test(property), this.featureStateID)); this.textCollisionBox = new CollisionBuffers(CollisionBoxLayoutArray, collisionBoxLayout.members, LineIndexArray); this.iconCollisionBox = new CollisionBuffers(CollisionBoxLayoutArray, collisionBoxLayout.members, LineIndexArray); diff --git a/src/data/feature_position_map.js b/src/data/feature_position_map.js index 1724ba5c015..9c01956c478 100644 --- a/src/data/feature_position_map.js +++ b/src/data/feature_position_map.js @@ -1,5 +1,6 @@ // @flow +import murmur3 from 'murmurhash-js'; import {register} from '../util/web_worker_transfer'; import assert from 'assert'; @@ -26,28 +27,30 @@ export default class FeaturePositionMap { this.indexed = false; } - add(id: number, index: number, start: number, end: number) { - this.ids.push(id); + add(id: mixed, index: number, start: number, end: number) { + this.ids.push(getIntegerId(id)); this.positions.push(index, start, end); } - getPositions(id: number): Array { + getPositions(id: mixed): Array { assert(this.indexed); + const intId = getIntegerId(id); + // binary search for the first occurrence of id in this.ids; // relies on ids/positions being sorted by id, which happens in serialization let i = 0; let j = this.ids.length - 1; while (i < j) { const m = (i + j) >> 1; - if (this.ids[m] >= id) { + if (this.ids[m] >= intId) { j = m; } else { i = m + 1; } } const positions = []; - while (this.ids[i] === id) { + while (this.ids[i] === intId) { const index = this.positions[3 * i]; const start = this.positions[3 * i + 1]; const end = this.positions[3 * i + 2]; @@ -79,6 +82,14 @@ export default class FeaturePositionMap { } } +export function getIntegerId(value: mixed) { + const numValue = +value; + if (!isNaN(numValue) && numValue % 1 === 0) { + return numValue; + } + return murmur3(String(value)); +} + // custom quicksort that sorts ids, indices and offsets together (by ids) function sort(ids, positions, left, right) { if (left >= right) return; diff --git a/src/data/program_configuration.js b/src/data/program_configuration.js index 3a0b7e78f9b..9001936b04c 100644 --- a/src/data/program_configuration.js +++ b/src/data/program_configuration.js @@ -624,7 +624,7 @@ export default class ProgramConfiguration { updatePaintArrays(featureStates: FeatureStates, featureMap: FeaturePositionMap, vtLayer: VectorTileLayer, layer: TypedStyleLayer, imagePositions: {[string]: ImagePosition}): boolean { let dirty: boolean = false; for (const id in featureStates) { - const positions = featureMap.getPositions(+id); + const positions = featureMap.getPositions(id); for (const pos of positions) { const feature = vtLayer.feature(pos.index); @@ -729,7 +729,7 @@ export class ProgramConfigurationSet { _featureMap: FeaturePositionMap; _bufferOffset: number; - constructor(layoutAttributes: Array, layers: $ReadOnlyArray, zoom: number, filterProperties: (string) => boolean = () => true) { + constructor(layoutAttributes: Array, layers: $ReadOnlyArray, zoom: number, filterProperties: (string) => boolean = () => true, featureStateID?: string) { this.programConfigurations = {}; for (const layer of layers) { this.programConfigurations[layer.id] = ProgramConfiguration.createDynamic(layer, zoom, filterProperties); @@ -737,6 +737,7 @@ export class ProgramConfigurationSet { } this.needsUpload = false; this._featureMap = new FeaturePositionMap(); + this._featureStateID = featureStateID; this._bufferOffset = 0; } @@ -745,8 +746,10 @@ export class ProgramConfigurationSet { this.programConfigurations[key].populatePaintArrays(length, feature, index, imagePositions, formattedSection); } - if (feature.id !== undefined) { - this._featureMap.add(+feature.id, index, this._bufferOffset, length); + const id = this._featureStateID ? feature.properties[this._featureStateID] : feature.id; + + if (id !== undefined) { + this._featureMap.add(id, index, this._bufferOffset, length); } this._bufferOffset = length; diff --git a/src/source/vector_tile_source.js b/src/source/vector_tile_source.js index 957c2ef6b72..69512678cba 100644 --- a/src/source/vector_tile_source.js +++ b/src/source/vector_tile_source.js @@ -55,7 +55,7 @@ class VectorTileSource extends Evented implements Source { this.isTileClipped = true; this._loaded = false; - extend(this, pick(options, ['url', 'scheme', 'tileSize'])); + extend(this, pick(options, ['url', 'scheme', 'tileSize', 'featureStateID'])); this._options = extend({type: 'vector'}, options); this._collectResourceTiming = options.collectResourceTiming; @@ -126,6 +126,7 @@ class VectorTileSource extends Evented implements Source { source: this.id, pixelRatio: browser.devicePixelRatio, showCollisionBoxes: this.map.showCollisionBoxes, + featureStateID: this.featureStateID }; params.request.collectResourceTiming = this._collectResourceTiming; diff --git a/src/source/worker_tile.js b/src/source/worker_tile.js index 17cd7f11b43..4adfe8c8edc 100644 --- a/src/source/worker_tile.js +++ b/src/source/worker_tile.js @@ -58,6 +58,7 @@ class WorkerTile { this.showCollisionBoxes = params.showCollisionBoxes; this.collectResourceTiming = !!params.collectResourceTiming; this.returnDependencies = !!params.returnDependencies; + this.featureStateID = params.featureStateID; } parse(data: VectorTile, layerIndex: StyleLayerIndex, availableImages: Array, actor: Actor, callback: WorkerTileCallback) { @@ -117,7 +118,8 @@ class WorkerTile { overscaling: this.overscaling, collisionBoxArray: this.collisionBoxArray, sourceLayerIndex, - sourceID: this.source + sourceID: this.source, + featureStateID: this.featureStateID && this.featureStateID[sourceLayerId] }); bucket.populate(features, options); diff --git a/src/style/style.js b/src/style/style.js index 6ab17885fb9..92a84650352 100644 --- a/src/style/style.js +++ b/src/style/style.js @@ -845,12 +845,11 @@ class Style extends Evented { return this.getLayer(layer).getPaintProperty(name); } - setFeatureState(feature: { source: string; sourceLayer?: string; id: string | number; }, state: Object) { + setFeatureState(target: { source: string; sourceLayer?: string; id: string | number; }, state: Object) { this._checkLoaded(); - const sourceId = feature.source; - const sourceLayer = feature.sourceLayer; + const sourceId = target.source; + const sourceLayer = target.sourceLayer; const sourceCache = this.sourceCaches[sourceId]; - const featureId = parseInt(feature.id, 10); if (sourceCache === undefined) { this.fire(new ErrorEvent(new Error(`The source '${sourceId}' does not exist in the map's style.`))); @@ -865,12 +864,8 @@ class Style extends Evented { this.fire(new ErrorEvent(new Error(`The sourceLayer parameter must be provided for vector source types.`))); return; } - if (isNaN(featureId) || featureId < 0) { - this.fire(new ErrorEvent(new Error(`The feature id parameter must be provided and non-negative.`))); - return; - } - sourceCache.setFeatureState(sourceLayer, featureId, state); + sourceCache.setFeatureState(sourceLayer, target.id, state); } removeFeatureState(target: { source: string; sourceLayer?: string; id?: string | number; }, key?: string) { @@ -885,32 +880,25 @@ class Style extends Evented { const sourceType = sourceCache.getSource().type; const sourceLayer = sourceType === 'vector' ? target.sourceLayer : undefined; - const featureId = parseInt(target.id, 10); if (sourceType === 'vector' && !sourceLayer) { this.fire(new ErrorEvent(new Error(`The sourceLayer parameter must be provided for vector source types.`))); return; } - if (target.id !== undefined && isNaN(featureId) || featureId < 0) { - this.fire(new ErrorEvent(new Error(`The feature id parameter must be non-negative.`))); - return; - } - if (key && (typeof target.id !== 'string' && typeof target.id !== 'number')) { this.fire(new ErrorEvent(new Error(`A feature id is requred to remove its specific state property.`))); return; } - sourceCache.removeFeatureState(sourceLayer, featureId, key); + sourceCache.removeFeatureState(sourceLayer, target.id, key); } - getFeatureState(feature: { source: string; sourceLayer?: string; id: string | number; }) { + getFeatureState(target: { source: string; sourceLayer?: string; id: string | number; }) { this._checkLoaded(); - const sourceId = feature.source; - const sourceLayer = feature.sourceLayer; + const sourceId = target.source; + const sourceLayer = target.sourceLayer; const sourceCache = this.sourceCaches[sourceId]; - const featureId = parseInt(feature.id, 10); if (sourceCache === undefined) { this.fire(new ErrorEvent(new Error(`The source '${sourceId}' does not exist in the map's style.`))); @@ -921,12 +909,8 @@ class Style extends Evented { this.fire(new ErrorEvent(new Error(`The sourceLayer parameter must be provided for vector source types.`))); return; } - if (isNaN(featureId) || featureId < 0) { - this.fire(new ErrorEvent(new Error(`The feature id parameter must be provided and non-negative.`))); - return; - } - return sourceCache.getFeatureState(sourceLayer, featureId); + return sourceCache.getFeatureState(sourceLayer, target.id); } getTransition() { From e7f8b79ff9a5b8d4cf9a73844713354713a42279 Mon Sep 17 00:00:00 2001 From: Vladimir Agafonkin Date: Mon, 18 Nov 2019 18:38:24 +0200 Subject: [PATCH 02/12] flow fixes --- bench/lib/tile_parser.js | 3 ++- src/data/bucket/symbol_bucket.js | 1 + src/data/program_configuration.js | 3 ++- src/source/source_cache.js | 12 ++++++------ src/source/source_state.js | 10 +++++----- src/source/vector_tile_source.js | 1 + src/source/worker_source.js | 1 + src/source/worker_tile.js | 1 + 8 files changed, 19 insertions(+), 13 deletions(-) diff --git a/bench/lib/tile_parser.js b/bench/lib/tile_parser.js index 1bba3e00182..6c0852bb6f9 100644 --- a/bench/lib/tile_parser.js +++ b/bench/lib/tile_parser.js @@ -129,7 +129,8 @@ export default class TileParser { pitch: 0, cameraToCenterDistance: 0, cameraToTileDistance: 0, - returnDependencies + returnDependencies, + featureStateID: undefined }); const vectorTile = new VT.VectorTile(new Protobuf(tile.buffer)); diff --git a/src/data/bucket/symbol_bucket.js b/src/data/bucket/symbol_bucket.js index 74ecaa0e9bc..e557845c777 100644 --- a/src/data/bucket/symbol_bucket.js +++ b/src/data/bucket/symbol_bucket.js @@ -302,6 +302,7 @@ class SymbolBucket implements Bucket { uploaded: boolean; sourceLayerIndex: number; sourceID: string; + featureStateID: ?string; symbolInstanceIndexes: Array; writingModes: Array; allowVerticalPlacement: boolean; diff --git a/src/data/program_configuration.js b/src/data/program_configuration.js index 9001936b04c..13d1d157b7d 100644 --- a/src/data/program_configuration.js +++ b/src/data/program_configuration.js @@ -727,9 +727,10 @@ export class ProgramConfigurationSet { programConfigurations: {[string]: ProgramConfiguration}; needsUpload: boolean; _featureMap: FeaturePositionMap; + _featureStateID: ?string; _bufferOffset: number; - constructor(layoutAttributes: Array, layers: $ReadOnlyArray, zoom: number, filterProperties: (string) => boolean = () => true, featureStateID?: string) { + constructor(layoutAttributes: Array, layers: $ReadOnlyArray, zoom: number, filterProperties: (string) => boolean = () => true, featureStateID: ?string) { this.programConfigurations = {}; for (const layer of layers) { this.programConfigurations[layer.id] = ProgramConfiguration.createDynamic(layer, zoom, filterProperties); diff --git a/src/source/source_cache.js b/src/source/source_cache.js index d602cec1eb7..05088daf87b 100644 --- a/src/source/source_cache.js +++ b/src/source/source_cache.js @@ -827,27 +827,27 @@ class SourceCache extends Evented { * Set the value of a particular state for a feature * @private */ - setFeatureState(sourceLayer?: string, feature: number, state: Object) { + setFeatureState(sourceLayer?: string, featureId: number | string, state: Object) { sourceLayer = sourceLayer || '_geojsonTileLayer'; - this._state.updateState(sourceLayer, feature, state); + this._state.updateState(sourceLayer, featureId, state); } /** * Resets the value of a particular state key for a feature * @private */ - removeFeatureState(sourceLayer?: string, feature?: number, key?: string) { + removeFeatureState(sourceLayer?: string, featureId?: number | string, key?: string) { sourceLayer = sourceLayer || '_geojsonTileLayer'; - this._state.removeFeatureState(sourceLayer, feature, key); + this._state.removeFeatureState(sourceLayer, featureId, key); } /** * Get the entire state object for a feature * @private */ - getFeatureState(sourceLayer?: string, feature: number) { + getFeatureState(sourceLayer?: string, featureId: number | string) { sourceLayer = sourceLayer || '_geojsonTileLayer'; - return this._state.getState(sourceLayer, feature); + return this._state.getState(sourceLayer, featureId); } } diff --git a/src/source/source_state.js b/src/source/source_state.js index e20851f5ad3..ff69cc0f87f 100644 --- a/src/source/source_state.js +++ b/src/source/source_state.js @@ -27,7 +27,7 @@ class SourceFeatureState { this.deletedStates = {}; } - updateState(sourceLayer: string, featureId: number, newState: Object) { + updateState(sourceLayer: string, featureId: number | string, newState: Object) { const feature = String(featureId); this.stateChanges[sourceLayer] = this.stateChanges[sourceLayer] || {}; this.stateChanges[sourceLayer][feature] = this.stateChanges[sourceLayer][feature] || {}; @@ -54,7 +54,7 @@ class SourceFeatureState { } } - removeFeatureState(sourceLayer: string, featureId?: number, key?: string) { + removeFeatureState(sourceLayer: string, featureId?: number | string, key?: string) { const sourceLayerDeleted = this.deletedStates[sourceLayer] === null; if (sourceLayerDeleted) return; @@ -62,12 +62,12 @@ class SourceFeatureState { this.deletedStates[sourceLayer] = this.deletedStates[sourceLayer] || {}; - if (key && featureId !== undefined && featureId >= 0) { + if (key && featureId !== undefined) { if (this.deletedStates[sourceLayer][feature] !== null) { this.deletedStates[sourceLayer][feature] = this.deletedStates[sourceLayer][feature] || {}; this.deletedStates[sourceLayer][feature][key] = null; } - } else if (featureId !== undefined && featureId >= 0) { + } else if (featureId !== undefined) { const updateInQueue = this.stateChanges[sourceLayer] && this.stateChanges[sourceLayer][feature]; if (updateInQueue) { this.deletedStates[sourceLayer][feature] = {}; @@ -82,7 +82,7 @@ class SourceFeatureState { } - getState(sourceLayer: string, featureId: number) { + getState(sourceLayer: string, featureId: number | string) { const feature = String(featureId); const base = this.state[sourceLayer] || {}; const changes = this.stateChanges[sourceLayer] || {}; diff --git a/src/source/vector_tile_source.js b/src/source/vector_tile_source.js index 69512678cba..33831d59b46 100644 --- a/src/source/vector_tile_source.js +++ b/src/source/vector_tile_source.js @@ -28,6 +28,7 @@ class VectorTileSource extends Evented implements Source { url: string; scheme: string; tileSize: number; + featureStateID: ?{[string]: string}; _options: VectorSourceSpecification; _collectResourceTiming: boolean; diff --git a/src/source/worker_source.js b/src/source/worker_source.js index c0b26deee20..cf942621ebd 100644 --- a/src/source/worker_source.js +++ b/src/source/worker_source.js @@ -23,6 +23,7 @@ export type WorkerTileParameters = TileParameters & { zoom: number, maxZoom: number, tileSize: number, + featureStateID: ?{[string]: string}, pixelRatio: number, showCollisionBoxes: boolean, collectResourceTiming?: boolean, diff --git a/src/source/worker_tile.js b/src/source/worker_tile.js index 4adfe8c8edc..322c3815ad7 100644 --- a/src/source/worker_tile.js +++ b/src/source/worker_tile.js @@ -34,6 +34,7 @@ class WorkerTile { pixelRatio: number; tileSize: number; source: string; + featureStateID: ?{[string]: string}; overscaling: number; showCollisionBoxes: boolean; collectResourceTiming: boolean; From 6c146ab9f843c8c0478ae650e57b5bf297317918 Mon Sep 17 00:00:00 2001 From: Vladimir Agafonkin Date: Wed, 27 Nov 2019 17:53:41 +0200 Subject: [PATCH 03/12] rework promoteId according to PR discussions --- bench/lib/tile_parser.js | 2 +- debug/featurestate.html | 2 +- src/data/bucket.js | 4 ++-- src/data/bucket/circle_bucket.js | 6 +++--- src/data/bucket/fill_bucket.js | 6 +++--- src/data/bucket/fill_extrusion_bucket.js | 5 +++-- src/data/bucket/line_bucket.js | 2 +- src/data/bucket/symbol_bucket.js | 12 ++++-------- src/data/feature_index.js | 10 +++++----- src/data/program_configuration.js | 10 +++------- src/source/vector_tile_source.js | 6 +++--- src/source/worker_source.js | 2 +- src/source/worker_tile.js | 16 ++++++++++------ 13 files changed, 40 insertions(+), 43 deletions(-) diff --git a/bench/lib/tile_parser.js b/bench/lib/tile_parser.js index 6c0852bb6f9..e28afd39f2f 100644 --- a/bench/lib/tile_parser.js +++ b/bench/lib/tile_parser.js @@ -130,7 +130,7 @@ export default class TileParser { cameraToCenterDistance: 0, cameraToTileDistance: 0, returnDependencies, - featureStateID: undefined + promoteId: undefined }); const vectorTile = new VT.VectorTile(new Protobuf(tile.buffer)); diff --git a/debug/featurestate.html b/debug/featurestate.html index fb58e2c3058..11c9d0f1317 100644 --- a/debug/featurestate.html +++ b/debug/featurestate.html @@ -30,7 +30,7 @@ map.addSource('counties', { "type": "vector", "url": "mapbox://mapbox.82pkq93d", - "featureStateID": {"original": "COUNTY"} + "promoteId": {"original": "COUNTY"} }); map.addLayer({ diff --git a/src/data/bucket.js b/src/data/bucket.js index d6675f55e7c..a6be33f9aa2 100644 --- a/src/data/bucket.js +++ b/src/data/bucket.js @@ -16,8 +16,7 @@ export type BucketParameters = { overscaling: number, collisionBoxArray: CollisionBoxArray, sourceLayerIndex: number, - sourceID: string, - featureStateID?: string + sourceID: string } export type PopulateParameters = { @@ -30,6 +29,7 @@ export type PopulateParameters = { export type IndexedFeature = { feature: VectorTileFeature, + id: number | string, index: number, sourceLayerIndex: number, } diff --git a/src/data/bucket/circle_bucket.js b/src/data/bucket/circle_bucket.js index d652d1c7b03..8feb187d1e0 100644 --- a/src/data/bucket/circle_bucket.js +++ b/src/data/bucket/circle_bucket.js @@ -71,7 +71,7 @@ class CircleBucket implements Bucke this.layoutVertexArray = new CircleLayoutArray(); this.indexArray = new TriangleIndexArray(); this.segments = new SegmentVector(); - this.programConfigurations = new ProgramConfigurationSet(layoutAttributes, options.layers, options.zoom, undefined, options.featureStateID); + this.programConfigurations = new ProgramConfigurationSet(layoutAttributes, options.layers, options.zoom); this.stateDependentLayerIds = this.layers.filter((l) => l.isStateDependent()).map((l) => l.id); } @@ -85,7 +85,7 @@ class CircleBucket implements Bucke circleSortKey = ((styleLayer: any): CircleStyleLayer).layout.get('circle-sort-key'); } - for (const {feature, index, sourceLayerIndex} of features) { + for (const {feature, id, index, sourceLayerIndex} of features) { if (this.layers[0]._featureFilter(new EvaluationParameters(this.zoom), feature)) { const geometry = loadGeometry(feature); const sortKey = circleSortKey ? @@ -93,7 +93,7 @@ class CircleBucket implements Bucke undefined; const bucketFeature: BucketFeature = { - id: feature.id, + id, properties: feature.properties, type: feature.type, sourceLayerIndex, diff --git a/src/data/bucket/fill_bucket.js b/src/data/bucket/fill_bucket.js index 681af69b25f..711d200c007 100644 --- a/src/data/bucket/fill_bucket.js +++ b/src/data/bucket/fill_bucket.js @@ -67,7 +67,7 @@ class FillBucket implements Bucket { this.layoutVertexArray = new FillLayoutArray(); this.indexArray = new TriangleIndexArray(); this.indexArray2 = new LineIndexArray(); - this.programConfigurations = new ProgramConfigurationSet(layoutAttributes, options.layers, options.zoom, undefined, options.featureStateID); + this.programConfigurations = new ProgramConfigurationSet(layoutAttributes, options.layers, options.zoom); this.segments = new SegmentVector(); this.segments2 = new SegmentVector(); this.stateDependentLayerIds = this.layers.filter((l) => l.isStateDependent()).map((l) => l.id); @@ -78,7 +78,7 @@ class FillBucket implements Bucket { const fillSortKey = this.layers[0].layout.get('fill-sort-key'); const bucketFeatures = []; - for (const {feature, index, sourceLayerIndex} of features) { + for (const {feature, id, index, sourceLayerIndex} of features) { if (!this.layers[0]._featureFilter(new EvaluationParameters(this.zoom), feature)) continue; const geometry = loadGeometry(feature); @@ -87,7 +87,7 @@ class FillBucket implements Bucket { undefined; const bucketFeature: BucketFeature = { - id: feature.id, + id, properties: feature.properties, type: feature.type, sourceLayerIndex, diff --git a/src/data/bucket/fill_extrusion_bucket.js b/src/data/bucket/fill_extrusion_bucket.js index 04e0a512305..901f9e8079b 100644 --- a/src/data/bucket/fill_extrusion_bucket.js +++ b/src/data/bucket/fill_extrusion_bucket.js @@ -81,7 +81,7 @@ class FillExtrusionBucket implements Bucket { this.layoutVertexArray = new FillExtrusionLayoutArray(); this.indexArray = new TriangleIndexArray(); - this.programConfigurations = new ProgramConfigurationSet(layoutAttributes, options.layers, options.zoom, undefined, options.featureStateID); + this.programConfigurations = new ProgramConfigurationSet(layoutAttributes, options.layers, options.zoom); this.segments = new SegmentVector(); this.stateDependentLayerIds = this.layers.filter((l) => l.isStateDependent()).map((l) => l.id); @@ -91,12 +91,13 @@ class FillExtrusionBucket implements Bucket { this.features = []; this.hasPattern = hasPattern('fill-extrusion', this.layers, options); - for (const {feature, index, sourceLayerIndex} of features) { + for (const {feature, id, index, sourceLayerIndex} of features) { if (!this.layers[0]._featureFilter(new EvaluationParameters(this.zoom), feature)) continue; const geometry = loadGeometry(feature); const patternFeature: BucketFeature = { + id, sourceLayerIndex, index, geometry, diff --git a/src/data/bucket/line_bucket.js b/src/data/bucket/line_bucket.js index 6e318eda877..4e1f31c65d7 100644 --- a/src/data/bucket/line_bucket.js +++ b/src/data/bucket/line_bucket.js @@ -110,7 +110,7 @@ class LineBucket implements Bucket { this.layoutVertexArray = new LineLayoutArray(); this.indexArray = new TriangleIndexArray(); - this.programConfigurations = new ProgramConfigurationSet(layoutAttributes, options.layers, options.zoom, undefined, options.featureStateID); + this.programConfigurations = new ProgramConfigurationSet(layoutAttributes, options.layers, options.zoom); this.segments = new SegmentVector(); this.stateDependentLayerIds = this.layers.filter((l) => l.isStateDependent()).map((l) => l.id); diff --git a/src/data/bucket/symbol_bucket.js b/src/data/bucket/symbol_bucket.js index e557845c777..3da6432922c 100644 --- a/src/data/bucket/symbol_bucket.js +++ b/src/data/bucket/symbol_bucket.js @@ -302,7 +302,6 @@ class SymbolBucket implements Bucket { uploaded: boolean; sourceLayerIndex: number; sourceID: string; - featureStateID: ?string; symbolInstanceIndexes: Array; writingModes: Array; allowVerticalPlacement: boolean; @@ -318,7 +317,6 @@ class SymbolBucket implements Bucket { this.index = options.index; this.pixelRatio = options.pixelRatio; this.sourceLayerIndex = options.sourceLayerIndex; - this.featureStateID = options.featureStateID; this.hasPattern = false; this.hasPaintOverrides = false; this.hasRTLText = false; @@ -350,8 +348,8 @@ class SymbolBucket implements Bucket { const layout = this.layers[0].layout; this.hasPaintOverrides = SymbolStyleLayer.hasPaintOverrides(layout); - this.text = new SymbolBuffers(new ProgramConfigurationSet(symbolLayoutAttributes.members, this.layers, this.zoom, property => /^text/.test(property), this.featureStateID)); - this.icon = new SymbolBuffers(new ProgramConfigurationSet(symbolLayoutAttributes.members, this.layers, this.zoom, property => /^icon/.test(property), this.featureStateID)); + this.text = new SymbolBuffers(new ProgramConfigurationSet(symbolLayoutAttributes.members, this.layers, this.zoom, property => /^text/.test(property))); + this.icon = new SymbolBuffers(new ProgramConfigurationSet(symbolLayoutAttributes.members, this.layers, this.zoom, property => /^icon/.test(property))); this.textCollisionBox = new CollisionBuffers(CollisionBoxLayoutArray, collisionBoxLayout.members, LineIndexArray); this.iconCollisionBox = new CollisionBuffers(CollisionBoxLayoutArray, collisionBoxLayout.members, LineIndexArray); @@ -403,7 +401,7 @@ class SymbolBucket implements Bucket { const availableImages = options.availableImages; const globalProperties = new EvaluationParameters(this.zoom); - for (const {feature, index, sourceLayerIndex} of features) { + for (const {feature, id, index, sourceLayerIndex} of features) { if (!layer._featureFilter(globalProperties, feature)) { continue; } @@ -449,6 +447,7 @@ class SymbolBucket implements Bucket { undefined; const symbolFeature: SymbolFeature = { + id, text, icon, index, @@ -458,9 +457,6 @@ class SymbolBucket implements Bucket { type: vectorTileFeatureTypes[feature.type], sortKey }; - if (typeof feature.id !== 'undefined') { - symbolFeature.id = feature.id; - } this.features.push(symbolFeature); if (icon) { diff --git a/src/data/feature_index.js b/src/data/feature_index.js index 157717014a5..93e3c9f6d02 100644 --- a/src/data/feature_index.js +++ b/src/data/feature_index.js @@ -46,6 +46,7 @@ class FeatureIndex { grid: Grid; grid3D: Grid; featureIndexArray: FeatureIndexArray; + promoteId: ?{[string]: string} | string; rawTileData: ArrayBuffer; bucketLayerIDs: Array>; @@ -53,16 +54,15 @@ class FeatureIndex { vtLayers: {[string]: VectorTileLayer}; sourceLayerCoder: DictionaryCoder; - constructor(tileID: OverscaledTileID, - grid?: Grid, - featureIndexArray?: FeatureIndexArray) { + constructor(tileID: OverscaledTileID, promoteId?: ?{[string]: string} | string) { this.tileID = tileID; this.x = tileID.canonical.x; this.y = tileID.canonical.y; this.z = tileID.canonical.z; - this.grid = grid || new Grid(EXTENT, 16, 0); + this.grid = new Grid(EXTENT, 16, 0); this.grid3D = new Grid(EXTENT, 16, 0); - this.featureIndexArray = featureIndexArray || new FeatureIndexArray(); + this.featureIndexArray = new FeatureIndexArray(); + this.promoteId = promoteId; } insert(feature: VectorTileFeature, geometry: Array>, featureIndex: number, sourceLayerIndex: number, bucketIndex: number, is3D?: boolean) { diff --git a/src/data/program_configuration.js b/src/data/program_configuration.js index 13d1d157b7d..8a8a62f623a 100644 --- a/src/data/program_configuration.js +++ b/src/data/program_configuration.js @@ -727,10 +727,9 @@ export class ProgramConfigurationSet { programConfigurations: {[string]: ProgramConfiguration}; needsUpload: boolean; _featureMap: FeaturePositionMap; - _featureStateID: ?string; _bufferOffset: number; - constructor(layoutAttributes: Array, layers: $ReadOnlyArray, zoom: number, filterProperties: (string) => boolean = () => true, featureStateID: ?string) { + constructor(layoutAttributes: Array, layers: $ReadOnlyArray, zoom: number, filterProperties: (string) => boolean = () => true) { this.programConfigurations = {}; for (const layer of layers) { this.programConfigurations[layer.id] = ProgramConfiguration.createDynamic(layer, zoom, filterProperties); @@ -738,7 +737,6 @@ export class ProgramConfigurationSet { } this.needsUpload = false; this._featureMap = new FeaturePositionMap(); - this._featureStateID = featureStateID; this._bufferOffset = 0; } @@ -747,10 +745,8 @@ export class ProgramConfigurationSet { this.programConfigurations[key].populatePaintArrays(length, feature, index, imagePositions, formattedSection); } - const id = this._featureStateID ? feature.properties[this._featureStateID] : feature.id; - - if (id !== undefined) { - this._featureMap.add(id, index, this._bufferOffset, length); + if (feature.id !== undefined) { + this._featureMap.add(feature.id, index, this._bufferOffset, length); } this._bufferOffset = length; diff --git a/src/source/vector_tile_source.js b/src/source/vector_tile_source.js index 33831d59b46..8c0feeb2130 100644 --- a/src/source/vector_tile_source.js +++ b/src/source/vector_tile_source.js @@ -28,7 +28,7 @@ class VectorTileSource extends Evented implements Source { url: string; scheme: string; tileSize: number; - featureStateID: ?{[string]: string}; + promoteId: ?{[string]: string}; _options: VectorSourceSpecification; _collectResourceTiming: boolean; @@ -56,7 +56,7 @@ class VectorTileSource extends Evented implements Source { this.isTileClipped = true; this._loaded = false; - extend(this, pick(options, ['url', 'scheme', 'tileSize', 'featureStateID'])); + extend(this, pick(options, ['url', 'scheme', 'tileSize', 'promoteId'])); this._options = extend({type: 'vector'}, options); this._collectResourceTiming = options.collectResourceTiming; @@ -127,7 +127,7 @@ class VectorTileSource extends Evented implements Source { source: this.id, pixelRatio: browser.devicePixelRatio, showCollisionBoxes: this.map.showCollisionBoxes, - featureStateID: this.featureStateID + promoteId: this.promoteId }; params.request.collectResourceTiming = this._collectResourceTiming; diff --git a/src/source/worker_source.js b/src/source/worker_source.js index cf942621ebd..feb8ee0f182 100644 --- a/src/source/worker_source.js +++ b/src/source/worker_source.js @@ -23,7 +23,7 @@ export type WorkerTileParameters = TileParameters & { zoom: number, maxZoom: number, tileSize: number, - featureStateID: ?{[string]: string}, + promoteId: ?{[string]: string} | string, pixelRatio: number, showCollisionBoxes: boolean, collectResourceTiming?: boolean, diff --git a/src/source/worker_tile.js b/src/source/worker_tile.js index 322c3815ad7..4ecd4dfc2f7 100644 --- a/src/source/worker_tile.js +++ b/src/source/worker_tile.js @@ -34,7 +34,7 @@ class WorkerTile { pixelRatio: number; tileSize: number; source: string; - featureStateID: ?{[string]: string}; + promoteId: ?{[string]: string} | string; overscaling: number; showCollisionBoxes: boolean; collectResourceTiming: boolean; @@ -59,7 +59,7 @@ class WorkerTile { this.showCollisionBoxes = params.showCollisionBoxes; this.collectResourceTiming = !!params.collectResourceTiming; this.returnDependencies = !!params.returnDependencies; - this.featureStateID = params.featureStateID; + this.promoteId = params.promoteId; } parse(data: VectorTile, layerIndex: StyleLayerIndex, availableImages: Array, actor: Actor, callback: WorkerTileCallback) { @@ -69,7 +69,7 @@ class WorkerTile { this.collisionBoxArray = new CollisionBoxArray(); const sourceLayerCoder = new DictionaryCoder(Object.keys(data.layers).sort()); - const featureIndex = new FeatureIndex(this.tileID); + const featureIndex = new FeatureIndex(this.tileID, this.promoteId); featureIndex.bucketLayerIDs = []; const buckets: {[string]: Bucket} = {}; @@ -98,7 +98,12 @@ class WorkerTile { const features = []; for (let index = 0; index < sourceLayer.length; index++) { const feature = sourceLayer.feature(index); - features.push({feature, index, sourceLayerIndex}); + let id = feature.id; + if (this.promoteId) { + const propName = typeof this.promoteId === 'string' ? this.promoteId : this.promoteId[sourceLayerId]; + id = feature.properties[propName]; + } + features.push({feature, id, index, sourceLayerIndex}); } for (const family of layerFamilies[sourceLayerId]) { @@ -119,8 +124,7 @@ class WorkerTile { overscaling: this.overscaling, collisionBoxArray: this.collisionBoxArray, sourceLayerIndex, - sourceID: this.source, - featureStateID: this.featureStateID && this.featureStateID[sourceLayerId] + sourceID: this.source }); bucket.populate(features, options); From ad09b9de2382da499a56d3f11b76e8d02a7d8454 Mon Sep 17 00:00:00 2001 From: Vladimir Agafonkin Date: Wed, 27 Nov 2019 18:49:32 +0200 Subject: [PATCH 04/12] properly expose promoted ids in query methods --- src/data/feature_index.js | 22 +++++++++++++++++----- src/source/tile.js | 8 +++++--- src/source/worker_tile.js | 6 +----- src/util/vectortile_to_geojson.js | 7 ++----- 4 files changed, 25 insertions(+), 18 deletions(-) diff --git a/src/data/feature_index.js b/src/data/feature_index.js index 93e3c9f6d02..1fa7d468530 100644 --- a/src/data/feature_index.js +++ b/src/data/feature_index.js @@ -146,12 +146,12 @@ class FeatureIndex { filter, params.layers, styleLayers, - (feature: VectorTileFeature, styleLayer: StyleLayer) => { + (feature: VectorTileFeature, styleLayer: StyleLayer, id: ?string | number) => { if (!featureGeometry) { featureGeometry = loadGeometry(feature); } let featureState = {}; - if (feature.id) { + if (id) { // `feature-state` expression evaluation requires feature state to be available featureState = sourceFeatureState.getState(styleLayer.sourceLayer || '_geojsonTileLayer', feature.id); } @@ -171,7 +171,7 @@ class FeatureIndex { filter: FeatureFilter, filterLayerIDs: Array, styleLayers: {[string]: StyleLayer}, - intersectionTest?: (feature: VectorTileFeature, styleLayer: StyleLayer) => boolean | number) { + intersectionTest?: (feature: VectorTileFeature, styleLayer: StyleLayer, id: ?string | number) => boolean | number) { const layerIDs = this.bucketLayerIDs[bucketIndex]; if (filterLayerIDs && !arraysIntersect(filterLayerIDs, layerIDs)) @@ -184,6 +184,8 @@ class FeatureIndex { if (!filter(new EvaluationParameters(this.tileID.overscaledZ), feature)) return; + let id = this.getId(feature, sourceLayerName); + for (let l = 0; l < layerIDs.length; l++) { const layerID = layerIDs[l]; @@ -194,13 +196,13 @@ class FeatureIndex { const styleLayer = styleLayers[layerID]; if (!styleLayer) continue; - const intersectionZ = !intersectionTest || intersectionTest(feature, styleLayer); + const intersectionZ = !intersectionTest || intersectionTest(feature, styleLayer, id); if (!intersectionZ) { // Only applied for non-symbol features continue; } - const geojsonFeature = new GeoJSONFeature(feature, this.z, this.x, this.y); + const geojsonFeature = new GeoJSONFeature(feature, this.z, this.x, this.y, id); (geojsonFeature: any).layer = styleLayer.serialize(); let layerResult = result[layerID]; if (layerResult === undefined) { @@ -247,6 +249,16 @@ class FeatureIndex { return false; } + + getId(feature: VectorTileFeature, sourceLayerId: string): string | number | void { + let id = feature.id; + if (this.promoteId) { + const propName = typeof this.promoteId === 'string' ? this.promoteId : this.promoteId[sourceLayerId]; + id = feature.properties[propName]; + if (typeof id === 'boolean') id = Number(id); + } + return id; + } } register( diff --git a/src/source/tile.js b/src/source/tile.js index e714007e2e1..46d18783bb7 100644 --- a/src/source/tile.js +++ b/src/source/tile.js @@ -297,9 +297,10 @@ class Tile { } querySourceFeatures(result: Array, params: any) { - if (!this.latestFeatureIndex || !this.latestFeatureIndex.rawTileData) return; + const featureIndex = this.latestFeatureIndex; + if (!featureIndex || !featureIndex.rawTileData) return; - const vtLayers = this.latestFeatureIndex.loadVTLayers(); + const vtLayers = featureIndex.loadVTLayers(); const sourceLayer = params ? params.sourceLayer : ''; const layer = vtLayers._geojsonTileLayer || vtLayers[sourceLayer]; @@ -313,7 +314,8 @@ class Tile { for (let i = 0; i < layer.length; i++) { const feature = layer.feature(i); if (filter(new EvaluationParameters(this.tileID.overscaledZ), feature)) { - const geojsonFeature = new GeoJSONFeature(feature, z, x, y); + const id = featureIndex.getId(feature, sourceLayer); + const geojsonFeature = new GeoJSONFeature(feature, z, x, y, id); (geojsonFeature: any).tile = coord; result.push(geojsonFeature); } diff --git a/src/source/worker_tile.js b/src/source/worker_tile.js index 4ecd4dfc2f7..118d93b600d 100644 --- a/src/source/worker_tile.js +++ b/src/source/worker_tile.js @@ -98,11 +98,7 @@ class WorkerTile { const features = []; for (let index = 0; index < sourceLayer.length; index++) { const feature = sourceLayer.feature(index); - let id = feature.id; - if (this.promoteId) { - const propName = typeof this.promoteId === 'string' ? this.promoteId : this.promoteId[sourceLayerId]; - id = feature.properties[propName]; - } + const id = featureIndex.getId(feature, sourceLayerId); features.push({feature, id, index, sourceLayerIndex}); } diff --git a/src/util/vectortile_to_geojson.js b/src/util/vectortile_to_geojson.js index 24c156e2a1b..87df3344a4f 100644 --- a/src/util/vectortile_to_geojson.js +++ b/src/util/vectortile_to_geojson.js @@ -9,7 +9,7 @@ class Feature { _vectorTileFeature: VectorTileFeature; - constructor(vectorTileFeature: VectorTileFeature, z: number, x: number, y: number) { + constructor(vectorTileFeature: VectorTileFeature, z: number, x: number, y: number, id: string | number | void) { this.type = 'Feature'; this._vectorTileFeature = vectorTileFeature; @@ -18,10 +18,7 @@ class Feature { (vectorTileFeature: any)._y = y; this.properties = vectorTileFeature.properties; - - if (vectorTileFeature.id != null) { - this.id = vectorTileFeature.id; - } + this.id = id; } get geometry(): ?GeoJSONGeometry { From bf1265318178e20ea2e7f38b96bc32b0566b9d74 Mon Sep 17 00:00:00 2001 From: Vladimir Agafonkin Date: Wed, 27 Nov 2019 18:50:46 +0200 Subject: [PATCH 05/12] fix lint --- src/data/feature_index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/data/feature_index.js b/src/data/feature_index.js index 1fa7d468530..74a02cc3ad4 100644 --- a/src/data/feature_index.js +++ b/src/data/feature_index.js @@ -184,7 +184,7 @@ class FeatureIndex { if (!filter(new EvaluationParameters(this.tileID.overscaledZ), feature)) return; - let id = this.getId(feature, sourceLayerName); + const id = this.getId(feature, sourceLayerName); for (let l = 0; l < layerIDs.length; l++) { const layerID = layerIDs[l]; From b7935e53278d9d826050cc020e1f69566043bc47 Mon Sep 17 00:00:00 2001 From: Vladimir Agafonkin Date: Fri, 29 Nov 2019 12:19:24 +0200 Subject: [PATCH 06/12] use state ids that can be cast as Float64 as is; update tests --- src/data/feature_position_map.js | 8 +-- src/style/style.js | 3 + test/unit/ui/map.test.js | 116 +++++++++---------------------- 3 files changed, 41 insertions(+), 86 deletions(-) diff --git a/src/data/feature_position_map.js b/src/data/feature_position_map.js index 9c01956c478..c48222e43c6 100644 --- a/src/data/feature_position_map.js +++ b/src/data/feature_position_map.js @@ -28,14 +28,14 @@ export default class FeaturePositionMap { } add(id: mixed, index: number, start: number, end: number) { - this.ids.push(getIntegerId(id)); + this.ids.push(getNumericId(id)); this.positions.push(index, start, end); } getPositions(id: mixed): Array { assert(this.indexed); - const intId = getIntegerId(id); + const intId = getNumericId(id); // binary search for the first occurrence of id in this.ids; // relies on ids/positions being sorted by id, which happens in serialization @@ -82,9 +82,9 @@ export default class FeaturePositionMap { } } -export function getIntegerId(value: mixed) { +export function getNumericId(value: mixed) { const numValue = +value; - if (!isNaN(numValue) && numValue % 1 === 0) { + if (!isNaN(numValue)) { return numValue; } return murmur3(String(value)); diff --git a/src/style/style.js b/src/style/style.js index 92a84650352..f7245dab1de 100644 --- a/src/style/style.js +++ b/src/style/style.js @@ -864,6 +864,9 @@ class Style extends Evented { this.fire(new ErrorEvent(new Error(`The sourceLayer parameter must be provided for vector source types.`))); return; } + if (target.id === undefined) { + this.fire(new ErrorEvent(new Error(`The feature id parameter must be provided.`))); + } sourceCache.setFeatureState(sourceLayer, target.id, state); } diff --git a/test/unit/ui/map.test.js b/test/unit/ui/map.test.js index cbb5c512b43..76cda8f792d 100755 --- a/test/unit/ui/map.test.js +++ b/test/unit/ui/map.test.js @@ -1446,6 +1446,23 @@ test('Map', (t) => { t.end(); }); }); + t.test('works with string ids', (t) => { + const map = createMap(t, { + style: { + "version": 8, + "sources": { + "geojson": createStyleSource() + }, + "layers": [] + } + }); + map.on('load', () => { + map.setFeatureState({source: 'geojson', id: 'foo'}, {'hover': true}); + const fState = map.getFeatureState({source: 'geojson', id: 'foo'}); + t.equal(fState.hover, true); + t.end(); + }); + }); t.test('parses feature id as an int', (t) => { const map = createMap(t, { style: { @@ -1539,54 +1556,31 @@ test('Map', (t) => { map.setFeatureState({source: 'vector', sourceLayer: "1"}, {'hover': true}); }); }); - t.test('fires an error if id is less than zero', (t) => { - const map = createMap(t, { - style: { - "version": 8, - "sources": { - "vector": { - "type": "vector", - "tiles": ["http://example.com/{z}/{x}/{y}.png"] - } - }, - "layers": [] - } - }); - map.on('load', () => { - map.on('error', ({error}) => { - t.match(error.message, /id/); - t.end(); - }); - map.setFeatureState({source: 'vector', sourceLayer: "1", id: -1}, {'hover': true}); - }); - }); - t.test('fires an error if id cannot be parsed as an int', (t) => { + t.end(); + }); + + t.test('#removeFeatureState', (t) => { + + t.test('accepts "0" id', (t) => { const map = createMap(t, { style: { "version": 8, "sources": { - "vector": { - "type": "vector", - "tiles": ["http://example.com/{z}/{x}/{y}.png"] - } + "geojson": createStyleSource() }, "layers": [] } }); map.on('load', () => { - map.on('error', ({error}) => { - t.match(error.message, /id/); - t.end(); - }); - map.setFeatureState({source: 'vector', sourceLayer: "1", id: 'abc'}, {'hover': true}); + map.setFeatureState({source: 'geojson', id: 0}, {'hover': true, 'click': true}); + map.removeFeatureState({source: 'geojson', id: 0}, 'hover'); + const fState = map.getFeatureState({source: 'geojson', id: 0}); + t.equal(fState.hover, undefined); + t.equal(fState.click, true); + t.end(); }); }); - t.end(); - }); - - t.test('#removeFeatureState', (t) => { - - t.test('accepts "0" id', (t) => { + t.test('accepts string id', (t) => { const map = createMap(t, { style: { "version": 8, @@ -1597,9 +1591,9 @@ test('Map', (t) => { } }); map.on('load', () => { - map.setFeatureState({source: 'geojson', id: 0}, {'hover': true, 'click': true}); - map.removeFeatureState({source: 'geojson', id: 0}, 'hover'); - const fState = map.getFeatureState({source: 'geojson', id: 0}); + map.setFeatureState({source: 'geojson', id: 'foo'}, {'hover': true, 'click': true}); + map.removeFeatureState({source: 'geojson', id: 'foo'}, 'hover'); + const fState = map.getFeatureState({source: 'geojson', id: 'foo'}); t.equal(fState.hover, undefined); t.equal(fState.click, true); t.end(); @@ -1852,48 +1846,6 @@ test('Map', (t) => { map.removeFeatureState({source: 'vector', sourceLayer: "1"}, {'hover': true}); }); }); - t.test('removeFeatureState fires an error if id is less than zero', (t) => { - const map = createMap(t, { - style: { - "version": 8, - "sources": { - "vector": { - "type": "vector", - "tiles": ["http://example.com/{z}/{x}/{y}.png"] - } - }, - "layers": [] - } - }); - map.on('load', () => { - map.on('error', ({error}) => { - t.match(error.message, /id/); - t.end(); - }); - map.removeFeatureState({source: 'vector', sourceLayer: "1", id: -1}, {'hover': true}); - }); - }); - t.test('fires an error if id cannot be parsed as an int', (t) => { - const map = createMap(t, { - style: { - "version": 8, - "sources": { - "vector": { - "type": "vector", - "tiles": ["http://example.com/{z}/{x}/{y}.png"] - } - }, - "layers": [] - } - }); - map.on('load', () => { - map.on('error', ({error}) => { - t.match(error.message, /id/); - t.end(); - }); - map.removeFeatureState({source: 'vector', sourceLayer: "1", id: 'abc'}, {'hover': true}); - }); - }); t.end(); }); From 8271bd0a52d965dc8d3fd4d38537235f62515afc Mon Sep 17 00:00:00 2001 From: Vladimir Agafonkin Date: Fri, 29 Nov 2019 12:49:53 +0200 Subject: [PATCH 07/12] promoteId: geojson support, better flow typing, v8 docs --- build/generate-flow-typed-style-spec.js | 2 ++ debug/featurestate.html | 21 ++++++--------------- src/data/feature_index.js | 6 +++--- src/source/geojson_source.js | 7 +++++-- src/source/vector_tile_source.js | 4 ++-- src/source/worker_source.js | 3 ++- src/source/worker_tile.js | 3 ++- src/style-spec/reference/v8.json | 14 +++++++++++--- src/style-spec/types.js | 8 ++++++-- 9 files changed, 39 insertions(+), 29 deletions(-) diff --git a/build/generate-flow-typed-style-spec.js b/build/generate-flow-typed-style-spec.js index bab7b6a405d..506f5ec532f 100644 --- a/build/generate-flow-typed-style-spec.js +++ b/build/generate-flow-typed-style-spec.js @@ -122,6 +122,8 @@ export type FormattedSpecification = string; export type ResolvedImageSpecification = string; +export type PromoteIdSpecification = {[string]: string} | string; + export type FilterSpecification = | ['has', string] | ['!has', string] diff --git a/debug/featurestate.html b/debug/featurestate.html index 11c9d0f1317..7d35702d5b3 100644 --- a/debug/featurestate.html +++ b/debug/featurestate.html @@ -48,11 +48,7 @@ function resetFeatureState() { if (selectedCounty) { - map.setFeatureState({ - source: 'counties', - sourceLayer: 'original', - id: selectedCounty - }, {hover: false}); + map.setFeatureState({source: 'counties', sourceLayer: 'original', id: selectedCounty}, {hover: false}); selectedCounty = null; } } @@ -62,17 +58,12 @@ }); map.on("mousemove", "counties", (e) => { - for (const feature of e.features) { - if (selectedCounty !== feature.properties.COUNTY) { - resetFeatureState(); - selectedCounty = feature.properties.COUNTY; - } + const feature = e.features[0]; - map.setFeatureState({ - source: 'counties', - sourceLayer: 'original', - id: selectedCounty - }, {hover: true}); + if (selectedCounty !== feature.id) { + resetFeatureState(); + map.setFeatureState({source: 'counties', sourceLayer: 'original', id: feature.id}, {hover: true}); + selectedCounty = feature.id; } }); }); diff --git a/src/data/feature_index.js b/src/data/feature_index.js index 74a02cc3ad4..0e77f9b4471 100644 --- a/src/data/feature_index.js +++ b/src/data/feature_index.js @@ -20,7 +20,7 @@ import {polygonIntersectsBox} from '../util/intersection_tests'; import type StyleLayer from '../style/style_layer'; import type {FeatureFilter} from '../style-spec/feature_filter'; import type Transform from '../geo/transform'; -import type {FilterSpecification} from '../style-spec/types'; +import type {FilterSpecification, PromoteIdSpecification} from '../style-spec/types'; import {FeatureIndexArray} from './array_types'; @@ -46,7 +46,7 @@ class FeatureIndex { grid: Grid; grid3D: Grid; featureIndexArray: FeatureIndexArray; - promoteId: ?{[string]: string} | string; + promoteId: ?PromoteIdSpecification; rawTileData: ArrayBuffer; bucketLayerIDs: Array>; @@ -54,7 +54,7 @@ class FeatureIndex { vtLayers: {[string]: VectorTileLayer}; sourceLayerCoder: DictionaryCoder; - constructor(tileID: OverscaledTileID, promoteId?: ?{[string]: string} | string) { + constructor(tileID: OverscaledTileID, promoteId?: ?PromoteIdSpecification) { this.tileID = tileID; this.x = tileID.canonical.x; this.y = tileID.canonical.y; diff --git a/src/source/geojson_source.js b/src/source/geojson_source.js index f9ce5f39ab9..b169bb534fb 100644 --- a/src/source/geojson_source.js +++ b/src/source/geojson_source.js @@ -14,7 +14,7 @@ import type Tile from './tile'; import type Actor from '../util/actor'; import type {Callback} from '../types/callback'; import type {GeoJSON, GeoJSONFeature} from '@mapbox/geojson-types'; -import type {GeoJSONSourceSpecification} from '../style-spec/types'; +import type {GeoJSONSourceSpecification, PromoteIdSpecification} from '../style-spec/types'; /** * A source containing GeoJSON. @@ -69,6 +69,7 @@ class GeoJSONSource extends Evented implements Source { maxzoom: number; tileSize: number; attribution: string; + promoteId: ?PromoteIdSpecification; isTileClipped: boolean; reparseOverscaled: boolean; @@ -114,6 +115,7 @@ class GeoJSONSource extends Evented implements Source { if (options.maxzoom !== undefined) this.maxzoom = options.maxzoom; if (options.type) this.type = options.type; if (options.attribution) this.attribution = options.attribution; + this.promoteId = options.promoteId; const scale = EXTENT / this.tileSize; @@ -295,7 +297,8 @@ class GeoJSONSource extends Evented implements Source { tileSize: this.tileSize, source: this.id, pixelRatio: browser.devicePixelRatio, - showCollisionBoxes: this.map.showCollisionBoxes + showCollisionBoxes: this.map.showCollisionBoxes, + promoteId: this.promoteId }; tile.request = this.actor.send(message, params, (err, data) => { diff --git a/src/source/vector_tile_source.js b/src/source/vector_tile_source.js index 8c0feeb2130..5854ab9e180 100644 --- a/src/source/vector_tile_source.js +++ b/src/source/vector_tile_source.js @@ -18,7 +18,7 @@ import type Dispatcher from '../util/dispatcher'; import type Tile from './tile'; import type {Callback} from '../types/callback'; import type {Cancelable} from '../types/cancelable'; -import type {VectorSourceSpecification} from '../style-spec/types'; +import type {VectorSourceSpecification, PromoteIdSpecification} from '../style-spec/types'; class VectorTileSource extends Evented implements Source { type: 'vector'; @@ -28,7 +28,7 @@ class VectorTileSource extends Evented implements Source { url: string; scheme: string; tileSize: number; - promoteId: ?{[string]: string}; + promoteId: ?PromoteIdSpecification; _options: VectorSourceSpecification; _collectResourceTiming: boolean; diff --git a/src/source/worker_source.js b/src/source/worker_source.js index feb8ee0f182..23f65855baa 100644 --- a/src/source/worker_source.js +++ b/src/source/worker_source.js @@ -11,6 +11,7 @@ import type {CollisionBoxArray} from '../data/array_types'; import type DEMData from '../data/dem_data'; import type {StyleGlyph} from '../style/style_glyph'; import type {StyleImage} from '../style/style_image'; +import type {PromoteIdSpecification} from '../style-spec/types'; export type TileParameters = { source: string, @@ -23,7 +24,7 @@ export type WorkerTileParameters = TileParameters & { zoom: number, maxZoom: number, tileSize: number, - promoteId: ?{[string]: string} | string, + promoteId: ?PromoteIdSpecification, pixelRatio: number, showCollisionBoxes: boolean, collectResourceTiming?: boolean, diff --git a/src/source/worker_tile.js b/src/source/worker_tile.js index 118d93b600d..45f43be32ef 100644 --- a/src/source/worker_tile.js +++ b/src/source/worker_tile.js @@ -26,6 +26,7 @@ import type { WorkerTileParameters, WorkerTileCallback, } from '../source/worker_source'; +import type {PromoteIdSpecification} from '../style-spec/types'; class WorkerTile { tileID: OverscaledTileID; @@ -34,7 +35,7 @@ class WorkerTile { pixelRatio: number; tileSize: number; source: string; - promoteId: ?{[string]: string} | string; + promoteId: ?PromoteIdSpecification; overscaling: number; showCollisionBoxes: boolean; collectResourceTiming: boolean; diff --git a/src/style-spec/reference/v8.json b/src/style-spec/reference/v8.json index 3b7c17d144e..4b0d0795e8d 100644 --- a/src/style-spec/reference/v8.json +++ b/src/style-spec/reference/v8.json @@ -317,6 +317,10 @@ "default": "mapbox", "doc": "The encoding used by this source. Mapbox Terrain RGB is used by default" }, + "promoteId": { + "type": "promoteId", + "doc": "A property to use as a feature id (for feature state). Either a property name, or an object of the form `{: }`." + }, "*": { "type": "*", "doc": "Other keys to configure the data source." @@ -383,9 +387,13 @@ "doc": "Whether to calculate line distance metrics. This is required for line layers that specify `line-gradient` values." }, "generateId": { - "type": "boolean", - "default": false, - "doc": "Whether to generate ids for the geojson features. When enabled, the `feature.id` property will be auto assigned based on its index in the `features` array, over-writing any previous values." + "type": "boolean", + "default": false, + "doc": "Whether to generate ids for the geojson features. When enabled, the `feature.id` property will be auto assigned based on its index in the `features` array, over-writing any previous values." + }, + "promoteId": { + "type": "promoteId", + "doc": "A property to use as a feature id (for feature state). Either a property name, or an object of the form `{: }`." } }, "source_video": { diff --git a/src/style-spec/types.js b/src/style-spec/types.js index 6be4d498266..bd74a4f62ba 100644 --- a/src/style-spec/types.js +++ b/src/style-spec/types.js @@ -8,6 +8,8 @@ export type FormattedSpecification = string; export type ResolvedImageSpecification = string; +export type PromoteIdSpecification = {[string]: string} | string; + export type FilterSpecification = | ['has', string] | ['!has', string] @@ -110,7 +112,8 @@ export type RasterDEMSourceSpecification = { "maxzoom"?: number, "tileSize"?: number, "attribution"?: string, - "encoding"?: "terrarium" | "mapbox" + "encoding"?: "terrarium" | "mapbox", + "promoteId"?: PromoteIdSpecification } export type GeoJSONSourceSpecification = {| @@ -125,7 +128,8 @@ export type GeoJSONSourceSpecification = {| "clusterMaxZoom"?: number, "clusterProperties"?: mixed, "lineMetrics"?: boolean, - "generateId"?: boolean + "generateId"?: boolean, + "promoteId"?: PromoteIdSpecification |} export type VideoSourceSpecification = {| From 3da8aa0ea6e447c2520cad3cbf6211fa47bf9fdb Mon Sep 17 00:00:00 2001 From: Vladimir Agafonkin Date: Fri, 29 Nov 2019 12:56:07 +0200 Subject: [PATCH 08/12] fix spec test --- test/unit/style-spec/spec.test.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/unit/style-spec/spec.test.js b/test/unit/style-spec/spec.test.js index 94f45686b23..62c5e7a4cd5 100644 --- a/test/unit/style-spec/spec.test.js +++ b/test/unit/style-spec/spec.test.js @@ -37,7 +37,8 @@ function validSchema(k, t, obj, ref, version, kind) { 'visibility-enum', 'property-type', 'formatted', - 'resolvedImage' + 'resolvedImage', + 'promoteId' ]); const keys = [ 'default', From 645608c9f33c520f298112c62b9d5dd0b5f50628 Mon Sep 17 00:00:00 2001 From: Vladimir Agafonkin Date: Fri, 29 Nov 2019 13:02:36 +0200 Subject: [PATCH 09/12] make sure id is provided in getFeatureState --- src/style/style.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/style/style.js b/src/style/style.js index f7245dab1de..858ed021fff 100644 --- a/src/style/style.js +++ b/src/style/style.js @@ -912,6 +912,9 @@ class Style extends Evented { this.fire(new ErrorEvent(new Error(`The sourceLayer parameter must be provided for vector source types.`))); return; } + if (target.id === undefined) { + this.fire(new ErrorEvent(new Error(`The feature id parameter must be provided.`))); + } return sourceCache.getFeatureState(sourceLayer, target.id); } From 8abdcd7c9e94ca22791f8fbd3272ea19cff2b840 Mon Sep 17 00:00:00 2001 From: Vladimir Agafonkin Date: Fri, 29 Nov 2019 23:55:23 +0200 Subject: [PATCH 10/12] minor fixes after review --- src/data/feature_index.js | 2 +- src/data/feature_position_map.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/data/feature_index.js b/src/data/feature_index.js index 0e77f9b4471..3f934776267 100644 --- a/src/data/feature_index.js +++ b/src/data/feature_index.js @@ -151,7 +151,7 @@ class FeatureIndex { featureGeometry = loadGeometry(feature); } let featureState = {}; - if (id) { + if (id !== undefined) { // `feature-state` expression evaluation requires feature state to be available featureState = sourceFeatureState.getState(styleLayer.sourceLayer || '_geojsonTileLayer', feature.id); } diff --git a/src/data/feature_position_map.js b/src/data/feature_position_map.js index c48222e43c6..389cbe5ff95 100644 --- a/src/data/feature_position_map.js +++ b/src/data/feature_position_map.js @@ -82,7 +82,7 @@ export default class FeaturePositionMap { } } -export function getNumericId(value: mixed) { +function getNumericId(value: mixed) { const numValue = +value; if (!isNaN(numValue)) { return numValue; From ce192148ca026b7c6074739e1b55c934d99637a0 Mon Sep 17 00:00:00 2001 From: Vladimir Agafonkin Date: Mon, 2 Dec 2019 11:54:17 +0200 Subject: [PATCH 11/12] hash ids > MAX_SAFE_INTEGER --- src/data/feature_index.js | 6 +++--- src/data/feature_position_map.js | 4 +++- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/data/feature_index.js b/src/data/feature_index.js index 3f934776267..eca434d7c1b 100644 --- a/src/data/feature_index.js +++ b/src/data/feature_index.js @@ -146,14 +146,14 @@ class FeatureIndex { filter, params.layers, styleLayers, - (feature: VectorTileFeature, styleLayer: StyleLayer, id: ?string | number) => { + (feature: VectorTileFeature, styleLayer: StyleLayer, id: string | number | void) => { if (!featureGeometry) { featureGeometry = loadGeometry(feature); } let featureState = {}; if (id !== undefined) { // `feature-state` expression evaluation requires feature state to be available - featureState = sourceFeatureState.getState(styleLayer.sourceLayer || '_geojsonTileLayer', feature.id); + featureState = sourceFeatureState.getState(styleLayer.sourceLayer || '_geojsonTileLayer', id); } return styleLayer.queryIntersectsFeature(queryGeometry, feature, featureState, featureGeometry, this.z, args.transform, pixelsToTileUnits, args.pixelPosMatrix); } @@ -171,7 +171,7 @@ class FeatureIndex { filter: FeatureFilter, filterLayerIDs: Array, styleLayers: {[string]: StyleLayer}, - intersectionTest?: (feature: VectorTileFeature, styleLayer: StyleLayer, id: ?string | number) => boolean | number) { + intersectionTest?: (feature: VectorTileFeature, styleLayer: StyleLayer, id: string | number | void) => boolean | number) { const layerIDs = this.bucketLayerIDs[bucketIndex]; if (filterLayerIDs && !arraysIntersect(filterLayerIDs, layerIDs)) diff --git a/src/data/feature_position_map.js b/src/data/feature_position_map.js index 389cbe5ff95..9db5e1f77a5 100644 --- a/src/data/feature_position_map.js +++ b/src/data/feature_position_map.js @@ -82,9 +82,11 @@ export default class FeaturePositionMap { } } +const MAX_SAFE_INTEGER = Math.pow(2, 53) - 1; + function getNumericId(value: mixed) { const numValue = +value; - if (!isNaN(numValue)) { + if (!isNaN(numValue) && numValue <= MAX_SAFE_INTEGER) { return numValue; } return murmur3(String(value)); From 71d135f05780201e50d8a41017b43a80679df741 Mon Sep 17 00:00:00 2001 From: Vladimir Agafonkin Date: Mon, 2 Dec 2019 12:40:51 +0200 Subject: [PATCH 12/12] overhaul promoteId validation, add render test --- src/style-spec/validate/validate_source.js | 24 ++++- .../feature-state/promote-id/expected.png | Bin 0 -> 349 bytes .../feature-state/promote-id/style.json | 99 ++++++++++++++++++ 3 files changed, 121 insertions(+), 2 deletions(-) create mode 100644 test/integration/render-tests/feature-state/promote-id/expected.png create mode 100644 test/integration/render-tests/feature-state/promote-id/style.json diff --git a/src/style-spec/validate/validate_source.js b/src/style-spec/validate/validate_source.js index cb7451ea5b7..820557d1487 100644 --- a/src/style-spec/validate/validate_source.js +++ b/src/style-spec/validate/validate_source.js @@ -4,6 +4,12 @@ import {unbundle} from '../util/unbundle_jsonlint'; import validateObject from './validate_object'; import validateEnum from './validate_enum'; import validateExpression from './validate_expression'; +import validateString from './validate_string'; +import getType from '../util/get_type'; + +const objectElementValidators = { + promoteId: validatePromoteId +}; export default function validateSource(options) { const value = options.value; @@ -27,7 +33,8 @@ export default function validateSource(options) { value, valueSpec: styleSpec[`source_${type.replace('-', '_')}`], style: options.style, - styleSpec + styleSpec, + objectElementValidators }); return errors; @@ -37,7 +44,8 @@ export default function validateSource(options) { value, valueSpec: styleSpec.source_geojson, style, - styleSpec + styleSpec, + objectElementValidators }); if (value.cluster) { for (const prop in value.clusterProperties) { @@ -89,3 +97,15 @@ export default function validateSource(options) { }); } } + +function validatePromoteId({key, value}) { + if (getType(value) === 'string') { + return validateString({key, value}); + } else { + const errors = []; + for (const prop in value) { + errors.push(...validateString({key: `${key}.${prop}`, value: value[prop]})); + } + return errors; + } +} diff --git a/test/integration/render-tests/feature-state/promote-id/expected.png b/test/integration/render-tests/feature-state/promote-id/expected.png new file mode 100644 index 0000000000000000000000000000000000000000..7e58decb017795d53ed5d1c1f6aa35c45c4bb55d GIT binary patch literal 349 zcmV-j0iyniP)SwsVZUn&SuWT!MR&4NM*7S?`*f^Q3}FwD#wTkknS z2qAaobUmKN|hOLZS8MZX;TA|7hS1Fcmj7p7RYvb02HOAd2R4JAo@uJ^7 z>CJ$|4y}G|VrSSJ?)7`Yqe7KwPO#P0kS>*3pw-o4q0A*3U0a-(Ao-SjigSG6mh@_( v%nU1E)`^|zj&OwoJm5(PA%qY@2%*G({Y^m