HARP-14633: island geometry disappears on certain zoom level

Signed-off-by: Daniele Bacarella <daniele.bacarella@here.com>

@here/harp-geometry/lib/ClipPolygon.ts

@@ -51,6 +51,8 @@ export abstract class ClippingEdge {
      *
      * @param polygon Clip the polygon against this edge.
      * @param extent The extent of the bounding box.
+     *
+     * @return The clipped polygon.
      */
     clipPolygon(polygon: Vector2[], extent: number): Vector2[] {
         const inputList = polygon;
@@ -205,7 +207,7 @@ const clipEdges = [
 ];
 
 /**
- * Clip the given polygon using the Sutherland-Hodgman algorithm.
+ * Clip the given polygon against a rectangle using the Sutherland-Hodgman algorithm.
  *
  * @remarks
  * The coordinates of the polygon must be integer numbers.

@here/harp-vectortile-datasource/lib/Ring.ts

@@ -17,6 +17,7 @@ export class Ring {
      * @remarks
      * The sign of the area depends on the projection and the axis orientation
      * of the ring coordinates.
+     * For example, given a ring with `CW winding`: `area > 0` with Y-axis that grows downwards and `area < 0` otherwise.
      */
     readonly area: number;
 

@here/harp-vectortile-datasource/lib/adapters/geojson-vt/GeoJsonVtDataAdapter.ts

@@ -7,7 +7,7 @@
 import { MapEnv, ValueMap } from "@here/harp-datasource-protocol/index-decoder";
 import { webMercatorProjection } from "@here/harp-geoutils";
 import { ILogger } from "@here/harp-utils";
-import { Vector2, Vector3 } from "three";
+import { ShapeUtils, Vector2, Vector3 } from "three";
 
 import { DataAdapter } from "../../DataAdapter";
 import { DecodeInfo } from "../../DecodeInfo";
@@ -181,20 +181,29 @@ export class GeoJsonVtDataAdapter implements DataAdapter {
                     break;
                 }
                 case VTJsonGeometryType.Polygon: {
-                    const polygon: IPolygonGeometry = { rings: [] };
+                    const polygons: IPolygonGeometry[] = [];
+                    let polygon: IPolygonGeometry | undefined;
                     for (const outline of feature.geometry as VTJsonPosition[][]) {
                         const ring: Vector2[] = [];
                         for (const [currX, currY] of outline) {
                             const position = new Vector2(currX, currY);
                             ring.push(position);
                         }
-                        polygon.rings.push(ring);
+                        // MVT spec defines that each exterior ring signals the beginning of a new polygon.
+                        // See https://github.com/mapbox/vector-tile-spec/tree/master/2.1
+                        if (ShapeUtils.area(ring) > 0) {
+                            // Create a new polygon and push it into the collection of polygons
+                            polygon = { rings: [] };
+                            polygons.push(polygon);
+                        }
+                        // Push the ring into the current polygon
+                        polygon?.rings.push(ring);
                     }
 
                     this.m_processor.processPolygonFeature(
                         tile.layer,
                         VT_JSON_EXTENTS,
-                        [polygon],
+                        polygons,
                         env,
                         tileKey.level
                     );

@here/harp-vectortile-datasource/lib/adapters/geojson/GeoJsonDataAdapter.ts

@@ -9,7 +9,7 @@ import { clipLineString } from "@here/harp-geometry/lib/ClipLineString";
 import { GeoCoordinates, GeoPointLike, webMercatorProjection } from "@here/harp-geoutils";
 import { Vector2Like } from "@here/harp-geoutils/lib/math/Vector2Like";
 import { ILogger } from "@here/harp-utils";
-import { Vector2, Vector3 } from "three";
+import { ShapeUtils, Vector2, Vector3 } from "three";
 
 import { DataAdapter } from "../../DataAdapter";
 import { DecodeInfo } from "../../DecodeInfo";
@@ -134,21 +134,11 @@ function convertLineGeometry(
         convertLineStringGeometry(lineString, decodeInfo)
     );
 }
-
-function signedPolygonArea(contour: GeoPointLike[]): number {
-    const n = contour.length;
-    let area = 0.0;
-    for (let p = n - 1, q = 0; q < n; p = q++) {
-        area += contour[p][0] * contour[q][1] - contour[q][0] * contour[p][1];
-    }
-    return area * 0.5;
-}
-
 function convertRings(coordinates: GeoPointLike[][], decodeInfo: DecodeInfo): IPolygonGeometry {
     const rings = coordinates.map((ring, i) => {
         const isOuterRing = i === 0;
-        const isClockWise = signedPolygonArea(ring) < 0;
         const { positions } = convertLineStringGeometry(ring, decodeInfo);
+        const isClockWise = ShapeUtils.area(positions) > 0;
         if ((isOuterRing && !isClockWise) || (!isOuterRing && isClockWise)) {
             positions.reverse();
         }

@here/harp-vectortile-datasource/lib/adapters/omv/OmvDataAdapter.ts

@@ -153,26 +153,38 @@ export function asGeometryType(feature: com.mapbox.pb.Tile.IFeature | undefined)
 
 // Ensures ring winding follows Mapbox Vector Tile specification: outer rings must be clockwise,
 // inner rings counter-clockwise.
+// See https://docs.mapbox.com/vector-tiles/specification/
 function checkWinding(multipolygon: IPolygonGeometry[]) {
-    if (multipolygon.length === 0) {
-        return;
-    }
-
     const firstPolygon = multipolygon[0];
 
-    if (firstPolygon.rings.length === 0) {
+    if (firstPolygon === undefined || firstPolygon.rings.length === 0) {
         return;
     }
 
     // Opposite sign to ShapeUtils.isClockWise, since webMercator tile space has top-left origin.
+    // For example:
+    // Given the ring = [(1,2), (2,2), (2,1), (1,1)]
+    // ShapeUtils.area(ring) > 0    -> false
+    // ShapeUtils.isClockWise(ring) -> true
+    // ^
+    // |  1,2 -> 2,2
+    // |          |
+    // |  1,1 <- 2,1
+    // |_______________>
+    //
+    // Tile space axis
+    //  ______________>
+    // |  1,1 <- 2,1
+    // |          |
+    // |  1,2 ->  2,2
+    // V
     const isOuterRingClockWise = ShapeUtils.area(firstPolygon.rings[0]) > 0;
-    if (isOuterRingClockWise) {
-        return;
-    }
 
-    for (const polygon of multipolygon) {
-        for (const ring of polygon.rings) {
-            ring.reverse();
+    if (!isOuterRingClockWise) {
+        for (const polygon of multipolygon) {
+            for (const ring of polygon.rings) {
+                ring.reverse();
+            }
         }
     }
 }
@@ -406,8 +418,9 @@ export class OmvDataAdapter implements DataAdapter, OmvVisitor {
         }
 
         const geometry: IPolygonGeometry[] = [];
-        const currentPolygon: IPolygonGeometry = { rings: [] };
+        let currentPolygon: IPolygonGeometry | undefined;
         let currentRing: Vector2[];
+        let exteriorWinding: number | undefined;
         this.m_geometryCommands.accept(feature.geometry, {
             type: "Polygon",
             visitCommand: command => {
@@ -417,17 +430,26 @@ export class OmvDataAdapter implements DataAdapter, OmvVisitor {
                     currentRing.push(command.position);
                 } else if (isClosePathCommand(command)) {
                     if (currentRing !== undefined && currentRing.length > 0) {
+                        const currentRingWinding = Math.sign(ShapeUtils.area(currentRing));
+                        // Winding order from XYZ spaces might be not MVT spec compliant, see HARP-11151.
+                        // We take the winding of the very first ring as reference.
+                        if (exteriorWinding === undefined) {
+                            exteriorWinding = currentRingWinding;
+                        }
+                        // MVT spec defines that each exterior ring signals the beginning of a new polygon.
+                        // see https://github.com/mapbox/vector-tile-spec/tree/master/2.1
+                        if (currentRingWinding === exteriorWinding) {
+                            // Create a new polygon and push it into the collection of polygons
+                            currentPolygon = { rings: [] };
+                            geometry.push(currentPolygon);
+                        }
+                        // Push the ring into the current polygon
                         currentRing.push(currentRing[0].clone());
-                        currentPolygon.rings.push(currentRing);
+                        currentPolygon?.rings.push(currentRing);
                     }
                 }
             }
         });
-
-        if (currentPolygon.rings.length > 0) {
-            geometry.push(currentPolygon);
-        }
-
         if (geometry.length === 0) {
             return;
         }

@here/harp-vectortile-datasource/test/GeoJsonVtDataAdapterTest.ts

@@ -20,6 +20,44 @@ enum VTJsonGeometryType {
     Polygon
 }
 
+// A multipolygon composed by 2 polygons:
+// Polygon#1: [exterior, interior]
+// Polygon#2: [exterior, interior, interior]
+const multipolygon = [
+    // START polygon with 1 exterior ring, 1 interior.
+    [
+        [1, 1],
+        [10, 1],
+        [10, 10],
+        [1, 10]
+    ], // exterior CW
+    [
+        [2, 8],
+        [8, 8],
+        [8, 2],
+        [2, 2]
+    ], // interior CCW
+    // END
+    // START polygon with 1 exterior ring, 2 interiors - note: whether they overlap it's not relevant
+    [
+        [1, 1],
+        [10, 1],
+        [10, 10],
+        [1, 10]
+    ], // exterior CW
+    [
+        [2, 8],
+        [8, 8],
+        [8, 2],
+        [2, 2]
+    ], // interior CCW
+    [
+        [2, 8],
+        [8, 8],
+        [8, 2],
+        [2, 2]
+    ] // interior CCW
+];
 const geojsonVtTile = {
     features: [
         {
@@ -45,13 +83,7 @@ const geojsonVtTile = {
             type: VTJsonGeometryType.Polygon,
             id: "polygon id",
             tags: {},
-            geometry: [
-                [
-                    [1, 2],
-                    [3, 4],
-                    [5, 6]
-                ]
-            ]
+            geometry: multipolygon
         }
     ],
     maxX: 0,
@@ -102,4 +134,13 @@ describe("GeoJsonVtDataAdapter", function () {
         const polygonEnv = polygonSpy.getCalls()[0].args[3];
         expect(polygonEnv.lookup("$id")).equals(geojsonVtTile.features[2].id);
     });
+
+    it("process tile's polygon geometries and create a single polygon from nested rings", function () {
+        const polygonSpy = sinon.spy(geometryProcessor, "processPolygonFeature");
+        adapter.process(geojsonVtTile as any, decodeInfo);
+
+        expect(polygonSpy.calledOnce);
+        const polygons = polygonSpy.getCalls()[0].args[2];
+        expect(polygons.length).equals(2);
+    });
 });