Skip to content

Commit

Permalink
Fix & Improve IsValidOp
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

JTS' commit locationtech/jts@fe1fe57
  • Loading branch information
@FObermaier
FObermaier committed Aug 3, 2021
1 parent eb9f554 commit fc11f21
Show file tree
Hide file tree
Showing 9 changed files with 580 additions and 538 deletions.
412 changes: 197 additions & 215 deletions data/NetTopologySuite.TestRunner.Tests/general/TestValid.xml
View file

Large diffs are not rendered by default.

61 changes: 31 additions & 30 deletions src/NetTopologySuite/Operation/Valid/IndexedNestedHoleTester.cs
View file
Expand Up @@ -10,10 +10,14 @@ namespace NetTopologySuite.Operation.Valid
/// nested inside another hole, using a spatial
/// index to speed up the comparisons.
/// <para/>
/// 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).
/// <para/>
/// 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.
/// </summary>
class IndexedNestedHoleTester
{
Expand All @@ -39,54 +43,51 @@ private void LoadIndex()
}
}

/// <summary>
/// Gets a value indicating a point on a nested hole, if one exists.
/// </summary>
/// <returns>A point on a nested hole, or <c>null</c> if none are nested</returns>
public Coordinate NestedPoint { get => _nestedPt; }

/// <summary>
/// Tests if any hole is nested (contained) within another hole.
/// <b>This is invalid</b>.
/// The <see cref="NestedPoint"/> will be set to reflect this.
/// </summary>
/// <returns><c>true</c> if some hole is nested.</returns>
public bool IsNested()
{
for (int i = 0; i < _polygon.NumInteriorRings; i++)
{
var hole = (LinearRing)_polygon.GetInteriorRingN(i);

var results = _index.Query(hole.EnvelopeInternal);
for (int j = 0; j < results.Count; j++)
foreach (var testHole in results)
{
var testHole = results[j];
if (hole == testHole)
continue;

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

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

private bool IsHoleInsideHole(LinearRing hole, LinearRing testHole)
{
var testPts = testHole.CoordinateSequence;
for (int i = 0; i < hole.NumPoints; i++)
{
var holePt = hole.GetCoordinateN(i);
var loc = PointLocation.LocateInRing(holePt, testPts);
switch (loc)
{
case Location.Exterior: return false;
case Location.Interior:
_nestedPt = holePt;
/*
* Checks nesting via a point-in-polygon test,
* or if the point lies on the boundary via
* the topology of the incident edges.
*/
var holePt0 = hole.GetCoordinateN(0);
var holePt1 = hole.GetCoordinateN(1);
if (PolygonTopologyAnalyzer.IsSegmentInRing(holePt0, holePt1, testHole))
{
_nestedPt = holePt0;
return true;
}
}
// location is BOUNDARY, so keep trying points
}
return false;
}


}
}
80 changes: 30 additions & 50 deletions src/NetTopologySuite/Operation/Valid/IsValidOp.cs
View file
Expand Up @@ -300,7 +300,7 @@ private bool IsValidGeometry(Polygon g)
CheckHolesOutsideShell(g);
if (HasInvalidError) return false;

CheckHolesNotNested(g);
CheckHolesNested(g);
if (HasInvalidError) return false;

CheckInteriorDisconnected(areaAnalyzer);
Expand Down Expand Up @@ -343,11 +343,11 @@ private bool IsValidGeometry(MultiPolygon g)
for (int i = 0; i < g.NumGeometries; i++)
{
var 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 @@ -486,26 +486,12 @@ private bool IsNonRepeatedSizeAtLeast(LineString line, int minSize)

private void CheckAreaIntersections(PolygonTopologyAnalyzer areaAnalyzer)
{
if (areaAnalyzer.HasIntersection)
if (areaAnalyzer.HasInvalidIntersection)
{
LogInvalid(TopologyValidationErrors.SelfIntersection,
LogInvalid(areaAnalyzer.InvalidCode,
areaAnalyzer.IntersectionLocation);
return;
}

if (areaAnalyzer.HasDoubleTouch)
{
LogInvalid(TopologyValidationErrors.DisconnectedInteriors,
areaAnalyzer.IntersectionLocation);
return;
}

if (areaAnalyzer.IsInteriorDisconnectedBySelfTouch())
{
LogInvalid(TopologyValidationErrors.DisconnectedInteriors,
areaAnalyzer.DisconnectionLocation);
}

}

/// <summary>
Expand Down Expand Up @@ -538,7 +524,6 @@ private void CheckHolesOutsideShell(Polygon poly)

var shell = poly.ExteriorRing;
bool isShellEmpty = shell.IsEmpty;
var pir = new IndexedPointInAreaLocator(shell);

for (int i = 0; i < poly.NumInteriorRings; i++)
{
Expand All @@ -552,7 +537,7 @@ private void CheckHolesOutsideShell(Polygon poly)
}
else
{
invalidPt = FindHoleOutsideShellPoint(pir, hole);
invalidPt = FindHoleOutsideShellPoint(hole, shell);
}

if (invalidPt != null)
Expand All @@ -566,41 +551,36 @@ private void CheckHolesOutsideShell(Polygon poly)

/// <summary>
/// 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.
/// </summary>
/// <param name="shellLocator"></param>
/// <param name="hole"></param>
/// <returns>A hole point outside the shell, or <c>null</c> if valid.</returns>
private Coordinate FindHoleOutsideShellPoint(IPointOnGeometryLocator shellLocator, LineString hole)
/// <param name="hole">The hole to test</param>
/// <param name="shell">The polygon shell to test against</param>
/// <returns>A hole point outside the shell, or <c>null</c> if it is inside.</returns>
private Coordinate FindHoleOutsideShellPoint(LineString hole, LineString shell)
{
for (int i = 0; i < hole.NumPoints - 1; i++)
{
var holePt = hole.GetCoordinateN(i);
var 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;
}
var holePt0 = hole.GetCoordinateN(0);
var holePt1 = hole.GetCoordinateN(1);
/*
* If hole envelope is not covered by shell, it must be outside
*/
if (!shell.EnvelopeInternal.Covers(hole.EnvelopeInternal))
return holePt0;

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

/// <summary>
/// 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.
/// </summary>
/// <param name="poly">The polygon with holes to test</param>
private void CheckHolesNotNested(Polygon poly)
private void CheckHolesNested(Polygon poly)
{
// skip test if no holes are present
if (poly.NumInteriorRings <= 0) return;
Expand All @@ -614,15 +594,15 @@ private void CheckHolesNotNested(Polygon poly)
}

/// <summary>
/// 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.
/// <para/>Preconditions:
/// <list type="bullet">
/// <item><description>shells do not partially overlap</description></item>
/// <item><description>shells do not touch along an edge</description></item>
/// <item><description>no duplicate rings exist</description></item></list>
/// These have been confirmed by the <see cref="PolygonTopologyAnalyzer"/>.
/// </summary>
private void CheckShellsNotNested(MultiPolygon mp)
private void CheckShellsNested(MultiPolygon mp)
{
for (int i = 0; i < mp.NumGeometries; i++)
{
Expand Down Expand Up @@ -689,11 +669,11 @@ private Coordinate FindShellSegmentInPolygon(LinearRing shell, Polygon poly)
return shell0;
}

private void CheckInteriorDisconnected(PolygonTopologyAnalyzer areaAnalyzer)
private void CheckInteriorDisconnected(PolygonTopologyAnalyzer analyzer)
{
if (areaAnalyzer.IsInteriorDisconnectedByRingCycle())
if (analyzer.IsInteriorDisconnected())
LogInvalid(TopologyValidationErrors.DisconnectedInteriors,
areaAnalyzer.DisconnectionLocation);
analyzer.DisconnectionLocation);
}

/// <summary>
Expand Down

0 comments on commit fc11f21

Please sign in to comment.