diff --git a/server/src/main/java/org/elasticsearch/common/geo/GeoUtils.java b/server/src/main/java/org/elasticsearch/common/geo/GeoUtils.java index 43ef22be0fe61..359b7781b895f 100644 --- a/server/src/main/java/org/elasticsearch/common/geo/GeoUtils.java +++ b/server/src/main/java/org/elasticsearch/common/geo/GeoUtils.java @@ -308,7 +308,7 @@ public static void normalizePoint(double[] lonLat, boolean normLon, boolean norm assert lonLat != null && lonLat.length == 2; normLat = normLat && (lonLat[1] > 90 || lonLat[1] < -90); - normLon = normLon && (lonLat[0] > 180 || lonLat[0] < -180); + normLon = normLon && (lonLat[0] > 180 || lonLat[0] < -180 || normLat); if (normLat) { lonLat[1] = centeredModulus(lonLat[1], 360); diff --git a/server/src/main/java/org/elasticsearch/index/mapper/GeoShapeIndexer.java b/server/src/main/java/org/elasticsearch/index/mapper/GeoShapeIndexer.java index efa490a2357bf..9cf2d7b630aa8 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/GeoShapeIndexer.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/GeoShapeIndexer.java @@ -225,15 +225,16 @@ protected static double intersection(double p1x, double p2x, double dateline) { * Splits the specified line by datelines and adds them to the supplied lines array */ private List<Line> decomposeGeometry(Line line, List<Line> lines) { - for (Line part : decompose(line)) { - double[] lats = new double[part.length()]; - double[] lons = new double[part.length()]; - for (int i = 0; i < part.length(); i++) { - lats[i] = normalizeLat(part.getY(i)); - lons[i] = normalizeLonMinus180Inclusive(part.getX(i)); - } - lines.add(new Line(lons, lats)); + double[] lons = new double[line.length()]; + double[] lats = new double[lons.length]; + + for (int i = 0; i < lons.length; i++) { + double[] lonLat = new double[] {line.getX(i), line.getY(i)}; + normalizePoint(lonLat,false, true); + lons[i] = lonLat[0]; + lats[i] = lonLat[1]; } + lines.addAll(decompose(lons, lats)); return lines; } @@ -253,12 +254,11 @@ private List<Line> decomposeGeometry(Line line, List<Line> lines) { /** * Decompose a linestring given as array of coordinates by anti-meridian. * - * @param line linestring that should be decomposed + * @param lons longitudes of the linestring that should be decomposed + * @param lats latitudes of the linestring that should be decomposed * @return array of linestrings given as coordinate arrays */ - private List<Line> decompose(Line line) { - double[] lons = line.getX(); - double[] lats = line.getY(); + private List<Line> decompose(double[] lons, double[] lats) { int offset = 0; ArrayList<Line> parts = new ArrayList<>(); diff --git a/server/src/test/java/org/elasticsearch/common/geo/GeometryIndexerTests.java b/server/src/test/java/org/elasticsearch/common/geo/GeometryIndexerTests.java index 24b135be5c6b7..b00b53ec18271 100644 --- a/server/src/test/java/org/elasticsearch/common/geo/GeometryIndexerTests.java +++ b/server/src/test/java/org/elasticsearch/common/geo/GeometryIndexerTests.java @@ -38,8 +38,10 @@ import java.io.IOException; import java.text.ParseException; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; +import java.util.List; import static org.hamcrest.Matchers.instanceOf; @@ -155,16 +157,40 @@ public void testLine() { */ public double length(Line line) { double distance = 0; + double[] prev = new double[]{line.getLon(0), line.getLat(0)}; + GeoUtils.normalizePoint(prev, false, true); for (int i = 1; i < line.length(); i++) { - distance += Math.sqrt((line.getLat(i) - line.getLat(i - 1)) * (line.getLat(i) - line.getLat(i - 1)) + - (line.getLon(i) - line.getLon(i - 1)) * (line.getLon(i) - line.getLon(i - 1))); + double[] cur = new double[]{line.getLon(i), line.getLat(i)}; + GeoUtils.normalizePoint(cur, false, true); + distance += Math.sqrt((cur[0] - prev[0]) * (cur[0] - prev[0]) + (cur[1] - prev[1]) * (cur[1] - prev[1])); + prev = cur; } return distance; } /** - * A simple tests that generates a random lines crossing anti-merdian and checks that the decomposed segments of this line + * Removes the points on the antimeridian that are introduced during linestring decomposition + */ + public static MultiPoint remove180s(MultiPoint points) { + List<Point> list = new ArrayList<>(); + points.forEach(point -> { + if (Math.abs(point.getLon()) - 180.0 > 0.000001) { + list.add(point); + } + }); + if (list.isEmpty()) { + return MultiPoint.EMPTY; + } + return new MultiPoint(list); + } + + /** + * A randomized test that generates a random lines crossing anti-merdian and checks that the decomposed segments of this line * have the same total length (measured using Euclidean distances between neighboring points) as the original line. + * + * It also extracts all points from these lines, performs normalization of these points and then compares that the resulting + * points of line normalization match the points of points normalization with the exception of points that were created on the + * antimeridian as the result of line decomposition. */ public void testRandomLine() { int size = randomIntBetween(2, 20); @@ -172,8 +198,10 @@ public void testRandomLine() { double[] originalLats = new double[size]; double[] originalLons = new double[size]; + // Generate a random line that goes over poles and stretches beyond -180 and +180 for (int i = 0; i < size; i++) { - originalLats[i] = GeometryTestUtils.randomLat(); + // from time to time go over poles + originalLats[i] = randomInt(4) == 0 ? GeometryTestUtils.randomLat() : GeometryTestUtils.randomLon(); originalLons[i] = GeometryTestUtils.randomLon() + shift * 360; if (randomInt(3) == 0) { shift += randomFrom(-2, -1, 1, 2); @@ -181,6 +209,7 @@ public void testRandomLine() { } Line original = new Line(originalLons, originalLats); + // Check that the length of original and decomposed lines is the same Geometry decomposed = indexer.prepareForIndexing(original); double decomposedLength = 0; if (decomposed instanceof Line) { @@ -192,8 +221,19 @@ public void testRandomLine() { decomposedLength += length(lines.get(i)); } } - assertEquals("Different Lengths between " + original + " and " + decomposed, length(original), decomposedLength, 0.001); + + // Check that normalized linestring generates the same points as the normalized multipoint based on the same set of points + MultiPoint decomposedViaLines = remove180s(GeometryTestUtils.toMultiPoint(decomposed)); + MultiPoint originalPoints = GeometryTestUtils.toMultiPoint(original); + MultiPoint decomposedViaPoint = remove180s(GeometryTestUtils.toMultiPoint(indexer.prepareForIndexing(originalPoints))); + assertEquals(decomposedViaPoint.size(), decomposedViaLines.size()); + for (int i=0; i<decomposedViaPoint.size(); i++) { + assertEquals("Difference between decomposing lines " + decomposedViaLines + " and points " + decomposedViaPoint + + " at the position " + i, decomposedViaPoint.get(i).getLat(), decomposedViaLines.get(i).getLat(), 0.0001); + assertEquals("Difference between decomposing lines " + decomposedViaLines + " and points " + decomposedViaPoint + + " at the position " + i, decomposedViaPoint.get(i).getLon(), decomposedViaLines.get(i).getLon(), 0.0001); + } } public void testMultiLine() { @@ -231,6 +271,9 @@ public void testPoint() { point = new Point(180, 180); assertEquals(new Point(0, 0), indexer.prepareForIndexing(point)); + + point = new Point(-180, -180); + assertEquals(new Point(0, 0), indexer.prepareForIndexing(point)); } public void testMultiPoint() { diff --git a/server/src/test/java/org/elasticsearch/index/search/geo/GeoUtilsTests.java b/server/src/test/java/org/elasticsearch/index/search/geo/GeoUtilsTests.java index a15c9b1dedffb..88e0b9fec123f 100644 --- a/server/src/test/java/org/elasticsearch/index/search/geo/GeoUtilsTests.java +++ b/server/src/test/java/org/elasticsearch/index/search/geo/GeoUtilsTests.java @@ -386,6 +386,10 @@ public void testNormalizePointEdgeCases() { assertNormalizedPoint(new GeoPoint(180.0, 360.0), new GeoPoint(0.0, 180.0)); assertNormalizedPoint(new GeoPoint(-90.0, -180.0), new GeoPoint(-90.0, -180.0)); assertNormalizedPoint(new GeoPoint(90.0, 180.0), new GeoPoint(90.0, 180.0)); + assertNormalizedPoint(new GeoPoint(100.0, 180.0), new GeoPoint(80.0, 0.0)); + assertNormalizedPoint(new GeoPoint(100.0, -180.0), new GeoPoint(80.0, 0.0)); + assertNormalizedPoint(new GeoPoint(-100.0, 180.0), new GeoPoint(-80.0, 0.0)); + assertNormalizedPoint(new GeoPoint(-100.0, -180.0), new GeoPoint(-80.0, 0.0)); } public void testParseGeoPoint() throws IOException { diff --git a/test/framework/src/main/java/org/elasticsearch/geo/GeometryTestUtils.java b/test/framework/src/main/java/org/elasticsearch/geo/GeometryTestUtils.java index 6555381f0c6e7..33983e8e39099 100644 --- a/test/framework/src/main/java/org/elasticsearch/geo/GeometryTestUtils.java +++ b/test/framework/src/main/java/org/elasticsearch/geo/GeometryTestUtils.java @@ -23,6 +23,7 @@ import org.elasticsearch.geometry.Circle; import org.elasticsearch.geometry.Geometry; import org.elasticsearch.geometry.GeometryCollection; +import org.elasticsearch.geometry.GeometryVisitor; import org.elasticsearch.geometry.Line; import org.elasticsearch.geometry.LinearRing; import org.elasticsearch.geometry.MultiLine; @@ -34,6 +35,8 @@ import org.elasticsearch.test.ESTestCase; import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; import java.util.List; import java.util.function.Function; @@ -186,4 +189,73 @@ protected static Geometry randomGeometry(int level, boolean hasAlt) { ); return geometry.apply(hasAlt); } + + /** + * Extracts all vertices of the supplied geometry + */ + public static MultiPoint toMultiPoint(Geometry geometry) { + return geometry.visit(new GeometryVisitor<MultiPoint, RuntimeException>() { + @Override + public MultiPoint visit(Circle circle) throws RuntimeException { + throw new UnsupportedOperationException("not supporting circles yet"); + } + + @Override + public MultiPoint visit(GeometryCollection<?> collection) throws RuntimeException { + List<Point> points = new ArrayList<>(); + collection.forEach(geometry -> toMultiPoint(geometry).forEach(points::add)); + return new MultiPoint(points); + } + + @Override + public MultiPoint visit(Line line) throws RuntimeException { + List<Point> points = new ArrayList<>(); + for (int i = 0; i < line.length(); i++) { + points.add(new Point(line.getX(i), line.getY(i), line.getZ(i))); + } + return new MultiPoint(points); + } + + @Override + public MultiPoint visit(LinearRing ring) throws RuntimeException { + return visit((Line) ring); + } + + @Override + public MultiPoint visit(MultiLine multiLine) throws RuntimeException { + return visit((GeometryCollection<?>) multiLine); + } + + @Override + public MultiPoint visit(MultiPoint multiPoint) throws RuntimeException { + return multiPoint; + } + + @Override + public MultiPoint visit(MultiPolygon multiPolygon) throws RuntimeException { + return visit((GeometryCollection<?>) multiPolygon); + } + + @Override + public MultiPoint visit(Point point) throws RuntimeException { + return new MultiPoint(Collections.singletonList(point)); + } + + @Override + public MultiPoint visit(Polygon polygon) throws RuntimeException { + List<Geometry> multiPoints = new ArrayList<>(); + multiPoints.add(toMultiPoint(polygon.getPolygon())); + for (int i = 0; i < polygon.getNumberOfHoles(); i++) { + multiPoints.add(toMultiPoint(polygon.getHole(i))); + } + return toMultiPoint(new GeometryCollection<>(multiPoints)); + } + + @Override + public MultiPoint visit(Rectangle rectangle) throws RuntimeException { + return new MultiPoint(Arrays.asList(new Point(rectangle.getMinX(), rectangle.getMinY(), rectangle.getMinZ()), + new Point(rectangle.getMaxX(), rectangle.getMaxY(), rectangle.getMaxZ()))); + } + }); + } }