You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by kw...@apache.org on 2018/04/10 15:12:27 UTC

lucene-solr:branch_6x: LUCENE-8245: Adopt a more-rigorous way of finding intersections with envelope planes.

Repository: lucene-solr
Updated Branches:
  refs/heads/branch_6x 3a0aef1fe -> 16e5ab161


LUCENE-8245: Adopt a more-rigorous way of finding intersections with envelope planes.


Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/16e5ab16
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/16e5ab16
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/16e5ab16

Branch: refs/heads/branch_6x
Commit: 16e5ab161cc97a46c4dd115c409e8ed4ddf3b40f
Parents: 3a0aef1
Author: Karl Wright <Da...@gmail.com>
Authored: Tue Apr 10 11:10:40 2018 -0400
Committer: Karl Wright <Da...@gmail.com>
Committed: Tue Apr 10 11:12:20 2018 -0400

----------------------------------------------------------------------
 .../spatial3d/geom/GeoComplexPolygon.java       | 187 ++++++++++++++++---
 .../lucene/spatial3d/geom/GeoPolygonTest.java   |   1 -
 2 files changed, 159 insertions(+), 29 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/16e5ab16/lucene/spatial3d/src/java/org/apache/lucene/spatial3d/geom/GeoComplexPolygon.java
----------------------------------------------------------------------
diff --git a/lucene/spatial3d/src/java/org/apache/lucene/spatial3d/geom/GeoComplexPolygon.java b/lucene/spatial3d/src/java/org/apache/lucene/spatial3d/geom/GeoComplexPolygon.java
index 5c3521c..3c63559 100644
--- a/lucene/spatial3d/src/java/org/apache/lucene/spatial3d/geom/GeoComplexPolygon.java
+++ b/lucene/spatial3d/src/java/org/apache/lucene/spatial3d/geom/GeoComplexPolygon.java
@@ -572,6 +572,144 @@ class GeoComplexPolygon extends GeoBasePolygon {
     return minimumDistance;
   }
 
