re-reverse inner-turned-outer if no matching outer is found

PDOK-16135

snap/snap.go

@@ -2,6 +2,7 @@ package snap
 
 import (
 	"fmt"
+	"log"
 	"math"
 	"slices"
 	"sort"
@@ -131,11 +132,9 @@ func addPointsAndSnap(ix *PointIndex, polygon geom.Polygon, levels []Level) map[
 			for _, outerRing := range outerRings { // (there are only outer rings if isOuter)
 				newPolygons[level] = append(newPolygons[level], [][][2]float64{outerRing})
 			}
-			if len(newPolygons[level]) == 0 && len(innerRings) > 0 { // should never happen
-				panicInnerRingsButNoOuterRings(level, polygon, innerRings)
-				continue
+			if len(innerRings) > 0 {
+				newPolygons[level] = matchInnersToPolygon(newPolygons[level], innerRings, len(polygon) > 1)
 			}
-			newPolygons[level] = matchInnersToPolygon(newPolygons[level], innerRings)
 			if keepPointsAndLines {
 				newPointsAndLines[level] = append(newPointsAndLines[level], pointsAndLines...)
 			}
@@ -150,14 +149,14 @@ func addPointsAndSnap(ix *PointIndex, polygon geom.Polygon, levels []Level) map[
 	return floatPolygonsToGeomPolygonsForAllLevels(newPolygons)
 }
 
-func matchInnersToPolygon(polygons [][][][2]float64, innerRings [][][2]float64) [][][][2]float64 {
+func matchInnersToPolygon(polygons [][][][2]float64, innerRings [][][2]float64, hasInners bool) [][][][2]float64 {
 	lenPolygons := len(polygons)
-	if lenPolygons == 0 {
-		return polygons
-	} else if lenPolygons == 1 {
-		polygons[0] = append(polygons[0], innerRings...)
+	if len(innerRings) == 0 {
 		return polygons
 	}
+	numDeduped, polygons := dedupePolygonsByOuters(polygons)
+
+	var innersTurnedOuters [][][2]float64
 matchInners:
 	for _, innerRing := range innerRings {
 		containsPerPolyI := make(map[int]uint, lenPolygons)
@@ -179,17 +178,60 @@ matchInners:
 				continue matchInners
 			}
 		}
-		// no (single) matching outer ring was found (should never happen)
 		if len(containsPerPolyI) == 0 {
-			panicNoMatchingOuterForInnerRing(polygons, innerRing)
+			// no (single) matching outer ring was found
+			// presumably because the inner ring's winding order is incorrect and it should have been an outer
+			// TODO is that presumption correct and is this really never a panic? // panicNoMatchingOuterForInnerRing(polygons, innerRing)
+			log.Printf("no matching outer for inner ring found, turned inner into outer. original has inners: %v", hasInners)
+			innersTurnedOuters = append(innersTurnedOuters, reverseClone(innerRing))
 			continue
 		}
-		panicMoreThanOneMatchingOuterRing(polygons, innerRing)
+		// multiple matching outer rings were found, possibly because there are duplicates
+		panicMoreThanOneMatchingOuterRing(polygons, innerRing, numDeduped)
 		continue
 	}
+	for i := range innersTurnedOuters {
+		polygons = append(polygons, [][][2]float64{innersTurnedOuters[i]})
+	}
 	return polygons
 }
 
+// helper for matchInnersToPolygon to delete duplicate polygons
+// comparing them only by their outers and asserting that a deleted polygon didn't have inner rings appended yet
+// yes it's implemented as ~O(n^2),
+// but it's expected that the (outer) rings are usually different even from the first point,
+// making it still more efficient than using a hashmap of the entire rings
+func dedupePolygonsByOuters(polygons [][][][2]float64) (int, [][][][2]float64) {
+	numPolygons := len(polygons)
+	numDeleted := 0
+	for i := 0; i < numPolygons; i++ {
+		ring := polygons[i][0] // only check the outer
+	compareToOther:
+		for j := i + 1; j < numPolygons; j++ {
+			ringLen := len(ring)
+			other := polygons[j][0]
+			otherLen := len(other)
+			if ringLen != otherLen {
+				continue
+			}
+			for k := 0; k < min(ringLen, otherLen); k++ {
+				if ring[k] != other[k] {
+					continue compareToOther
+				}
+			}
+			// delete
+			if len(polygons[j]) > 1 {
+				panicDeletedPolygonHasInnerRing(polygons[j])
+			}
+			polygons = append(polygons[:j], polygons[j+1:]...)
+			j--
+			numPolygons--
+			numDeleted++
+		}
+	}
+	return numDeleted, polygons
+}
+
 // from paulmach/orb, modified to also return whether it's on the boundary
 // ringContains returns true if the point is inside the ring.
 // Points on the boundary are also considered in. In which case the second returned var is true too.
@@ -725,10 +767,28 @@ func panicMoreThanOneMatchingOuterRing(polygons [][][][2]float64, innerRing [][2
 	panic(panicMsg)
 }
 
+func panicDeletedPolygonHasInnerRing(polygon [][][2]float64) {
+	panicMsg := fmt.Sprintf("a deleted dupe polygon had more than one ring, %v",
+		truncatedWkt(floatPolygonToGeomPolygon(polygon), 100))
+	panic(panicMsg)
+}
+
 func truncatedWkt(geom geom.Geometry, width uint) string {
 	return truncate.StringWithTail(wkt.MustEncode(geom), width, "...")
 }
 
+func reverseClone[S ~[]E, E any](s S) S {
+	if s == nil {
+		return nil
+	}
+	l := len(s)
+	c := make(S, l)
+	for i := 0; i < l; i++ {
+		c[l-1-i] = c[i]
+	}
+	return c
+}
+
 func roundFloat(f float64, p uint) float64 {
 	r := math.Pow(10, float64(p))
 	return math.Round(f*r) / r

snap/snap_test.go

@@ -589,6 +589,13 @@ func TestSnap_snapPolygon(t *testing.T) {
 				{{{179407.04, 408272.32}, {179192, 408272.32}}}, {{{179192, 408272.32}, {179192, 408487.36}}}, {{{179407.04, 408272.32}, {179192, 408272.32}}}, {{{179192, 408272.32}, {178976.96, 408487.36}}}, {{{178976.96, 408487.36}, {178761.92, 408272.32}}}, {{{178761.92, 408272.32}, {178546.88, 408057.28}}}, {{{178546.88, 408057.28}, {178546.88, 407842.24}}},
 			}},
 		},
+		{
+			name:    "one of three split outer rings is cw and turned outer after no matching outer",
+			tms:     loadEmbeddedTileMatrixSet(t, "NetherlandsRDNewQuad"),
+			tmIDs:   []tms20.TMID{0},
+			polygon: geom.Polygon{{{88580.011, 439678.996}, {88337.73, 439237.216}, {89273.964, 438026.4}, {89386.079, 438023.335}, {90251.524, 438784.15}, {89852.567, 439284.421}, {89425.263, 439355.284}, {89247.228, 439563.507}, {89089.95, 439692.364}, {88959.832, 439729.531}, {89055.886, 439819.684}, {89466.904, 439382.346}, {89899.488, 439311.969}, {90170.183, 438911.775}, {90329.354, 438821.391}, {90651.094, 438796.963}, {91473.854, 439243.296}, {90632.307, 438747.518}, {90270.708, 438757.632}, {89555.357, 437677.283}, {90499.163, 436096.427}, {91435.651, 435963.019}, {91404.334, 436039.088}, {91254.337, 436091.084}, {90500.745, 436098.362}, {90076.214, 437042.706}, {89870.055, 437307.816}, {89768.94, 437363.42}, {89650.683, 437521.434}, {89640.994, 437568.838}, {89558.222, 437677.647}, {90269.467, 438753.387}, {90632.85, 438744.94}, {91313.174, 439143.369}, {91477.748, 439241.657}, {91475.353, 439245.66}, {91457.592, 439266.852}, {91243.008, 439179.921}, {90710.843, 438897.924}, {90650.175, 438799.288}, {90440.729, 438846.985}, {90395.019, 438846.967}, {90329.938, 438823.822}, {90287.474, 438885.328}, {90172.086, 438913.396}, {90044.257, 439125.421}, {89901.052, 439313.924}, {89885.113, 439321.991}, {89835.824, 439335.083}, {89468.228, 439384.467}, {89173.832, 439758.873}, {89061.413, 439821.909}, {89054.68, 439821.883}, {89023.222, 439801.24}, {88989.659, 439763.597}, {88949.781, 439739.428}, {88958.959, 439726.203}, {89088.39, 439690.41}, {89245.45, 439561.75}, {89388.248, 439376.01}, {89424.081, 439353.075}, {89566.906, 439317.631}, {89851.03, 439282.45}, {90111.766, 438914.525}, {90249.027, 438784.029}, {90211.25, 438760.51}, {90183.492, 438736.293}, {89584.683, 438207.656}, {89384.579, 438025.335}, {89274.819, 438028.749}, {88339.974, 439238.317}, {88419.861, 439377.057}, {88447.454, 439387.602}, {88485.231, 439376.209}, {88505.9, 439379.802}, {88564.366, 439441.722}, {88589.428, 439478.721}, {88598.844, 439504.106}, {88608.517, 439561.563}, {88582.418, 439679.669}, {88565.692, 439724.97}, {88480.367, 439857.335}, {88409.981, 439938.527}, {88412.431, 439940.265}, {88366.171, 440033.682}, {88353.723, 440046.457}, {88356.08, 440054.25}, {88342.856, 440086.861}, {88266.552, 440224.799}, {88252.681, 440243.646}, {88196.44, 440306.135}, {87992.789, 440467.453}, {88250.595, 440274.14}, {88508.083, 439845.775}, {88270.249, 440256.888}, {88194.893, 440335.659}, {88010.485, 440474.349}, {87996.213, 440475.679}, {87990.894, 440469.07}, {88580.011, 439678.996}}},
+			want:    map[tms20.TMID][]geom.Polygon{}, // want no panicNoMatchingOuterForInnerRing
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {