GeoPolygonDecomposer might fail due to numerical errors when calculat…

…ing intersection with the dateline (elastic#82953) This change make sure that our intersection point lies either +180 or -180 always.
vijay2308 · Jan 25, 2022 · 3f723a4 · 3f723a4
1 parent 9be18cb
commit 3f723a4
Show file tree

Hide file tree

Showing 2 changed files with 46 additions and 18 deletions.
diff --git a/server/src/main/java/org/elasticsearch/common/geo/GeoPolygonDecomposer.java b/server/src/main/java/org/elasticsearch/common/geo/GeoPolygonDecomposer.java
@@ -160,18 +160,6 @@ private static void validateHole(LinearRing shell, LinearRing hole) {
         }
     }
 
-    private static Point position(Point p1, Point p2, double position) {
-        if (position == 0) {
-            return p1;
-        } else if (position == 1) {
-            return p2;
-        } else {
-            final double x = p1.getX() + position * (p2.getX() - p1.getX());
-            final double y = p1.getY() + position * (p2.getY() - p1.getY());
-            return new Point(x, y);
-        }
-    }
-
     private static int createEdges(
         int component,
         boolean orientation,
@@ -420,7 +408,7 @@ private static int intersections(double dateline, Edge[] edges) {
 
             double position = intersection(p1.getX(), p2.getX(), dateline);
             if (Double.isNaN(position) == false) {
-                edges[i].intersection(position);
+                edges[i].setIntersection(position, dateline);
                 numIntersections++;
                 maxComponent = Math.max(maxComponent, edges[i].component);
             }
@@ -781,13 +769,20 @@ void setNext(Edge next) {
         }
 
         /**
-         * Set the intersection of this line segment to the given position
+         * Set the intersection of this line segment with the given dateline
          *
          * @param position position of the intersection [0..1]
-         * @return the {@link Point} of the intersection
+         * @param dateline of the intersection
          */
-        Point intersection(double position) {
-            return intersect = position(coordinate, next.coordinate, position);
+        void setIntersection(double position, double dateline) {
+            if (position == 0) {
+                this.intersect = coordinate;
+            } else if (position == 1) {
+                this.intersect = next.coordinate;
+            } else {
+                final double y = coordinate.getY() + position * (next.coordinate.getY() - coordinate.getY());
+                this.intersect = new Point(dateline, y);
+            }
         }
 
         @Override

diff --git a/server/src/test/java/org/elasticsearch/common/geo/GeometryNormalizerTests.java b/server/src/test/java/org/elasticsearch/common/geo/GeometryNormalizerTests.java
@@ -358,7 +358,7 @@ public void testPolygon() {
         polygon = new Polygon(new LinearRing(new double[] { 1, 0, 0, 1, 1 }, new double[] { 1, 1, 0, 0, 1 }));
         // for some reason, the normalizer always changes the order of the points
         indexed = new Polygon(new LinearRing(new double[] { 0, 0, 1, 1, 0 }, new double[] { 1, 0, 0, 1, 1 }));
-        ;
+
         assertEquals(indexed, GeometryNormalizer.apply(Orientation.CCW, polygon));
         assertEquals(false, GeometryNormalizer.needsNormalize(Orientation.CCW, polygon));
 
@@ -426,4 +426,37 @@ public void testMultiPolygon() {
         assertEquals(indexed, GeometryNormalizer.apply(Orientation.CCW, multiPolygon));
         assertEquals(true, GeometryNormalizer.needsNormalize(Orientation.CCW, multiPolygon));
     }
+
+    public void testIssue82840() {
+        Polygon polygon = new Polygon(
+            new LinearRing(
+                new double[] { -143.10690080319134, -143.10690080319134, 62.41055750853541, -143.10690080319134 },
+                new double[] { -90.0, -30.033129816260214, -30.033129816260214, -90.0 }
+            )
+        );
+        MultiPolygon indexedCCW = new MultiPolygon(
+            List.of(
+                new Polygon(
+                    new LinearRing(
+                        new double[] { 180.0, 180.0, 62.41055750853541, 180.0 },
+                        new double[] { -75.67887564489237, -30.033129816260214, -30.033129816260214, -75.67887564489237 }
+                    )
+                ),
+                new Polygon(
+                    new LinearRing(
+                        new double[] { -180.0, -180.0, -143.10690080319134, -143.10690080319134, -180.0 },
+                        new double[] { -30.033129816260214, -75.67887564489237, -90.0, -30.033129816260214, -30.033129816260214 }
+                    )
+                )
+            )
+        );
+        assertEquals(indexedCCW, GeometryNormalizer.apply(Orientation.CCW, polygon));
+        Polygon indexedCW = new Polygon(
+            new LinearRing(
+                new double[] { -143.10690080319134, 62.41055750853541, -143.10690080319134, -143.10690080319134 },
+                new double[] { -30.033129816260214, -30.033129816260214, -90.0, -30.033129816260214 }
+            )
+        );
+        assertEquals(indexedCW, GeometryNormalizer.apply(Orientation.CW, polygon));
+    }
 }