Fix using within expression filter when query features (#9933)

* Fix within query source feature * Fix queryRenderedFeature when using within filter * Add a unit test case * Add a util helper function to construct evaluation feature used for expression
mapbox · Sep 9, 2020 · f2117b4 · f2117b4
1 parent 38f0072
commit f2117b4
Show file tree

Hide file tree

Showing 9 changed files with 61 additions and 36 deletions.
diff --git a/src/data/bucket/circle_bucket.js b/src/data/bucket/circle_bucket.js
@@ -7,6 +7,7 @@ import SegmentVector from '../segment';
 import {ProgramConfigurationSet} from '../program_configuration';
 import {TriangleIndexArray} from '../index_array_type';
 import loadGeometry from '../load_geometry';
+import toEvaluationFeature from '../evaluation_feature';
 import EXTENT from '../extent';
 import {register} from '../../util/web_worker_transfer';
 import EvaluationParameters from '../../style/evaluation_parameters';
@@ -88,14 +89,10 @@ class CircleBucket<Layer: CircleStyleLayer | HeatmapStyleLayer> implements Bucke
 
  for (const {feature, id, index, sourceLayerIndex} of features) {
  const needGeometry = this.layers[0]._featureFilter.needGeometry;
- const evaluationFeature = {type: feature.type,
- id,
- properties: feature.properties,
- geometry: needGeometry ? loadGeometry(feature) : []};
+ const evaluationFeature = toEvaluationFeature(feature, needGeometry);
 
  if (!this.layers[0]._featureFilter.filter(new EvaluationParameters(this.zoom), evaluationFeature, canonical)) continue;
 
- if (!needGeometry) evaluationFeature.geometry = loadGeometry(feature);
  const sortKey = circleSortKey ?
  circleSortKey.evaluate(evaluationFeature, {}, canonical) :
  undefined;
@@ -106,7 +103,7 @@ class CircleBucket<Layer: CircleStyleLayer | HeatmapStyleLayer> implements Bucke
  type: feature.type,
  sourceLayerIndex,
  index,
- geometry : evaluationFeature.geometry,
+ geometry: needGeometry ? evaluationFeature.geometry : loadGeometry(feature),
  patterns: {},
  sortKey
  };

diff --git a/src/data/bucket/fill_bucket.js b/src/data/bucket/fill_bucket.js
@@ -13,6 +13,7 @@ const EARCUT_MAX_RINGS = 500;
 import {register} from '../../util/web_worker_transfer';
 import {hasPattern, addPatternDependencies} from './pattern_bucket_features';
 import loadGeometry from '../load_geometry';
+import toEvaluationFeature from '../evaluation_feature';
 import EvaluationParameters from '../../style/evaluation_parameters';
 
 import type {CanonicalTileID} from '../../source/tile_id';
@@ -81,15 +82,10 @@ class FillBucket implements Bucket {
 
  for (const {feature, id, index, sourceLayerIndex} of features) {
  const needGeometry = this.layers[0]._featureFilter.needGeometry;
- const evaluationFeature = {type: feature.type,
- id,
- properties: feature.properties,
- geometry: needGeometry ? loadGeometry(feature) : []};
+ const evaluationFeature = toEvaluationFeature(feature, needGeometry);
 
  if (!this.layers[0]._featureFilter.filter(new EvaluationParameters(this.zoom), evaluationFeature, canonical)) continue;
 
- if (!needGeometry) evaluationFeature.geometry = loadGeometry(feature);
-
  const sortKey = fillSortKey ?
  fillSortKey.evaluate(evaluationFeature, {}, canonical, options.availableImages) :
  undefined;
@@ -100,7 +96,7 @@ class FillBucket implements Bucket {
  type: feature.type,
  sourceLayerIndex,
  index,
- geometry: evaluationFeature.geometry,
+ geometry: needGeometry ? evaluationFeature.geometry : loadGeometry(feature),
  patterns: {},
  sortKey
  };

diff --git a/src/data/bucket/fill_extrusion_bucket.js b/src/data/bucket/fill_extrusion_bucket.js
@@ -16,6 +16,7 @@ const EARCUT_MAX_RINGS = 500;
 import {register} from '../../util/web_worker_transfer';
 import {hasPattern, addPatternDependencies} from './pattern_bucket_features';
 import loadGeometry from '../load_geometry';
+import toEvaluationFeature from '../evaluation_feature';
 import EvaluationParameters from '../../style/evaluation_parameters';
 
 import type {CanonicalTileID} from '../../source/tile_id';
@@ -94,10 +95,7 @@ class FillExtrusionBucket implements Bucket {
 
  for (const {feature, id, index, sourceLayerIndex} of features) {
  const needGeometry = this.layers[0]._featureFilter.needGeometry;
- const evaluationFeature = {type: feature.type,
- id,
- properties: feature.properties,
- geometry: needGeometry ? loadGeometry(feature) : []};
+ const evaluationFeature = toEvaluationFeature(feature, needGeometry);
 
  if (!this.layers[0]._featureFilter.filter(new EvaluationParameters(this.zoom), evaluationFeature, canonical)) continue;
 

diff --git a/src/data/bucket/line_bucket.js b/src/data/bucket/line_bucket.js
@@ -13,6 +13,7 @@ const vectorTileFeatureTypes = mvt.VectorTileFeature.types;
 import {register} from '../../util/web_worker_transfer';
 import {hasPattern, addPatternDependencies} from './pattern_bucket_features';
 import loadGeometry from '../load_geometry';
+import toEvaluationFeature from '../evaluation_feature';
 import EvaluationParameters from '../../style/evaluation_parameters';
 
 import type {CanonicalTileID} from '../../source/tile_id';
@@ -149,15 +150,10 @@ class LineBucket implements Bucket {
 
  for (const {feature, id, index, sourceLayerIndex} of features) {
  const needGeometry = this.layers[0]._featureFilter.needGeometry;
- const evaluationFeature = {type: feature.type,
- id,
- properties: feature.properties,
- geometry: needGeometry ? loadGeometry(feature) : []};
+ const evaluationFeature = toEvaluationFeature(feature, needGeometry);
 
  if (!this.layers[0]._featureFilter.filter(new EvaluationParameters(this.zoom), evaluationFeature, canonical)) continue;
 
- if (!needGeometry) evaluationFeature.geometry = loadGeometry(feature);
-
  const sortKey = lineSortKey ?
  lineSortKey.evaluate(evaluationFeature, {}, canonical) :
  undefined;
@@ -168,7 +164,7 @@ class LineBucket implements Bucket {
  type: feature.type,
  sourceLayerIndex,
  index,
- geometry: evaluationFeature.geometry,
+ geometry: needGeometry ? evaluationFeature.geometry : loadGeometry(feature),
  patterns: {},
  sortKey
  };

diff --git a/src/data/bucket/symbol_bucket.js b/src/data/bucket/symbol_bucket.js
@@ -26,6 +26,7 @@ import mergeLines from '../../symbol/mergelines';
 import {allowsVerticalWritingMode, stringContainsRTLText} from '../../util/script_detection';
 import {WritingMode} from '../../symbol/shaping';
 import loadGeometry from '../load_geometry';
+import toEvaluationFeature from '../evaluation_feature';
 import mvt from '@mapbox/vector-tile';
 const vectorTileFeatureTypes = mvt.VectorTileFeature.types;
 import {verticalizedCharacterMap} from '../../util/verticalize_punctuation';
@@ -440,11 +441,7 @@ class SymbolBucket implements Bucket {
  for (const {feature, id, index, sourceLayerIndex} of features) {
 
  const needGeometry = layer._featureFilter.needGeometry;
- const evaluationFeature = {type: feature.type,
- id,
- properties: feature.properties,
- geometry: needGeometry ? loadGeometry(feature) : []};
-
+ const evaluationFeature = toEvaluationFeature(feature, needGeometry);
  if (!layer._featureFilter.filter(globalProperties, evaluationFeature, canonical)) {
  continue;
  }
@@ -496,7 +493,7 @@ class SymbolBucket implements Bucket {
  icon,
  index,
  sourceLayerIndex,
- geometry: loadGeometry(feature),
+ geometry: evaluationFeature.geometry,
  properties: feature.properties,
  type: vectorTileFeatureTypes[feature.type],
  sortKey

diff --git a/src/data/evaluation_feature.js b/src/data/evaluation_feature.js
@@ -0,0 +1,25 @@
+// @flow
+
+import loadGeometry from './load_geometry';
+
+type EvaluationFeature = {
+ +type: 1 | 2 | 3 | 'Unknown' | 'Point' | 'MultiPoint' | 'LineString' | 'MultiLineString' | 'Polygon' | 'MultiPolygon',
+ +id?: any,
+ +properties: {[_: string]: any},
+ +patterns?: {[_: string]: {"min": string, "mid": string, "max": string}},
+ geometry: Array<Array<Point>>
+};
+
+/**
+ * Construct a new feature based on a VectorTileFeature for expression evaluation, the geometry of which
+ * will be loaded based on necessity.
+ * @param {VectorTileFeature} feature
+ * @param {boolean} needGeometry
+ * @private
+ */
+export default function toEvaluationFeature(feature: VectorTileFeature, needGeometry: boolean): EvaluationFeature {
+ return {type: feature.type,
+ id: feature.id,
+ properties:feature.properties,
+ geometry: needGeometry ? loadGeometry(feature) : []};
+}
diff --git a/src/data/feature_index.js b/src/data/feature_index.js
@@ -3,6 +3,7 @@
 import Point from '@mapbox/point-geometry';
 
 import loadGeometry from './load_geometry';
+import toEvaluationFeature from './evaluation_feature';
 import EXTENT from './extent';
 import featureFilter from '../style-spec/feature_filter';
 import Grid from 'grid-index';
@@ -185,8 +186,14 @@ class FeatureIndex {
  const sourceLayer = this.vtLayers[sourceLayerName];
  const feature = sourceLayer.feature(featureIndex);
 
- if (!filter.filter(new EvaluationParameters(this.tileID.overscaledZ), feature))
+ if (filter.needGeometry) {
+ const evaluationFeature = toEvaluationFeature(feature, true);
+ if (!filter.filter(new EvaluationParameters(this.tileID.overscaledZ), evaluationFeature, this.tileID.canonical)) {
+ return;
+ }
+ } else if (!filter.filter(new EvaluationParameters(this.tileID.overscaledZ), feature)) {
  return;
+ }
 
  const id = this.getId(feature, sourceLayerName);
 

diff --git a/src/source/tile.js b/src/source/tile.js
@@ -9,6 +9,7 @@ import SymbolBucket from '../data/bucket/symbol_bucket';
 import {CollisionBoxArray} from '../data/array_types';
 import Texture from '../render/texture';
 import browser from '../util/browser';
+import toEvaluationFeature from '../data/evaluation_feature';
 import EvaluationParameters from '../style/evaluation_parameters';
 import SourceFeatureState from '../source/source_state';
 import {lazyLoadRTLTextPlugin} from './rtl_text_plugin';
@@ -307,12 +308,16 @@ class Tile {
 
  for (let i = 0; i < layer.length; i++) {
  const feature = layer.feature(i);
- if (filter.filter(new EvaluationParameters(this.tileID.overscaledZ), feature)) {
- const id = featureIndex.getId(feature, sourceLayer);
- const geojsonFeature = new GeoJSONFeature(feature, z, x, y, id);
-  (geojsonFeature: any).tile = coord;
- result.push(geojsonFeature);
+ if (filter.needGeometry) {
+ const evaluationFeature = toEvaluationFeature(feature, true);
+ if (!filter.filter(new EvaluationParameters(this.tileID.overscaledZ), evaluationFeature, this.tileID.canonical)) continue;
+ } else if (!filter.filter(new EvaluationParameters(this.tileID.overscaledZ), feature)) {
+ continue;
  }
+ 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/test/unit/source/tile.test.js b/test/unit/source/tile.test.js
@@ -47,6 +47,10 @@ test('querySourceFeatures', (t) => {
  result = [];
  tile.querySourceFeatures(result, {filter: ['!=', 'oneway', true]});
  t.equal(result.length, 0);
+ result = [];
+ const polygon = {type: "Polygon", coordinates: [[[-91, -1], [-89, -1], [-89, 1], [-91, 1], [-91, -1]]]};
+ tile.querySourceFeatures(result, {filter: ['within', polygon]});
+ t.equal(result.length, 1);
  t.end();
  });