Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use the determinant formula for calculating the orientation of a polygon #27967

Merged
merged 9 commits into from
Jul 31, 2018
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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there's an extra 'if so' here.

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());
}
}