Skip to content

Commit

Permalink
GeometryNormalizer should not fail if it cannot compute signed area (#…
Browse files Browse the repository at this point in the history
…84051) (#84082)

This commit removes the exception throwing and assumes that when the area is zero, the polygon has 
the right orientation. The exception will be thrown at indexing time when the polygon is invalid .
  • Loading branch information
iverase authored Feb 17, 2022
1 parent 9b14a98 commit a4acbe1
Show file tree
Hide file tree
Showing 8 changed files with 47 additions and 17 deletions.
6 changes: 6 additions & 0 deletions docs/changelog/84051.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
pr: 84051
summary: '`GeometryNormalizer` should not fail if it cannot compute signed area'
area: Geo
type: bug
issues:
- 83946
Original file line number Diff line number Diff line change
Expand Up @@ -721,11 +721,10 @@ private static Edge[] ring(
minX = Math.min(minX, points[i].x);
maxX = Math.max(maxX, points[i].x);
}
if (signedArea == 0) {
// Points are collinear or self-intersection
throw new InvalidShapeException("Cannot determine orientation: signed area equal to 0");
}
boolean orientation = signedArea < 0;

// if the polygon is tiny, the computed area can result in zero. In that case
// we assume orientation is correct
boolean orientation = signedArea == 0 ? handedness != false : signedArea < 0;

// OGC requires shell as ccw (Right-Handedness) and holes as cw (Left-Handedness)
// since GeoJSON doesn't specify (and doesn't need to) GEO core will assume OGC standards
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ public void testPolygonSelfIntersection() {
new CoordinatesBuilder().coordinate(-40.0, 50.0).coordinate(40.0, 50.0).coordinate(-40.0, -50.0).coordinate(40.0, -50.0).close()
);
Exception e = expectThrows(InvalidShapeException.class, () -> newPolygon.buildS4J());
assertThat(e.getMessage(), containsString("Cannot determine orientation: signed area equal to 0"));
assertThat(e.getMessage(), containsString("Self-intersection at or near point (0.0, 0.0, NaN)"));
}

/** note: only supported by S4J at the moment */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ public void testPolygonWithUndefinedOrientationDueToCollinearPoints() {
new CoordinatesBuilder().coordinate(0.0, 0.0).coordinate(1.0, 1.0).coordinate(-1.0, -1.0).close()
);
InvalidShapeException e = expectThrows(InvalidShapeException.class, pb::buildS4J);
assertEquals("Cannot determine orientation: signed area equal to 0", e.getMessage());
assertEquals("Self-intersection at or near point (-1.0, -1.0, NaN)", e.getMessage());
}

public void testCrossingDateline() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -213,13 +213,9 @@ private static Edge[] ring(
minX = Math.min(minX, points[i].getX());
maxX = Math.max(maxX, points[i].getX());
}
if (signedArea == 0) {
// Points are collinear or self-intersection
throw new IllegalArgumentException(
"Cannot determine orientation: signed area equal to 0." + " Points are collinear or polygon self-intersects."
);
}
boolean orientation = signedArea < 0;
// if the polygon is tiny, the computed area can result in zero. In that case
// we assume orientation is correct
boolean orientation = signedArea == 0 ? handedness != false : signedArea < 0;

// OGC requires shell as ccw (Right-Handedness) and holes as cw (Left-Handedness)
// since GeoJSON doesn't specify (and doesn't need to) GEO core will assume OGC standards
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,11 @@ public void testPolygonOrientation() throws IOException, ParseException {
expected("POLYGON ((180 29, 180 38, 180 56, 180 53, 178 47, 177 23, 180 29))"),
actual("POLYGON ((180 38, 180.0 56, 180.0 53, 178 47, 177 23, 180 29, 180 36, 180 37, 180 38))", randomBoolean())
);

assertEquals(
expected("POLYGON ((-135 85, 135 85, 45 85, -45 85, -135 85))"),
actual("POLYGON ((-45 85, -135 85, 135 85, 45 85, -45 85))", randomBoolean())
);
}

public void testInvalidSelfCrossingPolygon() {
Expand All @@ -218,6 +223,15 @@ public void testCrossingDateline() {
assertTrue(geometry instanceof MultiPolygon);
}

public void testPolygonAllCollinearPoints() {
Polygon polygon = new Polygon(new LinearRing(new double[] { 0, 1, -1, 0 }, new double[] { 0, 1, -1, 0 }));
IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> indexer.indexShape(polygon));
assertEquals(
"Unable to Tessellate shape [[1.0, 1.0] [-1.0, -1.0] [0.0, 0.0] [1.0, 1.0] ]. Possible malformed shape detected.",
e.getMessage()
);
}

private XContentBuilder polygon(Boolean orientation, double... val) throws IOException {
XContentBuilder pointGeoJson = XContentFactory.jsonBuilder().startObject();
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -375,6 +375,21 @@ public void testPolygon() {
polygon = new Polygon(new LinearRing(new double[] { 170, 190, 190, 170, 170 }, new double[] { -10, -10, 10, 10, -10 }));
assertEquals(indexed, GeometryNormalizer.apply(Orientation.CCW, polygon));
assertEquals(true, GeometryNormalizer.needsNormalize(Orientation.CCW, polygon));

polygon = new Polygon(
new LinearRing(
new double[] { -107.88180702965093, -107.88179936541891, -107.88180701456989, -107.88180702965093 },
new double[] { 37.289285907909985, 37.289278246132682, 37.289285918063491, 37.289285907909985 }
)
);
indexed = new Polygon(
new LinearRing(
new double[] { -107.88179936541891, -107.88180701456989, -107.88180702965093, -107.88179936541891 },
new double[] { 37.289278246132682, 37.289285918063491, 37.289285907909985, 37.289278246132682 }
)
);
assertEquals(indexed, GeometryNormalizer.apply(Orientation.CCW, polygon));

}

public void testMultiPolygon() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -245,16 +245,16 @@ public void testIgnoreMalformedValues() throws IOException {
.startObject()
.field(
"field",
"POLYGON ((18.9401790919516 -33.9681188869036, 18.9401790919516 -33.9681188869037, 18.9401790919517 "
+ "-33.9681188869037, 18.9401790919517 -33.9681188869036, 18.9401790919516 -33.9681188869036))"
"POLYGON ((18.9401790919516 -33.9681188869036, 18.9401790919516 -33.9681188869036, 18.9401790919517 "
+ "-33.9681188869036, 18.9401790919517 -33.9681188869036, 18.9401790919516 -33.9681188869036))"
)
.endObject()
);
SourceToParse sourceToParse = new SourceToParse("1", arrayedDoc, XContentType.JSON);
ParsedDocument document = ignoreMapper.parse(sourceToParse);
assertThat(document.docs().get(0).getFields("field").length, equalTo(0));
MapperParsingException exception = expectThrows(MapperParsingException.class, () -> failMapper.parse(sourceToParse));
assertThat(exception.getCause().getMessage(), containsString("Cannot determine orientation"));
assertThat(exception.getCause().getMessage(), containsString("at least 4 polygon points required"));
}
}

Expand Down

0 comments on commit a4acbe1

Please sign in to comment.