GeometryPrecisionReducer inverts self-intersecting inner ring

[msbarry]

While investigating onthegomap/planetiler#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

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]

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.

[msbarry]

Thanks for getting back to me. Maybe you could give some advice on the fastest correct way to accomplish the goal:

We are starting from coordinate sequences that define the outer shell and inner rings of polygons that may be invalid to data issues or rounding at previous stages of the pipeline. All of the coordinates fall between (0,0) and (4096,4096) and we want to end up with valid polygons that are snapped to integer coordinates in that grid.

Currently pseudocode for each polygon is:

Polygon input;
if (!input.isValid()) {
  input = fix(input);
}
return GeometryPrecisionReducer.reduce(input, precisionModel);

Is that the best way to accomplish this? Would it be more efficient to set a precision model on the geometry earlier on? Or are there a different sequence of steps you would recommend to accomplish this?

Thanks!