An option to use a feature property as ID for feature state (mapbox#8987

) * add ability to id by property for feature state * flow fixes * rework promoteId according to PR discussions * properly expose promoted ids in query methods * fix lint * use state ids that can be cast as Float64 as is; update tests * promoteId: geojson support, better flow typing, v8 docs * fix spec test * make sure id is provided in getFeatureState * minor fixes after review * hash ids > MAX_SAFE_INTEGER * overhaul promoteId validation, add render test
cgcs2000 · Dec 2, 2019 · b3046ea · b3046ea
1 parent e4dcc6b
commit b3046ea
Show file tree

Hide file tree

Showing 27 changed files with 352 additions and 167 deletions.
diff --git 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
@@ -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
@@ -0,0 +1,73 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <title>Mapbox GL JS debug page</title>
+    <meta charset='utf-8'>
+    <meta name="viewport" content="width=device-width, initial-scale=1.0, user-scalable=no">
+    <link rel='stylesheet' href='../dist/mapbox-gl.css' />
+    <style>
+        body { margin: 0; padding: 0; }
+        html, body, #map { height: 100%; }
+    </style>
+</head>
+
+<body>
+<div id='map'></div>
+
+<script src='../dist/mapbox-gl-dev.js'></script>
+<script src='../debug/access_token_generated.js'></script>
+<script>
+
+var map = window.map = new mapboxgl.Map({
+    container: 'map',
+    zoom: 4,
+    center: [-96, 38],
+    style: {version: 8, layers: [], sources: {}}
+});
+
+map.on('load', () => {
+
+    map.addSource('counties', {
+        "type": "vector",
+        "url": "mapbox://mapbox.82pkq93d",
+        "promoteId": {"original": "COUNTY"}
+    });
+
+    map.addLayer({
+        "id": "counties",
+        "type": "fill",
+        "source": "counties",
+        "source-layer": "original",
+        "paint": {
+            "fill-outline-color": "black",
+            "fill-color": ["case", ["boolean", ["feature-state", "hover"], false], "red", "lightgrey"]
+        }
+    });
+
+    let selectedCounty = null;
+
+    function resetFeatureState() {
+        if (selectedCounty) {
+            map.setFeatureState({source: 'counties', sourceLayer: 'original', id: selectedCounty}, {hover: false});
+            selectedCounty = null;
+        }
+    }
+
+    map.on("mouseleave", "counties", () => {
+        resetFeatureState();
+    });
+
+    map.on("mousemove", "counties", (e) => {
+        const feature = e.features[0];
+
+        if (selectedCounty !== feature.id) {
+            resetFeatureState();
+            map.setFeatureState({source: 'counties', sourceLayer: 'original', id: feature.id}, {hover: true});
+            selectedCounty = feature.id;
+        }
+    });
+});
+
+</script>
+</body>
+</html>
diff --git 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
@@ -85,15 +85,15 @@ class CircleBucket<Layer: CircleStyleLayer | HeatmapStyleLayer> 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 ?
                     circleSortKey.evaluate(feature, {}) :
                     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
@@ -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
@@ -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
@@ -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
@@ -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,23 +46,23 @@ class FeatureIndex {
     grid: Grid;
     grid3D: Grid;
     featureIndexArray: FeatureIndexArray;
+    promoteId: ?PromoteIdSpecification;
 
     rawTileData: ArrayBuffer;
     bucketLayerIDs: Array<Array<string>>;
 
     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<Array<Point>>, 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<string>,
         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
@@ -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<FeaturePosition> {
+    getPositions(id: mixed): Array<FeaturePosition> {
         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
@@ -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<Layer: TypedStyleLayer> {
         }
 
         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
@@ -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) => {