diff --git a/build-tools-internal/version.properties b/build-tools-internal/version.properties index 14acb95ed9b9a..93e1070eb1d7c 100644 --- a/build-tools-internal/version.properties +++ b/build-tools-internal/version.properties @@ -5,7 +5,7 @@ bundled_jdk_vendor = openjdk bundled_jdk = 25.0.1+8@2fbf10d8c78e40bd87641c434705079d # optional dependencies spatial4j = 0.7 -jts = 1.15.0 +jts = 1.20.0 jackson = 2.15.0 snakeyaml = 2.0 icu4j = 77.1 diff --git a/docs/changelog/138351.yaml b/docs/changelog/138351.yaml new file mode 100644 index 0000000000000..c63c0bdbca8be --- /dev/null +++ b/docs/changelog/138351.yaml @@ -0,0 +1,5 @@ +pr: 138351 +summary: Bumps jts version to 1.20.0 +area: Geo +type: upgrade +issues: [] diff --git a/gradle/verification-metadata.xml b/gradle/verification-metadata.xml index 116721c29993b..54fa25ff2f516 100644 --- a/gradle/verification-metadata.xml +++ b/gradle/verification-metadata.xml @@ -4478,9 +4478,9 @@ - - - + + + diff --git a/modules/legacy-geo/src/main/java/module-info.java b/modules/legacy-geo/src/main/java/module-info.java index b5c62ae18b066..8968e3b82418c 100644 --- a/modules/legacy-geo/src/main/java/module-info.java +++ b/modules/legacy-geo/src/main/java/module-info.java @@ -15,7 +15,7 @@ requires org.apache.lucene.core; requires org.apache.lucene.spatial_extras; requires org.apache.logging.log4j; - requires jts.core; + requires org.locationtech.jts; requires spatial4j; exports org.elasticsearch.legacygeo.mapper; diff --git a/modules/legacy-geo/src/test/java/org/elasticsearch/legacygeo/builders/PolygonBuilderTests.java b/modules/legacy-geo/src/test/java/org/elasticsearch/legacygeo/builders/PolygonBuilderTests.java index 463f795c13800..c2417854cbc00 100644 --- a/modules/legacy-geo/src/test/java/org/elasticsearch/legacygeo/builders/PolygonBuilderTests.java +++ b/modules/legacy-geo/src/test/java/org/elasticsearch/legacygeo/builders/PolygonBuilderTests.java @@ -168,7 +168,7 @@ public void testPolygonWithUndefinedOrientationDueToCollinearPoints() { new CoordinatesBuilder().coordinate(0.0, 0.0).coordinate(1.0, 1.0).coordinate(-1.0, -1.0).close() ); InvalidShapeException e = expectThrows(InvalidShapeException.class, pb::buildS4J); - assertEquals("Self-intersection at or near point (-1.0, -1.0, NaN)", e.getMessage()); + assertEquals("Self-intersection at or near point (0.0, 0.0, NaN)", e.getMessage()); } public void testCrossingDateline() { diff --git a/x-pack/plugin/vector-tile/src/main/java/org/elasticsearch/xpack/vectortile/feature/FeatureFactory.java b/x-pack/plugin/vector-tile/src/main/java/org/elasticsearch/xpack/vectortile/feature/FeatureFactory.java index b5f9088edc4be..040c6e081cd03 100644 --- a/x-pack/plugin/vector-tile/src/main/java/org/elasticsearch/xpack/vectortile/feature/FeatureFactory.java +++ b/x-pack/plugin/vector-tile/src/main/java/org/elasticsearch/xpack/vectortile/feature/FeatureFactory.java @@ -111,7 +111,7 @@ public byte[] points(List multiPoint) { * Returns a List {@code byte[]} containing the mvt representation of the provided geometry */ public List getFeatures(Geometry geometry) { - // Get geometry in pixel coordinates + // Get clipped and simplified geometry in pixel coordinates final org.locationtech.jts.geom.Geometry mvtGeometry = geometry.visit(mvtGeometryBuilder); if (mvtGeometry == null) { return List.of(); @@ -127,6 +127,15 @@ public List getFeatures(Geometry geometry) { return byteFeatures; } + /** + * Testing purposes only, allowing us to test the impact of the geometry clipping and simplification + * without needing to parse the protobuffer results. + */ + org.locationtech.jts.geom.Geometry getMvtGeometry(Geometry geometry) { + // Get clipped and simplified geometry in pixel coordinates + return geometry.visit(mvtGeometryBuilder); + } + private record MVTGeometryBuilder( GeometryFactory geomFactory, org.locationtech.jts.geom.Geometry clipTile, @@ -332,8 +341,13 @@ private MvtCoordinateSequenceFilter(Envelope tileEnvelope, int extent) { @Override public void filter(CoordinateSequence seq, int i) { - seq.setOrdinate(i, 0, lon(seq.getOrdinate(i, 0))); - seq.setOrdinate(i, 1, lat(seq.getOrdinate(i, 1))); + // JTS can sometimes close polygons using references to the starting coordinate, which can lead to that coordinate + // being converted twice. We need to detect if the end is the same actual Coordinate, and not do the conversion again. + if (i != 0 && i == seq.size() - 1 && seq.getCoordinate(0) == seq.getCoordinate(i)) { + return; + } + seq.setOrdinate(i, CoordinateSequence.X, lon(seq.getOrdinate(i, CoordinateSequence.X))); + seq.setOrdinate(i, CoordinateSequence.Y, lat(seq.getOrdinate(i, CoordinateSequence.Y))); } @Override diff --git a/x-pack/plugin/vector-tile/src/test/java/org/elasticsearch/xpack/vectortile/feature/FeatureFactoryTests.java b/x-pack/plugin/vector-tile/src/test/java/org/elasticsearch/xpack/vectortile/feature/FeatureFactoryTests.java index e8ea9bf06a4cc..aebb1bf585648 100644 --- a/x-pack/plugin/vector-tile/src/test/java/org/elasticsearch/xpack/vectortile/feature/FeatureFactoryTests.java +++ b/x-pack/plugin/vector-tile/src/test/java/org/elasticsearch/xpack/vectortile/feature/FeatureFactoryTests.java @@ -34,6 +34,7 @@ import java.nio.charset.StandardCharsets; import java.text.ParseException; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.function.Consumer; @@ -292,6 +293,44 @@ private void assertParsing(InputStream is) throws IOException, ParseException { assertThat(builder.getFeatures(geometry), iterableWithSize(1)); } + public void testVectorTilesDoNotContainOriginWithSimplifiableGeometries() { + Polygon polygon = new Polygon( + new LinearRing( + new double[] { -94.491034, -94.493892, -94.399587, -94.483604, -94.491034 }, + new double[] { -72.522177, -72.491761, -72.607412, -72.545519, -72.522177 } + ) + ); + + final FeatureFactory builder = new FeatureFactory(0, 0, 0, 4096, 5); + + var mvtGeometry = builder.getMvtGeometry(polygon); + var coordinates = mvtGeometry.getCoordinates(); + var containsCenter = Arrays.stream(coordinates).anyMatch(c -> c.x == 2048 && c.y == 2048); + assertFalse("Coordinate (2048,2048) only occurs if we are converting the first / last coordinate twice", containsCenter); + } + + public void testVectorTilesDoNotContainOriginWithComplexGeometries() throws IOException, ParseException { + // make sure we can parse big polygons + var is = new GZIPInputStream(getClass().getResourceAsStream("Antarctica.wkt.gz")); + final BufferedReader reader = new BufferedReader(new InputStreamReader(is, StandardCharsets.UTF_8)); + final Geometry geometry = WellKnownText.fromWKT(StandardValidator.instance(true), true, reader.readLine()); + final FeatureFactory builder = new FeatureFactory(0, 0, 0, 4096, 5); + + var mvtGeometry = builder.getMvtGeometry(geometry); + var coordinates = mvtGeometry.getCoordinates(); + var containsCenter = Arrays.stream(coordinates).anyMatch(c -> c.x == 2048 && c.y == 2048); + assertFalse("Coordinate (2048,2048) only occurs if we are converting the first / last coordinate twice", containsCenter); + } + + public void testVectorTilesDoNotContainOriginWithNonSimplifiableGeometries() { + Polygon polygon = new Polygon(new LinearRing(new double[] { -10, 10, 10, -10, -10 }, new double[] { -10, -10, 10, 10, -10 })); + final FeatureFactory builder = new FeatureFactory(0, 0, 0, 4096, 5); + var mvtGeometry = builder.getMvtGeometry(polygon); + var coordinates = mvtGeometry.getCoordinates(); + var containsCenter = Arrays.stream(coordinates).anyMatch(c -> c.x == 2048 && c.y == 2048); + assertFalse("Coordinate (2048,2048) only occurs if we are converting the first / last coordinate twice", containsCenter); + } + public void testPolygonOrientation() throws IOException { Polygon polygon = new Polygon( new LinearRing(new double[] { -10, 10, 10, -10, -10 }, new double[] { -10, -10, 10, 10, -10 }),