+  /** Create a linear crossing edge iterator with the appropriate cutoff planes given the geometry.
+   */
+  private CountingEdgeIterator createLinearCrossingEdgeIterator(final Plane plane, final Plane abovePlane, final Plane belowPlane, final double thePointX, final double thePointY, final double thePointZ) {
+    // If thePoint and testPoint are parallel, we won't be able to determine sidedness of the bounding planes.  So detect that case, and build the iterator differently if we find it.
+    // This didn't work; not sure why not:
+    //if (testPoint.isParallel(thePointX, thePointY, thePointZ)) {
+    //  return new FullLinearCrossingEdgeIterator(plane, abovePlane, belowPlane, thePointX, thePointY, thePointZ);
+    //}
+    //return new SectorLinearCrossingEdgeIterator(plane, abovePlane, belowPlane, thePointX, thePointY, thePointZ);
+    //
+    try {
+      return new SectorLinearCrossingEdgeIterator(plane, abovePlane, belowPlane, thePointX, thePointY, thePointZ);
+    } catch (IllegalArgumentException e) {
+      // Assume we failed because we could not construct bounding planes, so do it another way.
+      return new FullLinearCrossingEdgeIterator(plane, abovePlane, belowPlane, thePointX, thePointY, thePointZ);
+    }
+  }
+
+  private final static GeoPoint[] NO_POINTS = new GeoPoint[0];
+  
+  /** Compute crossings of an envelope plane by an edge.
+  */
+  private GeoPoint[] computeCrossings(final Plane envelopePlane, final Edge edge, final Membership... envelopeBounds) {
+    // Note: there is a possibility that one or both endpoints of the edge actually lies in the inside/outside planes.  If this is the case, those endpoints may or may not show up as crossings.
+    // And yet, we absolutely cannot count each crossing more than once.  So, how do we proceed?
+    // The solution is to recognize that excluded crossings will be excluded for two reasons: (1) bounds, and (2) because there's only one solution to the intersection equation, which means
+    // we never actually crossed the envelope plane.  So, the way we proceed is to look for intersections, but NOT do edge bounds at all.  Then, we consider the results in the context of
+    // the plane we're trying to assess.
+    //System.out.println(" Computing crossings between "+envelopePlane+" and ["+edge.startPoint+"->"+edge.endPoint+"]");
+    
+    final GeoPoint[] unboundedIntersectionPoints = envelopePlane.findIntersections(planetModel, edge.plane);
+    // Go through the intersection points one at a time.  Notes:
+    // (1) So that we don't double-count, we can only include at most one point in the result per intersection.
+    // (2) Single-solution results imply that the plane was not crossed.  The only time we consider them is if the edge ends on the plane, in which case we count it as a crossing.
+    // (3) We tried to detect the case where the edge ends on the envelope plane by seeing if the intersection point was numerically identical to an endpoint, but that
+    //    was still too strict.
+    // (4) The intersection points will be on both planes, for sure.  The question is whether an intersection point "lines up" with an edge endpoint.  If the edge endpoint
+    //    lies in the envelope plane, then we have the possibility of a detection.  The detection is confirmed if the distance "is small" between the edge endpoint and
+    //    the intersection point.  I see squared linear distance numbers of about 1.35e-24, which is still barely outside of the 1e-12 envelope, so a straight distance
+    //    check won't work.  So this is what I think we need to do:
+    //    (a) Check if endpoint is on envelope plane; if not, we keep going.
+    //    (b) If on envelope plane, we confirm that entire section of plane between intersection point and endpoint lies within envelope plane.  (How??)
+    
+    // If no points, just return.  (I'm not even sure this can happen)
+    if (unboundedIntersectionPoints.length == 0) {
+      //System.out.println("  None found.");
+      return unboundedIntersectionPoints;
+    }
+
+    // Single solution has special logic
+    if (unboundedIntersectionPoints.length == 1) {
+      //System.out.println("  One found.");
+      final GeoPoint thePoint = unboundedIntersectionPoints[0];
+      if (withinBounds(thePoint, envelopeBounds) &&
+        (pointMatches(envelopePlane, thePoint, edge.startPoint) || pointMatches(envelopePlane, thePoint, edge.endPoint) ||
+          (edge.startPlane.isWithin(thePoint) && edge.endPlane.isWithin(thePoint)))) {
+        return unboundedIntersectionPoints;
+      }
+      return NO_POINTS;
+    }
+      
+    // Two solutions: we could return none, one, the other one, or both.
+    //System.out.println("  Two found.");
+
+    final GeoPoint firstPoint = unboundedIntersectionPoints[0];
+    final GeoPoint secondPoint = unboundedIntersectionPoints[1];
+    
+    final boolean useFirstPoint;
+    if (withinBounds(firstPoint, envelopeBounds) &&
+        (pointMatches(envelopePlane, firstPoint, edge.startPoint) || pointMatches(envelopePlane, firstPoint, edge.endPoint) ||
+          (edge.startPlane.isWithin(firstPoint) && edge.endPlane.isWithin(firstPoint)))) {
+      //System.out.println("  Point "+firstPoint+" accepted.");
+      useFirstPoint = true;
+    } else {
+      /*System.out.println("  Point "+firstPoint+" rejected; withinBounds="+withinBounds(firstPoint, envelopeBounds)+
+        "; edgeBounds="+(edge.startPlane.isWithin(firstPoint) && edge.endPlane.isWithin(firstPoint))+
+        "; startPointDist="+edge.startPoint.linearDistanceSquared(firstPoint)+"; endPointDist="+edge.endPoint.linearDistanceSquared(firstPoint)); */
+      useFirstPoint = false;
+    }
+    
+    final boolean useSecondPoint;
+    if (withinBounds(secondPoint, envelopeBounds) &&
+        (pointMatches(envelopePlane, secondPoint, edge.startPoint) || pointMatches(envelopePlane, secondPoint, edge.endPoint) ||
+          (edge.startPlane.isWithin(secondPoint) && edge.endPlane.isWithin(secondPoint)))) {
+      //System.out.println("  Point "+secondPoint+" accepted.");
+      useSecondPoint = true;
+    } else {
+      /*System.out.println("  Point "+secondPoint+" rejected; withinBounds="+withinBounds(secondPoint, envelopeBounds)+
+        "; edgeBounds="+(edge.startPlane.isWithin(secondPoint) && edge.endPlane.isWithin(secondPoint))+
+        "; startPointDist="+edge.startPoint.linearDistanceSquared(secondPoint)+"; endPointDist="+edge.endPoint.linearDistanceSquared(secondPoint)); */
+      useSecondPoint = false;
+    }
+    
+    if (useFirstPoint && useSecondPoint) {
+      return unboundedIntersectionPoints;
+    }
+    
+    if (useFirstPoint) {
+      return new GeoPoint[]{firstPoint};
+    }
+    
+    if (useSecondPoint) {
+      return new GeoPoint[]{secondPoint};
+    }
+    
+    return NO_POINTS;
+  }
+
+  /** This distance is arbitrary, but it must NOT allow non-intersections to be detected.
+  */
+  private final static double MATCH_MAXIMUM_DISTANCE_SQUARED = Vector.MINIMUM_RESOLUTION_SQUARED * 2.0;
+
+  /** Return true of the point matches the edge endpoint, or false otherwise.
+  * This method is here to compensate for the fact that we don't always detect an intersection due to the bounds interfering.
+  */
+  private static boolean pointMatches(final Plane envelopePlane, final GeoPoint intersectionPoint, final GeoPoint edgePoint) {
+    // If edge isn't on the envelope plane, no match
+    if (!envelopePlane.evaluateIsZero(edgePoint)) {
+      return false;
+    }
+    // As a proxy for staying "within" the envelope plane,  compute linear squared distance.  If clearly too close to be anything other than local, we can
+    // just return true.  Otherwise, we'll need to add more complicated fallback computations.
+    if (edgePoint.linearDistanceSquared(intersectionPoint) <= MATCH_MAXIMUM_DISTANCE_SQUARED) {
+      return true;
+    }
+    // More to be done?  Cross that bridge if we come to it.
+    return false;
+  }
+  
+  private static boolean withinBounds(final GeoPoint point, final Membership[] bounds) {
+    for (final Membership bound : bounds) {
+      if (!bound.isWithin(point)) {
+        return false;
+      }
+    }
+    return true;
+  }
+  
   /**
    * An instance of this class describes a single edge, and includes what is necessary to reliably determine intersection
    * in the context of the even/odd algorithm used.
@@ -942,8 +1080,8 @@ class GeoComplexPolygon extends GeoBasePolygon {
       }
       
       // Determine crossings of this edge against all inside/outside planes.  There's no further need to look at the actual travel plane itself.
-      final GeoPoint[] aboveCrossings = abovePlane.findCrossings(planetModel, edge.plane, bound, edge.startPlane, edge.endPlane);
-      final GeoPoint[] belowCrossings = belowPlane.findCrossings(planetModel, edge.plane, bound, edge.startPlane, edge.endPlane);
+      final GeoPoint[] aboveCrossings = computeCrossings(abovePlane, edge, bound);
+      final GeoPoint[] belowCrossings = computeCrossings(belowPlane, edge, bound);
       
       if (aboveCrossings != null) {
         aboveCrossingCount += aboveCrossings.length;
@@ -957,24 +1095,6 @@ class GeoComplexPolygon extends GeoBasePolygon {
 
   }
 
-  /** Create a linear crossing edge iterator with the appropriate cutoff planes given the geometry.
-   */
-  private CountingEdgeIterator createLinearCrossingEdgeIterator(final Plane plane, final Plane abovePlane, final Plane belowPlane, final double thePointX, final double thePointY, final double thePointZ) {
-    // If thePoint and testPoint are parallel, we won't be able to determine sidedness of the bounding planes.  So detect that case, and build the iterator differently if we find it.
-    // This didn't work; not sure why not:
-    //if (testPoint.isParallel(thePointX, thePointY, thePointZ)) {
-    //  return new FullLinearCrossingEdgeIterator(plane, abovePlane, belowPlane, thePointX, thePointY, thePointZ);
-    //}
-    //return new SectorLinearCrossingEdgeIterator(plane, abovePlane, belowPlane, thePointX, thePointY, thePointZ);
-    //
-    try {
-      return new SectorLinearCrossingEdgeIterator(plane, abovePlane, belowPlane, thePointX, thePointY, thePointZ);
-    } catch (IllegalArgumentException e) {
-      // Assume we failed because we could not construct bounding planes, so do it another way.
-      return new FullLinearCrossingEdgeIterator(plane, abovePlane, belowPlane, thePointX, thePointY, thePointZ);
-    }
-  }
-
   /** Count the number of verifiable edge crossings for less than 1/2 a world.
    */
   private class SectorLinearCrossingEdgeIterator implements CountingEdgeIterator {
@@ -1030,8 +1150,8 @@ class GeoComplexPolygon extends GeoBasePolygon {
       }
       
       // Determine crossings of this edge against all inside/outside planes.  There's no further need to look at the actual travel plane itself.
-      final GeoPoint[] aboveCrossings = abovePlane.findCrossings(planetModel, edge.plane, bound1, bound2, edge.startPlane, edge.endPlane);
-      final GeoPoint[] belowCrossings = belowPlane.findCrossings(planetModel, edge.plane, bound1, bound2, edge.startPlane, edge.endPlane);
+      final GeoPoint[] aboveCrossings = computeCrossings(abovePlane, edge, bound1, bound2);
+      final GeoPoint[] belowCrossings = computeCrossings(belowPlane, edge, bound1, bound2);
       
       if (aboveCrossings != null) {
         aboveCrossingCount += aboveCrossings.length;
@@ -1047,7 +1167,7 @@ class GeoComplexPolygon extends GeoBasePolygon {
   
   /** Count the number of verifiable edge crossings for a dual-leg journey.
    */
-  private class DualCrossingEdgeIterator implements EdgeIterator {
+  private class DualCrossingEdgeIterator implements CountingEdgeIterator {
     
     // This is a hash of which edges we've already looked at and tallied, so we don't repeat ourselves.
     // It is lazily initialized since most transitions cross no edges at all.
@@ -1218,6 +1338,7 @@ class GeoComplexPolygon extends GeoBasePolygon {
       }
     }
     
+    @Override
     public int getCrossingCount() {
       // Doesn't return the actual crossing count -- just gets the even/odd part right
       if (innerCrossingCount < outerCrossingCount) {
@@ -1272,25 +1393,35 @@ class GeoComplexPolygon extends GeoBasePolygon {
           break;
         }
       }
-      System.out.println("");
       */
       
+      //System.out.println("");
       //System.out.println("Considering edge "+(edge.startPoint)+" -> "+(edge.endPoint));
 
       // Some edges are going to be given to us even when there's no real intersection, so do that as a sanity check, first.
       final GeoPoint[] travelCrossings = travelPlane.findIntersections(planetModel, edge.plane, checkPointCutoffPlane, checkPointOtherCutoffPlane, edge.startPlane, edge.endPlane);
       if (travelCrossings != null && travelCrossings.length == 0) {
+        //System.out.println(" No intersections with travel plane...");
         final GeoPoint[] testPointCrossings = testPointPlane.findIntersections(planetModel, edge.plane, testPointCutoffPlane, testPointOtherCutoffPlane, edge.startPlane, edge.endPlane);
         if (testPointCrossings != null && testPointCrossings.length == 0) {
           // As a last resort, see if the edge endpoints are on either plane.  This is sometimes necessary because the
           // intersection computation logic might not detect near-miss edges otherwise.
+          //System.out.println(" No intersections with testpoint plane...");
           if (!travelPlane.evaluateIsZero(edge.startPoint) && !travelPlane.evaluateIsZero(edge.endPoint) &&
             !testPointPlane.evaluateIsZero(edge.startPoint) && !testPointPlane.evaluateIsZero(edge.endPoint)) {
             return true;
+          } else {
+            //System.out.println(" Startpoint/travelPlane="+travelPlane.evaluate(edge.startPoint)+" Startpoint/testPointPlane="+testPointPlane.evaluate(edge.startPoint));
+            //System.out.println(" Endpoint/travelPlane="+travelPlane.evaluate(edge.endPoint)+" Endpoint/testPointPlane="+testPointPlane.evaluate(edge.endPoint));
           }
+        } else {
+          //System.out.println(" Intersection found with testPoint plane...");
         }
+      } else {
+        //System.out.println(" Intersection found with travel plane...");
       }
 
+      //System.out.println(" Edge intersects travel or testPoint plane");
       /*
       System.out.println(
         " start point travel dist="+travelPlane.evaluate(edge.startPoint)+"; end point travel dist="+travelPlane.evaluate(edge.endPoint));
@@ -1307,10 +1438,10 @@ class GeoComplexPolygon extends GeoBasePolygon {
       */
       
       // Determine crossings of this edge against all inside/outside planes.  There's no further need to look at the actual travel plane itself.
-      final GeoPoint[] travelInnerCrossings = travelInsidePlane.findCrossings(planetModel, edge.plane, checkPointCutoffPlane, insideTravelCutoffPlane, edge.startPlane, edge.endPlane);
-      final GeoPoint[] travelOuterCrossings = travelOutsidePlane.findCrossings(planetModel, edge.plane, checkPointCutoffPlane, outsideTravelCutoffPlane, edge.startPlane, edge.endPlane);
-      final GeoPoint[] testPointInnerCrossings = testPointInsidePlane.findCrossings(planetModel, edge.plane, testPointCutoffPlane, insideTestPointCutoffPlane, edge.startPlane, edge.endPlane);
-      final GeoPoint[] testPointOuterCrossings = testPointOutsidePlane.findCrossings(planetModel, edge.plane, testPointCutoffPlane, outsideTestPointCutoffPlane, edge.startPlane, edge.endPlane);
+      final GeoPoint[] travelInnerCrossings = computeCrossings(travelInsidePlane, edge, checkPointCutoffPlane, insideTravelCutoffPlane);
+      final GeoPoint[] travelOuterCrossings = computeCrossings(travelOutsidePlane, edge, checkPointCutoffPlane, outsideTravelCutoffPlane);
+      final GeoPoint[] testPointInnerCrossings = computeCrossings(testPointInsidePlane, edge, testPointCutoffPlane, insideTestPointCutoffPlane);
+      final GeoPoint[] testPointOuterCrossings = computeCrossings(testPointOutsidePlane, edge, testPointCutoffPlane, outsideTestPointCutoffPlane);
       
       // If the edge goes through the inner-inner intersection point, or the outer-outer intersection point, we need to be sure we count that only once.
       // It may appear in both lists.  Use a hash for this right now.

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/16e5ab16/lucene/spatial3d/src/test/org/apache/lucene/spatial3d/geom/GeoPolygonTest.java
----------------------------------------------------------------------
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 46750d4..455eee6 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
@@ -1522,7 +1522,6 @@ shape:
   }
 
   @Test
-  @AwaitsFix(bugUrl="https://issues.apache.org/jira/browse/LUCENE-8245")
   public void testLUCENE8245_case2() {
     //POLYGON((5.512285089810178 -26.833721534785912,12.13983320542565 -16.085163683089583,4.868755337835201 -9.167423203860656,0.0 -5.261747514529465,-15.696549288211289 -21.362181191487718,5.512285089810178 -26.833721534785912))
     final List<GeoPoint> points = new ArrayList<>();