Skip to content

Commit

Permalink
Fix & Improve IsValidOp (#748)
Browse files Browse the repository at this point in the history 
* Improve invalidity categorization
* Refactoring, logic improvement
* Fix hole cycle detection algorithm
* Improve unit tests

Signed-off-by: Martin Davis <mtnclimb@gmail.com>
  • Loading branch information
@dr-jts
dr-jts committed Jun 25, 2021
1 parent 2d12640 commit fe1fe57
Show file tree
Hide file tree
Showing 8 changed files with 332 additions and 246 deletions.
63 changes: 33 additions & 30 deletions modules/core/src/main/java/org/locationtech/jts/operation/valid/IndexedNestedHoleTester.java
View file
Expand Up @@ -13,11 +13,9 @@

import java.util.List;

import org.locationtech.jts.algorithm.PointLocation;
import org.locationtech.jts.geom.Coordinate;
import org.locationtech.jts.geom.Envelope;
import org.locationtech.jts.geom.LinearRing;
import org.locationtech.jts.geom.Location;
import org.locationtech.jts.geom.Polygon;
import org.locationtech.jts.index.SpatialIndex;
import org.locationtech.jts.index.strtree.STRtree;
Expand All @@ -27,10 +25,14 @@
* nested inside another hole, using a spatial
* index to speed up the comparisons.
* <p>
* Assumes that the holes and polygon shell do not cross
* (are properly nested).
* Does not check the case where every vertex of a hole touches another
* hole; this is invalid, and must be checked elsewhere.
* The logic assumes that the holes do not overlap and have no collinear segments
* (so they are properly nested, and there are no duplicate holes).
* <p>
* The situation where every vertex of a hole touches another hole
* is invalid because either the hole is nested,
* or else it disconnects the polygon interior.
* This class detects the nested situation.
* The disconnected interior situation must be checked elsewhere.
*
* @version 1.7
*/
Expand All @@ -57,48 +59,49 @@ private void loadIndex()
}
}

/**
* Gets a point on a nested hole, if one exists.
*
* @return a point on a nested hole, or null if none are nested
*/
public Coordinate getNestedPoint() { return nestedPt; }

/**
* Tests if any hole is nested (contained) within another hole.
* This is invalid.
* The nested point will be set to reflect this.
* @return true if some hole is nested
*/
public boolean isNested()
{
for (int i = 0; i < polygon.getNumInteriorRing(); i++) {
LinearRing hole = (LinearRing) polygon.getInteriorRingN(i);

List results = index.query(hole.getEnvelopeInternal());
for (int j = 0; j < results.size(); j++) {
LinearRing testHole = (LinearRing) results.get(j);
List<LinearRing> results = index.query(hole.getEnvelopeInternal());
for (LinearRing testHole : results) {
if (hole == testHole)
continue;

/**
* Hole is not covered by in test hole,
* so cannot be inside
* Hole is not fully covered by test hole, so cannot be nested
*/
if (! testHole.getEnvelopeInternal().covers( hole.getEnvelopeInternal()) )
continue;

if (isHoleInsideHole(hole, testHole))
return true;
}
}
return false;
}

private boolean isHoleInsideHole(LinearRing hole, LinearRing testHole) {
Coordinate[] testPts = testHole.getCoordinates();
for (int i = 0; i < hole.getNumPoints(); i++) {
Coordinate holePt = hole.getCoordinateN(i);
int loc = PointLocation.locateInRing(holePt, testPts);
switch (loc) {
case Location.EXTERIOR: return false;
case Location.INTERIOR:
nestedPt = holePt;
return true;
/**
* Checks nesting via a point-in-polygon test,
* or if the point lies on the boundary 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;
return true;
}
}
// location is BOUNDARY, so keep trying points
}
return false;
}


}
83 changes: 35 additions & 48 deletions modules/core/src/main/java/org/locationtech/jts/operation/valid/IsValidOp.java
View file
Expand Up @@ -246,7 +246,7 @@ private boolean isValid(Polygon g)
checkHolesOutsideShell(g);
if (hasInvalidError()) return false;

checkHolesNotNested(g);
checkHolesNested(g);
if (hasInvalidError()) return false;

checkInteriorDisconnected(areaAnalyzer);
Expand Down Expand Up @@ -286,10 +286,10 @@ private boolean isValid(MultiPolygon g)
}
for (int i = 0; i < g.getNumGeometries(); i++) {
Polygon p = (Polygon) g.getGeometryN(i);
checkHolesNotNested(p);
checkHolesNested(p);
if (hasInvalidError()) return false;
}
checkShellsNotNested(g);
checkShellsNested(g);
if (hasInvalidError()) return false;

checkInteriorDisconnected(areaAnalyzer);
Expand Down Expand Up @@ -403,22 +403,11 @@ private boolean isNonRepeatedSizeAtLeast(LineString line, int minSize) {
}

private void checkAreaIntersections(PolygonTopologyAnalyzer areaAnalyzer) {
if (areaAnalyzer.hasIntersection()) {
logInvalid(TopologyValidationError.SELF_INTERSECTION,
areaAnalyzer.getIntersectionLocation());
if (areaAnalyzer.hasInvalidIntersection()) {
logInvalid(areaAnalyzer.getInvalidCode(),
areaAnalyzer.getInvalidLocation());
return;
}
if (areaAnalyzer.hasDoubleTouch()) {
logInvalid(TopologyValidationError.DISCONNECTED_INTERIOR,
areaAnalyzer.getIntersectionLocation());
return;
}
if (areaAnalyzer.isInteriorDisconnectedBySelfTouch()) {
logInvalid(TopologyValidationError.DISCONNECTED_INTERIOR,
areaAnalyzer.getDisconnectionLocation());
return;
}

}

/**
Expand Down Expand Up @@ -452,7 +441,6 @@ private void checkHolesOutsideShell(Polygon poly)

LinearRing shell = poly.getExteriorRing();
boolean isShellEmpty = shell.isEmpty();
PointOnGeometryLocator pir = new IndexedPointInAreaLocator(shell);

for (int i = 0; i < poly.getNumInteriorRing(); i++) {
LinearRing hole = poly.getInteriorRingN(i);
Expand All @@ -463,7 +451,7 @@ private void checkHolesOutsideShell(Polygon poly)
invalidPt = hole.getCoordinate();
}
else {
invalidPt = findHoleOutsideShellPoint(pir, hole);
invalidPt = findHoleOutsideShellPoint(hole, shell);
}
if (invalidPt != null) {
logInvalid(TopologyValidationError.HOLE_OUTSIDE_SHELL,
Expand All @@ -475,40 +463,37 @@ private void checkHolesOutsideShell(Polygon poly)

/**
* Checks if a polygon hole lies inside its shell
* and if not returns the point indicating this.
* and if not returns a point indicating this.
* The hole is known to be wholly inside or outside the shell,
* so it suffices to find a single point which is interior or exterior.
* A valid hole may only have a single point touching the shell
* (since otherwise it creates a disconnected interior).
* So there should be at least one point which is interior or exterior,
* and this should be the first or second point tested.
* so it suffices to find a single point which is interior or exterior,
* or check the edge topology at a point on the boundary of the shell.
*
* @param shellLocator
* @param hole
* @return a hole point outside the shell, or null if valid
* @param hole the hole to test
* @param shell the polygon shell to test against
* @return a hole point outside the shell, or null if it is inside
*/
private Coordinate findHoleOutsideShellPoint(PointOnGeometryLocator shellLocator, LinearRing hole) {
for (int i = 0; i < hole.getNumPoints() - 1; i++) {
Coordinate holePt = hole.getCoordinateN(i);
int loc = shellLocator.locate(holePt);
if (loc== Location.BOUNDARY) continue;
if (loc== Location.INTERIOR) return null;
/**
* Location is EXTERIOR, so hole is outside shell
*/
return holePt;
}
return null;
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;

if (PolygonTopologyAnalyzer.isSegmentInRing(holePt0, holePt1, shell))
return null;
return holePt0;
}

/**
* Tests if any polygon hole is nested inside another.
* Checks if any polygon hole is nested inside another.
* Assumes that holes do not cross (overlap),
* This is checked earlier.
*
* @param poly the polygon with holes to test
*/
private void checkHolesNotNested(Polygon poly)
private void checkHolesNested(Polygon poly)
{
// skip test if no holes are present
if (poly.getNumInteriorRing() <= 0) return;
Expand All @@ -521,7 +506,7 @@ private void checkHolesNotNested(Polygon poly)
}

/**
* Tests that no element polygon is in the interior of another element polygon.
* Checks that no element polygon is in the interior of another element polygon.
* <p>
* Preconditions:
* <ul>
Expand All @@ -531,7 +516,7 @@ private void checkHolesNotNested(Polygon poly)
* </ul>
* These have been confirmed by the {@link PolygonTopologyAnalyzer}.
*/
private void checkShellsNotNested(MultiPolygon mp)
private void checkShellsNested(MultiPolygon mp)
{
for (int i = 0; i < mp.getNumGeometries(); i++) {
Polygon p = (Polygon) mp.getGeometryN(i);
Expand Down Expand Up @@ -594,9 +579,11 @@ private Coordinate findShellSegmentInPolygon(LinearRing shell, Polygon poly)
return shell0;
}

private void checkInteriorDisconnected(PolygonTopologyAnalyzer areaAnalyzer) {
if (areaAnalyzer.isInteriorDisconnectedByRingCycle())
private void checkInteriorDisconnected(PolygonTopologyAnalyzer analyzer) {
if (analyzer.isInteriorDisconnected()) {
logInvalid(TopologyValidationError.DISCONNECTED_INTERIOR,
areaAnalyzer.getDisconnectionLocation());
analyzer.getDisconnectionLocation());
}
}

}

0 comments on commit fe1fe57

Please sign in to comment.