Revert geometry-type implementation changes, bring spec in line wit…

…h historic behavior (#966) * revert geometry-type changes * Revert "Fixes geometry type filter (#924)" This reverts commit 2ed393e. * update description * Update changelog, scratch out previous change. * Use backticks in v8.json * Update CHANGELOG.md --------- Co-authored-by: Bart Louwers <[email protected]> Co-authored-by: Harel M <[email protected]>
maplibre · Jan 12, 2025 · 2c5e5cd · 2c5e5cd
1 parent 1a8b7a5
commit 2c5e5cd
Show file tree

Hide file tree

Showing 14 changed files with 33 additions and 444 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,7 +1,8 @@
 ## main
 
 ### ✨ Features and improvements
-- _...Add new stuff here..._
+
+- Adjust `geometry-type` expression specification and MapLibre GL JS implementation to the historic behavior of only returning `Point`, `LineString` or `Polygon`, not the `Multi...` variants of these. This reverts a recent change to the implementation, which was causing issues in a [large number of styles](https://github.com/maplibre/maplibre-style-spec/issues/965).
 
 ### 🐞 Bug fixes
 - _...Add new stuff here..._
@@ -10,7 +11,7 @@
 
 ### 🐞 Bug fixes
 
-- Fix issue with `geomtry-type` filter ([#924](https://github.com/maplibre/maplibre-style-spec/pull/924))
+- ~~Fix issue with `geomtry-type` filter ([#924](https://github.com/maplibre/maplibre-style-spec/pull/924))~~ This was reverted in 23.0.0 due to causing issues in [a large number of styles](https://github.com/maplibre/maplibre-style-spec/issues/965).
 
 ## 22.0.0
 
@@ -33,7 +34,7 @@
 
 ### ✨ Features and improvements
 
-- 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))
+- ~~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))~~ This was reverted in 23.0.0 due to causing issues in [a large number of styles](https://github.com/maplibre/maplibre-style-spec/issues/965).
 
 ## 20.4.0
 

diff --git a/src/expression/compound_expression.ts b/src/expression/compound_expression.ts
@@ -461,7 +461,7 @@ CompoundExpression.register(expressions, {
     'filter-type-==': [
         BooleanType,
         [StringType],
-        (ctx, [v]) => ctx.geometryDollarType() === (v as any).value
+        (ctx, [v]) => ctx.geometryType() === (v as any).value
     ],
     'filter-<': [
         BooleanType,
@@ -548,7 +548,7 @@ CompoundExpression.register(expressions, {
     'filter-type-in': [
         BooleanType,
         [array(StringType)],
-        (ctx, [v]) => (v as any).value.indexOf(ctx.geometryDollarType()) >= 0
+        (ctx, [v]) => (v as any).value.indexOf(ctx.geometryType()) >= 0
     ],
     'filter-id-in': [
         BooleanType,

diff --git a/src/expression/definitions/within.ts b/src/expression/definitions/within.ts
@@ -192,9 +192,9 @@ export class Within implements Expression {
 
     evaluate(ctx: EvaluationContext) {
         if (ctx.geometry() != null && ctx.canonicalID() != null) {
-            if (ctx.geometryDollarType() === 'Point') {
+            if (ctx.geometryType() === 'Point') {
                 return pointsWithinPolygons(ctx, this.geometries);
-            } else if (ctx.geometryDollarType() === 'LineString') {
+            } else if (ctx.geometryType() === 'LineString') {
                 return linesWithinPolygons(ctx, this.geometries);
             }
         }

diff --git a/src/expression/evaluation_context.ts b/src/expression/evaluation_context.ts
@@ -1,19 +1,9 @@
 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';
 import {Color} from './types/color';
 
 const geometryTypes = ['Unknown', 'Point', 'LineString', 'Polygon'];
-const simpleGeometryType = {
-    'Unknown': 'Unknown',
-    'Point': 'Point',
-    'MultiPoint': 'Point',
-    'LineString': 'LineString',
-    'MultiLineString': 'LineString',
-    'Polygon': 'Polygon',
-    'MultiPolygon': 'Polygon'
-};
 
 export class EvaluationContext {
     globals: GlobalProperties;
@@ -24,7 +14,6 @@ export class EvaluationContext {
     canonical: ICanonicalTileID;
 
     _parseColorCache: {[_: string]: Color};
-    _geometryType: string;
 
     constructor() {
         this.globals = null;
@@ -40,33 +29,8 @@ export 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() {
-        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';
+        return this.feature ? typeof this.feature.type === 'number' ? geometryTypes[this.feature.type] : this.feature.type : null;
     }
 
     geometry() {
@@ -89,3 +53,4 @@ export class EvaluationContext {
         return cached;
     }
 }
+
diff --git a/src/feature_filter/convert.ts b/src/feature_filter/convert.ts
@@ -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 convertInOp('$type', [value], op === '!=');
+        return [op, ['geometry-type'], value] as ExpressionFilterSpecification;
     } else if (property === '$id') {
         get = ['id'];
     } else {
@@ -162,10 +162,6 @@ 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'];