From efb3993464a6d2fea5cb0eb76b67bd48aadbf3fb Mon Sep 17 00:00:00 2001 From: Mike Barry Date: Thu, 29 Oct 2026 06:13:32 -0400 Subject: [PATCH 1/3] Add index for holes-in-shell validity check Signed-off-by: Mike Barry --- .../jts/operation/valid/IsValidOp.java | 38 ++++++++- .../jts/operation/valid/IsValidTest.java | 28 +++++++ .../valid/IsValidHolesInShellPerfTest.java | 78 +++++++++++++++++++ 3 files changed, 142 insertions(+), 2 deletions(-) create mode 100644 modules/core/src/test/java/test/jts/perf/operation/valid/IsValidHolesInShellPerfTest.java 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 5bd7904316..7791f820d5 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 @@ -11,11 +11,13 @@ */ package org.locationtech.jts.operation.valid; +import org.locationtech.jts.algorithm.locate.IndexedPointInAreaLocator; import org.locationtech.jts.geom.Coordinate; import org.locationtech.jts.geom.Geometry; import org.locationtech.jts.geom.GeometryCollection; import org.locationtech.jts.geom.LineString; import org.locationtech.jts.geom.LinearRing; +import org.locationtech.jts.geom.Location; import org.locationtech.jts.geom.MultiPoint; import org.locationtech.jts.geom.MultiPolygon; import org.locationtech.jts.geom.Point; @@ -32,6 +34,13 @@ public class IsValidOp { private static final int MIN_SIZE_LINESTRING = 2; private static final int MIN_SIZE_RING = 4; + /** + * Polygons with at least this many holes use an indexed shell locator + * to check that holes are inside the shell. + * Building the index costs about as much as scanning the shell for 20 holes + * (see IsValidHolesInShellPerfTest). + */ + private static final int MIN_HOLES_INDEXED_SHELL = 20; /** * Tests whether a {@link Geometry} is valid. @@ -71,6 +80,8 @@ public static boolean isValid(Coordinate coord) */ private boolean isInvertedRingValid = false; + private int minHolesIndexedShell = MIN_HOLES_INDEXED_SHELL; + private TopologyValidationError validErr; /** @@ -117,6 +128,17 @@ public void setSelfTouchingRingFormingHoleValid(boolean isValid) isInvertedRingValid = isValid; } + /** + * Sets the number of holes at which a polygon uses an indexed shell locator. + * Allows testing the indexed code path with small polygons. + * + * @param minHoles the minimum number of holes to use an indexed shell locator + */ + void setMinHolesIndexedShell(int minHoles) + { + minHolesIndexedShell = minHoles; + } + /** * Tests the validity of the input geometry. * @@ -438,6 +460,10 @@ private void checkHolesInShell(Polygon poly) LinearRing shell = poly.getExteriorRing(); boolean isShellEmpty = shell.isEmpty(); + IndexedPointInAreaLocator shellLocator = null; + if (! isShellEmpty && poly.getNumInteriorRing() >= minHolesIndexedShell) { + shellLocator = new IndexedPointInAreaLocator(shell); + } for (int i = 0; i < poly.getNumInteriorRing(); i++) { LinearRing hole = poly.getInteriorRingN(i); @@ -448,7 +474,7 @@ private void checkHolesInShell(Polygon poly) invalidPt = hole.getCoordinate(); } else { - invalidPt = findHoleOutsideShellPoint(hole, shell); + invalidPt = findHoleOutsideShellPoint(hole, shell, shellLocator); } if (invalidPt != null) { logInvalid(TopologyValidationError.HOLE_OUTSIDE_SHELL, @@ -467,9 +493,11 @@ private void checkHolesInShell(Polygon poly) * * @param hole the hole to test * @param shell the polygon shell to test against + * @param shellLocator the indexed locator for the shell, or null to scan the shell * @return a hole point outside the shell, or null if it is inside */ - private Coordinate findHoleOutsideShellPoint(LinearRing hole, LinearRing shell) { + private Coordinate findHoleOutsideShellPoint(LinearRing hole, LinearRing shell, + IndexedPointInAreaLocator shellLocator) { Coordinate holePt0 = hole.getCoordinateN(0); /** * If hole envelope is not covered by shell, it must be outside @@ -478,6 +506,12 @@ private Coordinate findHoleOutsideShellPoint(LinearRing hole, LinearRing shell) //TODO: find hole pt outside shell env return holePt0; + if (shellLocator != null) { + int loc = shellLocator.locate(holePt0); + if (loc == Location.EXTERIOR) return holePt0; + if (loc == Location.INTERIOR) return null; + //-- hole point is on the shell, so check which side of the shell the hole edge from it goes + } if (PolygonTopologyAnalyzer.isRingNested(hole, shell)) return null; //TODO: find hole point outside shell 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 b73d28ecf1..b6064cd6aa 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 @@ -176,6 +176,20 @@ public void testPolygonHoleWithRepeatedTouchEndPoint() { 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))"); } + + public void testPolygonHolesInConcaveShell() { + checkValidIndexedShell( "POLYGON ((10 10, 10 90, 40 90, 40 30, 60 30, 60 90, 90 90, 90 10, 10 10), (15 70, 25 70, 25 80, 15 80, 15 70), (70 50, 80 50, 80 60, 70 60, 70 50), (45 15, 55 15, 55 25, 45 25, 45 15), (40 50, 30 45, 30 55, 40 50))"); + } + + public void testPolygonHoleOutsideShellInConcavity() { + checkInvalidIndexedShell(TopologyValidationError.HOLE_OUTSIDE_SHELL, + "POLYGON ((10 10, 10 90, 40 90, 40 30, 60 30, 60 90, 90 90, 90 10, 10 10), (45 60, 55 60, 55 80, 45 80, 45 60))"); + } + + public void testPolygonHoleOutsideShellInConcavityTouch() { + checkInvalidIndexedShell(TopologyValidationError.HOLE_OUTSIDE_SHELL, + "POLYGON ((10 10, 10 90, 40 90, 40 30, 60 30, 60 90, 90 90, 90 10, 10 10), (15 70, 25 70, 25 80, 15 80, 15 70), (70 50, 80 50, 80 60, 70 60, 70 50), (40 50, 50 45, 50 55, 40 50))"); + } //============================================= @@ -194,6 +208,20 @@ private void checkInvalid(String wkt) { checkValid(false, wkt); } + private void checkValidIndexedShell(String wkt) { + assertNull( validateIndexedShell(wkt) ); + } + + private void checkInvalidIndexedShell(int expectedErrType, String wkt) { + assertEquals( expectedErrType, validateIndexedShell(wkt).getErrorType() ); + } + + private TopologyValidationError validateIndexedShell(String wkt) { + IsValidOp validOp = new IsValidOp(read(wkt)); + validOp.setMinHolesIndexedShell(0); + return validOp.getValidationError(); + } + private void checkInvalid(int exepctedErrType, String wkt) { Geometry geom = read(wkt); IsValidOp validOp = new IsValidOp(geom); diff --git a/modules/core/src/test/java/test/jts/perf/operation/valid/IsValidHolesInShellPerfTest.java b/modules/core/src/test/java/test/jts/perf/operation/valid/IsValidHolesInShellPerfTest.java new file mode 100644 index 0000000000..1db4b6acce --- /dev/null +++ b/modules/core/src/test/java/test/jts/perf/operation/valid/IsValidHolesInShellPerfTest.java @@ -0,0 +1,78 @@ +/* + * Copyright (c) 2020 Martin Davis. + * + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License 2.0 + * and Eclipse Distribution License v. 1.0 which accompanies this distribution. + * The Eclipse Public License is available at http://www.eclipse.org/legal/epl-v20.html + * and the Eclipse Distribution License is available at + * + * http://www.eclipse.org/org/documents/edl-v10.php. + */ +package test.jts.perf.operation.valid; + +import org.locationtech.jts.geom.Coordinate; +import org.locationtech.jts.geom.Geometry; +import org.locationtech.jts.geom.LinearRing; +import org.locationtech.jts.geom.Polygon; + +import test.jts.geom.TestShapeFactory; +import test.jts.perf.PerformanceTestCase; +import test.jts.perf.PerformanceTestRunner; + +/** + * Tests performance of IsValidOp.checkHolesInShell + * for polygons with a large shell and many holes. + */ +public class IsValidHolesInShellPerfTest extends PerformanceTestCase { + + static final int N_ITER = 10; + + static final int N_PTS_SMALL_SHELL = 1000; + static final int N_PTS_LARGE_SHELL = 100_000; + + public static void main(String args[]) { + PerformanceTestRunner.run(IsValidHolesInShellPerfTest.class); + } + + public IsValidHolesInShellPerfTest(String name) + { + super(name); + // number of holes; must be squares + setRunSize(new int[] { 1, 4, 16, 64, 256, 1024, 4096 }); + setRunIterations(N_ITER); + } + + Geometry smallShellGeom; + Geometry largeShellGeom; + + @Override + public void startRun(int nHoles) + { + System.out.println("\nRunning with # holes " + nHoles ); + smallShellGeom = createCircleWithHoles(N_PTS_SMALL_SHELL, nHoles); + largeShellGeom = createCircleWithHoles(N_PTS_LARGE_SHELL, nHoles); + } + + private Geometry createCircleWithHoles(int nPtsShell, int nHoles) { + Coordinate origin = new Coordinate(0, 0); + Polygon circle = TestShapeFactory.createCircle(origin, 100, nPtsShell); + // holes are placed in the square inscribed in the circle + Polygon square = (Polygon) TestShapeFactory.createSquareWithCircleHoles(origin, 70, nHoles, 8); + LinearRing[] holes = new LinearRing[square.getNumInteriorRing()]; + for (int i = 0; i < holes.length; i++) { + holes[i] = square.getInteriorRingN(i); + } + return circle.getFactory().createPolygon(circle.getExteriorRing(), holes); + } + + public void runValidateSmallShell() + { + smallShellGeom.isValid(); + } + + public void runValidateLargeShell() + { + largeShellGeom.isValid(); + } +} From 5847d01340782fee03f6a62aa39aac396cd7bed4 Mon Sep 17 00:00:00 2001 From: Mike Barry Date: Sat, 7 Nov 2026 17:42:35 -0500 Subject: [PATCH 2/3] Make all test cases go through indexed and non-indexed versions Signed-off-by: Mike Barry --- .../jts/operation/valid/IsValidTest.java | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) 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 b6064cd6aa..9da4335241 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 @@ -178,16 +178,16 @@ public void testPolygonHoleWithRepeatedHolePointTouch() { } public void testPolygonHolesInConcaveShell() { - checkValidIndexedShell( "POLYGON ((10 10, 10 90, 40 90, 40 30, 60 30, 60 90, 90 90, 90 10, 10 10), (15 70, 25 70, 25 80, 15 80, 15 70), (70 50, 80 50, 80 60, 70 60, 70 50), (45 15, 55 15, 55 25, 45 25, 45 15), (40 50, 30 45, 30 55, 40 50))"); + checkValid( "POLYGON ((10 10, 10 90, 40 90, 40 30, 60 30, 60 90, 90 90, 90 10, 10 10), (15 70, 25 70, 25 80, 15 80, 15 70), (70 50, 80 50, 80 60, 70 60, 70 50), (45 15, 55 15, 55 25, 45 25, 45 15), (40 50, 30 45, 30 55, 40 50))"); } public void testPolygonHoleOutsideShellInConcavity() { - checkInvalidIndexedShell(TopologyValidationError.HOLE_OUTSIDE_SHELL, + checkInvalid(TopologyValidationError.HOLE_OUTSIDE_SHELL, "POLYGON ((10 10, 10 90, 40 90, 40 30, 60 30, 60 90, 90 90, 90 10, 10 10), (45 60, 55 60, 55 80, 45 80, 45 60))"); } public void testPolygonHoleOutsideShellInConcavityTouch() { - checkInvalidIndexedShell(TopologyValidationError.HOLE_OUTSIDE_SHELL, + checkInvalid(TopologyValidationError.HOLE_OUTSIDE_SHELL, "POLYGON ((10 10, 10 90, 40 90, 40 30, 60 30, 60 90, 90 90, 90 10, 10 10), (15 70, 25 70, 25 80, 15 80, 15 70), (70 50, 80 50, 80 60, 70 60, 70 50), (40 50, 50 45, 50 55, 40 50))"); } @@ -202,19 +202,17 @@ private void checkValid(boolean isExpectedValid, String wkt) { Geometry geom = read(wkt); boolean isValid = geom.isValid(); assertEquals( isExpectedValid, isValid ); + TopologyValidationError indexedCheck = validateIndexedShell(wkt); + if (isExpectedValid) { + assertNull( indexedCheck ); + } else { + assertNotNull( indexedCheck ); + } } private void checkInvalid(String wkt) { checkValid(false, wkt); } - - private void checkValidIndexedShell(String wkt) { - assertNull( validateIndexedShell(wkt) ); - } - - private void checkInvalidIndexedShell(int expectedErrType, String wkt) { - assertEquals( expectedErrType, validateIndexedShell(wkt).getErrorType() ); - } private TopologyValidationError validateIndexedShell(String wkt) { IsValidOp validOp = new IsValidOp(read(wkt)); @@ -226,6 +224,8 @@ private void checkInvalid(int exepctedErrType, String wkt) { Geometry geom = read(wkt); IsValidOp validOp = new IsValidOp(geom); TopologyValidationError err = validOp.getValidationError(); + // validate through both indexed and non-indexed versions assertEquals( exepctedErrType, err.getErrorType() ); + assertEquals( exepctedErrType, validateIndexedShell(wkt).getErrorType() ); } } From 10e81552625a7d672940804687592f273d87c2f3 Mon Sep 17 00:00:00 2001 From: Mike Barry Date: Sat, 7 Nov 2026 17:51:35 -0500 Subject: [PATCH 3/3] Rm comment Signed-off-by: Mike Barry --- .../java/org/locationtech/jts/operation/valid/IsValidTest.java | 1 - 1 file changed, 1 deletion(-) 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 9da4335241..d0b9659288 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 @@ -224,7 +224,6 @@ private void checkInvalid(int exepctedErrType, String wkt) { Geometry geom = read(wkt); IsValidOp validOp = new IsValidOp(geom); TopologyValidationError err = validOp.getValidationError(); - // validate through both indexed and non-indexed versions assertEquals( exepctedErrType, err.getErrorType() ); assertEquals( exepctedErrType, validateIndexedShell(wkt).getErrorType() ); }