locationtech / jts

The JTS Topology Suite is a Java library for creating and manipulating vector geometry.
Other
1.94k stars 442 forks source link

GeometryPrecisionReducer inverts self-intersecting inner ring #974

Closed msbarry closed 1 year ago

msbarry commented 1 year ago

While investigating https://github.com/onthegomap/planetiler/issues/546 I found that GeometryPrecisionReducer can sometimes invert a hole of a polygon that has a small self-intersection and treat it as a separate polygon. It looks like the root cause is that EdgeNodingBuilder uses Orientation.isCCW on the inner ring, which gives the opposite result for that inner ring. Here is a simple test-case to reproduce:

  public void testSelfIntersectingInnerRing()
          throws Exception
  {
    checkReduce(16, "POLYGON (( 0 0, 2 0, 2 1, 0 1, 0 0 ), ( 1.190536 0.902969, 1.064392 0.921844, 1.427876 0.825773, 0.945790 0.561750, 1.190536 0.902969 ))",
            "POLYGON ((0 0, 0 1, 2 1, 2 0, 0 0), (0.9375 0.5625, 1.1875 0.875, 1.4375 0.8125, 0.9375 0.5625))");
  }

Which fails because GeometryPrecisionReducer returns MULTIPOLYGON (((0 0, 0 1, 2 1, 2 0, 0 0)), ((0.9375 0.5625, 1.1875 0.875, 1.4375 0.8125, 0.9375 0.5625))) instead.

input actual output
image image

Would it make sense to switch EdgeNodingBuilder to use isCCWArea instead of isCCW which handles this case as we would expect? Or at least expose which orientation method it uses as an option to OverlayNG and PrecisionReducer ? From a user perspective, it would also be fine for precision reducer to throw an exception in this case so we could re-try after fixing the polygon.

I'd be happy to make the change if any of those sound like a feasible path forward?

dr-jts commented 1 year ago

Like many JTS operations, GeometryPrecisionReducer required valid input. You can check validity using Geometry.isValid, and if invalid make it valid using GeometryFixer.

Invalid geometry can present in a multitude of different ways, and GeometryFixer contains extensive logic to determine how to fix it. Centralizing that logic in one place avoids a lot of code duplication and complexity.