Skip to content

Commit 3ae8bd8

Browse files
committed
Fix calculation of orientation of polygons (#27967)
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.
1 parent 4638b7c commit 3ae8bd8

File tree

2 files changed

+51
-8
lines changed

2 files changed

+51
-8
lines changed

server/src/main/java/org/elasticsearch/common/geo/builders/PolygonBuilder.java

Lines changed: 33 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,8 @@
4646
import java.util.Objects;
4747
import java.util.concurrent.atomic.AtomicBoolean;
4848

49+
import static org.apache.lucene.geo.GeoUtils.orient;
50+
4951
/**
5052
* The {@link PolygonBuilder} implements the groundwork to create polygons. This contains
5153
* Methods to wrap polygons at the dateline and building shapes from the data held by the
@@ -642,14 +644,8 @@ private static int createEdges(int component, Orientation orientation, LineStrin
642644
*/
643645
private static Edge[] ring(int component, boolean direction, boolean handedness,
644646
Coordinate[] points, int offset, Edge[] edges, int toffset, int length, final AtomicBoolean translated) {
645-
// calculate the direction of the points:
646-
// find the point a the top of the set and check its
647-
// neighbors orientation. So direction is equivalent
648-
// to clockwise/counterclockwise
649-
final int top = top(points, offset, length);
650-
final int prev = (offset + ((top + length - 1) % length));
651-
final int next = (offset + ((top + 1) % length));
652-
boolean orientation = points[offset + prev].x > points[offset + next].x;
647+
648+
boolean orientation = getOrientation(points, offset, length);
653649

654650
// OGC requires shell as ccw (Right-Handedness) and holes as cw (Left-Handedness)
655651
// since GeoJSON doesn't specify (and doesn't need to) GEO core will assume OGC standards
@@ -678,6 +674,35 @@ private static Edge[] ring(int component, boolean direction, boolean handedness,
678674
return concat(component, direction ^ orientation, points, offset, edges, toffset, length);
679675
}
680676

677+
/**
678+
* @return whether the points are clockwise (true) or anticlockwise (false)
679+
*/
680+
private static boolean getOrientation(Coordinate[] points, int offset, int length) {
681+
// calculate the direction of the points: find the southernmost point
682+
// and check its neighbors orientation.
683+
684+
final int top = top(points, offset, length);
685+
final int prev = (top + length - 1) % length;
686+
final int next = (top + 1) % length;
687+
688+
final int determinantSign = orient(
689+
points[offset + prev].x, points[offset + prev].y,
690+
points[offset + top].x, points[offset + top].y,
691+
points[offset + next].x, points[offset + next].y);
692+
693+
if (determinantSign == 0) {
694+
// Points are collinear, but `top` is not in the middle if so, so the edges either side of `top` are intersecting.
695+
throw new InvalidShapeException("Cannot determine orientation: edges adjacent to ("
696+
+ points[offset + top].x + "," + points[offset +top].y + ") coincide");
697+
}
698+
699+
return determinantSign < 0;
700+
}
701+
702+
/**
703+
* @return the (offset) index of the point that is furthest west amongst
704+
* those points that are the furthest south in the set.
705+
*/
681706
private static int top(Coordinate[] points, int offset, int length) {
682707
int top = 0; // we start at 1 here since top points to 0
683708
for (int i = 1; i < length; i++) {

server/src/test/java/org/elasticsearch/common/geo/builders/PolygonBuilderTests.java

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,4 +143,22 @@ public void testHoleThatIsNorthOfPolygon() {
143143

144144
assertEquals("Hole lies outside shell at or near point (4.0, 3.0, NaN)", e.getMessage());
145145
}
146+
147+
public void testWidePolygonWithConfusingOrientation() {
148+
// A valid polygon that is oriented correctly (anticlockwise) but which
149+
// confounds a naive algorithm for determining its orientation leading
150+
// ES to believe that it crosses the dateline and "fixing" it in a way
151+
// that self-intersects.
152+
153+
PolygonBuilder pb = new PolygonBuilder(new CoordinatesBuilder()
154+
.coordinate(10, -20).coordinate(100, 0).coordinate(-100, 0).coordinate(20, -45).coordinate(40, -60).close());
155+
pb.build(); // Should not throw an exception
156+
}
157+
158+
public void testPolygonWithUndefinedOrientationDueToCollinearPoints() {
159+
PolygonBuilder pb = new PolygonBuilder(new CoordinatesBuilder()
160+
.coordinate(0.0, 0.0).coordinate(1.0, 1.0).coordinate(-1.0, -1.0).close());
161+
InvalidShapeException e = expectThrows(InvalidShapeException.class, pb::build);
162+
assertEquals("Cannot determine orientation: edges adjacent to (-1.0,-1.0) coincide", e.getMessage());
163+
}
146164
}

0 commit comments

Comments
 (0)