Skip to content

Commit

Permalink
Fix calculation of orientation of polygons (#27967)
Browse files Browse the repository at this point in the history
The method for working out whether a polygon is clockwise or anticlockwise is
mostly correct but doesn't work in some rare cases such as the included test
case. This commit fixes that.
  • Loading branch information
DaveCTurner authored Jul 31, 2018
1 parent d4ea440 commit 8b57e2e
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@
import java.util.Objects;
import java.util.concurrent.atomic.AtomicBoolean;

import static org.apache.lucene.geo.GeoUtils.orient;

/**
* The {@link PolygonBuilder} implements the groundwork to create polygons. This contains
* Methods to wrap polygons at the dateline and building shapes from the data held by the
Expand Down Expand Up @@ -642,14 +644,8 @@ private static int createEdges(int component, Orientation orientation, LineStrin
*/
private static Edge[] ring(int component, boolean direction, boolean handedness,
Coordinate[] points, int offset, Edge[] edges, int toffset, int length, final AtomicBoolean translated) {
// calculate the direction of the points:
// find the point a the top of the set and check its
// neighbors orientation. So direction is equivalent
// to clockwise/counterclockwise
final int top = top(points, offset, length);
final int prev = (offset + ((top + length - 1) % length));
final int next = (offset + ((top + 1) % length));
boolean orientation = points[offset + prev].x > points[offset + next].x;

boolean orientation = getOrientation(points, offset, length);

// 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 Expand Up @@ -678,6 +674,35 @@ private static Edge[] ring(int component, boolean direction, boolean handedness,
return concat(component, direction ^ orientation, points, offset, edges, toffset, length);
}

/**
* @return whether the points are clockwise (true) or anticlockwise (false)
*/
private static boolean getOrientation(Coordinate[] points, int offset, int length) {
// calculate the direction of the points: find the southernmost point
// and check its neighbors orientation.

final int top = top(points, offset, length);
final int prev = (top + length - 1) % length;
final int next = (top + 1) % length;

final int determinantSign = orient(
points[offset + prev].x, points[offset + prev].y,
points[offset + top].x, points[offset + top].y,
points[offset + next].x, points[offset + next].y);

if (determinantSign == 0) {
// Points are collinear, but `top` is not in the middle if so, so the edges either side of `top` are intersecting.
throw new InvalidShapeException("Cannot determine orientation: edges adjacent to ("
+ points[offset + top].x + "," + points[offset +top].y + ") coincide");
}

return determinantSign < 0;
}

/**
* @return the (offset) index of the point that is furthest west amongst
* those points that are the furthest south in the set.
*/
private static int top(Coordinate[] points, int offset, int length) {
int top = 0; // we start at 1 here since top points to 0
for (int i = 1; i < length; i++) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,4 +143,22 @@ public void testHoleThatIsNorthOfPolygon() {

assertEquals("Hole lies outside shell at or near point (4.0, 3.0, NaN)", e.getMessage());
}

public void testWidePolygonWithConfusingOrientation() {
// A valid polygon that is oriented correctly (anticlockwise) but which
// confounds a naive algorithm for determining its orientation leading
// ES to believe that it crosses the dateline and "fixing" it in a way
// that self-intersects.

PolygonBuilder pb = new PolygonBuilder(new CoordinatesBuilder()
.coordinate(10, -20).coordinate(100, 0).coordinate(-100, 0).coordinate(20, -45).coordinate(40, -60).close());
pb.build(); // Should not throw an exception
}

public void testPolygonWithUndefinedOrientationDueToCollinearPoints() {
PolygonBuilder pb = new PolygonBuilder(new CoordinatesBuilder()
.coordinate(0.0, 0.0).coordinate(1.0, 1.0).coordinate(-1.0, -1.0).close());
InvalidShapeException e = expectThrows(InvalidShapeException.class, pb::build);
assertEquals("Cannot determine orientation: edges adjacent to (-1.0,-1.0) coincide", e.getMessage());
}
}

0 comments on commit 8b57e2e

Please sign in to comment.