Apache
/
lucene
mirror of https://github.com/apache/lucene.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

LUCENE-7642: Multi-pronged approach to fixing this problem. Main fix is a better detection of parallelness in polygon adjoining edge planes. We deliberately make parallel determination less strict now than technically needed to avoid pathological cases. Other downstream changes devolve from that.

This commit is contained in:
Karl Wright 2018-03-31 10:03:49 -04:00
parent 0ef68f7d7c
commit 590e67158c
6 changed files with 179 additions and 49 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
lucene/spatial3d/src/java/org/apache/lucene/spatial3d/geom/GeoComplexPolygon.java
View File

@ -394,6 +394,12 @@ class GeoComplexPolygon extends GeoBasePolygon {
for (final GeoPoint point : notablePoints) {
xyzBounds.addPoint(point);
}
// If we have no bounds at all then the answer is "false"
if (xyzBounds.getMaximumX() == null || xyzBounds.getMinimumX() == null ||
xyzBounds.getMaximumY() == null || xyzBounds.getMinimumY() == null ||
xyzBounds.getMaximumZ() == null || xyzBounds.getMinimumZ() == null) {
return false;
}
// Figure out which tree likely works best
final double xDelta = xyzBounds.getMaximumX() - xyzBounds.getMinimumX();
final double yDelta = xyzBounds.getMaximumY() - xyzBounds.getMinimumY();

16
lucene/spatial3d/src/java/org/apache/lucene/spatial3d/geom/GeoConcavePolygon.java
View File

@ -235,15 +235,19 @@ class GeoConcavePolygon extends GeoBasePolygon {
final SidedPlane edge = edges[edgeIndex];
final SidedPlane invertedEdge = invertedEdges[edgeIndex];
int bound1Index = legalIndex(edgeIndex+1);
while (invertedEdges[legalIndex(bound1Index)].isNumericallyIdentical(invertedEdge)) {
bound1Index++;
while (invertedEdges[bound1Index].isNumericallyIdentical(invertedEdge)) {
if (bound1Index == edgeIndex) {
throw new IllegalArgumentException("Constructed planes are all coplanar: "+points);
}
bound1Index = legalIndex(bound1Index + 1);
}
int bound2Index = legalIndex(edgeIndex-1);
while (invertedEdges[legalIndex(bound2Index)].isNumericallyIdentical(invertedEdge)) {
bound2Index--;
while (invertedEdges[bound2Index].isNumericallyIdentical(invertedEdge)) {
if (bound2Index == edgeIndex) {
throw new IllegalArgumentException("Constructed planes are all coplanar: "+points);
}
bound2Index = legalIndex(bound2Index - 1);
}
bound1Index = legalIndex(bound1Index);
bound2Index = legalIndex(bound2Index);
// Also confirm that all interior points are within the bounds
int startingIndex = bound2Index;
while (true) {

20
lucene/spatial3d/src/java/org/apache/lucene/spatial3d/geom/GeoConvexPolygon.java
View File

@ -218,7 +218,6 @@ class GeoConvexPolygon extends GeoBasePolygon {
}
final GeoPoint check = points.get(endPointIndex);
final SidedPlane sp = new SidedPlane(check, start, end);
//System.out.println("Created edge "+sp+" using start="+start+" end="+end+" check="+check);
edges[i] = sp;
notableEdgePoints[i] = new GeoPoint[]{start, end};
}
@ -230,16 +229,20 @@ class GeoConvexPolygon extends GeoBasePolygon {
for (int edgeIndex = 0; edgeIndex < edges.length; edgeIndex++) {
final SidedPlane edge = edges[edgeIndex];
int bound1Index = legalIndex(edgeIndex+1);
while (edges[legalIndex(bound1Index)].isNumericallyIdentical(edge)) {
bound1Index++;
while (edges[bound1Index].isNumericallyIdentical(edge)) {
if (bound1Index == edgeIndex) {
throw new IllegalArgumentException("Constructed planes are all coplanar: "+points);
}
bound1Index = legalIndex(bound1Index + 1);
}
int bound2Index = legalIndex(edgeIndex-1);
// Look for bound2
while (edges[legalIndex(bound2Index)].isNumericallyIdentical(edge)) {
bound2Index--;
while (edges[bound2Index].isNumericallyIdentical(edge)) {
if (bound2Index == edgeIndex) {
throw new IllegalArgumentException("Constructed planes are all coplanar: "+points);
}
bound2Index = legalIndex(bound2Index - 1);
}
bound1Index = legalIndex(bound1Index);
bound2Index = legalIndex(bound2Index);
// Also confirm that all interior points are within the bounds
int startingIndex = bound2Index;
while (true) {
@ -305,8 +308,9 @@ class GeoConvexPolygon extends GeoBasePolygon {
*@return the normalized index.
*/
protected int legalIndex(int index) {
while (index >= points.size())
while (index >= points.size()) {
index -= points.size();
}
while (index < 0) {
index += points.size();
}

7
lucene/spatial3d/src/java/org/apache/lucene/spatial3d/geom/GeoPolygonFactory.java
View File

@ -1219,7 +1219,7 @@ public class GeoPolygonFactory {
final GeoCompositePolygon rval,
final EdgeBuffer edgeBuffer,
final List<GeoPolygon> holes,
final GeoPoint testPoint) {
final GeoPoint testPoint) throws TileException {
//System.out.println("Looking at edge "+currentEdge+" with startpoint "+currentEdge.startPoint+" endpoint "+currentEdge.endPoint);
@ -1237,6 +1237,11 @@ public class GeoPolygonFactory {
break;
}
final Edge newLastEdge = edgeBuffer.getNext(lastEdge);
// Planes that are almost identical cannot be properly handled by the standard polygon logic. Detect this case and, if found,
// give up on the tiling -- we'll need to create a large poly instead.
if (lastEdge.plane.isNumericallyIdentical(newLastEdge.plane) /*isNearlyIdentical(lastEdge.plane, newLastEdge.plane) */) {
throw new TileException("Two adjacent edge planes are effectively parallel despite filtering; give up on tiling");
}
if (Plane.arePointsCoplanar(lastEdge.startPoint, lastEdge.endPoint, newLastEdge.endPoint)) {
break;
}

20
lucene/spatial3d/src/java/org/apache/lucene/spatial3d/geom/Plane.java
View File

@ -1275,7 +1275,9 @@ public class Plane extends Vector {
// Since a==b==0, any plane including the Z axis suffices.
//System.err.println(" Perpendicular to z");
final GeoPoint[] points = findIntersections(planetModel, normalYPlane, NO_BOUNDS, NO_BOUNDS);
boundsInfo.addZValue(points[0]);
if (points.length > 0) {
boundsInfo.addZValue(points[0]);
}
}
}
@ -2348,15 +2350,25 @@ public class Plane extends Vector {
* @return true if the planes are numerically identical.
*/
public boolean isNumericallyIdentical(final Plane p) {
// We can get the correlation by just doing a parallel plane check. If that passes, then compute a point on the plane
// (using D) and see if it also on the other plane.
// We can get the correlation by just doing a parallel plane check. That's basically finding
// out if the magnitude of the cross-product is "zero".
final double cross1 = this.y * p.z - this.z * p.y;
final double cross2 = this.z * p.x - this.x * p.z;
final double cross3 = this.x * p.y - this.y * p.x;
//System.out.println("cross product magnitude = "+(cross1 * cross1 + cross2 * cross2 + cross3 * cross3));
// Technically should be MINIMUM_RESOLUTION_SQUARED, but that gives us planes that are *almost* parallel, and those are problematic too
if (cross1 * cross1 + cross2 * cross2 + cross3 * cross3 >= MINIMUM_RESOLUTION) {
return false;
}
/* Old method
if (Math.abs(this.y * p.z - this.z * p.y) >= MINIMUM_RESOLUTION)
return false;
if (Math.abs(this.z * p.x - this.x * p.z) >= MINIMUM_RESOLUTION)
return false;
if (Math.abs(this.x * p.y - this.y * p.x) >= MINIMUM_RESOLUTION)
return false;
*/
// Now, see whether the parallel planes are in fact on top of one another.
// The math:
// We need a single point that fulfills:

159
lucene/spatial3d/src/test/org/apache/lucene/spatial3d/geom/GeoPolygonTest.java
View File

@ -756,19 +756,9 @@ shape:
final GeoPoint point = new GeoPoint(PlanetModel.WGS84, -0.41518838180529244, 3.141592653589793);
final GeoPoint encodedPoint = new GeoPoint(-0.9155623168963972, 2.3309121299774915E-10, -0.40359240449795253);
//System.out.println("point = "+point);
//System.out.println("encodedPoint = "+encodedPoint);
assertTrue(p.isWithin(point));
assertTrue(solid.isWithin(point));
assertTrue(p.isWithin(point)?solid.isWithin(point):true);
//System.out.println("bounds1 = "+bounds1);
//System.out.println("bounds2 = "+bounds2);
//assertTrue(poly1.isWithin(point));
//assertTrue(poly2.isWithin(point));
//assertTrue(solid2.isWithin(point));
//assertTrue(poly2.isWithin(encodedPoint));
}
@Test
@ -967,8 +957,10 @@ shape:
poly1List.add(new GeoPoint(PlanetModel.WGS84, 1.079437865394857, -1.720224083538152E-11));
poly1List.add(new GeoPoint(PlanetModel.WGS84, -1.5707963267948966, 0.017453291479645996));
poly1List.add(new GeoPoint(PlanetModel.WGS84, 0.017453291479645996, 2.4457272005608357E-47));
final GeoPolygonFactory.PolygonDescription pd = new GeoPolygonFactory.PolygonDescription(poly1List);
final GeoConvexPolygon poly1 = new GeoConvexPolygon(PlanetModel.WGS84, poly1List);
final GeoPolygon poly1 = GeoPolygonFactory.makeGeoPolygon(PlanetModel.WGS84, pd);
/*
[junit4] 1> unquantized=[lat=-1.5316724989005415, lon=3.141592653589793([X=-0.03902652216795768, Y=4.779370545484258E-18, Z=-0.9970038705813589])]
@ -977,17 +969,12 @@ shape:
final GeoPoint point = new GeoPoint(PlanetModel.WGS84, -1.5316724989005415, 3.141592653589793);
assertTrue(poly1.isWithin(point));
final XYZBounds actualBounds1 = new XYZBounds();
poly1.getBounds(actualBounds1);
final XYZSolid solid = XYZSolidFactory.makeXYZSolid(PlanetModel.WGS84,
actualBounds1.getMinimumX(), actualBounds1.getMaximumX(),
actualBounds1.getMinimumY(), actualBounds1.getMaximumY(),
actualBounds1.getMinimumZ(), actualBounds1.getMaximumZ());
final XYZSolid solid = XYZSolidFactory.makeXYZSolid(PlanetModel.WGS84, actualBounds1);
assertTrue(solid.isWithin(point));
assertTrue(poly1.isWithin(point)?solid.isWithin(point):true);
}
@Test
@ -1283,10 +1270,6 @@ shape:
points.add(new GeoPoint(PlanetModel.WGS84, 0.2839194570254642, -1.2434404554202965));
GeoPolygonFactory.PolygonDescription pd = new GeoPolygonFactory.PolygonDescription(points);
for (int i = 0; i < points.size(); i++) {
System.out.println("Point "+i+": "+points.get(i));
}
final GeoPoint unquantized = new GeoPoint(PlanetModel.WGS84, 2.4457272005608357E-47, -3.1404077424936307);
final GeoPoint quantized = new GeoPoint(-1.0011181510675629, -0.001186236379718708, 2.3309121299774915E-10);
@ -1300,27 +1283,20 @@ shape:
// Construct a standard polygon first to see what that does. This winds up being a large polygon under the covers.
GeoPolygon standard = GeoPolygonFactory.makeGeoPolygon(PlanetModel.WGS84, pd);
System.out.println("Shape = "+standard);
// This should be true, by inspection, but is false. That's the cause for the failure.
assertTrue(standard.isWithin(negativeX));
System.out.println("Negative x pole in set? "+standard.isWithin(negativeX));
System.out.println("Test point in set? "+standard.isWithin(testPoint));
assertTrue(standard.isWithin(testPoint));
// This is in-set because it's on an edge
System.out.println("North pole in set? "+standard.isWithin(northPole));
assertTrue(standard.isWithin(northPole));
// This is in-set
System.out.println("Plus-Y pole in set? "+standard.isWithin(positiveY));
assertTrue(standard.isWithin(positiveY));
final XYZBounds standardBounds = new XYZBounds();
standard.getBounds(standardBounds);
System.out.println("Bounds = "+standardBounds);
final XYZSolid standardSolid = XYZSolidFactory.makeXYZSolid(PlanetModel.WGS84, standardBounds);
// If within shape, should be within bounds
@ -1329,4 +1305,127 @@ shape:
}
@Test
public void testLUCENE7642() {
// Construct XYZ solid
final XYZSolid solid = XYZSolidFactory.makeXYZSolid(PlanetModel.WGS84,
0.1845405855034623, 0.2730694323646922,
-1.398547277986495E-9, 0.020766291030223535,
0.7703937553371503, 0.9977622932859774);
/*
[junit4] 1> individual planes
[junit4] 1> notableMinXPoints=[
[X=0.1845405855034623, Y=-1.398547277986495E-9, Z=0.9806642352600131],
[X=0.1845405855034623, Y=0.020766291030223535, Z=0.9804458120424796]]
notableMaxXPoints=[
[X=0.2730694323646922, Y=-1.398547277986495E-9, Z=0.959928047174481],
[X=0.2730694323646922, Y=0.020766291030223535, Z=0.9597049045335464]]
notableMinYPoints=[
[X=0.1845405855034623, Y=-1.398547277986495E-9, Z=0.9806642352600131],
[X=0.2730694323646922, Y=-1.398547277986495E-9, Z=0.959928047174481]]
notableMaxYPoints=[
[X=0.1845405855034623, Y=0.020766291030223535, Z=0.9804458120424796],
[X=0.2730694323646922, Y=0.020766291030223535, Z=0.9597049045335464]]
notableMinZPoints=[]
notableMaxZPoints=[]
[junit4] 1> All edge points=[
[X=0.1845405855034623, Y=-1.398547277986495E-9, Z=0.9806642352600131],
[X=0.1845405855034623, Y=0.020766291030223535, Z=0.9804458120424796],
[X=0.2730694323646922, Y=-1.398547277986495E-9, Z=0.959928047174481],
[X=0.2730694323646922, Y=0.020766291030223535, Z=0.9597049045335464]]
*/
final GeoPoint edge1 = new GeoPoint(0.1845405855034623, -1.398547277986495E-9, 0.9806642352600131);
final GeoPoint edge2 = new GeoPoint(0.1845405855034623, 0.020766291030223535, 0.9804458120424796);
final GeoPoint edge3 = new GeoPoint(0.2730694323646922, -1.398547277986495E-9, 0.959928047174481);
final GeoPoint edge4 = new GeoPoint(0.2730694323646922, 0.020766291030223535, 0.9597049045335464);
// The above says that none of these intersect the surface: minZmaxX, minZminX, minZmaxY, minZminY, or
// maxZmaxX, maxZminX, maxZmaxY, maxZminY.
// So what about minZ and maxZ all by themselves?
//
// [junit4] 1> Outside world: minXminYminZ=false minXminYmaxZ=true minXmaxYminZ=false minXmaxYmaxZ=true maxXminYminZ=false
// maxXminYmaxZ=true maxXmaxYminZ=false maxXmaxYmaxZ=true
//
// So the minz plane does not intersect the world because it's all inside. The maxZ plane is all outside but may intersect the world still.
// But it doesn't because it's too far north.
// So it looks like these are our edge points, and they are correct.
/*
GeoConvexPolygon: {planetmodel=PlanetModel.WGS84, points=[
[lat=-1.2267098126036888, lon=3.141592653589793([X=-0.33671029227864785, Y=4.123511816790159E-17, Z=-0.9396354281810864])],
[lat=0.2892272352400239, lon=0.017453291479645996([X=0.9591279281485559, Y=0.01674163926221766, Z=0.28545251693892165])],
[lat=-1.5707963267948966, lon=1.6247683074702402E-201([X=6.109531986173988E-17, Y=9.926573944611206E-218, Z=-0.997762292022105])]], internalEdges={2}},
GeoConvexPolygon: {planetmodel=PlanetModel.WGS84, points=[
[lat=-1.2267098126036888, lon=3.141592653589793([X=-0.33671029227864785, Y=4.123511816790159E-17, Z=-0.9396354281810864])],
[lat=-1.5707963267948966, lon=1.6247683074702402E-201([X=6.109531986173988E-17, Y=9.926573944611206E-218, Z=-0.997762292022105])],
[lat=0.6723906085905078, lon=-3.0261581679831E-12([X=0.7821883235431606, Y=-2.367025584191143E-12, Z=0.6227413298552851])]], internalEdges={0}}]}
*/
final List<GeoPoint> points = new ArrayList<>();
points.add(new GeoPoint(PlanetModel.WGS84, -1.2267098126036888, 3.141592653589793));
points.add(new GeoPoint(PlanetModel.WGS84, 0.2892272352400239, 0.017453291479645996));
points.add(new GeoPoint(PlanetModel.WGS84, -1.5707963267948966, 1.6247683074702402E-201));
points.add(new GeoPoint(PlanetModel.WGS84, 0.6723906085905078, -3.0261581679831E-12));
final GeoPolygonFactory.PolygonDescription pd = new GeoPolygonFactory.PolygonDescription(points);
final GeoPolygon shape = GeoPolygonFactory.makeGeoPolygon(PlanetModel.WGS84, pd);
final List<GeoPolygonFactory.PolygonDescription> pdList = new ArrayList<>(1);
pdList.add(pd);
final GeoPolygon largeShape = GeoPolygonFactory.makeLargeGeoPolygon(PlanetModel. WGS84, pdList);
/* This is the output:
[junit4] 1> shape = GeoCompositePolygon: {[
GeoConvexPolygon: {planetmodel=PlanetModel.WGS84, points=[
[lat=-1.2267098126036888, lon=3.141592653589793([X=-0.33671029227864785, Y=4.123511816790159E-17, Z=-0.9396354281810864])],
[lat=0.2892272352400239, lon=0.017453291479645996([X=0.9591279281485559, Y=0.01674163926221766, Z=0.28545251693892165])],
[lat=-1.5707963267948966, lon=1.6247683074702402E-201([X=6.109531986173988E-17, Y=9.926573944611206E-218, Z=-0.997762292022105])]], internalEdges={2}},
GeoConvexPolygon: {planetmodel=PlanetModel.WGS84, points=[
[lat=-1.2267098126036888, lon=3.141592653589793([X=-0.33671029227864785, Y=4.123511816790159E-17, Z=-0.9396354281810864])],
[lat=-1.5707963267948966, lon=1.6247683074702402E-201([X=6.109531986173988E-17, Y=9.926573944611206E-218, Z=-0.997762292022105])],
[lat=0.6723906085905078, lon=-3.0261581679831E-12([X=0.7821883235431606, Y=-2.367025584191143E-12, Z=0.6227413298552851])]], internalEdges={0}}]}
*/
final GeoPoint quantized = new GeoPoint(0.24162356556559528, 2.3309121299774915E-10, 0.9682657049003708);
final GeoPoint unquantized = new GeoPoint(PlanetModel.WGS84, 1.3262481806651818, 2.4457272005608357E-47);
// This passes; the point is definitely within the solid.
assertTrue(solid.isWithin(unquantized));
// This passes, so I assume that this is the correct response.
assertFalse(largeShape.isWithin(unquantized));
// This fails because the point is within the shape but apparently shouldn't be.
// Instrumenting isWithin finds that the point is on three edge planes somehow:
/*
[junit4] 1> localIsWithin start for point [0.2416235655409041,5.90945326539883E-48,0.9682657046994557]
[junit4] 1> For edge [A=-1.224646799147353E-16, B=-1.0, C=-7.498798913309287E-33, D=0.0, side=1.0] the point evaluation is -2.959035261382389E-17
[junit4] 1> For edge [A=-3.0261581679831E-12, B=-0.9999999999999999, C=-1.8529874570670608E-28, D=0.0, side=1.0] the point evaluation is -7.31191126438807E-13
[junit4] 1> For edge [A=4.234084035470679E-12, B=1.0, C=-1.5172037954732973E-12, D=0.0, side=1.0] the point evaluation is -4.460019207463956E-13
*/
// These are too close to parallel. The only solution is to prevent the poly from being created. Let's see if Geo3d thinks they are parallel.
final Plane p1 = new Plane(-1.224646799147353E-16, -1.0, -7.498798913309287E-33, 0.0);
final Plane p2 = new Plane(-3.0261581679831E-12, -0.9999999999999999, -1.8529874570670608E-28, 0.0);
final Plane p3 = new Plane(4.234084035470679E-12, 1.0, -1.5172037954732973E-12, 0.0);
assertFalse(shape.isWithin(unquantized));
// This point is indeed outside the shape but it doesn't matter
assertFalse(shape.isWithin(quantized));
// Sanity check with different poly implementation
assertTrue(shape.isWithin(edge1) == largeShape.isWithin(edge1));
assertTrue(shape.isWithin(edge2) == largeShape.isWithin(edge2));
assertTrue(shape.isWithin(edge3) == largeShape.isWithin(edge3));
assertTrue(shape.isWithin(edge4) == largeShape.isWithin(edge4));
// Verify both shapes give the same relationship
int intersection = solid.getRelationship(shape);
int largeIntersection = solid.getRelationship(largeShape);
assertTrue(intersection == largeIntersection);
}
}