From c0698952b20e7252728f28a4b788713c45ea3726 Mon Sep 17 00:00:00 2001 From: Karl Wright Date: Wed, 13 Apr 2016 02:51:25 -0400 Subject: [PATCH] LUCENE-7204: Add a test for (and make a fix for) legitimately coplanar polygon points. --- .../lucene/spatial3d/geom/GeoPolygonFactory.java | 10 ++++++---- .../apache/lucene/spatial3d/geom/GeoPolygonTest.java | 8 ++++++++ 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/lucene/spatial3d/src/java/org/apache/lucene/spatial3d/geom/GeoPolygonFactory.java b/lucene/spatial3d/src/java/org/apache/lucene/spatial3d/geom/GeoPolygonFactory.java index 9865ac0db5c..6bf876650c9 100755 --- a/lucene/spatial3d/src/java/org/apache/lucene/spatial3d/geom/GeoPolygonFactory.java +++ b/lucene/spatial3d/src/java/org/apache/lucene/spatial3d/geom/GeoPolygonFactory.java @@ -1094,10 +1094,10 @@ public class GeoPolygonFactory { final boolean isNewPointWithin; final GeoPoint pointToPresent; if (currentEdge.plane.evaluateIsZero(newPoint)) { - // The new point is colinear with the current edge. We'll have to look for the first point that isn't. + // The new point is colinear with the current edge. We'll have to look backwards for the first point that isn't. int checkPointIndex = -1; - // Compute the arc distance before we try to extend - double accumulatedDistance = 0.0; + // Compute the arc distance before we try to extend, so that we note backtracking when we see it + double accumulatedDistance = newPoint.arcDistance(pointList.get(startIndex)); final Plane checkPlane = new Plane(pointList.get(startIndex), newPoint); for (int i = 0; i < pointList.size(); i++) { final int index = getLegalIndex(startIndex - 1 - i, pointList.size()); @@ -1106,11 +1106,13 @@ public class GeoPolygonFactory { break; } else { accumulatedDistance += pointList.get(getLegalIndex(index+1, pointList.size())).arcDistance(pointList.get(index)); - final double actualDistance = pointList.get(getLegalIndex(startIndex-1, pointList.size())).arcDistance(pointList.get(index)); + final double actualDistance = newPoint.arcDistance(pointList.get(index)); if (Math.abs(actualDistance - accumulatedDistance) >= Vector.MINIMUM_RESOLUTION) { throw new IllegalArgumentException("polygon backtracks over itself"); } } + } + if (checkPointIndex == -1) { throw new IllegalArgumentException("polygon is illegal (linear)"); } pointToPresent = pointList.get(checkPointIndex); diff --git a/lucene/spatial3d/src/test/org/apache/lucene/spatial3d/geom/GeoPolygonTest.java b/lucene/spatial3d/src/test/org/apache/lucene/spatial3d/geom/GeoPolygonTest.java index 2da93cfbe16..e72129966ee 100755 --- a/lucene/spatial3d/src/test/org/apache/lucene/spatial3d/geom/GeoPolygonTest.java +++ b/lucene/spatial3d/src/test/org/apache/lucene/spatial3d/geom/GeoPolygonTest.java @@ -416,6 +416,14 @@ shape: } assertTrue(backtracks); + // Now make sure a legit poly with coplanar points works. + polyPoints.clear(); + polyPoints.add(new GeoPoint(pm, -0.5516194571595735, 0.0)); + polyPoints.add(new GeoPoint(pm, -1.5707963267948966, -2.2780601241431375)); + polyPoints.add(new GeoPoint(pm, 0.2669499069140678, -0.31249902828113546)); + polyPoints.add(new GeoPoint(pm, 1.538559019421765, 0.0)); + GeoPolygonFactory.makeGeoPolygon(pm, polyPoints, 3, null); + } }