From bc40f6c7e219c3def81e5d3bee6d5123cc4141e6 Mon Sep 17 00:00:00 2001 From: Karl Wright Date: Fri, 30 Mar 2018 06:43:42 -0400 Subject: [PATCH] LUCENE-8227: Redevelop path iterator implementations to make them robust against edges on paths. --- .../spatial3d/geom/GeoComplexPolygon.java | 696 ++++++------------ .../lucene/spatial3d/TestGeo3DPoint.java | 11 +- .../lucene/spatial3d/geom/GeoPolygonTest.java | 21 +- 3 files changed, 231 insertions(+), 497 deletions(-) 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 de12348de43..c8d643526fd 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 @@ -19,6 +19,8 @@ package org.apache.lucene.spatial3d.geom; import java.util.Arrays; import java.util.List; import java.util.ArrayList; +import java.util.Set; +import java.util.HashSet; import java.io.InputStream; import java.io.OutputStream; import java.io.IOException; @@ -157,6 +159,7 @@ class GeoComplexPolygon extends GeoBasePolygon { @Override public boolean isWithin(final double x, final double y, final double z) { + //System.out.println("\nIswithin called for ["+x+","+y+","+z+"]"); // If we're right on top of the point, we know the answer. if (testPoint.isNumericallyIdentical(x, y, z)) { return testPointInSet; @@ -366,11 +369,12 @@ class GeoComplexPolygon extends GeoBasePolygon { if (!firstLegTree.traverse(edgeIterator, firstLegValue)) { return true; } - edgeIterator.setSecondLeg(); + //edgeIterator.setSecondLeg(); if (!secondLegTree.traverse(edgeIterator, secondLegValue)) { return true; } - return ((edgeIterator.crossingCount & 1) == 0)?testPointInSet:!testPointInSet; + //System.out.println("Polarity vs. test point: "+(((edgeIterator.getCrossingCount() & 1) == 0)?"same":"different")+"; testPointInSet: "+testPointInSet); + return ((edgeIterator.getCrossingCount() & 1) == 0)?testPointInSet:!testPointInSet; } } @@ -502,6 +506,8 @@ class GeoComplexPolygon extends GeoBasePolygon { this.planeBounds.addPlane(pm, this.plane, this.startPlane, this.endPlane); //System.err.println("Recording edge "+this+" from "+startPoint+" to "+endPoint+"; bounds = "+planeBounds); } + + // Hashcode and equals are system default!! } /** @@ -798,7 +804,8 @@ class GeoComplexPolygon extends GeoBasePolygon { private final double thePointY; private final double thePointZ; - private int crossingCount = 0; + private int aboveCrossingCount = 0; + private int belowCrossingCount = 0; public FullLinearCrossingEdgeIterator(final Plane plane, final Plane abovePlane, final Plane belowPlane, final double thePointX, final double thePointY, final double thePointZ) { this.plane = plane; @@ -813,7 +820,11 @@ class GeoComplexPolygon extends GeoBasePolygon { @Override public int getCrossingCount() { - return crossingCount; + if (aboveCrossingCount < belowCrossingCount) { + return aboveCrossingCount; + } else { + return belowCrossingCount; + } } @Override @@ -822,131 +833,29 @@ class GeoComplexPolygon extends GeoBasePolygon { if (edge.plane.evaluateIsZero(thePointX, thePointY, thePointZ) && edge.startPlane.isWithin(thePointX, thePointY, thePointZ) && edge.endPlane.isWithin(thePointX, thePointY, thePointZ)) { return false; } - final GeoPoint[] crossingPoints = plane.findCrossings(planetModel, edge.plane, bound, edge.startPlane, edge.endPlane); - if (crossingPoints != null) { - // We need to handle the endpoint case, which is quite tricky. - for (final GeoPoint crossingPoint : crossingPoints) { - countCrossingPoint(crossingPoint, edge); - } + + // This should precisely mirror what is in DualCrossingIterator, but without the dual crossings. + // 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[] planeCrossings = plane.findIntersections(planetModel, edge.plane, bound, edge.startPlane, edge.endPlane); + if (planeCrossings != null && planeCrossings.length == 0) { + // No actual crossing + return true; } + + // 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); + + if (aboveCrossings != null) { + aboveCrossingCount += aboveCrossings.length; + } + if (belowCrossings != null) { + belowCrossingCount += belowCrossings.length; + } + return true; } - private void countCrossingPoint(final GeoPoint crossingPoint, final Edge edge) { - if (crossingPoint.isNumericallyIdentical(edge.startPoint)) { - // We have to figure out if this crossing should be counted. - - // Does the crossing for this edge go up, or down? Or can't we tell? - final GeoPoint[] aboveIntersections = abovePlane.findIntersections(planetModel, edge.plane, edge.startPlane, edge.endPlane); - final GeoPoint[] belowIntersections = belowPlane.findIntersections(planetModel, edge.plane, edge.startPlane, edge.endPlane); - - assert !(aboveIntersections.length > 0 && belowIntersections.length > 0) : "edge that ends in a crossing can't both up and down"; - - if (aboveIntersections.length == 0 && belowIntersections.length == 0) { - return; - } - - final boolean edgeCrossesAbove = aboveIntersections.length > 0; - - // This depends on the previous edge that first departs from identicalness. - Edge assessEdge = edge; - GeoPoint[] assessAboveIntersections; - GeoPoint[] assessBelowIntersections; - while (true) { - assessEdge = assessEdge.previous; - assessAboveIntersections = abovePlane.findIntersections(planetModel, assessEdge.plane, assessEdge.startPlane, assessEdge.endPlane); - assessBelowIntersections = belowPlane.findIntersections(planetModel, assessEdge.plane, assessEdge.startPlane, assessEdge.endPlane); - - assert !(assessAboveIntersections.length > 0 && assessBelowIntersections.length > 0) : "assess edge that ends in a crossing can't both up and down"; - - if (assessAboveIntersections.length == 0 && assessBelowIntersections.length == 0) { - continue; - } - break; - } - - // Basically, we now want to assess whether both edges that come together at this endpoint leave the plane in opposite - // directions. If they do, then we should count it as a crossing; if not, we should not. We also have to remember that - // each edge we look at can also be looked at again if it, too, seems to cross the plane. - - // To handle the latter situation, we need to know if the other edge will be looked at also, and then we can make - // a decision whether to count or not based on that. - - // Compute the crossing points of this other edge. - final GeoPoint[] otherCrossingPoints = plane.findCrossings(planetModel, assessEdge.plane, bound, assessEdge.startPlane, assessEdge.endPlane); - - // Look for a matching endpoint. If the other endpoint doesn't show up, it is either out of bounds (in which case the - // transition won't be counted for that edge), or it is not a crossing for that edge (so, same conclusion). - for (final GeoPoint otherCrossingPoint : otherCrossingPoints) { - if (otherCrossingPoint.isNumericallyIdentical(assessEdge.endPoint)) { - // Found it! - // Both edges will try to contribute to the crossing count. By convention, we'll only include the earlier one. - // Since we're the latter point, we exit here in that case. - return; - } - } - - // Both edges will not count the same point, so we can proceed. We need to determine the direction of both edges at the - // point where they hit the plane. This may be complicated by the 3D geometry; it may not be safe just to look at the endpoints of the edges - // and make an assessment that way, since a single edge can intersect the plane at more than one point. - - final boolean assessEdgeAbove = assessAboveIntersections.length > 0; - if (assessEdgeAbove != edgeCrossesAbove) { - crossingCount++; - } - - } else if (crossingPoint.isNumericallyIdentical(edge.endPoint)) { - // Figure out if the crossing should be counted. - - // Does the crossing for this edge go up, or down? Or can't we tell? - final GeoPoint[] aboveIntersections = abovePlane.findIntersections(planetModel, edge.plane, edge.startPlane, edge.endPlane); - final GeoPoint[] belowIntersections = belowPlane.findIntersections(planetModel, edge.plane, edge.startPlane, edge.endPlane); - - assert !(aboveIntersections.length > 0 && belowIntersections.length > 0) : "edge that ends in a crossing can't both up and down"; - - if (aboveIntersections.length == 0 && belowIntersections.length == 0) { - return; - } - - final boolean edgeCrossesAbove = aboveIntersections.length > 0; - - // This depends on the previous edge that first departs from identicalness. - Edge assessEdge = edge; - GeoPoint[] assessAboveIntersections; - GeoPoint[] assessBelowIntersections; - while (true) { - assessEdge = assessEdge.next; - assessAboveIntersections = abovePlane.findIntersections(planetModel, assessEdge.plane, assessEdge.startPlane, assessEdge.endPlane); - assessBelowIntersections = belowPlane.findIntersections(planetModel, assessEdge.plane, assessEdge.startPlane, assessEdge.endPlane); - - assert !(assessAboveIntersections.length > 0 && assessBelowIntersections.length > 0) : "assess edge that ends in a crossing can't both up and down"; - - if (assessAboveIntersections.length == 0 && assessBelowIntersections.length == 0) { - continue; - } - break; - } - - // Basically, we now want to assess whether both edges that come together at this endpoint leave the plane in opposite - // directions. If they do, then we should count it as a crossing; if not, we should not. We also have to remember that - // each edge we look at can also be looked at again if it, too, seems to cross the plane. - - // By definition, we're the earlier plane in this case, so any crossing we detect we must count, by convention. It is unnecessary - // to consider what the other edge does, because when we get to it, it will look back and figure out what we did for this one. - - // We need to determine the direction of both edges at the - // point where they hit the plane. This may be complicated by the 3D geometry; it may not be safe just to look at the endpoints of the edges - // and make an assessment that way, since a single edge can intersect the plane at more than one point. - - final boolean assessEdgeAbove = assessAboveIntersections.length > 0; - if (assessEdgeAbove != edgeCrossesAbove) { - crossingCount++; - } - - } else { - crossingCount++; - } - } } /** Create a linear crossing edge iterator with the appropriate cutoff planes given the geometry. @@ -980,7 +889,8 @@ class GeoComplexPolygon extends GeoBasePolygon { private final double thePointY; private final double thePointZ; - private int crossingCount = 0; + private int aboveCrossingCount = 0; + private int belowCrossingCount = 0; public SectorLinearCrossingEdgeIterator(final Plane plane, final Plane abovePlane, final Plane belowPlane, final double thePointX, final double thePointY, final double thePointZ) { this.plane = plane; @@ -996,7 +906,11 @@ class GeoComplexPolygon extends GeoBasePolygon { @Override public int getCrossingCount() { - return crossingCount; + if (aboveCrossingCount < belowCrossingCount) { + return aboveCrossingCount; + } else { + return belowCrossingCount; + } } @Override @@ -1005,139 +919,38 @@ class GeoComplexPolygon extends GeoBasePolygon { if (edge.plane.evaluateIsZero(thePointX, thePointY, thePointZ) && edge.startPlane.isWithin(thePointX, thePointY, thePointZ) && edge.endPlane.isWithin(thePointX, thePointY, thePointZ)) { return false; } - final GeoPoint[] crossingPoints = plane.findCrossings(planetModel, edge.plane, bound1, bound2, edge.startPlane, edge.endPlane); - if (crossingPoints != null) { - // We need to handle the endpoint case, which is quite tricky. - for (final GeoPoint crossingPoint : crossingPoints) { - countCrossingPoint(crossingPoint, edge); - } + + // This should precisely mirror what is in DualCrossingIterator, but without the dual crossings. + // 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[] planeCrossings = plane.findIntersections(planetModel, edge.plane, bound1, bound2, edge.startPlane, edge.endPlane); + if (planeCrossings != null && planeCrossings.length == 0) { + // No actual crossing + return true; } + + // 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); + + if (aboveCrossings != null) { + aboveCrossingCount += aboveCrossings.length; + } + if (belowCrossings != null) { + belowCrossingCount += belowCrossings.length; + } + return true; } - private void countCrossingPoint(final GeoPoint crossingPoint, final Edge edge) { - if (crossingPoint.isNumericallyIdentical(edge.startPoint)) { - - // The crossing point is shared by two edges. If we are going to count it, this is the edge we'll count it on. - // We have to figure out if this crossing should be counted. - - // We look at the above plane and the below plane and see if we cross either of them. - // If we cross NEITHER of them: we're in the "zone" between the planes, and this edge doesn't count. - - // Does the crossing for this edge go up, or down? Or can't we tell? - final GeoPoint[] aboveIntersections = abovePlane.findCrossings(planetModel, edge.plane, edge.startPlane, edge.endPlane); - final GeoPoint[] belowIntersections = belowPlane.findCrossings(planetModel, edge.plane, edge.startPlane, edge.endPlane); - - if ((aboveIntersections == null || aboveIntersections.length == 0) && (belowIntersections == null || belowIntersections.length == 0)) { - return; - } - - // A null value means we have a situation where the edge is numerically identical. That's not counted as a "crossing". - - assert !(aboveIntersections != null && aboveIntersections.length > 0 && belowIntersections != null && belowIntersections.length > 0) : "edge that ends in a crossing can't be both up and down!"; - - final boolean edgeCrossesAbove = aboveIntersections != null && aboveIntersections.length > 0; - - // This depends on the previous edge that first departs from identicalness. - Edge assessEdge = edge; - GeoPoint[] assessAboveIntersections; - GeoPoint[] assessBelowIntersections; - while (true) { - assessEdge = assessEdge.previous; - assessAboveIntersections = abovePlane.findCrossings(planetModel, assessEdge.plane, assessEdge.startPlane, assessEdge.endPlane); - assessBelowIntersections = belowPlane.findCrossings(planetModel, assessEdge.plane, assessEdge.startPlane, assessEdge.endPlane); - - if ((assessAboveIntersections == null || assessAboveIntersections.length == 0) && (assessBelowIntersections == null || assessBelowIntersections.length == 0)) { - continue; - } - break; - } - - // Basically, we now want to assess whether both edges that come together at this endpoint leave the plane in opposite - // directions. If they do, then we should count it as a crossing; if not, we should not. We also have to remember that - // each edge we look at can also be looked at again if it, too, seems to cross the plane. - - // To handle the latter situation, we need to know if the other edge will be looked at also, and then we can make - // a decision whether to count or not based on that. - - // Compute the crossing points of this other edge. - final GeoPoint[] otherCrossingPoints = plane.findCrossings(planetModel, assessEdge.plane, bound1, bound2, assessEdge.startPlane, assessEdge.endPlane); - - // Look for a matching endpoint. If the other endpoint doesn't show up, it is either out of bounds (in which case the - // transition won't be counted for that edge), or it is not a crossing for that edge (so, same conclusion). - for (final GeoPoint otherCrossingPoint : otherCrossingPoints) { - if (otherCrossingPoint.isNumericallyIdentical(assessEdge.endPoint)) { - // Found it! - // Both edges will try to contribute to the crossing count. By convention, we'll only include the earlier one. - // Since we're the latter point, we exit here in that case. - return; - } - } - - // Both edges will not count the same point, so we can proceed. We need to determine the direction of both edges at the - // point where they hit the plane. This may be complicated by the 3D geometry; it may not be safe just to look at the endpoints of the edges - // and make an assessment that way, since a single edge can intersect the plane at more than one point. - - final boolean assessEdgeAbove = assessAboveIntersections != null && assessAboveIntersections.length > 0; - if (assessEdgeAbove != edgeCrossesAbove) { - crossingCount++; - } - - } else if (crossingPoint.isNumericallyIdentical(edge.endPoint)) { - // Figure out if the crossing should be counted. - - // Does the crossing for this edge go up, or down? Or can't we tell? - final GeoPoint[] aboveIntersections = abovePlane.findCrossings(planetModel, edge.plane, edge.startPlane, edge.endPlane); - final GeoPoint[] belowIntersections = belowPlane.findCrossings(planetModel, edge.plane, edge.startPlane, edge.endPlane); - - if ((aboveIntersections == null || aboveIntersections.length == 0) && (belowIntersections == null || belowIntersections.length == 0)) { - return; - } - - final boolean edgeCrossesAbove = aboveIntersections != null && aboveIntersections.length > 0; - - // This depends on the previous edge that first departs from identicalness. - Edge assessEdge = edge; - GeoPoint[] assessAboveIntersections; - GeoPoint[] assessBelowIntersections; - while (true) { - assessEdge = assessEdge.next; - assessAboveIntersections = abovePlane.findCrossings(planetModel, assessEdge.plane, assessEdge.startPlane, assessEdge.endPlane); - assessBelowIntersections = belowPlane.findCrossings(planetModel, assessEdge.plane, assessEdge.startPlane, assessEdge.endPlane); - - if (assessAboveIntersections != null && assessAboveIntersections.length == 0 && assessBelowIntersections != null && assessBelowIntersections.length == 0) { - continue; - } - break; - } - - // Basically, we now want to assess whether both edges that come together at this endpoint leave the plane in opposite - // directions. If they do, then we should count it as a crossing; if not, we should not. We also have to remember that - // each edge we look at can also be looked at again if it, too, seems to cross the plane. - - // By definition, we're the earlier plane in this case, so any crossing we detect we must count, by convention. It is unnecessary - // to consider what the other edge does, because when we get to it, it will look back and figure out what we did for this one. - - // We need to determine the direction of both edges at the - // point where they hit the plane. This may be complicated by the 3D geometry; it may not be safe just to look at the endpoints of the edges - // and make an assessment that way, since a single edge can intersect the plane at more than one point. - - final boolean assessEdgeAbove = assessAboveIntersections != null && assessAboveIntersections.length > 0; - if (assessEdgeAbove != edgeCrossesAbove) { - crossingCount++; - } - - } else { - crossingCount++; - } - } } /** Count the number of verifiable edge crossings for a dual-leg journey. */ private class DualCrossingEdgeIterator implements EdgeIterator { - private boolean isSecondLeg = false; + // 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. + private Set seenEdges = null; private final Plane testPointPlane; private final Plane testPointAbovePlane; @@ -1163,10 +976,12 @@ class GeoComplexPolygon extends GeoBasePolygon { private Plane travelOutsidePlane; private SidedPlane insideTestPointCutoffPlane; private SidedPlane insideTravelCutoffPlane; + private SidedPlane outsideTestPointCutoffPlane; + private SidedPlane outsideTravelCutoffPlane; - // The counter - - public int crossingCount = 0; + // The counters + public int innerCrossingCount = 0; + public int outerCrossingCount = 0; public DualCrossingEdgeIterator(final Plane testPointPlane, final Plane testPointAbovePlane, final Plane testPointBelowPlane, final Plane travelPlane, final double thePointX, final double thePointY, final double thePointZ, final GeoPoint intersectionPoint) { @@ -1179,7 +994,7 @@ class GeoComplexPolygon extends GeoBasePolygon { this.thePointZ = thePointZ; this.intersectionPoint = intersectionPoint; - //System.err.println("Intersection point = "+intersectionPoint); + //System.out.println("Intersection point = "+intersectionPoint); assert travelPlane.evaluateIsZero(intersectionPoint) : "intersection point must be on travel plane"; assert testPointPlane.evaluateIsZero(intersectionPoint) : "intersection point must be on test point plane"; @@ -1216,6 +1031,9 @@ class GeoComplexPolygon extends GeoBasePolygon { final Plane travelAbovePlane = new Plane(travelPlane, true); final Plane travelBelowPlane = new Plane(travelPlane, false); + // Each of these can generate two solutions. We need to refine them to generate only one somehow -- the one in the same area of the world as intersectionPoint. + // Since the travel/testpoint planes have one fixed coordinate, and that is represented by the plane's D value, it should be possible to choose based on the + // point's coordinates. final GeoPoint[] aboveAbove = travelAbovePlane.findIntersections(planetModel, testPointAbovePlane, intersectionBound1, intersectionBound2); assert aboveAbove != null : "Above + above should not be coplanar"; final GeoPoint[] aboveBelow = travelAbovePlane.findIntersections(planetModel, testPointBelowPlane, intersectionBound1, intersectionBound2); @@ -1227,265 +1045,197 @@ class GeoComplexPolygon extends GeoBasePolygon { assert ((aboveAbove.length > 0)?1:0) + ((aboveBelow.length > 0)?1:0) + ((belowBelow.length > 0)?1:0) + ((belowAbove.length > 0)?1:0) == 1 : "Can be exactly one inside point, instead was: aa="+aboveAbove.length+" ab=" + aboveBelow.length+" bb="+ belowBelow.length+" ba=" + belowAbove.length; + final GeoPoint[] insideInsidePoints; if (aboveAbove.length > 0) { travelInsidePlane = travelAbovePlane; testPointInsidePlane = testPointAbovePlane; travelOutsidePlane = travelBelowPlane; testPointOutsidePlane = testPointBelowPlane; + insideInsidePoints = aboveAbove; } else if (aboveBelow.length > 0) { travelInsidePlane = travelAbovePlane; testPointInsidePlane = testPointBelowPlane; travelOutsidePlane = travelBelowPlane; testPointOutsidePlane = testPointAbovePlane; + insideInsidePoints = aboveBelow; } else if (belowBelow.length > 0) { travelInsidePlane = travelBelowPlane; testPointInsidePlane = testPointBelowPlane; travelOutsidePlane = travelAbovePlane; testPointOutsidePlane = testPointAbovePlane; + insideInsidePoints = belowBelow; } else { travelInsidePlane = travelBelowPlane; testPointInsidePlane = testPointAbovePlane; travelOutsidePlane = travelAbovePlane; testPointOutsidePlane = testPointBelowPlane; + insideInsidePoints = belowAbove; } - insideTravelCutoffPlane = new SidedPlane(thePointX, thePointY, thePointZ, testPointInsidePlane, testPointInsidePlane.D); - insideTestPointCutoffPlane = new SidedPlane(testPoint, travelInsidePlane, travelInsidePlane.D); + // Get the inside-inside intersection point + // Picking which point, out of two, that corresponds to the already-selected intersectionPoint, is tricky, but it must be done. + // We expect the choice to be within a small delta of the intersection point in 2 of the dimensions, but not the third + final GeoPoint insideInsidePoint = pickProximate(insideInsidePoints); + + // Get the outside-outside intersection point + final GeoPoint[] outsideOutsidePoints = testPointOutsidePlane.findIntersections(planetModel, travelOutsidePlane); //these don't add anything: , checkPointCutoffPlane, testPointCutoffPlane); + final GeoPoint outsideOutsidePoint = pickProximate(outsideOutsidePoints); + + insideTravelCutoffPlane = new SidedPlane(thePointX, thePointY, thePointZ, travelInsidePlane, insideInsidePoint); + outsideTravelCutoffPlane = new SidedPlane(thePointX, thePointY, thePointZ, travelInsidePlane, outsideOutsidePoint); + insideTestPointCutoffPlane = new SidedPlane(testPoint, testPointInsidePlane, insideInsidePoint); + outsideTestPointCutoffPlane = new SidedPlane(testPoint, testPointOutsidePlane, outsideOutsidePoint); + + /* + System.out.println("insideTravelCutoffPlane = "+insideTravelCutoffPlane); + System.out.println("outsideTravelCutoffPlane = "+outsideTravelCutoffPlane); + System.out.println("insideTestPointCutoffPlane = "+insideTestPointCutoffPlane); + System.out.println("outsideTestPointCutoffPlane = "+outsideTestPointCutoffPlane); + */ + computedInsideOutside = true; } } - public void setSecondLeg() { - isSecondLeg = true; + private GeoPoint pickProximate(final GeoPoint[] points) { + if (points.length == 0) { + throw new IllegalArgumentException("No off-plane intersection points were found; can't compute traversal"); + } else if (points.length == 1) { + return points[0]; + } else { + final double p1dist = computeSquaredDistance(points[0], intersectionPoint); + final double p2dist = computeSquaredDistance(points[1], intersectionPoint); + if (p1dist < p2dist) { + return points[0]; + } else if (p2dist < p1dist) { + return points[1]; + } else { + throw new IllegalArgumentException("Neither off-plane intersection point matched intersection point; intersection = "+intersectionPoint+"; offplane choice 0: "+points[0]+"; offplane choice 1: "+points[1]); + } + } + } + + public int getCrossingCount() { + // Doesn't return the actual crossing count -- just gets the even/odd part right + if (innerCrossingCount < outerCrossingCount) { + return innerCrossingCount; + } else { + return outerCrossingCount; + } } @Override public boolean matches(final Edge edge) { - //System.err.println("Processing edge "+edge+", startpoint="+edge.startPoint+" endpoint="+edge.endPoint); - // Early exit if the point is on the edge. + // Early exit if the point is on the edge, in which case we accidentally discovered the answer. if (edge.plane.evaluateIsZero(thePointX, thePointY, thePointZ) && edge.startPlane.isWithin(thePointX, thePointY, thePointZ) && edge.endPlane.isWithin(thePointX, thePointY, thePointZ)) { - //System.err.println(" Check point is on edge: isWithin = true"); return false; } - // If the intersection point lies on this edge, we should still be able to consider crossing points only. - // Even if an intersection point is eliminated because it's not a crossing of one plane, it will have to be a crossing - // for at least one of the two planes in order to be a legitimate crossing of the combined path. - final GeoPoint[] crossingPoints; - if (isSecondLeg) { - //System.err.println(" check point plane = "+travelPlane); - crossingPoints = travelPlane.findCrossings(planetModel, edge.plane, checkPointCutoffPlane, checkPointOtherCutoffPlane, edge.startPlane, edge.endPlane); - } else { - //System.err.println(" test point plane = "+testPointPlane); - crossingPoints = testPointPlane.findCrossings(planetModel, edge.plane, testPointCutoffPlane, testPointOtherCutoffPlane, edge.startPlane, edge.endPlane); + + // All edges that touch the travel planes get assessed the same. So, for each intersecting edge on both legs: + // (1) If the edge contains the intersection point, we analyze it on only one leg. For the other leg, we do nothing. + // (2) We compute the crossings of the edge with ALL FOUR inner and outer bounding planes. + // (3) We add the numbers of each kind of crossing to the total for that class of crossing (innerTotal and outerTotal). + // (4) When done all edges tallied in this way, we take min(innerTotal, outerTotal) and assume that is the number of crossings. + // + // Q: What if we see the same edge in both traversals? + // A: We should really evaluate it only in one. Keep a hash of the edges we've looked at already and don't process edges twice. + + // Every edge should be looked at only once. + if (seenEdges != null && seenEdges.contains(edge)) { + return true; } - if (crossingPoints != null) { - // We need to handle the endpoint case, which is quite tricky. - for (final GeoPoint crossingPoint : crossingPoints) { - countCrossingPoint(crossingPoint, edge); + if (seenEdges == null) { + seenEdges = new HashSet<>(); + } + seenEdges.add(edge); + + //System.out.println("Considering edge "+(edge.startPoint)+" -> "+(edge.endPoint)); + + // We've never seen this edge before. Evaluate it in the context of inner and outer planes. + computeInsideOutside(); + + /* + System.out.println("\nThe following edges should intersect the travel/testpoint planes:"); + Edge thisEdge = edge; + while (true) { + final GeoPoint[] travelCrossings = travelPlane.findIntersections(planetModel, thisEdge.plane, checkPointCutoffPlane, checkPointOtherCutoffPlane, thisEdge.startPlane, thisEdge.endPlane); + if (travelCrossings == null || travelCrossings.length > 0) { + System.out.println("Travel plane: "+thisEdge.startPoint+" -> "+thisEdge.endPoint); + } + final GeoPoint[] testPointCrossings = testPointPlane.findIntersections(planetModel, thisEdge.plane, testPointCutoffPlane, testPointOtherCutoffPlane, thisEdge.startPlane, thisEdge.endPlane); + if (testPointCrossings == null || testPointCrossings.length > 0) { + System.out.println("Test point plane: "+thisEdge.startPoint+" -> "+thisEdge.endPoint); + } + thisEdge = thisEdge.next; + if (thisEdge == edge) { + break; } - //System.err.println(" All crossing points processed"); - } else { - //System.err.println(" No crossing points!"); } + System.out.println(""); + */ + + // 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) { + final GeoPoint[] testPointCrossings = testPointPlane.findIntersections(planetModel, edge.plane, testPointCutoffPlane, testPointOtherCutoffPlane, edge.startPlane, edge.endPlane); + if (testPointCrossings != null && testPointCrossings.length == 0) { + return true; + } + } + + // 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); + + // 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. + final Set countingHash = new HashSet<>(2); + + if (travelInnerCrossings != null) { + for (final GeoPoint crossing : travelInnerCrossings) { + //System.out.println(" Travel inner point "+crossing); + countingHash.add(crossing); + } + } + if (testPointInnerCrossings != null) { + for (final GeoPoint crossing : testPointInnerCrossings) { + //System.out.println(" Test point inner point "+crossing); + countingHash.add(crossing); + } + } + //System.out.println(" Edge added "+countingHash.size()+" to innerCrossingCount"); + innerCrossingCount += countingHash.size(); + + countingHash.clear(); + if (travelOuterCrossings != null) { + for (final GeoPoint crossing : travelOuterCrossings) { + //System.out.println(" Travel outer point "+crossing); + countingHash.add(crossing); + } + } + if (testPointOuterCrossings != null) { + for (final GeoPoint crossing : testPointOuterCrossings) { + //System.out.println(" Test point outer point "+crossing); + countingHash.add(crossing); + } + } + //System.out.println(" Edge added "+countingHash.size()+" to outerCrossingCount"); + outerCrossingCount += countingHash.size(); + return true; } - private void countCrossingPoint(final GeoPoint crossingPoint, final Edge edge) { - //System.err.println(" Crossing point "+crossingPoint); - // We consider crossing points only in this method. - // Unlike the linear case, there are additional cases when: - // (1) The crossing point and the intersection point are the same, but are not the endpoint of an edge; - // (2) The crossing point and the intersection point are the same, and they *are* the endpoint of an edge. - // The other logical difference is that crossings of all kinds have to be considered so that: - // (a) both inside edges are considered together at all times; - // (b) both outside edges are considered together at all times; - // (c) inside edge crossings that are between the other leg's inside and outside edge are ignored. - - // Intersection point crossings are either simple, or a crossing on an endpoint. - // In either case, we have to be sure to count each edge only once, since it might appear in both the - // first leg and the second. If the first leg can process it, it should, and the second should skip it. - if (crossingPoint.isNumericallyIdentical(intersectionPoint)) { - //System.err.println(" Crosses intersection point."); - if (isSecondLeg) { - // See whether this edge would have been processed in the first leg; if so, we skip it. - final GeoPoint[] firstLegCrossings = testPointPlane.findCrossings(planetModel, edge.plane, testPointCutoffPlane, testPointOtherCutoffPlane, edge.startPlane, edge.endPlane); - for (final GeoPoint firstLegCrossing : firstLegCrossings) { - if (firstLegCrossing.isNumericallyIdentical(intersectionPoint)) { - // We already processed it, so we're done here. - //System.err.println(" Already processed on previous leg: exit"); - return; - } - } - } - } - - // Plane crossing, either first leg or second leg - - if (crossingPoint.isNumericallyIdentical(edge.startPoint)) { - //System.err.println(" Crossing point = edge.startPoint"); - // We have to figure out if this crossing should be counted. - computeInsideOutside(); - - // Does the crossing for this edge go up, or down? Or can't we tell? - final GeoPoint[] insideTestPointPlaneIntersections = testPointInsidePlane.findCrossings(planetModel, edge.plane, edge.startPlane, edge.endPlane, insideTestPointCutoffPlane); - final GeoPoint[] insideTravelPlaneIntersections = travelInsidePlane.findCrossings(planetModel, edge.plane, edge.startPlane, edge.endPlane, insideTravelCutoffPlane); - final GeoPoint[] outsideTestPointPlaneIntersections = testPointOutsidePlane.findCrossings(planetModel, edge.plane, edge.startPlane, edge.endPlane); - final GeoPoint[] outsideTravelPlaneIntersections = travelOutsidePlane.findCrossings(planetModel, edge.plane, edge.startPlane, edge.endPlane); - - if ((insideTestPointPlaneIntersections == null || insideTestPointPlaneIntersections.length == 0) && - (insideTravelPlaneIntersections == null || insideTravelPlaneIntersections.length == 0) && - (outsideTestPointPlaneIntersections == null || outsideTestPointPlaneIntersections.length == 0) && - (outsideTravelPlaneIntersections == null || outsideTravelPlaneIntersections.length == 0)) { - //System.err.println(" No inside or outside crossings found"); - return; - } - - final boolean edgeCrossesInside = insideTestPointPlaneIntersections.length + insideTravelPlaneIntersections.length > 0; - - // This depends on the previous edge that first departs from identicalness. - Edge assessEdge = edge; - GeoPoint[] assessInsideTestPointIntersections; - GeoPoint[] assessInsideTravelIntersections; - GeoPoint[] assessOutsideTestPointIntersections; - GeoPoint[] assessOutsideTravelIntersections; - while (true) { - assessEdge = assessEdge.previous; - assessInsideTestPointIntersections = testPointInsidePlane.findIntersections(planetModel, assessEdge.plane, assessEdge.startPlane, assessEdge.endPlane, insideTestPointCutoffPlane); - assessInsideTravelIntersections = travelInsidePlane.findIntersections(planetModel, assessEdge.plane, assessEdge.startPlane, assessEdge.endPlane, insideTravelCutoffPlane); - assessOutsideTestPointIntersections = testPointOutsidePlane.findIntersections(planetModel, assessEdge.plane, assessEdge.startPlane, assessEdge.endPlane); - assessOutsideTravelIntersections = travelOutsidePlane.findIntersections(planetModel, assessEdge.plane, assessEdge.startPlane, assessEdge.endPlane); - - // If the assess edge is numerically identical to the edge we're trying to find the intersections with, there's not really a crossing, so count it as zero. - - if ((assessInsideTestPointIntersections == null || assessInsideTestPointIntersections.length == 0) && - (assessInsideTravelIntersections == null || assessInsideTravelIntersections.length == 0) && - (assessOutsideTestPointIntersections == null || assessOutsideTestPointIntersections.length == 0) && - (assessOutsideTravelIntersections == null || assessOutsideTravelIntersections.length == 0)) { - continue; - } - break; - } - - // Basically, we now want to assess whether both edges that come together at this endpoint leave the plane in opposite - // directions. If they do, then we should count it as a crossing; if not, we should not. We also have to remember that - // each edge we look at can also be looked at again if it, too, seems to cross the plane. - - // To handle the latter situation, we need to know if the other edge will be looked at also, and then we can make - // a decision whether to count or not based on that. - - // Compute the crossing points of this other edge. - final GeoPoint[] otherCrossingPoints; - if (isSecondLeg) { - otherCrossingPoints = travelPlane.findCrossings(planetModel, assessEdge.plane, checkPointCutoffPlane, checkPointOtherCutoffPlane, assessEdge.startPlane, assessEdge.endPlane); - } else { - otherCrossingPoints = testPointPlane.findCrossings(planetModel, assessEdge.plane, testPointCutoffPlane, testPointOtherCutoffPlane, assessEdge.startPlane, assessEdge.endPlane); - } - - if (otherCrossingPoints == null) { - // The assessEdge plane is the same as the travel plane. We consider this the same as "no crossing". - return; - } - - // Look for a matching endpoint. If the other endpoint doesn't show up, it is either out of bounds (in which case the - // transition won't be counted for that edge), or it is not a crossing for that edge (so, same conclusion). - for (final GeoPoint otherCrossingPoint : otherCrossingPoints) { - if (otherCrossingPoint.isNumericallyIdentical(assessEdge.endPoint)) { - // Found it! - // Both edges will try to contribute to the crossing count. By convention, we'll only include the earlier one. - // Since we're the latter point, we exit here in that case. - //System.err.println(" Earlier point fired, so this one shouldn't"); - return; - } - } - - // Both edges will not count the same point, so we can proceed. We need to determine the direction of both edges at the - // point where they hit the plane. This may be complicated by the 3D geometry; it may not be safe just to look at the endpoints of the edges - // and make an assessment that way, since a single edge can intersect the plane at more than one point. - - final boolean assessEdgeInside = (assessInsideTestPointIntersections != null && assessInsideTestPointIntersections.length > 0) || - (assessInsideTravelIntersections != null && assessInsideTravelIntersections.length > 0); - if (assessEdgeInside != edgeCrossesInside) { - //System.err.println(" Incrementing crossing count"); - crossingCount++; - } else { - //System.err.println(" Entered and exited on same side"); - } - - } else if (crossingPoint.isNumericallyIdentical(edge.endPoint)) { - //System.err.println(" Crossing point = edge.endPoint"); - // Figure out if the crossing should be counted. - computeInsideOutside(); - - // If the assess edge is numerically identical to the edge we're trying to find the intersections with, there's not really a crossing, so count it as zero. - - // Does the crossing for this edge go up, or down? Or can't we tell? - final GeoPoint[] insideTestPointPlaneIntersections = testPointInsidePlane.findIntersections(planetModel, edge.plane, edge.startPlane, edge.endPlane, insideTestPointCutoffPlane); - final GeoPoint[] insideTravelPlaneIntersections = travelInsidePlane.findIntersections(planetModel, edge.plane, edge.startPlane, edge.endPlane, insideTravelCutoffPlane); - final GeoPoint[] outsideTestPointPlaneIntersections = testPointOutsidePlane.findIntersections(planetModel, edge.plane, edge.startPlane, edge.endPlane); - final GeoPoint[] outsideTravelPlaneIntersections = travelOutsidePlane.findIntersections(planetModel, edge.plane, edge.startPlane, edge.endPlane); - - // An edge can cross both outside and inside, because of the corner. But it can be considered to cross the inside ONLY if it crosses either of the inside edges. - - if ((insideTestPointPlaneIntersections == null || insideTestPointPlaneIntersections.length == 0) && - (insideTravelPlaneIntersections == null || insideTravelPlaneIntersections.length == 0) && - (outsideTestPointPlaneIntersections == null || outsideTestPointPlaneIntersections.length == 0) && - (outsideTravelPlaneIntersections == null || outsideTravelPlaneIntersections.length == 0)) { - //System.err.println(" No inside or outside crossings found"); - return; - } - - final boolean edgeCrossesInside = (insideTestPointPlaneIntersections !=null && insideTestPointPlaneIntersections.length > 0) || - (insideTravelPlaneIntersections != null && insideTravelPlaneIntersections.length > 0); - - // This depends on the previous edge that first departs from identicalness. - Edge assessEdge = edge; - GeoPoint[] assessInsideTestPointIntersections; - GeoPoint[] assessInsideTravelIntersections; - GeoPoint[] assessOutsideTestPointIntersections; - GeoPoint[] assessOutsideTravelIntersections; - while (true) { - assessEdge = assessEdge.next; - assessInsideTestPointIntersections = testPointInsidePlane.findIntersections(planetModel, assessEdge.plane, assessEdge.startPlane, assessEdge.endPlane, insideTestPointCutoffPlane); - assessInsideTravelIntersections = travelInsidePlane.findIntersections(planetModel, assessEdge.plane, assessEdge.startPlane, assessEdge.endPlane, insideTravelCutoffPlane); - assessOutsideTestPointIntersections = testPointOutsidePlane.findIntersections(planetModel, assessEdge.plane, assessEdge.startPlane, assessEdge.endPlane); - assessOutsideTravelIntersections = travelOutsidePlane.findIntersections(planetModel, assessEdge.plane, assessEdge.startPlane, assessEdge.endPlane); - - if ((assessInsideTestPointIntersections == null || assessInsideTestPointIntersections.length == 0) && - (assessInsideTravelIntersections == null || assessInsideTravelIntersections.length == 0) && - (assessOutsideTestPointIntersections == null || assessOutsideTestPointIntersections.length == 0) && - (assessOutsideTravelIntersections == null || assessOutsideTravelIntersections.length == 0)) { - continue; - } - break; - } - - // Basically, we now want to assess whether both edges that come together at this endpoint leave the plane in opposite - // directions. If they do, then we should count it as a crossing; if not, we should not. We also have to remember that - // each edge we look at can also be looked at again if it, too, seems to cross the plane. - - // By definition, we're the earlier plane in this case, so any crossing we detect we must count, by convention. It is unnecessary - // to consider what the other edge does, because when we get to it, it will look back and figure out what we did for this one. - - // We need to determine the direction of both edges at the - // point where they hit the plane. This may be complicated by the 3D geometry; it may not be safe just to look at the endpoints of the edges - // and make an assessment that way, since a single edge can intersect the plane at more than one point. - - final boolean assessEdgeInside = (assessInsideTestPointIntersections !=null && assessInsideTestPointIntersections.length > 0) || - (assessInsideTravelIntersections != null && assessInsideTravelIntersections.length > 0); - if (assessEdgeInside != edgeCrossesInside) { - //System.err.println(" Incrementing crossing count"); - crossingCount++; - } else { - //System.err.println(" Entered and exited on same side"); - } - } else { - //System.err.println(" Not a special case: incrementing crossing count"); - // Not a special case, so we can safely count a crossing. - crossingCount++; - } - } } - + + private static double computeSquaredDistance(final GeoPoint checkPoint, final GeoPoint intersectionPoint) { + final double distanceX = checkPoint.x - intersectionPoint.x; + final double distanceY = checkPoint.y - intersectionPoint.y; + final double distanceZ = checkPoint.z - intersectionPoint.z; + return distanceX * distanceX + distanceY * distanceY + distanceZ * distanceZ; + } + @Override public boolean equals(Object o) { if (!(o instanceof GeoComplexPolygon)) diff --git a/lucene/spatial3d/src/test/org/apache/lucene/spatial3d/TestGeo3DPoint.java b/lucene/spatial3d/src/test/org/apache/lucene/spatial3d/TestGeo3DPoint.java index 38b4114a4f3..5d58d5ebe1f 100644 --- a/lucene/spatial3d/src/test/org/apache/lucene/spatial3d/TestGeo3DPoint.java +++ b/lucene/spatial3d/src/test/org/apache/lucene/spatial3d/TestGeo3DPoint.java @@ -81,8 +81,6 @@ import org.apache.lucene.util.NumericUtils; import org.apache.lucene.util.StringHelper; import org.apache.lucene.util.TestUtil; -import org.junit.Ignore; - import com.carrotsearch.randomizedtesting.generators.RandomNumbers; public class TestGeo3DPoint extends LuceneTestCase { @@ -190,8 +188,7 @@ public class TestGeo3DPoint extends LuceneTestCase { } /** Tests consistency of GeoArea.getRelationship vs GeoShape.isWithin */ - //@AwaitsFix("https://issues.apache.org/jira/browse/LUCENE-8227") - @Ignore + @AwaitsFix(bugUrl="https://issues.apache.org/jira/browse/LUCENE-8227") public void testGeo3DRelations() throws Exception { int numDocs = atLeast(1000); @@ -471,22 +468,16 @@ public class TestGeo3DPoint extends LuceneTestCase { } } - //@AwaitsFix("https://issues.apache.org/jira/browse/LUCENE-8227") - @Ignore public void testRandomTiny() throws Exception { // Make sure single-leaf-node case is OK: doTestRandom(10); } - //@AwaitsFix("https://issues.apache.org/jira/browse/LUCENE-8227") - @Ignore public void testRandomMedium() throws Exception { doTestRandom(10000); } @Nightly - //@AwaitsFix("https://issues.apache.org/jira/browse/LUCENE-8227") - @Ignore public void testRandomBig() throws Exception { doTestRandom(50000); } 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 ebfb0f4b3a3..65659b3accd 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 @@ -253,6 +253,7 @@ public class GeoPolygonTest { shapes.add(pd); c = GeoPolygonFactory.makeLargeGeoPolygon(PlanetModel.SPHERE, shapes); + //System.out.println("Large polygon = "+c); // Sample some points within gp = new GeoPoint(PlanetModel.SPHERE, 0.0, -0.45); @@ -1217,7 +1218,6 @@ shape: [junit4] 1> quantized=[X=-0.9951793580415914, Y=-0.10888987641797832, Z=-2.3309121299774915E-10] */ @Test - @Ignore public void testLUCENE8227() throws Exception { List points = new ArrayList<>(); points.add(new GeoPoint(PlanetModel.WGS84, -0.63542308910253, 0.9853722928232957)); @@ -1227,9 +1227,11 @@ shape: points.add(new GeoPoint(PlanetModel.WGS84, -1.2205765069413237, 3.141592653589793)); 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, -3.1780051348770987E-74, -3.032608859187692); final GeoPoint quantized = new GeoPoint(-0.9951793580415914, -0.10888987641797832, -2.3309121299774915E-10); @@ -1237,31 +1239,22 @@ shape: final GeoPoint negativeX = new GeoPoint(PlanetModel.WGS84, 0.0, Math.PI); final GeoPoint negativeY = new GeoPoint(PlanetModel.WGS84, 0.0, -Math.PI * 0.5); - // Construct a standard polygon first to see what that does + // 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("Standard polygon: "+standard); - // This shows y < 0 hemisphere is all in-set //assertTrue(standard.isWithin(negativeY)); // This should be in-set too, but isn't!! assertTrue(standard.isWithin(negativeX)); -/* final XYZBounds standardBounds = new XYZBounds(); standard.getBounds(standardBounds); final XYZSolid standardSolid = XYZSolidFactory.makeXYZSolid(PlanetModel.WGS84, standardBounds); - System.out.println("Standard bounds: "+standardBounds); + // If within shape, should be within bounds + assertTrue(standard.isWithin(quantized)?standardSolid.isWithin(quantized):true); + assertTrue(standard.isWithin(unquantized)?standardSolid.isWithin(unquantized):true); - assertFalse(standardSolid.isWithin(quantized)); - assertFalse(standardSolid.isWithin(unquantized)); -*/ - // Now, both points should also not be in the poly - assertFalse(standard.isWithin(unquantized)); - assertFalse(standard.isWithin(quantized)); - - } }