Skip to content

Add index for holes-in-shell validity check #1251

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
msbarry wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Add index for holes-in-shell validity check #1251

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
efb3993
Add index for holes-in-shell validity check
msbarry Oct 29, 2026
5847d01
Make all test cases go through indexed and non-indexed versions
msbarry Nov 7, 2026
10e8155
Rm comment
msbarry Nov 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 36 additions & 2 deletions modules/core/src/main/java/org/locationtech/jts/operation/valid/IsValidOp.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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.
Expand Down Expand Up @@ -71,6 +80,8 @@ public static boolean isValid(Coordinate coord)
*/
private boolean isInvertedRingValid = false;

private int minHolesIndexedShell = MIN_HOLES_INDEXED_SHELL;

private TopologyValidationError validErr;

/**
Expand Down Expand Up @@ -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.
*
Expand Down Expand Up @@ -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);
Expand All @@ -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,
Expand All @@ -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
Expand All @@ -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
Expand Down
27 changes: 27 additions & 0 deletions modules/core/src/test/java/org/locationtech/jts/operation/valid/IsValidTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
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() {
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() {
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))");
}

@msbarry msbarry Oct 29, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These test add some extra coverage around the indexed version:

Image


//=============================================

Expand All @@ -188,16 +202,29 @@ 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 TopologyValidationError validateIndexedShell(String wkt) {
IsValidOp validOp = new IsValidOp(read(wkt));
validOp.setMinHolesIndexedShell(0);

@msbarry msbarry Oct 29, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added this to keep the tests verifying index behavior smaller, if you want I could set the min holes to 0 on all tests in this file so they all exercise the new code path, or make the indexed ones create 20 extra holes to force it without the setMinHoles method?

@msbarry msbarry Nov 7, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also changed this to validate all test cases with and without an index.

return validOp.getValidationError();
}

private void checkInvalid(int exepctedErrType, String wkt) {
Geometry geom = read(wkt);
IsValidOp validOp = new IsValidOp(geom);
TopologyValidationError err = validOp.getValidationError();
assertEquals( exepctedErrType, err.getErrorType() );
assertEquals( exepctedErrType, validateIndexedShell(wkt).getErrorType() );
}
}
78 changes: 78 additions & 0 deletions modules/core/src/test/java/test/jts/perf/operation/valid/IsValidHolesInShellPerfTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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 {

@msbarry msbarry Oct 29, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On my m1 macbook:

BeforeAfter
Running with # holes 1
runValidateSmallShell : 16 ms
runValidateLargeShell : 21 ms

Running with # holes 4
runValidateSmallShell : 3 ms  ( 0.2x - size 4.0x)
runValidateLargeShell : 9 ms  ( 0.4x - size 4.0x)

Running with # holes 16
runValidateSmallShell : 3 ms  ( 1.0x - size 4.0x)
runValidateLargeShell : 26 ms  ( 2.9x - size 4.0x)

Running with # holes 64
runValidateSmallShell : 6 ms  ( 2.0x - size 4.0x)
runValidateLargeShell : 93 ms  ( 3.6x - size 4.0x)

Running with # holes 256
runValidateSmallShell : 16 ms  ( 2.7x - size 4.0x)
runValidateLargeShell : 356 ms  ( 3.8x - size 4.0x)

Running with # holes 1024
runValidateSmallShell : 50 ms  ( 3.1x - size 4.0x)
runValidateLargeShell : 1389 ms  ( 3.9x - size 4.0x)

Running with # holes 4096
runValidateSmallShell : 194 ms  ( 3.9x - size 4.0x)
runValidateLargeShell : 5587 ms  ( 4.0x - size 4.0x)

Running with # holes 1
runValidateSmallShell : 10 ms
runValidateLargeShell : 18 ms

Running with # holes 4
runValidateSmallShell : 3 ms  ( 0.3x - size 4.0x)
runValidateLargeShell : 10 ms  ( 0.6x - size 4.0x)

Running with # holes 16
runValidateSmallShell : 3 ms  ( 1.0x - size 4.0x)
runValidateLargeShell : 26 ms  ( 2.6x - size 4.0x)

Running with # holes 64
runValidateSmallShell : 11 ms  ( 3.7x - size 4.0x)
runValidateLargeShell : 91 ms  ( 3.5x - size 4.0x)

Running with # holes 256
runValidateSmallShell : 15 ms  ( 1.4x - size 4.0x)
runValidateLargeShell : 41 ms  ( 0.5x - size 4.0x)

Running with # holes 1024
runValidateSmallShell : 41 ms  ( 2.7x - size 4.0x)
runValidateLargeShell : 58 ms  ( 1.4x - size 4.0x)

Running with # holes 4096
runValidateSmallShell : 127 ms  ( 3.1x - size 4.0x)
runValidateLargeShell : 155 ms  ( 2.7x - size 4.0x)


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();
}
}
Loading