Skip to content

Commit

Permalink
"geometry-type" to identify Multi- features (#519)
Browse files Browse the repository at this point in the history 
* "geometry-type" to identify Milti- features

* Fix and extend tests

- Add mapping from geometry-type to $type
- Remove caching to geometry analysis results

* Lint fixes, incomplete

* Linting, continued

Reverted one lint "fix" that caused test failure

* Linting, var -> let, const

* Linting, completed

* Reuse `calculateSignedArea`

* Simplify `geometryType()` code

Use `return` often instead of nesting condisions

* Split test

* Use variables for sample geometries.

Make comments part of the code

* Extract code into `hasMultipleOuterRings()`

* Update src/util/classify_rings.ts

Co-authored-by: Harel M <[email protected]>

* Updated `Changelog.md`

* Add `["geomerty-type"]` tests

* Rename test.json to test.json

* Rename test.json to test.json

* Rename test.json to test.json

* Rename test.json to test.json

* Rename test.json to test.json

---------

Co-authored-by: Harel M <[email protected]>
  • Loading branch information
@zstadler @HarelM
zstadler and HarelM authored Oct 22, 2024
1 parent 00b5831 commit 3f6e6d6
Show file tree
Hide file tree
Showing 13 changed files with 418 additions and 37 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
## main

### ✨ Features and improvements
- _...Add new stuff here..._
- Aligned the implementation of `["geometry-type"]` with [its spec](https://maplibre.org/maplibre-style-spec/expressions/#geometry-type). Now, when applicable, `["geometry-type"]` returns values not available while using `"$type"`: `"MultiPoint"`, `"MultiLineString"`, and `"MultiPolygon"`. The behaviour of `"$type"` has not changed. ([#519](https://github.com/maplibre/maplibre-style-spec/pull/519))

### 🐞 Bug fixes
- _...Add new stuff here..._
Expand Down
4 changes: 2 additions & 2 deletions src/expression/compound_expression.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -461,7 +461,7 @@ CompoundExpression.register(expressions, {
'filter-type-==': [
BooleanType,
[StringType],
(ctx, [v]) => ctx.geometryType() === (v as any).value
(ctx, [v]) => ctx.geometryDollarType() === (v as any).value
],
'filter-<': [
BooleanType,
Expand Down Expand Up @@ -548,7 +548,7 @@ CompoundExpression.register(expressions, {
'filter-type-in': [
BooleanType,
[array(StringType)],
(ctx, [v]) => (v as any).value.indexOf(ctx.geometryType()) >= 0
(ctx, [v]) => (v as any).value.indexOf(ctx.geometryDollarType()) >= 0
],
'filter-id-in': [
BooleanType,
Expand Down
4 changes: 2 additions & 2 deletions src/expression/definitions/within.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -192,9 +192,9 @@ class Within implements Expression {

evaluate(ctx: EvaluationContext) {
if (ctx.geometry() != null && ctx.canonicalID() != null) {
if (ctx.geometryType() === 'Point') {
if (ctx.geometryDollarType() === 'Point') {
return pointsWithinPolygons(ctx, this.geometries);
} else if (ctx.geometryType() === 'LineString') {
} else if (ctx.geometryDollarType() === 'LineString') {
return linesWithinPolygons(ctx, this.geometries);
}
}
Expand Down
38 changes: 37 additions & 1 deletion src/expression/evaluation_context.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,18 @@ import {Color} from './values';
import type {FormattedSection} from './types/formatted';
import type {GlobalProperties, Feature, FeatureState} from './index';
import {ICanonicalTileID} from '../tiles_and_coordinates';
import {hasMultipleOuterRings} from '../util/classify_rings';

const geometryTypes = ['Unknown', 'Point', 'LineString', 'Polygon'];
const simpleGeometryType = {
'Unknown': 'Unknown',
'Point': 'Point',
'MultiPoint': 'Point',
'LineString': 'LineString',
'MultiLineString': 'LineString',
'Polygon': 'Polygon',
'MultiPolygon': 'Polygon'
};

class EvaluationContext {
globals: GlobalProperties;
Expand All @@ -14,6 +24,7 @@ class EvaluationContext {
canonical: ICanonicalTileID;

_parseColorCache: {[_: string]: Color};
_geometryType: string;

constructor() {
this.globals = null;
Expand All @@ -29,8 +40,33 @@ class EvaluationContext {
return this.feature && 'id' in this.feature ? this.feature.id : null;
}

geometryDollarType() {
return this.feature ?
typeof this.feature.type === 'number' ? geometryTypes[this.feature.type] : simpleGeometryType[this.feature.type] :
null;
}

geometryType() {
return this.feature ? typeof this.feature.type === 'number' ? geometryTypes[this.feature.type] : this.feature.type : null;
let geometryType = this.feature.type;
if (typeof geometryType !== 'number') {
return geometryType;
}
geometryType = geometryTypes[this.feature.type];
if (geometryType === 'Unknown') {
return geometryType;
}
const geom = this.geometry();
const len = geom.length;
if (len === 1) {
return geometryType;
}
if (geometryType !== 'Polygon') {
return `Multi${geometryType}`;
}
if (hasMultipleOuterRings(geom)) {
return 'MultiPolygon';
}
return 'Polygon';
}

geometry() {
Expand Down
6 changes: 5 additions & 1 deletion src/feature_filter/convert.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ function runtimeTypeChecks(expectedTypes: ExpectedTypes): ExpressionFilterSpecif
function convertComparisonOp(property: string, value: any, op: string, expectedTypes?: ExpectedTypes | null): ExpressionFilterSpecification {
let get;
if (property === '$type') {
return [op, ['geometry-type'], value] as ExpressionFilterSpecification;
return convertInOp('$type', [value], op === '!=');
} else if (property === '$id') {
get = ['id'];
} else {
Expand Down Expand Up @@ -162,6 +162,10 @@ function convertInOp(property: string, values: Array<any>, negate = false): Expr

let get: ExpressionSpecification;
if (property === '$type') {
const len = values.length;
for (let i = 0; i < len; i++) {
values.push(`Multi${values[i]}`);
}
get = ['geometry-type'];
} else if (property === '$id') {
get = ['id'];
Expand Down
210 changes: 191 additions & 19 deletions src/feature_filter/feature_filter.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ describe('convert legacy filters to expressions', () => {
[
'match',
['geometry-type'],
['LineString', 'Point', 'Polygon'],
['LineString', 'MultiLineString', 'MultiPoint', 'MultiPolygon', 'Point', 'Polygon'],
true,
false
],
Expand Down Expand Up @@ -328,10 +328,96 @@ function legacyFilterTests(createFilterExpr) {
expect(f({zoom: 0}, {properties: {}})).toBe(false);
});

test('==, $type', () => {
const f = createFilterExpr(['==', '$type', 'LineString']).filter;
expect(f({zoom: 0}, {type: 1})).toBe(false);
expect(f({zoom: 0}, {type: 2})).toBe(true);
const UnknownGeometry = {type: 0};
const PointGeometry = {type: 1, geometry: [
[{x: 0, y: 0}]]};
const MultiPointGeometry = {type: 1, geometry: [
[{x: 0, y: 0}],
[{x: 1, y: 1}]]};
const LinestringGeometry = {type: 2, geometry: [
[{x: 0, y: 0}, {x: 1, y: 1}]]};
const MultiLineStringGeometry = {type: 2, geometry: [
[{x: 0, y: 0}, {x: 1, y: 1}],
[{x: 2, y: 0}, {x: 1, y: 0}]]};
const PolygonGeometry = {type: 3, geometry: [
[{x: 0, y: 0}, {x: 3, y: 0}, {x: 3, y: 3}, {x: 0, y: 3}, {x: 0, y: 0}],
[{x: 0, y: 0}, {x: 0, y: 2}, {x: 2, y: 2}, {x: 2, y: 0}, {x: 0, y: 0}]]};
const MultiPolygonGeometry = {type: 3, geometry: [
[{x: 0, y: 0}, {x: 3, y: 0}, {x: 3, y: 3}, {x: 0, y: 3}, {x: 0, y: 0}],
[{x: 0, y: 0}, {x: 0, y: 2}, {x: 2, y: 2}, {x: 2, y: 0}, {x: 0, y: 0}],
[{x: 0, y: 0}, {x: 1, y: 0}, {x: 1, y: 1}, {x: 0, y: 1}, {x: 0, y: 0}]]};

test('==, $type, Point', () => {
const fPoint = createFilterExpr(['==', '$type', 'Point']).filter;
expect(fPoint({zoom: 0}, UnknownGeometry)).toBe(false);
expect(fPoint({zoom: 0}, PointGeometry)).toBe(true);
expect(fPoint({zoom: 0}, MultiPointGeometry)).toBe(true);
expect(fPoint({zoom: 0}, LinestringGeometry)).toBe(false);
expect(fPoint({zoom: 0}, MultiLineStringGeometry)).toBe(false);
expect(fPoint({zoom: 0}, PolygonGeometry)).toBe(false);
expect(fPoint({zoom: 0}, MultiPolygonGeometry)).toBe(false);
expect(fPoint({zoom: 0}, {type: 'Unknown'})).toBe(false);
expect(fPoint({zoom: 0}, {type: 'Point'})).toBe(true);
expect(fPoint({zoom: 0}, {type: 'MultiPoint'})).toBe(true);
expect(fPoint({zoom: 0}, {type: 'LineString'})).toBe(false);
expect(fPoint({zoom: 0}, {type: 'MultiLineString'})).toBe(false);
expect(fPoint({zoom: 0}, {type: 'Polygon'})).toBe(false);
expect(fPoint({zoom: 0}, {type: 'MultiPolygon'})).toBe(false);
});

test('==, $type, LineString', () => {
const fLineString = createFilterExpr(['==', '$type', 'LineString']).filter;
expect(fLineString({zoom: 0}, UnknownGeometry)).toBe(false);
expect(fLineString({zoom: 0}, PointGeometry)).toBe(false);
expect(fLineString({zoom: 0}, MultiPointGeometry)).toBe(false);
expect(fLineString({zoom: 0}, LinestringGeometry)).toBe(true);
expect(fLineString({zoom: 0}, MultiLineStringGeometry)).toBe(true);
expect(fLineString({zoom: 0}, PolygonGeometry)).toBe(false);
expect(fLineString({zoom: 0}, MultiPolygonGeometry)).toBe(false);
expect(fLineString({zoom: 0}, {type: 'Unknown'})).toBe(false);
expect(fLineString({zoom: 0}, {type: 'Point'})).toBe(false);
expect(fLineString({zoom: 0}, {type: 'MultiPoint'})).toBe(false);
expect(fLineString({zoom: 0}, {type: 'LineString'})).toBe(true);
expect(fLineString({zoom: 0}, {type: 'MultiLineString'})).toBe(true);
expect(fLineString({zoom: 0}, {type: 'Polygon'})).toBe(false);
expect(fLineString({zoom: 0}, {type: 'MultiPolygon'})).toBe(false);
});

test('==, $type, Polygon', () => {

const fPolygon = createFilterExpr(['==', '$type', 'Polygon']).filter;
expect(fPolygon({zoom: 0}, UnknownGeometry)).toBe(false);
expect(fPolygon({zoom: 0}, PointGeometry)).toBe(false);
expect(fPolygon({zoom: 0}, MultiPointGeometry)).toBe(false);
expect(fPolygon({zoom: 0}, LinestringGeometry)).toBe(false);
expect(fPolygon({zoom: 0}, MultiLineStringGeometry)).toBe(false);
expect(fPolygon({zoom: 0}, PolygonGeometry)).toBe(true);
expect(fPolygon({zoom: 0}, MultiPolygonGeometry)).toBe(true);
expect(fPolygon({zoom: 0}, {type: 'Unknown'})).toBe(false);
expect(fPolygon({zoom: 0}, {type: 'Point'})).toBe(false);
expect(fPolygon({zoom: 0}, {type: 'MultiPoint'})).toBe(false);
expect(fPolygon({zoom: 0}, {type: 'LineString'})).toBe(false);
expect(fPolygon({zoom: 0}, {type: 'MultiLineString'})).toBe(false);
expect(fPolygon({zoom: 0}, {type: 'Polygon'})).toBe(true);
expect(fPolygon({zoom: 0}, {type: 'MultiPolygon'})).toBe(true);
});

test('==, $type, Unknown', () => {
const fUnknown = createFilterExpr(['==', '$type', 'Unknown']).filter;
expect(fUnknown({zoom: 0}, UnknownGeometry)).toBe(true);
expect(fUnknown({zoom: 0}, PointGeometry)).toBe(false);
expect(fUnknown({zoom: 0}, MultiPointGeometry)).toBe(false);
expect(fUnknown({zoom: 0}, LinestringGeometry)).toBe(false);
expect(fUnknown({zoom: 0}, MultiLineStringGeometry)).toBe(false);
expect(fUnknown({zoom: 0}, PolygonGeometry)).toBe(false);
expect(fUnknown({zoom: 0}, MultiPolygonGeometry)).toBe(false);
expect(fUnknown({zoom: 0}, {type: 'Unknown'})).toBe(true);
expect(fUnknown({zoom: 0}, {type: 'Point'})).toBe(false);
expect(fUnknown({zoom: 0}, {type: 'MultiPoint'})).toBe(false);
expect(fUnknown({zoom: 0}, {type: 'LineString'})).toBe(false);
expect(fUnknown({zoom: 0}, {type: 'MultiLineString'})).toBe(false);
expect(fUnknown({zoom: 0}, {type: 'Polygon'})).toBe(false);
expect(fUnknown({zoom: 0}, {type: 'MultiPolygon'})).toBe(false);
});

test('==, $id', () => {
Expand Down Expand Up @@ -373,10 +459,76 @@ function legacyFilterTests(createFilterExpr) {
expect(f({zoom: 0}, {properties: {}})).toBe(true);
});

test('!=, $type', () => {
const f = createFilterExpr(['!=', '$type', 'LineString']).filter;
expect(f({zoom: 0}, {type: 1})).toBe(true);
expect(f({zoom: 0}, {type: 2})).toBe(false);
test('!=, $type, Point', () => {
const fPoint = createFilterExpr(['!=', '$type', 'Point']).filter;
expect(fPoint({zoom: 0}, UnknownGeometry)).toBe(true);
expect(fPoint({zoom: 0}, PointGeometry)).toBe(false);
expect(fPoint({zoom: 0}, MultiPointGeometry)).toBe(false);
expect(fPoint({zoom: 0}, LinestringGeometry)).toBe(true);
expect(fPoint({zoom: 0}, MultiLineStringGeometry)).toBe(true);
expect(fPoint({zoom: 0}, PolygonGeometry)).toBe(true);
expect(fPoint({zoom: 0}, MultiPolygonGeometry)).toBe(true);
expect(fPoint({zoom: 0}, {type: 'Unknown'})).toBe(true);
expect(fPoint({zoom: 0}, {type: 'Point'})).toBe(false);
expect(fPoint({zoom: 0}, {type: 'MultiPoint'})).toBe(false);
expect(fPoint({zoom: 0}, {type: 'LineString'})).toBe(true);
expect(fPoint({zoom: 0}, {type: 'MultiLineString'})).toBe(true);
expect(fPoint({zoom: 0}, {type: 'Polygon'})).toBe(true);
expect(fPoint({zoom: 0}, {type: 'MultiPolygon'})).toBe(true);
});

test('!=, $type, LineString', () => {
const fLineString = createFilterExpr(['!=', '$type', 'LineString']).filter;
expect(fLineString({zoom: 0}, UnknownGeometry)).toBe(true);
expect(fLineString({zoom: 0}, PointGeometry)).toBe(true);
expect(fLineString({zoom: 0}, MultiPointGeometry)).toBe(true);
expect(fLineString({zoom: 0}, LinestringGeometry)).toBe(false);
expect(fLineString({zoom: 0}, MultiLineStringGeometry)).toBe(false);
expect(fLineString({zoom: 0}, PolygonGeometry)).toBe(true);
expect(fLineString({zoom: 0}, MultiPolygonGeometry)).toBe(true);
expect(fLineString({zoom: 0}, {type: 'Unknown'})).toBe(true);
expect(fLineString({zoom: 0}, {type: 'Point'})).toBe(true);
expect(fLineString({zoom: 0}, {type: 'MultiPoint'})).toBe(true);
expect(fLineString({zoom: 0}, {type: 'LineString'})).toBe(false);
expect(fLineString({zoom: 0}, {type: 'MultiLineString'})).toBe(false);
expect(fLineString({zoom: 0}, {type: 'Polygon'})).toBe(true);
expect(fLineString({zoom: 0}, {type: 'MultiPolygon'})).toBe(true);
});

test('!=, $type, Polygon', () => {
const fPolygon = createFilterExpr(['!=', '$type', 'Polygon']).filter;
expect(fPolygon({zoom: 0}, UnknownGeometry)).toBe(true);
expect(fPolygon({zoom: 0}, PointGeometry)).toBe(true);
expect(fPolygon({zoom: 0}, MultiPointGeometry)).toBe(true);
expect(fPolygon({zoom: 0}, LinestringGeometry)).toBe(true);
expect(fPolygon({zoom: 0}, MultiLineStringGeometry)).toBe(true);
expect(fPolygon({zoom: 0}, PolygonGeometry)).toBe(false);
expect(fPolygon({zoom: 0}, MultiPolygonGeometry)).toBe(false);
expect(fPolygon({zoom: 0}, {type: 'Unknown'})).toBe(true);
expect(fPolygon({zoom: 0}, {type: 'Point'})).toBe(true);
expect(fPolygon({zoom: 0}, {type: 'MultiPoint'})).toBe(true);
expect(fPolygon({zoom: 0}, {type: 'LineString'})).toBe(true);
expect(fPolygon({zoom: 0}, {type: 'MultiLineString'})).toBe(true);
expect(fPolygon({zoom: 0}, {type: 'Polygon'})).toBe(false);
expect(fPolygon({zoom: 0}, {type: 'MultiPolygon'})).toBe(false);
});

test('!=, $type, Unknown', () => {
const fUnknown = createFilterExpr(['!=', '$type', 'Unknown']).filter;
expect(fUnknown({zoom: 0}, UnknownGeometry)).toBe(false);
expect(fUnknown({zoom: 0}, PointGeometry)).toBe(true);
expect(fUnknown({zoom: 0}, MultiPointGeometry)).toBe(true);
expect(fUnknown({zoom: 0}, LinestringGeometry)).toBe(true);
expect(fUnknown({zoom: 0}, MultiLineStringGeometry)).toBe(true);
expect(fUnknown({zoom: 0}, PolygonGeometry)).toBe(true);
expect(fUnknown({zoom: 0}, MultiPolygonGeometry)).toBe(true);
expect(fUnknown({zoom: 0}, {type: 'Unknown'})).toBe(false);
expect(fUnknown({zoom: 0}, {type: 'Point'})).toBe(true);
expect(fUnknown({zoom: 0}, {type: 'MultiPoint'})).toBe(true);
expect(fUnknown({zoom: 0}, {type: 'LineString'})).toBe(true);
expect(fUnknown({zoom: 0}, {type: 'MultiLineString'})).toBe(true);
expect(fUnknown({zoom: 0}, {type: 'Polygon'})).toBe(true);
expect(fUnknown({zoom: 0}, {type: 'MultiPolygon'})).toBe(true);
});

test('<, number', () => {
Expand Down Expand Up @@ -563,15 +715,22 @@ function legacyFilterTests(createFilterExpr) {

test('in, $type', () => {
const f = createFilterExpr(['in', '$type', 'LineString', 'Polygon']).filter;
expect(f({zoom: 0}, {type: 1})).toBe(false);
expect(f({zoom: 0}, {type: 2})).toBe(true);
expect(f({zoom: 0}, {type: 3})).toBe(true);
expect(f({zoom: 0}, {type: 'Unknown'})).toBe(false);
expect(f({zoom: 0}, {type: 'Point'})).toBe(false);
expect(f({zoom: 0}, {type: 'MultiPoint'})).toBe(false);
expect(f({zoom: 0}, {type: 'LineString'})).toBe(true);
expect(f({zoom: 0}, {type: 'MultiLineString'})).toBe(true);
expect(f({zoom: 0}, {type: 'Polygon'})).toBe(true);
expect(f({zoom: 0}, {type: 'MultiPolygon'})).toBe(true);

const f1 = createFilterExpr(['in', '$type', 'Polygon', 'LineString', 'Point']).filter;
expect(f1({zoom: 0}, {type: 1})).toBe(true);
expect(f1({zoom: 0}, {type: 2})).toBe(true);
expect(f1({zoom: 0}, {type: 3})).toBe(true);

expect(f1({zoom: 0}, {type: 'Unknown'})).toBe(false);
expect(f1({zoom: 0}, {type: 'Point'})).toBe(true);
expect(f1({zoom: 0}, {type: 'MultiPoint'})).toBe(true);
expect(f1({zoom: 0}, {type: 'LineString'})).toBe(true);
expect(f1({zoom: 0}, {type: 'MultiLineString'})).toBe(true);
expect(f1({zoom: 0}, {type: 'Polygon'})).toBe(true);
expect(f1({zoom: 0}, {type: 'MultiPolygon'})).toBe(true);
});

test('!in, degenerate', () => {
Expand Down Expand Up @@ -621,9 +780,22 @@ function legacyFilterTests(createFilterExpr) {

test('!in, $type', () => {
const f = createFilterExpr(['!in', '$type', 'LineString', 'Polygon']).filter;
expect(f({zoom: 0}, {type: 1})).toBe(true);
expect(f({zoom: 0}, {type: 2})).toBe(false);
expect(f({zoom: 0}, {type: 3})).toBe(false);
expect(f({zoom: 0}, {type: 'Unknown'})).toBe(true);
expect(f({zoom: 0}, {type: 'Point'})).toBe(true);
expect(f({zoom: 0}, {type: 'MultiPoint'})).toBe(true);
expect(f({zoom: 0}, {type: 'LineString'})).toBe(false);
expect(f({zoom: 0}, {type: 'MultiLineString'})).toBe(false);
expect(f({zoom: 0}, {type: 'Polygon'})).toBe(false);
expect(f({zoom: 0}, {type: 'MultiPolygon'})).toBe(false);

const f1 = createFilterExpr(['!in', '$type', 'Polygon', 'LineString', 'Point']).filter;
expect(f1({zoom: 0}, {type: 'Unknown'})).toBe(true);
expect(f1({zoom: 0}, {type: 'Point'})).toBe(false);
expect(f1({zoom: 0}, {type: 'MultiPoint'})).toBe(false);
expect(f1({zoom: 0}, {type: 'LineString'})).toBe(false);
expect(f1({zoom: 0}, {type: 'MultiLineString'})).toBe(false);
expect(f1({zoom: 0}, {type: 'Polygon'})).toBe(false);
expect(f1({zoom: 0}, {type: 'MultiPolygon'})).toBe(false);
});

test('any', () => {
Expand Down
24 changes: 24 additions & 0 deletions src/util/classify_rings.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,3 +69,27 @@ function calculateSignedArea(ring: Point2D[]): number {
}
return sum;
}

/**
* Returns if there are multiple outer rings.
* The first ring is an outer ring. Its direction, cw or ccw, defines the direction of outer rings.
*
* @param rings - List of rings
* @returns Are there multiple outer rings
*/
export function hasMultipleOuterRings(rings: Point2D[][]): boolean {
// Following https://github.com/mapbox/vector-tile-js/blob/77851380b63b07fd0af3d5a3f144cc86fb39fdd1/lib/vectortilefeature.js#L197
const len = rings.length;
for (let i = 0, direction; i < len; i++) {
const area = calculateSignedArea(rings[i]);
if (area === 0) continue;
if (direction === undefined) {
// Keep the direction of the first ring
direction = area < 0;
} else if (direction === area < 0) {
// Same direction as the first ring -> a second outer ring
return true;
}
}
return false;
}
Loading

0 comments on commit 3f6e6d6

Please sign in to comment.