From e23102342072e0039b3668c9b85351b953078019 Mon Sep 17 00:00:00 2001 From: Hrishi Bakshi Date: Sat, 7 Mar 2020 18:44:41 -0500 Subject: [PATCH 1/6] Improve conversion of GeoCircle to Shape(Polygon) Details of the problem are explained here: https://github.com/locationtech/spatial4j/issues/177 The new test-case fails without the code change and passes after the code change. Signed-off-by: Hrishi Bakshi --- .../spatial4j/shape/jts/JtsShapeFactory.java | 3 +- .../shape/jts/JtsShapeFactoryTest.java | 43 +++++++++++++++++++ 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/locationtech/spatial4j/shape/jts/JtsShapeFactory.java b/src/main/java/org/locationtech/spatial4j/shape/jts/JtsShapeFactory.java index e8b7f7e1d..ac1afb030 100755 --- a/src/main/java/org/locationtech/spatial4j/shape/jts/JtsShapeFactory.java +++ b/src/main/java/org/locationtech/spatial4j/shape/jts/JtsShapeFactory.java @@ -160,7 +160,8 @@ public Geometry getGeometryFrom(Shape shape) { if (circle.getBoundingBox().getCrossesDateLine()) throw new IllegalArgumentException("Doesn't support dateline cross yet: "+circle);//TODO GeometricShapeFactory gsf = new GeometricShapeFactory(geometryFactory); - gsf.setSize(circle.getBoundingBox().getWidth()); + gsf.setWidth(circle.getBoundingBox().getWidth()); + gsf.setHeight(circle.getBoundingBox().getHeight()); gsf.setNumPoints(4*25);//multiple of 4 is best gsf.setCentre(new Coordinate(circle.getCenter().getX(), circle.getCenter().getY())); return gsf.createCircle(); diff --git a/src/test/java/org/locationtech/spatial4j/shape/jts/JtsShapeFactoryTest.java b/src/test/java/org/locationtech/spatial4j/shape/jts/JtsShapeFactoryTest.java index 9c803c0c5..df4415a61 100644 --- a/src/test/java/org/locationtech/spatial4j/shape/jts/JtsShapeFactoryTest.java +++ b/src/test/java/org/locationtech/spatial4j/shape/jts/JtsShapeFactoryTest.java @@ -17,11 +17,19 @@ package org.locationtech.spatial4j.shape.jts; +import org.junit.Assert; import org.locationtech.jts.geom.Coordinate; import org.locationtech.jts.geom.Geometry; import org.junit.Test; +import org.locationtech.jts.geom.Polygon; +import org.locationtech.spatial4j.context.SpatialContext; import org.locationtech.spatial4j.context.jts.JtsSpatialContext; import org.locationtech.spatial4j.context.jts.JtsSpatialContextFactory; +import org.locationtech.spatial4j.distance.DistanceUtils; +import org.locationtech.spatial4j.distance.GeodesicSphereDistCalc; +import org.locationtech.spatial4j.shape.Point; +import org.locationtech.spatial4j.shape.impl.GeoCircle; +import org.locationtech.spatial4j.shape.impl.PointImpl; import static org.junit.Assert.assertTrue; @@ -43,4 +51,39 @@ public void testIndex() { JtsGeometry jtsGeom2 = ctx.getShapeFactory().makeShape(g); assertTrue(jtsGeom2.isIndexed()); } + + @Test + public void testCircleGeometryConversions() { + // Hawaii (Far West) + circleGeometryConversionTest(-155.84, 19.74, 50); + // Nunavat (Far North) + circleGeometryConversionTest(-83.10, 70.30, 100); + // Sydney (South East) + circleGeometryConversionTest(151.21, 33.87, 1); + } + + private void circleGeometryConversionTest(double x, double y, double radiusKm) { + Point circleCenter = new PointImpl(x, y, SpatialContext.GEO); + double radiusDeg = DistanceUtils.dist2Degrees(radiusKm, DistanceUtils.EARTH_EQUATORIAL_RADIUS_KM); + GeoCircle geoCircle = new GeoCircle(circleCenter, radiusDeg, SpatialContext.GEO); + + JtsShapeFactory shapeFactory = JtsSpatialContext.GEO.getShapeFactory(); + // Let's ensure the circle-to-polygon conversion is accurate accounting for geodesics. + Geometry geometry = shapeFactory.getGeometryFrom(geoCircle); + Assert.assertTrue(geometry instanceof Polygon); + Polygon polygon = (Polygon) geometry; + Coordinate[] coordinates = polygon.getExteriorRing().getCoordinates(); + int size = coordinates.length; + Assert.assertTrue(size >= 100); + GeodesicSphereDistCalc distCalc = new GeodesicSphereDistCalc.Haversine(); + double maxDeltaKm = radiusKm / 100; // allow 1% inaccuracy + for (Coordinate coordinate : coordinates) { + // Check distance from center of each point + Point point = new PointImpl(coordinate.x, coordinate.y, SpatialContext.GEO); + double distance = distCalc.distance(point, circleCenter); + double distanceKm = DistanceUtils.degrees2Dist(distance, DistanceUtils.EARTH_MEAN_RADIUS_KM); + Assert.assertEquals(String.format("Distance from point to center: %.2f km. Expected: %.2f km", distanceKm, + radiusKm), radiusKm, distanceKm, maxDeltaKm); + } + } } From 8f8fbc5aaf81d5645f70569b1d698f9397cc6821 Mon Sep 17 00:00:00 2001 From: Hrishi Bakshi Date: Sun, 8 Mar 2020 16:15:27 -0400 Subject: [PATCH 2/6] Update CHANGES.md with geocircle conversion fix Signed-off-by: Hrishi Bakshi --- CHANGES.md | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/CHANGES.md b/CHANGES.md index 532298f5b..08ce8853c 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,3 +1,14 @@ +## VERSION 0.8 + +DATE: March 8 2020 + +* \#177: Improve conversion of a Circle to Shape. JtsShapeFactory allows converting from a Shape + object to a JTS Geometry object. Geodetic circles now translate to a polygon that has points + equidistant from the center. Before the change, there was potentially a large inaccuracy. + (Hrishi Bakshi) + +--------------------------------------- + ## VERSION 0.7 DATE: 27 December 2017 From 5df42343b547c8d13daed51f89a7421fd5bc5ff3 Mon Sep 17 00:00:00 2001 From: David Smiley Date: Sun, 8 Mar 2020 14:39:14 -0400 Subject: [PATCH 3/6] EMPTY points in JTS are now convertible to a Shape instead of throwing an exception. Fixes #163 --- CHANGES.md | 7 ++++--- .../spatial4j/shape/jts/JtsShapeFactory.java | 6 +++++- .../spatial4j/shape/jts/JtsShapeFactoryTest.java | 14 +++++++++++++- 3 files changed, 22 insertions(+), 5 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 08ce8853c..8613c7383 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,13 +1,14 @@ ## VERSION 0.8 -DATE: March 8 2020 +DATE: unreleased * \#177: Improve conversion of a Circle to Shape. JtsShapeFactory allows converting from a Shape object to a JTS Geometry object. Geodetic circles now translate to a polygon that has points equidistant from the center. Before the change, there was potentially a large inaccuracy. (Hrishi Bakshi) - ---------------------------------------- + +* \#163: EMPTY points in JTS are now convertible to a Spatial4j Shape instead of throwing an exception. + (David Smiley) ## VERSION 0.7 diff --git a/src/main/java/org/locationtech/spatial4j/shape/jts/JtsShapeFactory.java b/src/main/java/org/locationtech/spatial4j/shape/jts/JtsShapeFactory.java index ac1afb030..eee200525 100755 --- a/src/main/java/org/locationtech/spatial4j/shape/jts/JtsShapeFactory.java +++ b/src/main/java/org/locationtech/spatial4j/shape/jts/JtsShapeFactory.java @@ -479,7 +479,11 @@ public Shape makeShapeFromGeometry(Geometry geom) { } } else if (geom instanceof org.locationtech.jts.geom.Point) { org.locationtech.jts.geom.Point pt = (org.locationtech.jts.geom.Point) geom; - return pointXY(pt.getX(), pt.getY()); + if (pt.isEmpty()) { + return pointXY(Double.NaN, Double.NaN); + } else { + return pointXY(pt.getX(), pt.getY()); + } } else if (geom instanceof LineString) { if (!useJtsLineString()) { LineString lineString = (LineString) geom; diff --git a/src/test/java/org/locationtech/spatial4j/shape/jts/JtsShapeFactoryTest.java b/src/test/java/org/locationtech/spatial4j/shape/jts/JtsShapeFactoryTest.java index df4415a61..7458cd7e8 100644 --- a/src/test/java/org/locationtech/spatial4j/shape/jts/JtsShapeFactoryTest.java +++ b/src/test/java/org/locationtech/spatial4j/shape/jts/JtsShapeFactoryTest.java @@ -18,9 +18,10 @@ package org.locationtech.spatial4j.shape.jts; import org.junit.Assert; +import org.junit.Test; import org.locationtech.jts.geom.Coordinate; import org.locationtech.jts.geom.Geometry; -import org.junit.Test; +import org.locationtech.jts.geom.GeometryFactory; import org.locationtech.jts.geom.Polygon; import org.locationtech.spatial4j.context.SpatialContext; import org.locationtech.spatial4j.context.jts.JtsSpatialContext; @@ -28,6 +29,7 @@ import org.locationtech.spatial4j.distance.DistanceUtils; import org.locationtech.spatial4j.distance.GeodesicSphereDistCalc; import org.locationtech.spatial4j.shape.Point; +import org.locationtech.spatial4j.shape.Shape; import org.locationtech.spatial4j.shape.impl.GeoCircle; import org.locationtech.spatial4j.shape.impl.PointImpl; @@ -52,6 +54,16 @@ public void testIndex() { assertTrue(jtsGeom2.isIndexed()); } + @Test + public void testEmptyPoint() { + JtsSpatialContextFactory jtsCtxFactory = new JtsSpatialContextFactory(); + JtsSpatialContext jtsCtx = jtsCtxFactory.newSpatialContext(); + GeometryFactory geometryFactory = jtsCtxFactory.getGeometryFactory(); + final org.locationtech.jts.geom.Point point = geometryFactory.createPoint();//empty + final Shape shape = jtsCtx.getShapeFactory().makeShapeFromGeometry(point); // don't throw + assertTrue(shape.isEmpty()); + } + @Test public void testCircleGeometryConversions() { // Hawaii (Far West) From 689b41d51125c2c24ba63089acd705215d5e4a0b Mon Sep 17 00:00:00 2001 From: Jeen Broekstra Date: Mon, 9 Mar 2020 13:32:49 +1100 Subject: [PATCH 4/6] GH-162 handle empty point / shape collection in toString Signed-off-by: Jeen Broekstra --- .../locationtech/spatial4j/io/WKTWriter.java | 27 ++++++++++---- .../spatial4j/io/WKTWriterTest.java | 37 +++++++++++++++++++ 2 files changed, 56 insertions(+), 8 deletions(-) create mode 100644 src/test/java/org/locationtech/spatial4j/io/WKTWriterTest.java diff --git a/src/main/java/org/locationtech/spatial4j/io/WKTWriter.java b/src/main/java/org/locationtech/spatial4j/io/WKTWriter.java index 38a49156a..d049a418a 100644 --- a/src/main/java/org/locationtech/spatial4j/io/WKTWriter.java +++ b/src/main/java/org/locationtech/spatial4j/io/WKTWriter.java @@ -8,6 +8,11 @@ package org.locationtech.spatial4j.io; +import java.io.IOException; +import java.io.Writer; +import java.math.RoundingMode; +import java.text.NumberFormat; +import java.util.Iterator; import org.locationtech.spatial4j.shape.Circle; import org.locationtech.spatial4j.shape.Point; import org.locationtech.spatial4j.shape.Rectangle; @@ -16,12 +21,6 @@ import org.locationtech.spatial4j.shape.impl.BufferedLine; import org.locationtech.spatial4j.shape.impl.BufferedLineString; -import java.io.IOException; -import java.io.Writer; -import java.math.RoundingMode; -import java.text.NumberFormat; -import java.util.Iterator; - public class WKTWriter implements ShapeWriter { @Override @@ -42,8 +41,13 @@ protected NumberFormat getNumberFormat() { public String toString(Shape shape) { NumberFormat nf = getNumberFormat(); if (shape instanceof Point) { + Point point = (Point)shape; + if (point.isEmpty()) { + return "POINT EMPTY"; + } + StringBuilder buffer = new StringBuilder(); - return append(buffer.append("POINT ("),(Point)shape,nf).append(")").toString(); + return append(buffer.append("POINT ("), point, nf).append(")").toString(); } if (shape instanceof Rectangle) { NumberFormat nfMIN = nf; @@ -103,10 +107,17 @@ public String toString(Shape shape) { return str.toString(); } if(shape instanceof ShapeCollection) { + @SuppressWarnings("unchecked") + ShapeCollection collection = (ShapeCollection) shape; + + if (collection.isEmpty()) { + return "GEOMETRYCOLLECTION EMPTY"; + } + StringBuilder buffer = new StringBuilder(); buffer.append("GEOMETRYCOLLECTION ("); boolean first = true; - for(Shape sub : ((ShapeCollection)shape).getShapes()) { + for (Shape sub : collection.getShapes()) { if(!first) { buffer.append(","); } diff --git a/src/test/java/org/locationtech/spatial4j/io/WKTWriterTest.java b/src/test/java/org/locationtech/spatial4j/io/WKTWriterTest.java new file mode 100644 index 000000000..a0c45d66f --- /dev/null +++ b/src/test/java/org/locationtech/spatial4j/io/WKTWriterTest.java @@ -0,0 +1,37 @@ +package org.locationtech.spatial4j.io; + +import static org.junit.Assert.assertEquals; +import java.util.ArrayList; +import org.junit.Test; +import org.locationtech.spatial4j.context.SpatialContext; +import org.locationtech.spatial4j.shape.Point; +import org.locationtech.spatial4j.shape.ShapeCollection; + +public class WKTWriterTest { + + private SpatialContext ctx; + + protected WKTWriterTest(SpatialContext ctx) { + this.ctx = ctx; + } + + public WKTWriterTest() { + this(SpatialContext.GEO); + } + + @Test + public void testToStringOnEmptyPoint() throws Exception { + ShapeWriter writer = ctx.getFormats().getWktWriter(); + Point emptyPoint = ctx.makePoint(Double.NaN, Double.NaN); + + assertEquals("POINT EMPTY", writer.toString(emptyPoint)); + } + + @Test + public void testToStringOnEmptyShapeCollection() throws Exception { + ShapeWriter writer = ctx.getFormats().getWktWriter(); + ShapeCollection emptyCollection = ctx.makeCollection(new ArrayList()); + + assertEquals("GEOMETRYCOLLECTION EMPTY", writer.toString(emptyCollection)); + } +} From 0485785d54b849ecfe21da10444c506288be9823 Mon Sep 17 00:00:00 2001 From: Jeen Broekstra Date: Thu, 12 Mar 2020 15:10:44 +1100 Subject: [PATCH 5/6] added changelog entry Signed-off-by: Jeen Broekstra --- CHANGES.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGES.md b/CHANGES.md index 8613c7383..3d0ff938f 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -10,6 +10,9 @@ DATE: unreleased * \#163: EMPTY points in JTS are now convertible to a Spatial4j Shape instead of throwing an exception. (David Smiley) +* \#162: Fixed WKT serialization of empty point/geometrycollection. + (Jeen Broekstra) + ## VERSION 0.7 DATE: 27 December 2017 From 3786ceb77994baff9c3e806a3d0de9ae75396697 Mon Sep 17 00:00:00 2001 From: David Smiley Date: Fri, 13 Mar 2020 23:56:25 -0400 Subject: [PATCH 6/6] Fix GeoJson too. Test round-trip. --- CHANGES.md | 6 +++--- .../locationtech/spatial4j/io/GeoJSONReader.java | 4 ---- .../locationtech/spatial4j/io/GeoJSONWriter.java | 3 +++ .../org/locationtech/spatial4j/io/WKTWriter.java | 6 +----- .../io/jackson/ShapeAsGeoJSONSerializer.java | 7 +++++-- .../spatial4j/io/jackson/ShapeDeserializer.java | 3 +++ .../spatial4j/io/GeneralPolyshapeTest.java | 6 ++++++ .../spatial4j/io/GeneralReadWriteShapeTest.java | 13 ++++++++++++- 8 files changed, 33 insertions(+), 15 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 3d0ff938f..98a103666 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -7,11 +7,11 @@ DATE: unreleased equidistant from the center. Before the change, there was potentially a large inaccuracy. (Hrishi Bakshi) -* \#163: EMPTY points in JTS are now convertible to a Spatial4j Shape instead of throwing an exception. +* \#163: "Empty" points in JTS are now convertible to a Spatial4j Shape instead of throwing an exception. (David Smiley) -* \#162: Fixed WKT serialization of empty point/geometrycollection. - (Jeen Broekstra) +* \#162: Fixed WKT & GeoJSON \[de\]serialization of "empty" points and geometrycollections. + (Jeen Broekstra, David Smiley) ## VERSION 0.7 diff --git a/src/main/java/org/locationtech/spatial4j/io/GeoJSONReader.java b/src/main/java/org/locationtech/spatial4j/io/GeoJSONReader.java index fab6012d5..21cc78d3b 100644 --- a/src/main/java/org/locationtech/spatial4j/io/GeoJSONReader.java +++ b/src/main/java/org/locationtech/spatial4j/io/GeoJSONReader.java @@ -262,10 +262,6 @@ protected Shape readShape(JSONParser parser) throws IOException, ParseException } sub = parser.nextEvent(); } - if (shapes.isEmpty()) { - throw new ParseException("Shape Collection with no geometries!", - (int) parser.getPosition()); - } return ctx.makeCollection(shapes); } else { diff --git a/src/main/java/org/locationtech/spatial4j/io/GeoJSONWriter.java b/src/main/java/org/locationtech/spatial4j/io/GeoJSONWriter.java index c00aec70c..223ee9d73 100644 --- a/src/main/java/org/locationtech/spatial4j/io/GeoJSONWriter.java +++ b/src/main/java/org/locationtech/spatial4j/io/GeoJSONWriter.java @@ -39,6 +39,9 @@ public String getFormatName() { protected void write(Writer output, NumberFormat nf, double... coords) throws IOException { output.write('['); for (int i = 0; i < coords.length; i++) { + if (Double.isNaN(coords[i])) { + break; // empty point or no more coordinates + } if (i > 0) { output.append(','); } diff --git a/src/main/java/org/locationtech/spatial4j/io/WKTWriter.java b/src/main/java/org/locationtech/spatial4j/io/WKTWriter.java index d049a418a..fc32e5983 100644 --- a/src/main/java/org/locationtech/spatial4j/io/WKTWriter.java +++ b/src/main/java/org/locationtech/spatial4j/io/WKTWriter.java @@ -13,11 +13,7 @@ import java.math.RoundingMode; import java.text.NumberFormat; import java.util.Iterator; -import org.locationtech.spatial4j.shape.Circle; -import org.locationtech.spatial4j.shape.Point; -import org.locationtech.spatial4j.shape.Rectangle; -import org.locationtech.spatial4j.shape.Shape; -import org.locationtech.spatial4j.shape.ShapeCollection; +import org.locationtech.spatial4j.shape.*; import org.locationtech.spatial4j.shape.impl.BufferedLine; import org.locationtech.spatial4j.shape.impl.BufferedLineString; diff --git a/src/main/java/org/locationtech/spatial4j/io/jackson/ShapeAsGeoJSONSerializer.java b/src/main/java/org/locationtech/spatial4j/io/jackson/ShapeAsGeoJSONSerializer.java index 568639958..5bef269e8 100644 --- a/src/main/java/org/locationtech/spatial4j/io/jackson/ShapeAsGeoJSONSerializer.java +++ b/src/main/java/org/locationtech/spatial4j/io/jackson/ShapeAsGeoJSONSerializer.java @@ -37,8 +37,11 @@ public class ShapeAsGeoJSONSerializer extends JsonSerializer protected void write(JsonGenerator gen, double... coords) throws IOException { gen.writeStartArray(); - for (int i = 0; i < coords.length; i++) { - gen.writeNumber(coords[i]); + for (double coord : coords) { + if (Double.isNaN(coord)) { + break; // empty + } + gen.writeNumber(coord); } gen.writeEndArray(); } diff --git a/src/main/java/org/locationtech/spatial4j/io/jackson/ShapeDeserializer.java b/src/main/java/org/locationtech/spatial4j/io/jackson/ShapeDeserializer.java index 42790af7b..7092d573a 100644 --- a/src/main/java/org/locationtech/spatial4j/io/jackson/ShapeDeserializer.java +++ b/src/main/java/org/locationtech/spatial4j/io/jackson/ShapeDeserializer.java @@ -38,6 +38,9 @@ public ShapeDeserializer(SpatialContext ctx) { } public Point readPoint(ArrayNode arr, ShapeFactory factory) { + if(arr.size()==0) { + return factory.pointXY(Double.NaN, Double.NaN); + } double x = arr.get(0).asDouble(); double y = arr.get(1).asDouble(); if(arr.size()==3) { diff --git a/src/test/java/org/locationtech/spatial4j/io/GeneralPolyshapeTest.java b/src/test/java/org/locationtech/spatial4j/io/GeneralPolyshapeTest.java index cc74f53de..c8d652b6f 100644 --- a/src/test/java/org/locationtech/spatial4j/io/GeneralPolyshapeTest.java +++ b/src/test/java/org/locationtech/spatial4j/io/GeneralPolyshapeTest.java @@ -19,6 +19,7 @@ import org.junit.Assert; import org.junit.Before; +import org.junit.Ignore; public class GeneralPolyshapeTest extends GeneralReadWriteShapeTest { @@ -61,4 +62,9 @@ public boolean shouldBeEqualAfterRoundTrip() { return false; // the polyline values will be off by a small fraction -- everything is rounded to: Math.round(value * 1e5) } + @Override + @Ignore + public void testEmptyGeometryCollection() throws Exception { + assumeTrue(false); // not supported + } } \ No newline at end of file diff --git a/src/test/java/org/locationtech/spatial4j/io/GeneralReadWriteShapeTest.java b/src/test/java/org/locationtech/spatial4j/io/GeneralReadWriteShapeTest.java index 434933f2e..78cdef522 100644 --- a/src/test/java/org/locationtech/spatial4j/io/GeneralReadWriteShapeTest.java +++ b/src/test/java/org/locationtech/spatial4j/io/GeneralReadWriteShapeTest.java @@ -27,6 +27,7 @@ import org.locationtech.spatial4j.util.GeomBuilder; import java.util.Arrays; +import java.util.Collections; public abstract class GeneralReadWriteShapeTest extends BaseRoundTripTest { @@ -70,7 +71,17 @@ protected void assertRoundTrip(Shape shape, boolean andEquals) throws Exception Assert.assertEquals(shape, out); } } - + + @Test + public void testEmptyPoint() throws Exception { + assertRoundTrip(ctx.getShapeFactory().pointXY(Double.NaN, Double.NaN)); + } + + @Test + public void testEmptyGeometryCollection() throws Exception { + assertRoundTrip(ctx.getShapeFactory().multiShape(Collections.emptyList())); + } + @Test public void testWriteThenReadPoint() throws Exception { assertRoundTrip(point());