LUCENE-8141: Do a better job of making sure polygon points are not coplanar. Committed on behalf of Ignacio Vera.

This commit is contained in:
Karl Wright 2018-01-31 08:20:58 -05:00
parent 0b0e8e5e7a
commit ebb346d582
2 changed files with 98 additions and 147 deletions

View File

@ -18,6 +18,7 @@ package org.apache.lucene.spatial3d.geom;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.BitSet; import java.util.BitSet;
import java.util.Collections;
import java.util.List; import java.util.List;
import java.util.Random; import java.util.Random;
import java.util.Iterator; import java.util.Iterator;
@ -443,18 +444,13 @@ public class GeoPolygonFactory {
*/ */
static List<GeoPoint> filterEdges(final List<GeoPoint> noIdenticalPoints, final double leniencyValue) { static List<GeoPoint> filterEdges(final List<GeoPoint> noIdenticalPoints, final double leniencyValue) {
// Now, do the depth-first search needed to find a path that has no coplanarities in it. // Now, do the search needed to find a path that has no coplanarities in it.
// This is, unfortunately, not easy, because coplanarity is not transitive as you walk around the polygon. // It is important to check coplanarities using the points that are further away so the
// If point C is not coplanar with edge A-B, there is no guarantee that A is not coplanar with B-C. // plane is more precise.
// But we have to produce a polygon that is safe no matter which way it is looked at.
// The approach I'm taking therefore is to do a depth-first search until we find a valid polygon.
// This algorithmically awful in the worst case, but luckily we can presume that real-life data
// does not require more than a couple of iterations.
for (int i = 0; i < noIdenticalPoints.size(); i++) { for (int i = 0; i < noIdenticalPoints.size(); i++) {
final SafePath startPath = new SafePath(null, noIdenticalPoints.get(i), i, null); //Search starting for current index.
// Search, with this as the start path. final SafePath resultPath = findSafePath(noIdenticalPoints, i, leniencyValue);
final SafePath resultPath = findSafePath(startPath, noIdenticalPoints, getLegalIndex(i+1, noIdenticalPoints.size()), i, leniencyValue);
if (resultPath != null && resultPath.previous != null) { if (resultPath != null && resultPath.previous != null) {
// Read out result, maintaining ordering // Read out result, maintaining ordering
final List<GeoPoint> rval = new ArrayList<>(noIdenticalPoints.size()); final List<GeoPoint> rval = new ArrayList<>(noIdenticalPoints.size());
@ -466,132 +462,65 @@ public class GeoPolygonFactory {
return null; return null;
} }
/** Recursive depth-first path search. In order to find a valid path, we must consider all possible legal extensions of /** Iterative path search through ordered list of points. The method merges together
* the current path. We discard any path that produces illegalities (meaning anything that would allow any coplanarity * all consecutive coplanar points and builds the plane using the first and the last point.
* to continue to exist no matter from which direction one looks at it), and take the first legal path we find. * It does not converge if the starting point is coplanar with the last and next point of the path.
* @param currentPath is the current path (not null). *
* @param points is the raw list of points under consideration. * @param points is the ordered raw list of points under consideration.
* @param pointIndex is the index of the point that represents the next possible point for consideration for path * @param startIndex is index of the point that starts the current path, so that we can know when we are done.
* extension. * @param leniencyValue is the allowed distance of a point from the plane to be considered coplanar.
* @param startPointIndex is index of the point that starts the current path, so that we can know when we are done. * @return null if the starting point is coplanar with the last and next point of the path.
* @param leniencyValue is the maximum allowed distance of a point being skipped from the revised polygon. Pass zero if
* no leniency desired.
* @return null if there was no safe path found, or the safe path if one was discovered.
*/ */
private static SafePath findSafePath(final SafePath currentPath, final List<GeoPoint> points, final int pointIndex, private static SafePath findSafePath(final List<GeoPoint> points, final int startIndex, final double leniencyValue) {
final int startPointIndex, final double leniencyValue) { SafePath safePath = null;
//System.err.println("extending path..."); for (int i = startIndex; i < startIndex + points.size(); i++) {
//get start point, always the same for an iteration
final int startPointIndex = getLegalIndex(i -1, points.size());
final GeoPoint startPoint = points.get(startPointIndex);
//get end point, can be coplanar and therefore change
int endPointIndex = getLegalIndex(i, points.size());
GeoPoint endPoint = points.get(endPointIndex);
// Loop across all possible path extensions, and consider each in turn if (startPoint.isNumericallyIdentical(endPoint)) {
int considerPointIndex = pointIndex; //go to next if identical
continue;
}
//Check if nextPoints are co-planar, if so advance to next point.
//if we go over the start index then we have no succeed.
while (true) { while (true) {
// Check if the extension of currentPath to considerPointIndex is workable int nextPointIndex = getLegalIndex(endPointIndex + 1, points.size());
final GeoPoint considerStartPoint = currentPath.lastPoint; final GeoPoint nextPoint = points.get(nextPointIndex);
final GeoPoint considerEndPoint = points.get(considerPointIndex); if (startPoint.isNumericallyIdentical(nextPoint)) {
final int nextPointIndex = getLegalIndex(considerPointIndex + 1, points.size()); //all coplanar
if (!considerStartPoint.isNumericallyIdentical(considerEndPoint)) {
// Create a plane including these two
final Plane considerPlane = new Plane(considerStartPoint, considerEndPoint);
boolean isChoiceLegal = true;
//System.err.println(" considering "+considerStartPoint+" to "+considerEndPoint);
if (isChoiceLegal) {
// Consider the previous plane/point
if (currentPath.lastPlane != null) {
if (currentPath.lastPlane.evaluateIsZero(considerEndPoint)) {
//System.err.println(" coplanar with last plane");
// no good
isChoiceLegal = false;
} else if (considerPlane.evaluateIsZero(currentPath.previous.lastPoint)) {
//System.err.println(" last point coplanar with this plane");
isChoiceLegal = false;
} else {
// To guarantee that no planes we build are coplanar with edge points, we need to verify everything back from
// considerEndPoint back to the start of the path. We build the edge from considerEndPoint back to each
// of the SafePath points already determined. Then, we need to look at all triangles that include that edge and
// the SafePath points in between. If all of those triangles are legal, we can be assured that adding the current
// proposed point is safe to do.
// This is, of course, a lot of work -- specifically, it's O(n^2) for each point in the path, which leads to an O(n^3)
// evaluation time overall!!
// The only alternative is to understand the cases under which these triangles would be introduced, and tailor the
// cleaning to catch those cases only. Still need to figure that out. The case that blows up is when *all* the points
// for a triangle are coplanar, so theoretically we don't even need to generate the triangle at all(!)
//
// Build a plane that represents the third edge in this triangle, to guarantee that we can compose
// the polygon from triangles
final Plane thirdPlane = new Plane(currentPath.previous.lastPoint, considerEndPoint);
if (thirdPlane.evaluateIsZero(considerStartPoint)) {
isChoiceLegal = false;
}
}
}
}
if (isChoiceLegal && considerPointIndex == startPointIndex) {
// Verify that the first plane (already recorded) works together with the last plane
final SafePath firstPlaneEndpoint = currentPath.findFirstEndpoint();
if (firstPlaneEndpoint == null) {
//System.err.println(" path not long enough");
isChoiceLegal = false;
} else {
if (firstPlaneEndpoint.lastPlane.evaluateIsZero(considerStartPoint)) {
//System.err.println(" last point is coplanar with start plane");
isChoiceLegal = false;
} else if (considerPlane.evaluateIsZero(firstPlaneEndpoint.lastPoint)) {
//System.err.println(" first point is coplanar with last plane");
isChoiceLegal = false;
} else {
// Build a plane that represents the third edge in this triangle, to guarantee that we can compose
// the polygon from triangles
final Plane thirdPlane = new Plane(considerStartPoint, firstPlaneEndpoint.lastPoint);
if (thirdPlane.evaluateIsZero(considerEndPoint)) {
isChoiceLegal = false;
}
}
}
}
if (isChoiceLegal) {
// All points between the start and end, if any, must be on the plane.
int checkIndex = getLegalIndex(currentPath.lastPointIndex + 1, points.size());
while (checkIndex != considerPointIndex) {
if (Math.abs(considerPlane.evaluate(points.get(checkIndex))) >= Vector.MINIMUM_RESOLUTION + leniencyValue) {
// This possibility is no good. But does it say anything about other possibilities? I think
// it may mean we don't have to consider any further extensions. I can't prove this, but
// it makes this algorithm complete in not an insane period of time at least...
//System.err.println(" interior point not coplanar with trial plane");
//isChoiceLegal = false;
//break;
return null; return null;
} }
checkIndex = getLegalIndex(checkIndex + 1, points.size()); Plane nextPlane = new Plane(startPoint, nextPoint);
} if (Math.abs(nextPlane.evaluate(endPoint)) > Vector.MINIMUM_RESOLUTION + leniencyValue) {
} //no coplanar.
if (isChoiceLegal) {
// Extend the path and call ourselves recursively.
if (considerPointIndex == startPointIndex) {
// Current path has been validated; return it
return currentPath;
}
//System.err.println(" adding to path: "+considerEndPoint+"; "+considerPlane);
final SafePath newPath = new SafePath(currentPath, considerEndPoint, considerPointIndex, considerPlane);
final SafePath result = findSafePath(newPath, points, nextPointIndex, startPointIndex, leniencyValue);
if (result != null) {
return result;
}
}
}
if (considerPointIndex == startPointIndex) {
break; break;
} }
considerPointIndex = nextPointIndex; if (endPointIndex == startIndex) {
} //we are over the path, we fail.
return null; return null;
} }
//advance
endPointIndex = nextPointIndex;
endPoint = nextPoint;
i++;
}
if (safePath != null && endPointIndex == startIndex) {
//We are already at the start, current point is coplanar with
//start point, no need to add this node.
break;
}
//Create node and move to next one
Plane currentPlane = new Plane(startPoint, endPoint);
safePath = new SafePath(safePath, endPoint, endPointIndex, currentPlane);
}
return safePath;
}
/** Pick a random pole that has a good chance of being inside the polygon described by the points. /** Pick a random pole that has a good chance of being inside the polygon described by the points.
* @param generator is the random number generator to use. * @param generator is the random number generator to use.
@ -1723,24 +1652,18 @@ public class GeoPolygonFactory {
this.previous = previous; this.previous = previous;
} }
/** Find the first endpoint */
public SafePath findFirstEndpoint() {
if (previous == null) {
return null;
}
if (previous.previous == null) {
return this;
}
return previous.findFirstEndpoint();
}
/** Fill in a list, in order, of safe points. /** Fill in a list, in order, of safe points.
*/ */
public void fillInList(final List<GeoPoint> pointList) { public void fillInList(final List<GeoPoint> pointList) {
if (previous != null) { //we don't use recursion because it can be problematic
previous.fillInList(pointList); //for polygons with many points.
SafePath safePath = this;
while (safePath.previous != null) {
pointList.add(safePath.lastPoint);
safePath = safePath.previous;
} }
pointList.add(lastPoint); pointList.add(safePath.lastPoint);
Collections.reverse(pointList);
} }
} }

View File

@ -92,6 +92,21 @@ public class GeoPolygonTest {
} }
@Test
public void testPolygonPointFiltering2() {
//all coplanar
GeoPoint point1 = new GeoPoint(PlanetModel.SPHERE, 1.1264101919629863, -0.9108307879480759);
GeoPoint point2 = new GeoPoint(PlanetModel.SPHERE, 1.1264147298190414, -0.9108309624810013);
GeoPoint point3 = new GeoPoint(PlanetModel.SPHERE, 1.1264056541069312, -0.9108306134151508);
final List<GeoPoint> originalPoints = new ArrayList<>();
originalPoints.add(point1);
originalPoints.add(point2);
originalPoints.add(point3);
final List<GeoPoint> filteredPoints = GeoPolygonFactory.filterEdges(GeoPolygonFactory.filterPoints(originalPoints), 0.0);
assertEquals(null, filteredPoints);
}
@Test @Test
public void testPolygonClockwise() { public void testPolygonClockwise() {
GeoPolygon c; GeoPolygon c;
@ -1050,5 +1065,18 @@ shape:
} }
} }
@Test
public void testLUCENE8140() throws Exception {
//POINT(15.426026 68.35078) is coplanar
//"POLYGON((15.426411 68.35069,15.4261 68.35078,15.426026 68.35078,15.425868 68.35078,15.425745 68.350746,15.426411 68.35069))";
List<GeoPoint> points = new ArrayList<>();
points.add(new GeoPoint(PlanetModel.SPHERE, Geo3DUtil.fromDegrees(68.35069), Geo3DUtil.fromDegrees(15.426411)));
points.add(new GeoPoint(PlanetModel.SPHERE, Geo3DUtil.fromDegrees(68.35078), Geo3DUtil.fromDegrees(15.4261)));
points.add(new GeoPoint(PlanetModel.SPHERE, Geo3DUtil.fromDegrees(68.35078), Geo3DUtil.fromDegrees(15.426026)));
points.add(new GeoPoint(PlanetModel.SPHERE, Geo3DUtil.fromDegrees(68.35078), Geo3DUtil.fromDegrees(15.425868)));
points.add(new GeoPoint(PlanetModel.SPHERE, Geo3DUtil.fromDegrees(68.350746), Geo3DUtil.fromDegrees(15.426411)));
assertTrue(GeoPolygonFactory.makeGeoPolygon(PlanetModel.SPHERE, points) != null);
}
} }