Skip to content

Commit

Permalink
queryRenderedFeatures doesnt take into account feature states (#6569)
Browse files Browse the repository at this point in the history
Update tile queryPadding with every feature state update and include feature state in expression evaluation for query intersection tests.
  • Loading branch information
Asheem Mamoowala authored May 3, 2018
1 parent db83b5e commit c9c4f61
Show file tree
Hide file tree
Showing 9 changed files with 113 additions and 18 deletions.
7 changes: 6 additions & 1 deletion src/data/feature_index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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 || {},
Expand Down Expand Up @@ -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);
}
);
Expand Down
1 change: 1 addition & 0 deletions src/source/query_features.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
15 changes: 7 additions & 8 deletions src/source/source_cache.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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}));

Expand Down Expand Up @@ -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];
Expand Down Expand Up @@ -792,18 +792,17 @@ 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);
}

/**
* 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);
}
}
Expand Down
8 changes: 4 additions & 4 deletions src/source/source_state.js
Original file line number Diff line number Diff line change
Expand Up @@ -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] || {};
Expand All @@ -59,7 +59,7 @@ class SourceFeatureState {

for (const id in tiles) {
const tile = tiles[id];
tile.setFeatureState(changes);
tile.setFeatureState(changes, painter);
}
}
}
Expand Down
9 changes: 7 additions & 2 deletions src/source/tile.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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<Array<Point>>,
scale: number,
params: { filter: FilterSpecification, layers: Array<string> },
Expand All @@ -258,7 +260,7 @@ class Tile {
transform: transform,
params: params,
queryPadding: this.queryPadding * maxPitchScaleFactor
}, layers);
}, layers, sourceFeatureState);
}

querySourceFeatures(result: Array<GeoJSONFeature>, params: any) {
Expand Down Expand Up @@ -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) {
Expand All @@ -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));
}
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/style/style.js
Original file line number Diff line number Diff line change
Expand Up @@ -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; }) {
Expand All @@ -805,7 +805,7 @@ class Style extends Evented {
return;
}

return sourceCache.getFeatureState(sourceLayer || '_geojsonTileLayer', feature.id);
return sourceCache.getFeatureState(sourceLayer, feature.id);
}

getTransition() {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
[
{
"type": "Feature",
"id": 1,
"geometry": {
"type": "Point",
"coordinates": [0, 0]
},
"properties": {},
"source": "mapbox",
"state": { "big": true }
}
]
Original file line number Diff line number Diff line change
@@ -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]
]
}
}
]
}
2 changes: 1 addition & 1 deletion test/unit/style/style.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down

0 comments on commit c9c4f61

Please sign in to comment.