From 8d486a1c044dca1c2c88208db0eb599472dc3138 Mon Sep 17 00:00:00 2001 From: zmiao Date: Tue, 21 Apr 2020 13:40:01 +0300 Subject: [PATCH 1/3] Fix within usage with othere expressions in the same filter --- src/style-spec/feature_filter/index.js | 18 ++++++++++++++---- src/style-spec/reference/v8.json | 3 +++ src/style-spec/validate/validate_filter.js | 10 ++++++++-- 3 files changed, 25 insertions(+), 6 deletions(-) diff --git a/src/style-spec/feature_filter/index.js b/src/style-spec/feature_filter/index.js index ffe1662d3b5..c5b2fd07841 100644 --- a/src/style-spec/feature_filter/index.js +++ b/src/style-spec/feature_filter/index.js @@ -77,15 +77,13 @@ function createFilter(filter: any): FeatureFilter { return {filter: () => true, needGeometry: false}; } - if (!isExpressionFilter(filter)) { - filter = convertFilter(filter); - } + filter = convertFilter(filter); const compiled = createExpression(filter, filterSpec); if (compiled.result === 'error') { throw new Error(compiled.value.map(err => `${err.key}: ${err.message}`).join(', ')); } else { - const needGeometry = Array.isArray(filter) && filter.length !== 0 && filter[0] === 'within'; + const needGeometry = geometryNeeded(filter); return {filter: (globalProperties: GlobalProperties, feature: Feature, canonical?: CanonicalTileID) => compiled.value.evaluate(globalProperties, feature, {}, canonical), needGeometry}; } @@ -96,7 +94,19 @@ function compare(a, b) { return a < b ? -1 : a > b ? 1 : 0; } +function geometryNeeded(filter) { + if (!Array.isArray(filter)) return false; + let needed = false; + for (let index = 0; index < filter.length; index++) { + const val = filter[index]; + const op = Array.isArray(val) ? val[0] : val; + needed = needed || (op === 'within'); + } + return needed; +} + function convertFilter(filter: ?Array): mixed { + if (isExpressionFilter(filter)) return filter; if (!filter) return true; const op = filter[0]; if (filter.length <= 1) return (op !== 'any'); diff --git a/src/style-spec/reference/v8.json b/src/style-spec/reference/v8.json index b7a7b35c70b..8b12accf811 100644 --- a/src/style-spec/reference/v8.json +++ b/src/style-spec/reference/v8.json @@ -2443,6 +2443,9 @@ }, "!has": { "doc": "`[\"!has\", key]` `feature[key]` does not exist" + }, + "within": { + "doc": "`[\"within\", object]` feature geometry is within object geometry" } }, "doc": "The filter operator." diff --git a/src/style-spec/validate/validate_filter.js b/src/style-spec/validate/validate_filter.js index 470ce40731a..804b7baac38 100644 --- a/src/style-spec/validate/validate_filter.js +++ b/src/style-spec/validate/validate_filter.js @@ -104,8 +104,14 @@ function validateNonExpressionFilter(options) { errors.push(new ValidationError(`${key}[1]`, value[1], `string expected, ${type} found`)); } break; - + case 'within': + type = getType(value[1]); + if (value.length !== 2) { + errors.push(new ValidationError(key, value, `filter array for "${value[0]}" operator must have 2 elements`)); + } else if (type !== 'object') { + errors.push(new ValidationError(`${key}[1]`, value[1], `object expected, ${type} found`)); + } + break; } - return errors; } From 12fe07bab5ffbc86ffc03aa290dd2e277b548ce4 Mon Sep 17 00:00:00 2001 From: zmiao Date: Tue, 21 Apr 2020 17:39:15 +0300 Subject: [PATCH 2/3] Add render tests --- .../within-in-complex-filter/expected.png | Bin 0 -> 393 bytes .../within-in-complex-filter/style.json | 102 ++++++++++++++++++ 2 files changed, 102 insertions(+) create mode 100644 test/integration/render-tests/within/within-in-complex-filter/expected.png create mode 100644 test/integration/render-tests/within/within-in-complex-filter/style.json diff --git a/test/integration/render-tests/within/within-in-complex-filter/expected.png b/test/integration/render-tests/within/within-in-complex-filter/expected.png new file mode 100644 index 0000000000000000000000000000000000000000..7f3f172cee2d680e5ec6ae12740877da4e81e079 GIT binary patch literal 393 zcmeAS@N?(olHy`uVBq!ia0vp^4j|0I1|(Ny7T#lEU<~kdaSW+oe0xwa`%0k<`-A6p zzol8*#Ud?I9eLThk6u?`Yfgw1Xui85+UNGCadCmjh zW1|0mJl^;JxuL`rvrRAdT{S Date: Wed, 22 Apr 2020 11:25:32 +0300 Subject: [PATCH 3/3] Fix filter operator checking for nested condition --- src/style-spec/feature_filter/index.js | 16 ++++++++-------- .../within/within-in-complex-filter/style.json | 8 ++++---- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/style-spec/feature_filter/index.js b/src/style-spec/feature_filter/index.js index c5b2fd07841..a096d14ed73 100644 --- a/src/style-spec/feature_filter/index.js +++ b/src/style-spec/feature_filter/index.js @@ -77,7 +77,9 @@ function createFilter(filter: any): FeatureFilter { return {filter: () => true, needGeometry: false}; } - filter = convertFilter(filter); + if (!isExpressionFilter(filter)) { + filter = convertFilter(filter); + } const compiled = createExpression(filter, filterSpec); if (compiled.result === 'error') { @@ -96,17 +98,14 @@ function compare(a, b) { function geometryNeeded(filter) { if (!Array.isArray(filter)) return false; - let needed = false; - for (let index = 0; index < filter.length; index++) { - const val = filter[index]; - const op = Array.isArray(val) ? val[0] : val; - needed = needed || (op === 'within'); + if (filter[0] === 'within') return true; + for (let index = 1; index < filter.length; index++) { + if (geometryNeeded(filter[index])) return true; } - return needed; + return false; } function convertFilter(filter: ?Array): mixed { - if (isExpressionFilter(filter)) return filter; if (!filter) return true; const op = filter[0]; if (filter.length <= 1) return (op !== 'any'); @@ -124,6 +123,7 @@ function convertFilter(filter: ?Array): mixed { op === '!in' ? convertNegation(convertInOp(filter[1], filter.slice(2))) : op === 'has' ? convertHasOp(filter[1]) : op === '!has' ? convertNegation(convertHasOp(filter[1])) : + op === 'within' ? filter : true; return converted; } diff --git a/test/integration/render-tests/within/within-in-complex-filter/style.json b/test/integration/render-tests/within/within-in-complex-filter/style.json index 1768e3023fa..f6c69364438 100644 --- a/test/integration/render-tests/within/within-in-complex-filter/style.json +++ b/test/integration/render-tests/within/within-in-complex-filter/style.json @@ -17,7 +17,7 @@ { "type": "Feature", "properties": { - "name": "ABC" + "number": [5] }, "geometry": { "type": "Point", @@ -30,7 +30,7 @@ { "type": "Feature", "properties": { - "name": "ABC" + "number": [5] }, "geometry": { "type": "Point", @@ -80,7 +80,7 @@ "id": "circle", "type": "circle", "source": "points", - "filter": ["all", ["in", "ABC", ["get", "name"]], ["within", { + "filter": ["all", ["in", 5, ["get", "number"]], ["==", ["within", { "type": "Polygon", "coordinates": [ [ @@ -91,7 +91,7 @@ [0, 0] ] ] - }] + }], true] ], "paint": { "circle-radius": 5,