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

LUCENE-7424: GeoPolygon computation of intersection bounds was incorrect.

This commit is contained in:
Karl Wright 2016-08-25 18:09:50 -04:00
parent da6a502f26
commit 6242717a29
3 changed files with 79 additions and 58 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -50,10 +50,10 @@ class GeoConcavePolygon extends GeoBasePolygon {
protected boolean isDone = false; protected boolean isDone = false;
/** A bounds object for each sided plane */ /** A bounds object for each sided plane */
protected Map<SidedPlane, Membership> eitherBounds = null; protected Map<SidedPlane, Membership> eitherBounds = null;
/** Edge plane for one side of intersection */ /** Map from edge to its previous non-coplanar brother */
protected Map<SidedPlane, Plane> edgePlanes = null; protected Map<SidedPlane, SidedPlane> prevBrotherMap = null;
/** Intersection bounds */ /** Map from edge to its next non-coplanar brother */
protected Map<SidedPlane, Membership> intersectionBounds = null; protected Map<SidedPlane, SidedPlane> nextBrotherMap = null;
/** /**
* Create a concave polygon from a list of points. The first point must be on the * Create a concave polygon from a list of points. The first point must be on the
@ -214,8 +214,8 @@ class GeoConcavePolygon extends GeoBasePolygon {
// For each edge, create a bounds object. // For each edge, create a bounds object.
eitherBounds = new HashMap<>(edges.length); eitherBounds = new HashMap<>(edges.length);
intersectionBounds = new HashMap<>(edges.length); prevBrotherMap = new HashMap<>(edges.length);
edgePlanes = new HashMap<>(edges.length); nextBrotherMap = new HashMap<>(edges.length);
for (int edgeIndex = 0; edgeIndex < edges.length; edgeIndex++) { for (int edgeIndex = 0; edgeIndex < edges.length; edgeIndex++) {
final SidedPlane edge = edges[edgeIndex]; final SidedPlane edge = edges[edgeIndex];
final SidedPlane invertedEdge = invertedEdges[edgeIndex]; final SidedPlane invertedEdge = invertedEdges[edgeIndex];
@ -224,16 +224,6 @@ class GeoConcavePolygon extends GeoBasePolygon {
bound1Index++; bound1Index++;
} }
int bound2Index = legalIndex(edgeIndex-1); int bound2Index = legalIndex(edgeIndex-1);
int otherIndex = bound2Index;
final SidedPlane otherEdge;
final SidedPlane otherInvertedEdge;
if (invertedEdges[legalIndex(otherIndex)].isNumericallyIdentical(invertedEdge)) {
otherInvertedEdge = null;
otherEdge = null;
} else {
otherInvertedEdge = invertedEdges[legalIndex(otherIndex)];
otherEdge = edges[legalIndex(otherIndex)];
}
while (invertedEdges[legalIndex(bound2Index)].isNumericallyIdentical(invertedEdge)) { while (invertedEdges[legalIndex(bound2Index)].isNumericallyIdentical(invertedEdge)) {
bound2Index--; bound2Index--;
} }
@ -252,15 +242,10 @@ class GeoConcavePolygon extends GeoBasePolygon {
} }
} }
eitherBounds.put(edge, new EitherBound(invertedEdges[bound1Index], invertedEdges[bound2Index])); eitherBounds.put(edge, new EitherBound(invertedEdges[bound1Index], invertedEdges[bound2Index]));
// For intersections, we look at the point at the intersection between the previous edge and this one. We need to locate the // When we are done with this cycle, we'll need to build the intersection bound for each edge and its brother.
// Intersection bounds needs to look even further forwards/backwards // For now, keep track of the relationships.
if (otherInvertedEdge != null) { nextBrotherMap.put(invertedEdge, invertedEdges[bound1Index]);
while (invertedEdges[legalIndex(otherIndex)].isNumericallyIdentical(otherInvertedEdge)) { prevBrotherMap.put(invertedEdge, invertedEdges[bound2Index]);
otherIndex--;
}
intersectionBounds.put(edge, new EitherBound(invertedEdges[legalIndex(otherIndex)], invertedEdges[legalIndex(bound2Index)]));
edgePlanes.put(edge, otherEdge);
}
} }
// Pick an edge point arbitrarily from the outer polygon. Glom this together with all edge points from // Pick an edge point arbitrarily from the outer polygon. Glom this together with all edge points from
@ -383,7 +368,7 @@ class GeoConcavePolygon extends GeoBasePolygon {
/** A membership implementation representing polygon edges that must apply. /** A membership implementation representing polygon edges that must apply.
*/ */
protected class EitherBound implements Membership { protected static class EitherBound implements Membership {
protected final SidedPlane sideBound1; protected final SidedPlane sideBound1;
protected final SidedPlane sideBound2; protected final SidedPlane sideBound2;
@ -406,6 +391,12 @@ class GeoConcavePolygon extends GeoBasePolygon {
public boolean isWithin(final double x, final double y, final double z) { public boolean isWithin(final double x, final double y, final double z) {
return sideBound1.isWithin(x,y,z) && sideBound2.isWithin(x,y,z); return sideBound1.isWithin(x,y,z) && sideBound2.isWithin(x,y,z);
} }
@Override
public String toString() {
return "(" + sideBound1 + "," + sideBound2 + ")";
}
} }
@Override @Override
@ -442,10 +433,10 @@ class GeoConcavePolygon extends GeoBasePolygon {
// Add planes with membership. // Add planes with membership.
for (final SidedPlane edge : edges) { for (final SidedPlane edge : edges) {
bounds.addPlane(planetModel, edge, eitherBounds.get(edge)); bounds.addPlane(planetModel, edge, eitherBounds.get(edge));
final Membership m = intersectionBounds.get(edge); }
if (m != null) { for (final SidedPlane invertedEdge : invertedEdges) {
bounds.addIntersection(planetModel, edgePlanes.get(edge), edge, m); final SidedPlane nextEdge = nextBrotherMap.get(invertedEdge);
} bounds.addIntersection(planetModel, invertedEdge, nextEdge, prevBrotherMap.get(invertedEdge), nextBrotherMap.get(nextEdge));
} }
} }

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

@ -48,10 +48,10 @@ class GeoConvexPolygon extends GeoBasePolygon {
protected boolean isDone = false; protected boolean isDone = false;
/** A bounds object for each sided plane */ /** A bounds object for each sided plane */
protected Map<SidedPlane, Membership> eitherBounds = null; protected Map<SidedPlane, Membership> eitherBounds = null;
/** Edge plane for one side of intersection */ /** Map from edge to its previous non-coplanar brother */
protected Map<SidedPlane, Plane> edgePlanes = null; protected Map<SidedPlane, SidedPlane> prevBrotherMap = null;
/** Intersection bounds */ /** Map from edge to its next non-coplanar brother */
protected Map<SidedPlane, Membership> intersectionBounds = null; protected Map<SidedPlane, SidedPlane> nextBrotherMap = null;
/** /**
* Create a convex polygon from a list of points. The first point must be on the * Create a convex polygon from a list of points. The first point must be on the
@ -210,8 +210,8 @@ class GeoConvexPolygon extends GeoBasePolygon {
// For each edge, create a bounds object. // For each edge, create a bounds object.
eitherBounds = new HashMap<>(edges.length); eitherBounds = new HashMap<>(edges.length);
intersectionBounds = new HashMap<>(edges.length); prevBrotherMap = new HashMap<>(edges.length);
edgePlanes = new HashMap<>(edges.length); nextBrotherMap = new HashMap<>(edges.length);
for (int edgeIndex = 0; edgeIndex < edges.length; edgeIndex++) { for (int edgeIndex = 0; edgeIndex < edges.length; edgeIndex++) {
final SidedPlane edge = edges[edgeIndex]; final SidedPlane edge = edges[edgeIndex];
int bound1Index = legalIndex(edgeIndex+1); int bound1Index = legalIndex(edgeIndex+1);
@ -219,13 +219,6 @@ class GeoConvexPolygon extends GeoBasePolygon {
bound1Index++; bound1Index++;
} }
int bound2Index = legalIndex(edgeIndex-1); int bound2Index = legalIndex(edgeIndex-1);
int otherIndex = bound2Index;
final SidedPlane otherEdge;
if (edges[legalIndex(otherIndex)].isNumericallyIdentical(edge)) {
otherEdge = null;
} else {
otherEdge = edges[legalIndex(otherIndex)];
}
// Look for bound2 // Look for bound2
while (edges[legalIndex(bound2Index)].isNumericallyIdentical(edge)) { while (edges[legalIndex(bound2Index)].isNumericallyIdentical(edge)) {
bound2Index--; bound2Index--;
@ -245,15 +238,10 @@ class GeoConvexPolygon extends GeoBasePolygon {
} }
} }
eitherBounds.put(edge, new EitherBound(edges[bound1Index], edges[bound2Index])); eitherBounds.put(edge, new EitherBound(edges[bound1Index], edges[bound2Index]));
// For intersections, we look at the point at the intersection between the previous edge and this one. We need to locate the // When we are done with this cycle, we'll need to build the intersection bound for each edge and its brother.
// Intersection bounds needs to look even further forwards/backwards // For now, keep track of the relationships.
if (otherEdge != null) { nextBrotherMap.put(edge, edges[bound1Index]);
while (edges[legalIndex(otherIndex)].isNumericallyIdentical(otherEdge)) { prevBrotherMap.put(edge, edges[bound2Index]);
otherIndex--;
}
intersectionBounds.put(edge, new EitherBound(edges[legalIndex(otherIndex)], edges[legalIndex(bound2Index)]));
edgePlanes.put(edge, otherEdge);
}
} }
// Pick an edge point arbitrarily from the outer polygon. Glom this together with all edge points from // Pick an edge point arbitrarily from the outer polygon. Glom this together with all edge points from
@ -370,7 +358,7 @@ class GeoConvexPolygon extends GeoBasePolygon {
/** A membership implementation representing polygon edges that must apply. /** A membership implementation representing polygon edges that must apply.
*/ */
protected class EitherBound implements Membership { protected static class EitherBound implements Membership {
protected final SidedPlane sideBound1; protected final SidedPlane sideBound1;
protected final SidedPlane sideBound2; protected final SidedPlane sideBound2;
@ -393,6 +381,11 @@ class GeoConvexPolygon extends GeoBasePolygon {
public boolean isWithin(final double x, final double y, final double z) { public boolean isWithin(final double x, final double y, final double z) {
return sideBound1.isWithin(x,y,z) && sideBound2.isWithin(x,y,z); return sideBound1.isWithin(x,y,z) && sideBound2.isWithin(x,y,z);
} }
@Override
public String toString() {
return "(" + sideBound1 + "," + sideBound2 + ")";
}
} }
@ -428,10 +421,8 @@ class GeoConvexPolygon extends GeoBasePolygon {
// Add planes with membership. // Add planes with membership.
for (final SidedPlane edge : edges) { for (final SidedPlane edge : edges) {
bounds.addPlane(planetModel, edge, eitherBounds.get(edge)); bounds.addPlane(planetModel, edge, eitherBounds.get(edge));
final Membership m = intersectionBounds.get(edge); final SidedPlane nextEdge = nextBrotherMap.get(edge);
if (m != null) { bounds.addIntersection(planetModel, edge, nextEdge, prevBrotherMap.get(edge), nextBrotherMap.get(nextEdge));
bounds.addIntersection(planetModel, edgePlanes.get(edge), edge, m);
}
} }
} }

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

@ -928,4 +928,43 @@ shape:
assertTrue(!result); assertTrue(!result);
} }
@Test
public void testPolygonFailureCase2() {
/*
[junit4] 1> shape=GeoCompositeMembershipShape: {[GeoConvexPolygon: {planetmodel=PlanetModel.WGS84, points=[
[lat=1.079437865394857, lon=-1.720224083538152E-11([X=0.47111944719262044, Y=-8.104310192839264E-12, Z=0.8803759987367299])],
[lat=-1.5707963267948966, lon=0.017453291479645996([X=6.108601474971234E-17, Y=1.066260290095308E-18, Z=-0.997762292022105])],
[lat=0.017453291479645996, lon=2.4457272005608357E-47([X=1.0009653513901666, Y=2.448088186713865E-47, Z=0.01747191415779267])]], internalEdges={2}},
GeoConvexPolygon: {planetmodel=PlanetModel.WGS84, points=[
[lat=1.079437865394857, lon=-1.720224083538152E-11([X=0.47111944719262044, Y=-8.104310192839264E-12, Z=0.8803759987367299])],
[lat=0.017453291479645996, lon=2.4457272005608357E-47([X=1.0009653513901666, Y=2.448088186713865E-47, Z=0.01747191415779267])],
[lat=0.0884233366943164, lon=0.4323234231678824([X=0.9054355304510789, Y=0.4178006803188124, Z=0.08840463683725623])]], internalEdges={0}}]}
*/
final List<GeoPoint> poly1List = new ArrayList<>();
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 GeoConvexPolygon poly1 = new GeoConvexPolygon(PlanetModel.WGS84, poly1List);
/*
[junit4] 1> unquantized=[lat=-1.5316724989005415, lon=3.141592653589793([X=-0.03902652216795768, Y=4.779370545484258E-18, Z=-0.9970038705813589])]
[junit4] 1> quantized=[X=-0.03902652216283731, Y=2.3309121299774915E-10, Z=-0.9970038706538652]
*/
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());
assertTrue(solid.isWithin(point));
}
} }