Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix using within expression filter when query features #9933

Merged
merged 4 commits into from
Sep 9, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 3 additions & 6 deletions src/data/bucket/circle_bucket.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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;
Expand All @@ -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
};
Expand Down
10 changes: 3 additions & 7 deletions src/data/bucket/fill_bucket.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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;
Expand All @@ -100,7 +96,7 @@ class FillBucket implements Bucket {
type: feature.type,
sourceLayerIndex,
index,
geometry: evaluationFeature.geometry,
geometry: needGeometry ? evaluationFeature.geometry : loadGeometry(feature),
patterns: {},
sortKey
};
Expand Down
6 changes: 2 additions & 4 deletions src/data/bucket/fill_extrusion_bucket.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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;

Expand Down
10 changes: 3 additions & 7 deletions src/data/bucket/line_bucket.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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;
Expand All @@ -168,7 +164,7 @@ class LineBucket implements Bucket {
type: feature.type,
sourceLayerIndex,
index,
geometry: evaluationFeature.geometry,
geometry: needGeometry ? evaluationFeature.geometry : loadGeometry(feature),
patterns: {},
sortKey
};
Expand Down
9 changes: 3 additions & 6 deletions src/data/bucket/symbol_bucket.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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
Expand Down
25 changes: 25 additions & 0 deletions src/data/evaluation_feature.js
Original file line number Diff line number Diff line change
@@ -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) : []};
}
9 changes: 8 additions & 1 deletion src/data/feature_index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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);

Expand Down
15 changes: 10 additions & 5 deletions src/source/tile.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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);
}
}

Expand Down
4 changes: 4 additions & 0 deletions test/unit/source/tile.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});

Expand Down