From fede781c18f9bb1ca19131cf45f8bcc076032fa9 Mon Sep 17 00:00:00 2001 From: Martin Davis Date: Fri, 25 Feb 2022 21:16:33 -0800 Subject: [PATCH 1/3] Fix IsValidOp for repeated points Signed-off-by: Martin Davis --- .../valid/IndexedNestedHoleTester.java | 10 +- .../valid/IndexedNestedPolygonTester.java | 11 +- .../jts/operation/valid/IsValidOp.java | 9 +- .../valid/PolygonTopologyAnalyzer.java | 113 ++++++++++++++---- .../jts/operation/valid/IsValidTest.java | 21 ++++ .../resources/testxml/general/TestValid.xml | 48 ++++++++ 6 files changed, 169 insertions(+), 43 deletions(-) diff --git a/modules/core/src/main/java/org/locationtech/jts/operation/valid/IndexedNestedHoleTester.java b/modules/core/src/main/java/org/locationtech/jts/operation/valid/IndexedNestedHoleTester.java index c8d4773385..f480b66d20 100644 --- a/modules/core/src/main/java/org/locationtech/jts/operation/valid/IndexedNestedHoleTester.java +++ b/modules/core/src/main/java/org/locationtech/jts/operation/valid/IndexedNestedHoleTester.java @@ -90,13 +90,11 @@ public boolean isNested() /** * Checks nesting via a point-in-polygon test, - * or if the point lies on the boundary via - * the topology of the incident edges. + * or via the topology of the incident edges. */ - Coordinate holePt0 = hole.getCoordinateN(0); - Coordinate holePt1 = hole.getCoordinateN(1); - if (PolygonTopologyAnalyzer.isSegmentInRing(holePt0, holePt1, testHole)) { - nestedPt = holePt0; + if (PolygonTopologyAnalyzer.isInside(hole, testHole)) { + //TODO: find a hole point known to be inside + nestedPt = hole.getCoordinateN(0); return true; } } diff --git a/modules/core/src/main/java/org/locationtech/jts/operation/valid/IndexedNestedPolygonTester.java b/modules/core/src/main/java/org/locationtech/jts/operation/valid/IndexedNestedPolygonTester.java index cce97b212a..e17289a6eb 100644 --- a/modules/core/src/main/java/org/locationtech/jts/operation/valid/IndexedNestedPolygonTester.java +++ b/modules/core/src/main/java/org/locationtech/jts/operation/valid/IndexedNestedPolygonTester.java @@ -136,7 +136,7 @@ private Coordinate findNestedPoint(LinearRing shell, /** * Finds a point of a shell segment which lies inside a polygon, if any. - * The shell is assume to touch the polyon only at shell vertices, + * The shell is assumed to touch the polygon only at shell vertices, * and does not cross the polygon. * * @param shell the shell to test @@ -148,10 +148,7 @@ private static Coordinate findSegmentInPolygon(LinearRing shell, Polygon poly) LinearRing polyShell = poly.getExteriorRing(); if (polyShell.isEmpty()) return null; - Coordinate shell0 = shell.getCoordinateN(0); - Coordinate shell1 = shell.getCoordinateN(1); - - if (! PolygonTopologyAnalyzer.isSegmentInRing(shell0, shell1, polyShell)) + if (! PolygonTopologyAnalyzer.isInside(shell, polyShell)) return null; /** @@ -161,7 +158,7 @@ private static Coordinate findSegmentInPolygon(LinearRing shell, Polygon poly) for (int i = 0; i < poly.getNumInteriorRing(); i++) { LinearRing hole = poly.getInteriorRingN(i); if (hole.getEnvelopeInternal().covers(shell.getEnvelopeInternal()) - && PolygonTopologyAnalyzer.isSegmentInRing(shell0, shell1, hole)) { + && PolygonTopologyAnalyzer.isInside(shell, hole)) { return null; } } @@ -170,6 +167,6 @@ private static Coordinate findSegmentInPolygon(LinearRing shell, Polygon poly) * The shell is contained in the polygon, but is not contained in a hole. * This is invalid. */ - return shell0; + return shell.getCoordinateN(0); } } diff --git a/modules/core/src/main/java/org/locationtech/jts/operation/valid/IsValidOp.java b/modules/core/src/main/java/org/locationtech/jts/operation/valid/IsValidOp.java index 5dee0eefff..1377532705 100644 --- a/modules/core/src/main/java/org/locationtech/jts/operation/valid/IsValidOp.java +++ b/modules/core/src/main/java/org/locationtech/jts/operation/valid/IsValidOp.java @@ -471,15 +471,16 @@ private void checkHolesOutsideShell(Polygon poly) */ private Coordinate findHoleOutsideShellPoint(LinearRing hole, LinearRing shell) { Coordinate holePt0 = hole.getCoordinateN(0); - Coordinate holePt1 = hole.getCoordinateN(1); /** * If hole envelope is not covered by shell, it must be outside */ if (! shell.getEnvelopeInternal().covers( hole.getEnvelopeInternal() )) - return holePt0; + //TODO: find hole pt outside shell env + return holePt0; - if (PolygonTopologyAnalyzer.isSegmentInRing(holePt0, holePt1, shell)) - return null; + if (PolygonTopologyAnalyzer.isInside(hole, shell)) + return null; + //TODO: find hole point outside shell return holePt0; } diff --git a/modules/core/src/main/java/org/locationtech/jts/operation/valid/PolygonTopologyAnalyzer.java b/modules/core/src/main/java/org/locationtech/jts/operation/valid/PolygonTopologyAnalyzer.java index 32149af73c..7b8cfa9d69 100644 --- a/modules/core/src/main/java/org/locationtech/jts/operation/valid/PolygonTopologyAnalyzer.java +++ b/modules/core/src/main/java/org/locationtech/jts/operation/valid/PolygonTopologyAnalyzer.java @@ -56,32 +56,47 @@ public static Coordinate findSelfIntersection(LinearRing ring) { } /** - * Tests whether a segment p0-p1 is inside or outside a ring. + * Tests whether a ring is inside another ring. *

* Preconditions: *

+ * If the start point is properly inside or outside, the provides the result. + * Otherwise, the start point is on the target ring, + * and the incident start segment (accounting for repeated points) is + * tested for its topology relative to the target ring. * - * @param p0 a segment vertex - * @param p1 a segment vertex - * @param ring the ring to test - * @return true if the segment lies inside the ring + * @param test the ring to test + * @param target the ring to test against + * @return true if the test ring lies inside the target ring */ - public static boolean isSegmentInRing(Coordinate p0, Coordinate p1, LinearRing ring) { - Coordinate[] ringPts = ring.getCoordinates(); - int loc = PointLocation.locateInRing(p0, ringPts); + public static boolean isInside(LinearRing test, LinearRing target) { + Coordinate p0 = test.getCoordinateN(0); + Coordinate[] targetPts = target.getCoordinates(); + int loc = PointLocation.locateInRing(p0, targetPts); if (loc == Location.EXTERIOR) return false; if (loc == Location.INTERIOR) return true; /** - * The segment point is on the boundary of the ring. + * The start point is on the boundary of the ring. * Use the topology at the node to check if the segment * is inside or outside the ring. */ - return isIncidentSegmentInRing(p0, p1, ringPts); + Coordinate p1 = findNonEqualVertex(test, p0); + return isIncidentSegmentInRing(p0, p1, targetPts); + } + + private static Coordinate findNonEqualVertex(LinearRing ring, Coordinate p) { + int i = 1; + Coordinate next = ring.getCoordinateN(i); + while (next.equals2D(p) && i < ring.getNumPoints() - 1) { + i += 1; + next = ring.getCoordinateN(i); + } + return next; } /** @@ -96,21 +111,18 @@ public static boolean isSegmentInRing(Coordinate p0, Coordinate p1, LinearRing r * This works for both shells and holes, but the caller must know * the ring role. * - * @param p0 the first vertex of the segment + * @param p0 the touching vertex of the segment * @param p1 the second vertex of the segment * @param ringPts the points of the ring * @return true if the segment is inside the ring. */ - public static boolean isIncidentSegmentInRing(Coordinate p0, Coordinate p1, Coordinate[] ringPts) { + private static boolean isIncidentSegmentInRing(Coordinate p0, Coordinate p1, Coordinate[] ringPts) { int index = intersectingSegIndex(ringPts, p0); if (index < 0) { throw new IllegalArgumentException("Segment vertex does not intersect ring"); } - Coordinate rPrev = ringPts[index]; - Coordinate rNext = ringPts[index + 1]; - if (p0.equals2D(ringPts[index])) { - rPrev = ringPts[ringIndexPrev(ringPts, index)]; - } + Coordinate rPrev = findRingVertexPrev(ringPts, index, p0); + Coordinate rNext = findRingVertexNext(ringPts, index, p0); /** * If ring orientation is not normalized, flip the corner orientation */ @@ -122,6 +134,61 @@ public static boolean isIncidentSegmentInRing(Coordinate p0, Coordinate p1, Coor } return PolygonNode.isInteriorSegment(p0, rPrev, rNext, p1); } + + /** + * Finds the ring vertex previous to a node point on a ring + * (which is contained in the index'th segment, + * as either the start vertex or an interior point). + * Repeated points are skipped over. + * @param ringPts the ring + * @param index the index of the segment containing the node + * @param node the node point + * + * @return the previous ring vertex + */ + private static Coordinate findRingVertexPrev(Coordinate[] ringPts, int index, Coordinate node) { + int iPrev = index; + Coordinate prev = ringPts[iPrev]; + while (node.equals2D(prev)) { + iPrev = ringIndexPrev(ringPts, iPrev); + prev = ringPts[iPrev]; + } + return prev; + } + + private static int ringIndexPrev(Coordinate[] ringPts, int index) { + int iPrev = index - 1; + if (iPrev < 0) iPrev = ringPts.length - 2; + return iPrev; + } + + /** + * Finds the ring vertex next from a node point on a ring + * (which is contained in the index'th segment, + * as either the start vertex or an interior point). + * Repeated points are skipped over. + * @param ringPts the ring + * @param index the index of the segment containing the node + * @param node the node point + * + * @return the next ring vertex + */ + private static Coordinate findRingVertexNext(Coordinate[] ringPts, int index, Coordinate node) { + //-- safe, since index is always the start of a ring segment + int iNext = index + 1; + Coordinate next = ringPts[iNext]; + while (node.equals2D(next)) { + iNext = ringIndexNext(ringPts, iNext); + next = ringPts[iNext]; + } + return next; + } + + private static int ringIndexNext(Coordinate[] ringPts, int index) { + int iNext = index + 1; + if (iNext > ringPts.length - 2) iNext = 0; + return iNext; + } /** * Computes the index of the segment which intersects a given point. @@ -143,12 +210,6 @@ private static int intersectingSegIndex(Coordinate[] ringPts, Coordinate pt) { } return -1; } - - private static int ringIndexPrev(Coordinate[] ringPts, int index) { - int iPrev = index - 1; - if (index == 0) iPrev = ringPts.length - 2; - return iPrev; - } private boolean isInvertedRingValid; diff --git a/modules/core/src/test/java/org/locationtech/jts/operation/valid/IsValidTest.java b/modules/core/src/test/java/org/locationtech/jts/operation/valid/IsValidTest.java index 5dc904da4e..b73d28ecf1 100644 --- a/modules/core/src/test/java/org/locationtech/jts/operation/valid/IsValidTest.java +++ b/modules/core/src/test/java/org/locationtech/jts/operation/valid/IsValidTest.java @@ -156,6 +156,27 @@ public void testLinearRingSelfCrossing2() { "LINEARRING (0 0, 100 100, 100 0, 0 100, 0 0)"); } + /** + * Tests that repeated points at nodes are handled correctly. + * + * See https://github.com/locationtech/jts/issues/843 + */ + public void testPolygonHoleWithRepeatedShellPointTouch() { + checkValid( "POLYGON ((90 10, 10 10, 50 90, 50 90, 90 10), (50 90, 60 30, 40 30, 50 90))"); + } + + public void testPolygonHoleWithRepeatedShellPointTouchMultiple() { + checkValid( "POLYGON ((90 10, 10 10, 50 90, 50 90, 50 90, 50 90, 90 10), (50 90, 60 30, 40 30, 50 90))"); + } + + public void testPolygonHoleWithRepeatedTouchEndPoint() { + checkValid( "POLYGON ((90 10, 10 10, 50 90, 90 10, 90 10), (90 10, 40 30, 60 50, 90 10))"); + } + + public void testPolygonHoleWithRepeatedHolePointTouch() { + checkValid( "POLYGON ((50 90, 10 10, 90 10, 50 90), (50 90, 50 90, 60 40, 60 40, 40 40, 50 90))"); + } + //============================================= private void checkValid(String wkt) { diff --git a/modules/tests/src/test/resources/testxml/general/TestValid.xml b/modules/tests/src/test/resources/testxml/general/TestValid.xml index 15924a222f..7173219169 100644 --- a/modules/tests/src/test/resources/testxml/general/TestValid.xml +++ b/modules/tests/src/test/resources/testxml/general/TestValid.xml @@ -171,6 +171,54 @@ POLYGON ((10 90, 90 90, 90 10, 10 10, 10 90), (40 80, 60 80, 50 50, 40 80), (20 true + + A - hole touches shell at repeated shell point (valid) + +POLYGON ((90 10, 10 10, 50 90, 50 90, 90 10), (50 90, 60 30, 40 30, 50 90)) + + true + + + + A - hole touches shell at repeated shell end point (valid) + +POLYGON ((90 10, 10 10, 50 90, 90 10, 90 10), (90 10, 40 30, 60 50, 90 10)) + + true + + + + A - hole touches shell at repeated hole point (valid) + +POLYGON ((50 90, 10 10, 90 10, 50 90), (60 40, 40 40, 50 90, 50 90, 60 40)) + + true + + + + A - hole touches shell at repeated hole end point (valid) + +POLYGON ((50 90, 10 10, 90 10, 50 90), (50 90, 50 90, 60 40, 60 40, 40 40, 50 90)) + + true + + + + A - hole touches hole at repeated point on hole (valid) + +POLYGON ((10 90, 90 90, 90 10, 10 10, 10 90), (70 80, 20 50, 80 20, 40 50, 40 50, 70 80), (40 50, 40 50, 70 40, 70 60, 40 50)) + + true + + + + mA - shell touches shell at repeated point (valid) + +MULTIPOLYGON (((70 80, 20 50, 80 20, 40 50, 40 50, 70 80)), ((40 50, 40 50, 70 40, 70 60, 40 50))) + + true + + mA - shell inside hole, no touch (valid) From c683189f3dc5e92afb32704726f33051abfe7911 Mon Sep 17 00:00:00 2001 From: Martin Davis Date: Sat, 26 Feb 2022 10:05:30 -0800 Subject: [PATCH 2/3] Rename function Signed-off-by: Martin Davis --- .../valid/IndexedNestedHoleTester.java | 6 +--- .../valid/IndexedNestedPolygonTester.java | 4 +-- .../jts/operation/valid/IsValidOp.java | 2 +- .../valid/PolygonTopologyAnalyzer.java | 36 +++++++++---------- 4 files changed, 22 insertions(+), 26 deletions(-) diff --git a/modules/core/src/main/java/org/locationtech/jts/operation/valid/IndexedNestedHoleTester.java b/modules/core/src/main/java/org/locationtech/jts/operation/valid/IndexedNestedHoleTester.java index f480b66d20..6806e19fc8 100644 --- a/modules/core/src/main/java/org/locationtech/jts/operation/valid/IndexedNestedHoleTester.java +++ b/modules/core/src/main/java/org/locationtech/jts/operation/valid/IndexedNestedHoleTester.java @@ -88,11 +88,7 @@ public boolean isNested() if (! testHole.getEnvelopeInternal().covers( hole.getEnvelopeInternal()) ) continue; - /** - * Checks nesting via a point-in-polygon test, - * or via the topology of the incident edges. - */ - if (PolygonTopologyAnalyzer.isInside(hole, testHole)) { + if (PolygonTopologyAnalyzer.isRingNested(hole, testHole)) { //TODO: find a hole point known to be inside nestedPt = hole.getCoordinateN(0); return true; diff --git a/modules/core/src/main/java/org/locationtech/jts/operation/valid/IndexedNestedPolygonTester.java b/modules/core/src/main/java/org/locationtech/jts/operation/valid/IndexedNestedPolygonTester.java index e17289a6eb..cb929c7f1d 100644 --- a/modules/core/src/main/java/org/locationtech/jts/operation/valid/IndexedNestedPolygonTester.java +++ b/modules/core/src/main/java/org/locationtech/jts/operation/valid/IndexedNestedPolygonTester.java @@ -148,7 +148,7 @@ private static Coordinate findSegmentInPolygon(LinearRing shell, Polygon poly) LinearRing polyShell = poly.getExteriorRing(); if (polyShell.isEmpty()) return null; - if (! PolygonTopologyAnalyzer.isInside(shell, polyShell)) + if (! PolygonTopologyAnalyzer.isRingNested(shell, polyShell)) return null; /** @@ -158,7 +158,7 @@ private static Coordinate findSegmentInPolygon(LinearRing shell, Polygon poly) for (int i = 0; i < poly.getNumInteriorRing(); i++) { LinearRing hole = poly.getInteriorRingN(i); if (hole.getEnvelopeInternal().covers(shell.getEnvelopeInternal()) - && PolygonTopologyAnalyzer.isInside(shell, hole)) { + && PolygonTopologyAnalyzer.isRingNested(shell, hole)) { return null; } } diff --git a/modules/core/src/main/java/org/locationtech/jts/operation/valid/IsValidOp.java b/modules/core/src/main/java/org/locationtech/jts/operation/valid/IsValidOp.java index 1377532705..8b69c4f297 100644 --- a/modules/core/src/main/java/org/locationtech/jts/operation/valid/IsValidOp.java +++ b/modules/core/src/main/java/org/locationtech/jts/operation/valid/IsValidOp.java @@ -478,7 +478,7 @@ private Coordinate findHoleOutsideShellPoint(LinearRing hole, LinearRing shell) //TODO: find hole pt outside shell env return holePt0; - if (PolygonTopologyAnalyzer.isInside(hole, shell)) + if (PolygonTopologyAnalyzer.isRingNested(hole, shell)) return null; //TODO: find hole point outside shell return holePt0; diff --git a/modules/core/src/main/java/org/locationtech/jts/operation/valid/PolygonTopologyAnalyzer.java b/modules/core/src/main/java/org/locationtech/jts/operation/valid/PolygonTopologyAnalyzer.java index 7b8cfa9d69..2d632bbfe1 100644 --- a/modules/core/src/main/java/org/locationtech/jts/operation/valid/PolygonTopologyAnalyzer.java +++ b/modules/core/src/main/java/org/locationtech/jts/operation/valid/PolygonTopologyAnalyzer.java @@ -43,29 +43,16 @@ class PolygonTopologyAnalyzer { /** - * Finds a self-intersection (if any) in a {@link LinearRing}. - * - * @param ring the ring to analyze - * @return a self-intersection point if one exists, or null - */ - public static Coordinate findSelfIntersection(LinearRing ring) { - PolygonTopologyAnalyzer ata = new PolygonTopologyAnalyzer(ring, false); - if (ata.hasInvalidIntersection()) - return ata.getInvalidLocation(); - return null; - } - - /** - * Tests whether a ring is inside another ring. + * Tests whether a ring is nested inside another ring. *

* Preconditions: *

    *
  • The rings do not cross (i.e. the test is wholly inside or outside the target) - *
  • The rings may touch (at a point or in a line) + *
  • The rings may touch at discrete points only *
  • The target ring does not self-cross, but it may self-touch *
- * If the start point is properly inside or outside, the provides the result. - * Otherwise, the start point is on the target ring, + * If the test ring start point is properly inside or outside, that provides the result. + * Otherwise the start point is on the target ring, * and the incident start segment (accounting for repeated points) is * tested for its topology relative to the target ring. * @@ -73,7 +60,7 @@ public static Coordinate findSelfIntersection(LinearRing ring) { * @param target the ring to test against * @return true if the test ring lies inside the target ring */ - public static boolean isInside(LinearRing test, LinearRing target) { + public static boolean isRingNested(LinearRing test, LinearRing target) { Coordinate p0 = test.getCoordinateN(0); Coordinate[] targetPts = target.getCoordinates(); int loc = PointLocation.locateInRing(p0, targetPts); @@ -211,6 +198,19 @@ private static int intersectingSegIndex(Coordinate[] ringPts, Coordinate pt) { return -1; } + /** + * Finds a self-intersection (if any) in a {@link LinearRing}. + * + * @param ring the ring to analyze + * @return a self-intersection point if one exists, or null + */ + public static Coordinate findSelfIntersection(LinearRing ring) { + PolygonTopologyAnalyzer ata = new PolygonTopologyAnalyzer(ring, false); + if (ata.hasInvalidIntersection()) + return ata.getInvalidLocation(); + return null; + } + private boolean isInvertedRingValid; private PolygonIntersectionAnalyzer intFinder; From 587542f7291d234760feab39ab30ee303b8f306d Mon Sep 17 00:00:00 2001 From: Martin Davis Date: Sat, 26 Feb 2022 11:39:09 -0800 Subject: [PATCH 3/3] Rename functions Signed-off-by: Martin Davis --- .../valid/IndexedNestedPolygonTester.java | 23 +++++++++++++------ 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/modules/core/src/main/java/org/locationtech/jts/operation/valid/IndexedNestedPolygonTester.java b/modules/core/src/main/java/org/locationtech/jts/operation/valid/IndexedNestedPolygonTester.java index cb929c7f1d..7435aa5455 100644 --- a/modules/core/src/main/java/org/locationtech/jts/operation/valid/IndexedNestedPolygonTester.java +++ b/modules/core/src/main/java/org/locationtech/jts/operation/valid/IndexedNestedPolygonTester.java @@ -25,11 +25,12 @@ /** * Tests whether a MultiPolygon has any element polygon - * nested inside another polygon, using a spatial + * improperly nested inside another polygon, using a spatial * index to speed up the comparisons. *

- * The logic assumes that the polygons do not overlap and have no collinear segments - * (so they are properly nested, and there are no duplicate rings). + * The logic assumes that the polygons do not overlap and have no collinear segments. + * So the polygon rings may touch at discrete points, + * but they are properly nested, and there are no duplicate rings. */ class IndexedNestedPolygonTester { @@ -75,7 +76,7 @@ private IndexedPointInAreaLocator getLocator(int polyIndex) { public Coordinate getNestedPoint() { return nestedPt; } /** - * Tests if any polygon is nested (contained) within another polygon. + * Tests if any polygon is improperly nested (contained) within another polygon. * This is invalid. * The nested point will be set to reflect this. * @return true if some polygon is nested @@ -106,6 +107,14 @@ public boolean isNested() return false; } + /** + * Finds an improperly nested point, if one exists. + * + * @param shell the test polygon shell + * @param possibleOuterPoly a polygon which may contain it + * @param locator the locator for the outer polygon + * @return a nested point, if one exists, or null + */ private Coordinate findNestedPoint(LinearRing shell, Polygon possibleOuterPoly, IndexedPointInAreaLocator locator) { @@ -119,7 +128,7 @@ private Coordinate findNestedPoint(LinearRing shell, return shellPt0; } - Coordinate shellPt1 = shell.getCoordinateN(0); + Coordinate shellPt1 = shell.getCoordinateN(1); int loc1 = locator.locate(shellPt1); if (loc1 == Location.EXTERIOR) return null; if (loc1 == Location.INTERIOR) { @@ -131,7 +140,7 @@ private Coordinate findNestedPoint(LinearRing shell, * the polygon. * Nesting can be checked via the topology of the incident edges. */ - return findSegmentInPolygon(shell, possibleOuterPoly); + return findIncidentSegmentNestedPoint(shell, possibleOuterPoly); } /** @@ -143,7 +152,7 @@ private Coordinate findNestedPoint(LinearRing shell, * @param poly the polygon to test against * @return an interior segment point, or null if the shell is nested correctly */ - private static Coordinate findSegmentInPolygon(LinearRing shell, Polygon poly) + private static Coordinate findIncidentSegmentNestedPoint(LinearRing shell, Polygon poly) { LinearRing polyShell = poly.getExteriorRing(); if (polyShell.isEmpty()) return null;