You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ds...@apache.org on 2015/06/04 14:32:46 UTC
svn commit: r1683532 - in /lucene/dev/trunk/lucene: CHANGES.txt
spatial/src/java/org/apache/lucene/spatial/spatial4j/geo3d/GeoPath.java
spatial/src/test/org/apache/lucene/spatial/spatial4j/geo3d/GeoPathTest.java
Author: dsmiley
Date: Thu Jun 4 12:32:45 2015
New Revision: 1683532
URL: http://svn.apache.org/r1683532
Log:
LUCENE-6520: Geo3D GeoPath.done() would throw an NPE if adjacent path segments were co-linear
Modified:
lucene/dev/trunk/lucene/CHANGES.txt
lucene/dev/trunk/lucene/spatial/src/java/org/apache/lucene/spatial/spatial4j/geo3d/GeoPath.java
lucene/dev/trunk/lucene/spatial/src/test/org/apache/lucene/spatial/spatial4j/geo3d/GeoPathTest.java
Modified: lucene/dev/trunk/lucene/CHANGES.txt
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/CHANGES.txt?rev=1683532&r1=1683531&r2=1683532&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/CHANGES.txt (original)
+++ lucene/dev/trunk/lucene/CHANGES.txt Thu Jun 4 12:32:45 2015
@@ -60,6 +60,9 @@ Bug fixes
closed listeners. This was fixed by LUCENE-6501.
(Adrien Grand, Uwe Schindler)
+* LUCENE-6520: Geo3D GeoPath.done() would throw an NPE if adjacent path
+ segments were co-linear. (Karl Wright via David Smiley)
+
Changes in Runtime Behavior
* LUCENE-6501: The subreader structure in ParallelCompositeReader
Modified: lucene/dev/trunk/lucene/spatial/src/java/org/apache/lucene/spatial/spatial4j/geo3d/GeoPath.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/spatial/src/java/org/apache/lucene/spatial/spatial4j/geo3d/GeoPath.java?rev=1683532&r1=1683531&r2=1683532&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/spatial/src/java/org/apache/lucene/spatial/spatial4j/geo3d/GeoPath.java (original)
+++ lucene/dev/trunk/lucene/spatial/src/java/org/apache/lucene/spatial/spatial4j/geo3d/GeoPath.java Thu Jun 4 12:32:45 2015
@@ -113,15 +113,21 @@ public class GeoPath extends GeoBaseExte
// General intersection case
final PathSegment prevSegment = segments.get(i-1);
- if (prevSegment.upperConnectingPlane.isNumericallyIdentical(currentSegment.upperConnectingPlane) &&
- prevSegment.lowerConnectingPlane.isNumericallyIdentical(currentSegment.lowerConnectingPlane)) {
+ // We construct four separate planes, and evaluate which one includes all interior points with least overlap
+ final SidedPlane candidate1 = SidedPlane.constructNormalizedThreePointSidedPlane(currentSegment.start, prevSegment.URHC, currentSegment.ULHC, currentSegment.LLHC);
+ final SidedPlane candidate2 = SidedPlane.constructNormalizedThreePointSidedPlane(currentSegment.start, currentSegment.ULHC, currentSegment.LLHC, prevSegment.LRHC);
+ final SidedPlane candidate3 = SidedPlane.constructNormalizedThreePointSidedPlane(currentSegment.start, currentSegment.LLHC, prevSegment.LRHC, prevSegment.URHC);
+ final SidedPlane candidate4 = SidedPlane.constructNormalizedThreePointSidedPlane(currentSegment.start, prevSegment.LRHC, prevSegment.URHC, currentSegment.ULHC);
+
+ if (candidate1 == null && candidate2 == null && candidate3 == null && candidate4 == null) {
// The planes are identical. We don't need a circle at all. Special constructor...
endPoints.add(new SegmentEndpoint(currentSegment.start));
} else {
endPoints.add(new SegmentEndpoint(currentSegment.start,
prevSegment.endCutoffPlane, currentSegment.startCutoffPlane,
prevSegment.URHC, prevSegment.LRHC,
- currentSegment.ULHC, currentSegment.LLHC));
+ currentSegment.ULHC, currentSegment.LLHC,
+ candidate1, candidate2, candidate3, candidate4));
}
}
// Do final endpoint
@@ -458,8 +464,9 @@ public class GeoPath extends GeoBaseExte
*/
public SegmentEndpoint(final GeoPoint point,
final SidedPlane prevCutoffPlane, final SidedPlane nextCutoffPlane,
- final GeoPoint prevUpperGeoPoint, final GeoPoint prevLowerGeoPoint,
- final GeoPoint nextUpperGeoPoint, final GeoPoint nextLowerGeoPoint) {
+ final GeoPoint notCand2Point, final GeoPoint notCand1Point,
+ final GeoPoint notCand3Point, final GeoPoint notCand4Point,
+ final SidedPlane candidate1, final SidedPlane candidate2, final SidedPlane candidate3, final SidedPlane candidate4) {
// Note: What we really need is a single plane that goes through all four points.
// Since that's not possible in the ellipsoid case (because three points determine a plane, not four), we
// need an approximation that at least creates a boundary that has no interruptions.
@@ -472,38 +479,35 @@ public class GeoPath extends GeoBaseExte
this.point = point;
// We construct four separate planes, and evaluate which one includes all interior points with least overlap
- final SidedPlane candidate1 = SidedPlane.constructNormalizedThreePointSidedPlane(point, prevUpperGeoPoint, nextUpperGeoPoint, nextLowerGeoPoint);
- final SidedPlane candidate2 = SidedPlane.constructNormalizedThreePointSidedPlane(point, nextUpperGeoPoint, nextLowerGeoPoint, prevLowerGeoPoint);
- final SidedPlane candidate3 = SidedPlane.constructNormalizedThreePointSidedPlane(point, nextLowerGeoPoint, prevLowerGeoPoint, prevUpperGeoPoint);
- final SidedPlane candidate4 = SidedPlane.constructNormalizedThreePointSidedPlane(point, prevLowerGeoPoint, prevUpperGeoPoint, nextUpperGeoPoint);
-
- final boolean cand1IsOtherWithin = candidate1.isWithin(prevLowerGeoPoint);
- final boolean cand2IsOtherWithin = candidate2.isWithin(prevUpperGeoPoint);
- final boolean cand3IsOtherWithin = candidate3.isWithin(nextUpperGeoPoint);
- final boolean cand4IsOtherWithin = candidate4.isWithin(nextLowerGeoPoint);
+ // (Constructed beforehand because we need them for degeneracy check)
+
+ final boolean cand1IsOtherWithin = candidate1!=null?candidate1.isWithin(notCand1Point):false;
+ final boolean cand2IsOtherWithin = candidate2!=null?candidate2.isWithin(notCand2Point):false;
+ final boolean cand3IsOtherWithin = candidate3!=null?candidate3.isWithin(notCand3Point):false;
+ final boolean cand4IsOtherWithin = candidate4!=null?candidate4.isWithin(notCand4Point):false;
if (cand1IsOtherWithin && cand2IsOtherWithin && cand3IsOtherWithin && cand4IsOtherWithin) {
// The only way we should see both within is if all four points are coplanar. In that case, we default to the simplest treatment.
this.circlePlane = candidate1; // doesn't matter which
- this.notablePoints = new GeoPoint[]{prevUpperGeoPoint, nextUpperGeoPoint, prevLowerGeoPoint, nextLowerGeoPoint};
+ this.notablePoints = new GeoPoint[]{notCand2Point, notCand3Point, notCand1Point, notCand4Point};
this.cutoffPlanes = new Membership[]{new SidedPlane(prevCutoffPlane), new SidedPlane(nextCutoffPlane)};
} else if (cand1IsOtherWithin) {
// Use candidate1, and DON'T include prevCutoffPlane in the cutoff planes list
this.circlePlane = candidate1;
- this.notablePoints = new GeoPoint[]{prevUpperGeoPoint, nextUpperGeoPoint, nextLowerGeoPoint};
+ this.notablePoints = new GeoPoint[]{notCand2Point, notCand3Point, notCand4Point};
this.cutoffPlanes = new Membership[]{new SidedPlane(nextCutoffPlane)};
} else if (cand2IsOtherWithin) {
// Use candidate2
this.circlePlane = candidate2;
- this.notablePoints = new GeoPoint[]{nextUpperGeoPoint, nextLowerGeoPoint, prevLowerGeoPoint};
+ this.notablePoints = new GeoPoint[]{notCand3Point, notCand4Point, notCand1Point};
this.cutoffPlanes = new Membership[]{new SidedPlane(nextCutoffPlane)};
} else if (cand3IsOtherWithin) {
this.circlePlane = candidate3;
- this.notablePoints = new GeoPoint[]{nextLowerGeoPoint, prevLowerGeoPoint, prevUpperGeoPoint};
+ this.notablePoints = new GeoPoint[]{notCand4Point, notCand1Point, notCand2Point};
this.cutoffPlanes = new Membership[]{new SidedPlane(prevCutoffPlane)};
} else if (cand4IsOtherWithin) {
this.circlePlane = candidate4;
- this.notablePoints = new GeoPoint[]{prevLowerGeoPoint, prevUpperGeoPoint, nextUpperGeoPoint};
+ this.notablePoints = new GeoPoint[]{notCand1Point, notCand2Point, notCand3Point};
this.cutoffPlanes = new Membership[]{new SidedPlane(prevCutoffPlane)};
} else {
// dunno what happened
Modified: lucene/dev/trunk/lucene/spatial/src/test/org/apache/lucene/spatial/spatial4j/geo3d/GeoPathTest.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/spatial/src/test/org/apache/lucene/spatial/spatial4j/geo3d/GeoPathTest.java?rev=1683532&r1=1683531&r2=1683532&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/spatial/src/test/org/apache/lucene/spatial/spatial4j/geo3d/GeoPathTest.java (original)
+++ lucene/dev/trunk/lucene/spatial/src/test/org/apache/lucene/spatial/spatial4j/geo3d/GeoPathTest.java Thu Jun 4 12:32:45 2015
@@ -19,6 +19,7 @@ package org.apache.lucene.spatial.spatia
import org.junit.Test;
+import static java.lang.Math.toRadians;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
@@ -182,4 +183,14 @@ public class GeoPathTest {
}
+ @Test
+ public void testCoLinear() {
+ // p1: (12,-90), p2: (11, -55), (129, -90)
+ GeoPath p = new GeoPath(PlanetModel.SPHERE, 0.1);
+ p.addPoint(toRadians(-90), toRadians(12));//south pole
+ p.addPoint(toRadians(-55), toRadians(11));
+ p.addPoint(toRadians(-90), toRadians(129));//south pole again
+ p.done();//at least test this doesn't bomb like it used too -- LUCENE-6520
+ }
+
}