diff --git a/bench/lib/tile_parser.js b/bench/lib/tile_parser.js index 1bba3e00182..e28afd39f2f 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, + promoteId: undefined }); const vectorTile = new VT.VectorTile(new Protobuf(tile.buffer)); 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 new file mode 100644 index 00000000000..7d35702d5b3 --- /dev/null +++ b/debug/featurestate.html @@ -0,0 +1,73 @@ + + + + Mapbox GL JS debug page + + + + + + + +
+ + + + + + diff --git a/src/data/bucket.js b/src/data/bucket.js index f0ecc4ef7c2..a6be33f9aa2 100644 --- a/src/data/bucket.js +++ b/src/data/bucket.js @@ -29,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 2ee8ade034b..8feb187d1e0 100644 --- a/src/data/bucket/circle_bucket.js +++ b/src/data/bucket/circle_bucket.js @@ -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 39d9901d013..711d200c007 100644 --- a/src/data/bucket/fill_bucket.js +++ b/src/data/bucket/fill_bucket.js @@ -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 4f8632dc384..901f9e8079b 100644 --- a/src/data/bucket/fill_extrusion_bucket.js +++ b/src/data/bucket/fill_extrusion_bucket.js @@ -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/symbol_bucket.js b/src/data/bucket/symbol_bucket.js index 2b7b751389e..936fac092df 100644 --- a/src/data/bucket/symbol_bucket.js +++ b/src/data/bucket/symbol_bucket.js @@ -411,7 +411,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; } @@ -457,6 +457,7 @@ class SymbolBucket implements Bucket { undefined; const symbolFeature: SymbolFeature = { + id, text, icon, index, @@ -466,9 +467,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..eca434d7c1b 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,6 +46,7 @@ class FeatureIndex { grid: Grid; grid3D: Grid; featureIndexArray: FeatureIndexArray; + promoteId: ?PromoteIdSpecification; 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?: ?PromoteIdSpecification) { 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) { @@ -146,14 +146,14 @@ class FeatureIndex { filter, params.layers, styleLayers, - (feature: VectorTileFeature, styleLayer: StyleLayer) => { + (feature: VectorTileFeature, styleLayer: StyleLayer, id: string | number | void) => { if (!featureGeometry) { featureGeometry = loadGeometry(feature); } let featureState = {}; - if (feature.id) { + 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) => boolean | number) { + intersectionTest?: (feature: VectorTileFeature, styleLayer: StyleLayer, id: string | number | void) => 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; + const 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/data/feature_position_map.js b/src/data/feature_position_map.js index 962bf0faadf..72bafce0058 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(getNumericId(id)); this.positions.push(index, start, end); } - getPositions(id: number): Array { + getPositions(id: mixed): Array { assert(this.indexed); + 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 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]; @@ -81,6 +84,16 @@ export default class FeaturePositionMap { } } +const MAX_SAFE_INTEGER = Math.pow(2, 53) - 1; + +function getNumericId(value: mixed) { + const numValue = +value; + if (!isNaN(numValue) && numValue <= MAX_SAFE_INTEGER) { + 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..8a8a62f623a 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); @@ -746,7 +746,7 @@ export class ProgramConfigurationSet { } if (feature.id !== undefined) { - this._featureMap.add(+feature.id, index, this._bufferOffset, length); + this._featureMap.add(feature.id, index, this._bufferOffset, length); } this._bufferOffset = length; diff --git a/src/source/geojson_source.js b/src/source/geojson_source.js index 16a9379f782..2b043f1ef91 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/source_cache.js b/src/source/source_cache.js index d2d097b130b..9eebec9d027 100644 --- a/src/source/source_cache.js +++ b/src/source/source_cache.js @@ -820,27 +820,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/tile.js b/src/source/tile.js index c4670332699..9602db15b43 100644 --- a/src/source/tile.js +++ b/src/source/tile.js @@ -287,9 +287,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]; @@ -303,7 +304,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/vector_tile_source.js b/src/source/vector_tile_source.js index 681c5b417d7..5ec313a6264 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,6 +28,7 @@ class VectorTileSource extends Evented implements Source { url: string; scheme: string; tileSize: number; + promoteId: ?PromoteIdSpecification; _options: VectorSourceSpecification; _collectResourceTiming: boolean; @@ -55,7 +56,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', 'promoteId'])); this._options = extend({type: 'vector'}, options); this._collectResourceTiming = options.collectResourceTiming; @@ -126,6 +127,7 @@ class VectorTileSource extends Evented implements Source { source: this.id, pixelRatio: browser.devicePixelRatio, showCollisionBoxes: this.map.showCollisionBoxes, + promoteId: this.promoteId }; params.request.collectResourceTiming = this._collectResourceTiming; diff --git a/src/source/worker_source.js b/src/source/worker_source.js index c0b26deee20..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,6 +24,7 @@ export type WorkerTileParameters = TileParameters & { zoom: number, maxZoom: number, tileSize: number, + promoteId: ?PromoteIdSpecification, pixelRatio: number, showCollisionBoxes: boolean, collectResourceTiming?: boolean, diff --git a/src/source/worker_tile.js b/src/source/worker_tile.js index 1aa8a5c5c68..2f07b0de416 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,6 +35,7 @@ class WorkerTile { pixelRatio: number; tileSize: number; source: string; + promoteId: ?PromoteIdSpecification; overscaling: number; showCollisionBoxes: boolean; collectResourceTiming: boolean; @@ -58,6 +60,7 @@ class WorkerTile { this.showCollisionBoxes = params.showCollisionBoxes; this.collectResourceTiming = !!params.collectResourceTiming; this.returnDependencies = !!params.returnDependencies; + this.promoteId = params.promoteId; } parse(data: VectorTile, layerIndex: StyleLayerIndex, availableImages: Array, actor: Actor, callback: WorkerTileCallback) { @@ -67,7 +70,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} = {}; @@ -96,7 +99,8 @@ class WorkerTile { const features = []; for (let index = 0; index < sourceLayer.length; index++) { const feature = sourceLayer.feature(index); - features.push({feature, index, sourceLayerIndex}); + const id = featureIndex.getId(feature, sourceLayerId); + features.push({feature, id, index, sourceLayerIndex}); } for (const family of layerFamilies[sourceLayerId]) { 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 = {| 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/src/style/style.js b/src/style/style.js index a80c91c9b30..fec8906d699 100644 --- a/src/style/style.js +++ b/src/style/style.js @@ -868,12 +868,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.`))); @@ -888,12 +887,11 @@ 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; + if (target.id === undefined) { + this.fire(new ErrorEvent(new Error(`The feature id parameter must be provided.`))); } - sourceCache.setFeatureState(sourceLayer, featureId, state); + sourceCache.setFeatureState(sourceLayer, target.id, state); } removeFeatureState(target: { source: string; sourceLayer?: string; id?: string | number; }, key?: string) { @@ -908,32 +906,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.`))); @@ -944,12 +935,11 @@ 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; + if (target.id === undefined) { + this.fire(new ErrorEvent(new Error(`The feature id parameter must be provided.`))); } - return sourceCache.getFeatureState(sourceLayer, featureId); + return sourceCache.getFeatureState(sourceLayer, target.id); } getTransition() { 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 { 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 00000000000..7e58decb017 Binary files /dev/null and b/test/integration/render-tests/feature-state/promote-id/expected.png differ diff --git a/test/integration/render-tests/feature-state/promote-id/style.json b/test/integration/render-tests/feature-state/promote-id/style.json new file mode 100644 index 00000000000..109309279d7 --- /dev/null +++ b/test/integration/render-tests/feature-state/promote-id/style.json @@ -0,0 +1,99 @@ +{ + "version": 8, + "metadata": { + "test": { + "width": 64, + "height": 32, + "operations": [ + [ + "setFeatureState", + { + "source": "geojson", + "id": "9007199254740992" + }, + { + "color": "red" + } + ], + [ + "setFeatureState", + { + "source": "geojson", + "id": "1.2" + }, + { + "color": "blue" + } + ], + [ + "wait" + ] + ] + } + }, + "zoom": 2, + "sources": { + "geojson": { + "type": "geojson", + "promoteId": "foo", + "data": { + "type": "FeatureCollection", + "features": [{ + "type": "Feature", + "geometry": { + "type": "Point", + "coordinates": [ + 0, + 0 + ] + }, + "properties": { + "foo": "9007199254740992" + } + }, { + "type": "Feature", + "geometry": { + "type": "Point", + "coordinates": [ + 3, + 0 + ] + }, + "properties": { + "foo": "9007199254740993" + } + }, { + "type": "Feature", + "geometry": { + "type": "Point", + "coordinates": [ + -3, + 0 + ] + }, + "properties": { + "foo": "1.2" + } + }] + } + } + }, + "layers": [ + { + "id": "circle", + "type": "circle", + "source": "geojson", + "paint": { + "circle-radius": 5, + "circle-color": [ + "coalesce", + [ + "feature-state", + "color" + ], + "black" + ] + } + } + ] +} 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', diff --git a/test/unit/ui/map.test.js b/test/unit/ui/map.test.js index ff5c36c22d5..47da135e998 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(); });