From 7080ba5b05b470248a1fdc505e9b37b64dabf1da Mon Sep 17 00:00:00 2001 From: Ignacio Vera Date: Wed, 17 Jun 2020 09:34:49 +0200 Subject: [PATCH] Check for degenerated lines when calculating the centroid (#58216) --- .../index/fielddata/CentroidCalculator.java | 44 +++++++++---------- .../fielddata/CentroidCalculatorTests.java | 10 ++++- .../index/fielddata/TriangleTreeTests.java | 1 - 3 files changed, 30 insertions(+), 25 deletions(-) diff --git a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/fielddata/CentroidCalculator.java b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/fielddata/CentroidCalculator.java index 40fefd11528..f52c8cd3641 100644 --- a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/fielddata/CentroidCalculator.java +++ b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/fielddata/CentroidCalculator.java @@ -224,18 +224,18 @@ public class CentroidCalculator { @Override public Void visit(Rectangle rectangle) { - double sumX = rectangle.getMaxX() + rectangle.getMinX(); - double sumY = rectangle.getMaxY() + rectangle.getMinY(); double diffX = rectangle.getMaxX() - rectangle.getMinX(); double diffY = rectangle.getMaxY() - rectangle.getMinY(); - if (diffX != 0 && diffY != 0) { - calculator.addCoordinate(sumX / 2, sumY / 2, Math.abs(diffX * diffY), DimensionalShapeType.POLYGON); - } else if (diffX != 0) { - calculator.addCoordinate(sumX / 2, rectangle.getMinY(), diffX, DimensionalShapeType.LINE); - } else if (diffY != 0) { - calculator.addCoordinate(rectangle.getMinX(), sumY / 2, diffY, DimensionalShapeType.LINE); + double rectWeight = Math.abs(diffX * diffY); + if (rectWeight != 0) { + double sumX = rectangle.getMaxX() + rectangle.getMinX(); + double sumY = rectangle.getMaxY() + rectangle.getMinY(); + calculator.addCoordinate(sumX / 2, sumY / 2, rectWeight, DimensionalShapeType.POLYGON); } else { - visitPoint(rectangle.getMinX(), rectangle.getMinY()); + // degenerated rectangle, transform to Line + Line line = new Line(new double[]{rectangle.getMinX(), rectangle.getMaxX()}, + new double[]{rectangle.getMinY(), rectangle.getMaxY()}); + visit(line); } return null; } @@ -246,22 +246,20 @@ public class CentroidCalculator { } private void visitLine(int length, CoordinateSupplier x, CoordinateSupplier y) { - // check line has length - double originDiffX = x.get(0) - x.get(1); - double originDiffY = y.get(0) - y.get(1); - if (originDiffX != 0 || originDiffY != 0) { - // a line's centroid is calculated by summing the center of each - // line segment weighted by the line segment's length in degrees - for (int i = 0; i < length - 1; i++) { - double diffX = x.get(i) - x.get(i + 1); - double diffY = y.get(i) - y.get(i + 1); - double xAvg = (x.get(i) + x.get(i + 1)) / 2; - double yAvg = (y.get(i) + y.get(i + 1)) / 2; - double weight = Math.sqrt(diffX * diffX + diffY * diffY); + // a line's centroid is calculated by summing the center of each + // line segment weighted by the line segment's length in degrees + for (int i = 0; i < length - 1; i++) { + double diffX = x.get(i) - x.get(i + 1); + double diffY = y.get(i) - y.get(i + 1); + double xAvg = (x.get(i) + x.get(i + 1)) / 2; + double yAvg = (y.get(i) + y.get(i + 1)) / 2; + double weight = Math.sqrt(diffX * diffX + diffY * diffY); + if (weight == 0) { + // degenerated line, it can be considered a point + visitPoint(x.get(i), y.get(i)); + } else { calculator.addCoordinate(xAvg, yAvg, weight, DimensionalShapeType.LINE); } - } else { - visitPoint(x.get(0), y.get(0)); } } diff --git a/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/index/fielddata/CentroidCalculatorTests.java b/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/index/fielddata/CentroidCalculatorTests.java index 917322d3650..3768a714c87 100644 --- a/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/index/fielddata/CentroidCalculatorTests.java +++ b/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/index/fielddata/CentroidCalculatorTests.java @@ -31,6 +31,7 @@ import static org.elasticsearch.xpack.spatial.index.fielddata.DimensionalShapeTy import static org.hamcrest.Matchers.anyOf; import static org.hamcrest.Matchers.closeTo; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.greaterThan; public class CentroidCalculatorTests extends ESTestCase { private static final double DELTA = 0.000000001; @@ -190,6 +191,13 @@ public class CentroidCalculatorTests extends ESTestCase { } } + public void testRectangle() { + for (int i = 0; i < 100; i++) { + CentroidCalculator calculator = new CentroidCalculator(GeometryTestUtils.randomRectangle()); + assertThat(calculator.sumWeight(), greaterThan(0.0)); + } + } + public void testLineAsClosedPoint() { double lon = GeometryTestUtils.randomLon(); double lat = GeometryTestUtils.randomLat(); @@ -250,7 +258,7 @@ public class CentroidCalculatorTests extends ESTestCase { CentroidCalculator calculator = new CentroidCalculator(polygon); assertThat(calculator.getX(), equalTo(GeoUtils.normalizeLon(point.getX()))); assertThat(calculator.getY(), equalTo(GeoUtils.normalizeLat(point.getY()))); - assertThat(calculator.sumWeight(), equalTo(1.0)); + assertThat(calculator.sumWeight(), equalTo(3.0)); assertThat(calculator.getDimensionalShapeType(), equalTo(POINT)); } diff --git a/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/index/fielddata/TriangleTreeTests.java b/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/index/fielddata/TriangleTreeTests.java index a2429d7c214..a8ff53bc57b 100644 --- a/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/index/fielddata/TriangleTreeTests.java +++ b/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/index/fielddata/TriangleTreeTests.java @@ -255,7 +255,6 @@ public class TriangleTreeTests extends ESTestCase { assertRelation(GeoRelation.QUERY_CROSSES, reader, getExtentFromBox(xMin, yMin, xMax, yMax)); } - @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/56755") public void testRandomMultiLineIntersections() throws IOException { GeoShapeIndexer indexer = new GeoShapeIndexer(true, "test"); MultiLine geometry = randomMultiLine(false);