From c9c4f614c5cc8e887027457f09a403710b610ff9 Mon Sep 17 00:00:00 2001 From: Asheem Mamoowala Date: Thu, 3 May 2018 16:11:33 -0700 Subject: [PATCH] queryRenderedFeatures doesnt take into account feature states (#6569) Update tile queryPadding with every feature state update and include feature state in expression evaluation for query intersection tests. --- src/data/feature_index.js | 7 +- src/source/query_features.js | 1 + src/source/source_cache.js | 15 ++-- src/source/source_state.js | 8 +-- src/source/tile.js | 9 ++- src/style/style.js | 4 +- .../circle-radius/feature-state/expected.json | 13 ++++ .../circle-radius/feature-state/style.json | 72 +++++++++++++++++++ test/unit/style/style.test.js | 2 +- 9 files changed, 113 insertions(+), 18 deletions(-) create mode 100644 test/integration/query-tests/circle-radius/feature-state/expected.json create mode 100644 test/integration/query-tests/circle-radius/feature-state/style.json diff --git a/src/data/feature_index.js b/src/data/feature_index.js index b8c62eb6421..694205c6f84 100644 --- a/src/data/feature_index.js +++ b/src/data/feature_index.js @@ -14,6 +14,7 @@ import { arraysIntersect } from '../util/util'; import { OverscaledTileID } from '../source/tile_id'; import { register } from '../util/web_worker_transfer'; import EvaluationParameters from '../style/evaluation_parameters'; +import SourceFeatureState from '../source/source_state'; import type StyleLayer from '../style/style_layer'; import type {FeatureFilter} from '../style-spec/feature_filter'; @@ -93,7 +94,7 @@ class FeatureIndex { } // Finds non-symbol features in this tile at a particular position. - query(args: QueryParameters, styleLayers: {[string]: StyleLayer}): {[string]: Array<{ featureIndex: number, feature: GeoJSONFeature }>} { + query(args: QueryParameters, styleLayers: {[string]: StyleLayer}, sourceFeatureState: SourceFeatureState): {[string]: Array<{ featureIndex: number, feature: GeoJSONFeature }>} { this.loadVTLayers(); const params = args.params || {}, @@ -143,6 +144,10 @@ class FeatureIndex { if (!featureGeometry) { featureGeometry = loadGeometry(feature); } + if (feature.id) { + // `feature-state` expression evaluation requires feature state to be available + (feature: any).state = sourceFeatureState.getState(styleLayer.sourceLayer || '_geojsonTileLayer', String(feature.id)); + } return styleLayer.queryIntersectsFeature(queryGeometry, feature, featureGeometry, this.z, args.transform, pixelsToTileUnits, args.posMatrix); } ); diff --git a/src/source/query_features.js b/src/source/query_features.js index eaab668d3b1..af56d7675bc 100644 --- a/src/source/query_features.js +++ b/src/source/query_features.js @@ -24,6 +24,7 @@ export function queryRenderedFeatures(sourceCache: SourceCache, wrappedTileID: tileIn.tileID.wrapped().key, queryResults: tileIn.tile.queryRenderedFeatures( styleLayers, + sourceCache._state, tileIn.queryGeometry, tileIn.scale, params, diff --git a/src/source/source_cache.js b/src/source/source_cache.js index 4069fd537c3..0721a1f9e9c 100644 --- a/src/source/source_cache.js +++ b/src/source/source_cache.js @@ -168,7 +168,7 @@ class SourceCache extends Evented { this._source.prepare(); } - this._state.coalesceChanges(this._tiles); + this._state.coalesceChanges(this._tiles, this.map ? this.map.painter : null); for (const i in this._tiles) { this._tiles[i].upload(context); } @@ -251,7 +251,7 @@ class SourceCache extends Evented { if (previousState === 'expired') tile.refreshedUponExpiration = true; this._setTileReloadTimer(id, tile); if (this.getSource().type === 'raster-dem' && tile.dem) this._backfillDEM(tile); - this._state.initializeTileState(tile); + this._state.initializeTileState(tile, this.map ? this.map.painter : null); this._source.fire(new Event('data', {dataType: 'source', tile: tile, coord: tile.tileID})); @@ -627,7 +627,7 @@ class SourceCache extends Evented { this._setTileReloadTimer(tileID.key, tile); // set the tileID because the cached tile could have had a different wrap value tile.tileID = tileID; - this._state.initializeTileState(tile); + this._state.initializeTileState(tile, this.map ? this.map.painter : null); if (this._cacheTimers[tileID.key]) { clearTimeout(this._cacheTimers[tileID.key]); delete this._cacheTimers[tileID.key]; @@ -792,7 +792,8 @@ class SourceCache extends Evented { * Set the value of a particular state for a feature * @private */ - setFeatureState(sourceLayer: string, feature: string, state: Object) { + setFeatureState(sourceLayer?: string, feature: string, state: Object) { + sourceLayer = sourceLayer || '_geojsonTileLayer'; this._state.updateState(sourceLayer, feature, state); } @@ -800,10 +801,8 @@ class SourceCache extends Evented { * Get the entire state object for a feature * @private */ - getFeatureState(sourceLayer: string, feature: string) { - if (this._source.type === 'geojson') { - sourceLayer = '_geojsonTileLayer'; - } + getFeatureState(sourceLayer?: string, feature: string) { + sourceLayer = sourceLayer || '_geojsonTileLayer'; return this._state.getState(sourceLayer, feature); } } diff --git a/src/source/source_state.js b/src/source/source_state.js index 9fe8c39bb61..89f4c09df4b 100644 --- a/src/source/source_state.js +++ b/src/source/source_state.js @@ -36,11 +36,11 @@ class SourceFeatureState { return extend({}, base[feature], changes[feature]); } - initializeTileState(tile: Tile) { - tile.setFeatureState(this.state); + initializeTileState(tile: Tile, painter: any) { + tile.setFeatureState(this.state, painter); } - coalesceChanges(tiles: {[any]: Tile}) { + coalesceChanges(tiles: {[any]: Tile}, painter: any) { const changes: LayerFeatureStates = {}; for (const sourceLayer in this.stateChanges) { this.state[sourceLayer] = this.state[sourceLayer] || {}; @@ -59,7 +59,7 @@ class SourceFeatureState { for (const id in tiles) { const tile = tiles[id]; - tile.setFeatureState(changes); + tile.setFeatureState(changes, painter); } } } diff --git a/src/source/tile.js b/src/source/tile.js index 5332c9b2f54..d30a826e82a 100644 --- a/src/source/tile.js +++ b/src/source/tile.js @@ -15,6 +15,7 @@ import SegmentVector from '../data/segment'; import { TriangleIndexArray } from '../data/index_array_type'; import browser from '../util/browser'; import EvaluationParameters from '../style/evaluation_parameters'; +import SourceFeatureState from '../source/source_state'; const CLOCK_SKEW_RETRY_TIMEOUT = 30000; @@ -241,6 +242,7 @@ class Tile { // Queries non-symbol features rendered for this tile. // Symbol features are queried globally queryRenderedFeatures(layers: {[string]: StyleLayer}, + sourceFeatureState: SourceFeatureState, queryGeometry: Array>, scale: number, params: { filter: FilterSpecification, layers: Array }, @@ -258,7 +260,7 @@ class Tile { transform: transform, params: params, queryPadding: this.queryPadding * maxPitchScaleFactor - }, layers); + }, layers, sourceFeatureState); } querySourceFeatures(result: Array, params: any) { @@ -412,7 +414,7 @@ class Tile { } } - setFeatureState(states: LayerFeatureStates) { + setFeatureState(states: LayerFeatureStates, painter: any) { if (!this.latestFeatureIndex || !this.latestFeatureIndex.rawTileData || Object.keys(states).length === 0) { @@ -430,6 +432,9 @@ class Tile { if (!sourceLayer || !sourceLayerStates || Object.keys(sourceLayerStates).length === 0) continue; bucket.update(sourceLayerStates, sourceLayer); + if (painter && painter.style) { + this.queryPadding = Math.max(this.queryPadding, painter.style.getLayer(bucket.layerIds[0]).queryRadius(bucket)); + } } } } diff --git a/src/style/style.js b/src/style/style.js index 29c033c5c99..0348181909d 100644 --- a/src/style/style.js +++ b/src/style/style.js @@ -786,7 +786,7 @@ class Style extends Evented { return; } - sourceCache.setFeatureState(sourceLayer || '_geojsonTileLayer', feature.id, state); + sourceCache.setFeatureState(sourceLayer, feature.id, state); } getFeatureState(feature: { source: string; sourceLayer?: string; id: string; }) { @@ -805,7 +805,7 @@ class Style extends Evented { return; } - return sourceCache.getFeatureState(sourceLayer || '_geojsonTileLayer', feature.id); + return sourceCache.getFeatureState(sourceLayer, feature.id); } getTransition() { diff --git a/test/integration/query-tests/circle-radius/feature-state/expected.json b/test/integration/query-tests/circle-radius/feature-state/expected.json new file mode 100644 index 00000000000..b35900c5e72 --- /dev/null +++ b/test/integration/query-tests/circle-radius/feature-state/expected.json @@ -0,0 +1,13 @@ +[ + { + "type": "Feature", + "id": 1, + "geometry": { + "type": "Point", + "coordinates": [0, 0] + }, + "properties": {}, + "source": "mapbox", + "state": { "big": true } + } +] \ No newline at end of file diff --git a/test/integration/query-tests/circle-radius/feature-state/style.json b/test/integration/query-tests/circle-radius/feature-state/style.json new file mode 100644 index 00000000000..45881d1bf2b --- /dev/null +++ b/test/integration/query-tests/circle-radius/feature-state/style.json @@ -0,0 +1,72 @@ +{ + "version": 8, + "metadata": { + "test": { + "width": 64, + "height": 64, + "operations": [ + [ + "wait" + ], + [ + "setFeatureState", + { "source": "mapbox", "id": 1}, + { "big": true } + ], + [ + "wait" + ] + ], + "queryGeometry": [ + 32, + 16 + ] + } + }, + "sources": { + "mapbox": { + "type": "geojson", + "data": { + "type": "FeatureCollection", + "features": [ + { + "id": 1, + "type": "Feature", + "geometry": { + "type": "Point", + "coordinates": [ + 0, + 0 + ] + } + }, + { + "id": 2, + "type": "Feature", + "geometry": { + "type": "Point", + "coordinates": [ + 5, + 15 + ] + } + } + ] + } + } + }, + "layers": [ + { + "id": "circle", + "type": "circle", + "source": "mapbox", + "paint": { + "circle-radius": ["case", + ["boolean", ["feature-state", "big"], false], + ["number", 20], + ["number", 3] + ] + } + } + ] +} \ No newline at end of file diff --git a/test/unit/style/style.test.js b/test/unit/style/style.test.js index 1f3c2e65fcc..2a0013a44ea 100644 --- a/test/unit/style/style.test.js +++ b/test/unit/style/style.test.js @@ -1664,7 +1664,7 @@ test('Style#queryRenderedFeatures', (t) => { const transform = new Transform(); transform.resize(512, 512); - function queryMapboxFeatures(layers, queryGeom, scale, params) { + function queryMapboxFeatures(layers, getFeatureState, queryGeom, scale, params) { const features = { 'land': [{ type: 'Feature',