Fix WKT for atomic multi-geometries with one or more empties

libgeos · Aug 29, 2023 · 6429ffd · 6429ffd
1 parent 15185e6
commit 6429ffd
Show file tree

Hide file tree

Showing 4 changed files with 70 additions and 15 deletions.
diff --git a/NEWS.md b/NEWS.md
@@ -11,6 +11,7 @@
   - ConvexHull: Performance improvement for larger geometries (JTS-985, Martin Davis)
   - Intersection: change to using DoubleDouble computation to improve robustness (GH-937, Martin Davis)
   - Fix LargestEmptyCircle to respect polygonal obstacles (GH-939, Martin Davis)
+  - Fix WKT for atomic multi-geometries with one or more empties (GH-952, Mike Taves)
 
 
 ## Changes in 3.12.0

diff --git a/src/io/WKTWriter.cpp b/src/io/WKTWriter.cpp
@@ -477,12 +477,13 @@ void
 WKTWriter::appendMultiPointText(const MultiPoint& multiPoint, OrdinateSet outputOrdinates,
                                 int /*level*/, Writer& writer) const
 {
-    if(multiPoint.isEmpty()) {
+    const std::size_t n = multiPoint.getNumGeometries();
+    if(n == 0) {
         writer.write("EMPTY");
     }
     else {
         writer.write("(");
-        for(std::size_t i = 0, n = multiPoint.getNumGeometries(); i < n; ++i) {
+        for(std::size_t i = 0; i < n; ++i) {
             if(i > 0) {
                 writer.write(", ");
             }
@@ -506,15 +507,15 @@ void
 WKTWriter::appendMultiLineStringText(const MultiLineString& multiLineString, OrdinateSet outputOrdinates, int p_level, bool indentFirst,
                                      Writer& writer) const
 {
-    if(multiLineString.isEmpty()) {
+    const std::size_t n = multiLineString.getNumGeometries();
+    if(n == 0) {
         writer.write("EMPTY");
     }
     else {
         int level2 = p_level;
         bool doIndent = indentFirst;
         writer.write("(");
-        for(std::size_t i = 0, n = multiLineString.getNumGeometries();
-                i < n; ++i) {
+        for(std::size_t i = 0; i < n; ++i) {
             if(i > 0) {
                 writer.write(", ");
                 level2 = p_level + 1;
@@ -530,15 +531,15 @@ WKTWriter::appendMultiLineStringText(const MultiLineString& multiLineString, Ord
 void
 WKTWriter::appendMultiPolygonText(const MultiPolygon& multiPolygon, OrdinateSet outputOrdinates, int p_level, Writer& writer) const
 {
-    if(multiPolygon.isEmpty()) {
+    const std::size_t n = multiPolygon.getNumGeometries();
+    if(n == 0) {
         writer.write("EMPTY");
     }
     else {
         int level2 = p_level;
         bool doIndent = false;
         writer.write("(");
-        for(std::size_t i = 0, n = multiPolygon.getNumGeometries();
-                i < n; ++i) {
+        for(std::size_t i = 0; i < n; ++i) {
             if(i > 0) {
                 writer.write(", ");
                 level2 = p_level + 1;
@@ -558,11 +559,14 @@ WKTWriter::appendGeometryCollectionText(
     int p_level,
     Writer& writer) const
 {
-    if(geometryCollection.getNumGeometries() > 0) {
+    const std::size_t n = geometryCollection.getNumGeometries();
+    if(n == 0) {
+        writer.write("EMPTY");
+    }
+    else {
         int level2 = p_level;
         writer.write("(");
-        for(std::size_t i = 0, n = geometryCollection.getNumGeometries();
-                i < n; ++i) {
+        for(std::size_t i = 0; i < n; ++i) {
             if(i > 0) {
                 writer.write(", ");
                 level2 = p_level + 1;
@@ -571,9 +575,6 @@ WKTWriter::appendGeometryCollectionText(
         }
         writer.write(")");
     }
-    else {
-        writer.write("EMPTY");
-    }
 }
 
 void

diff --git a/tests/unit/io/GeoJSONReaderTest.cpp b/tests/unit/io/GeoJSONReaderTest.cpp
@@ -434,7 +434,7 @@ void object::test<28>
 {
     std::string geojson { "{\"type\":\"MultiLineString\",\"coordinates\":[[],[],[]]}" };
     GeomPtr geom(geojsonreader.read(geojson));
-    ensure_equals(geom->toText(), "MULTILINESTRING EMPTY");
+    ensure_equals(geom->toText(), "MULTILINESTRING (EMPTY, EMPTY, EMPTY)");
     ensure_equals(static_cast<size_t>(geom->getCoordinateDimension()), 2u);
 }
 

diff --git a/tests/unit/io/WKTWriterTest.cpp b/tests/unit/io/WKTWriterTest.cpp
@@ -405,4 +405,57 @@ void object::test<14>
     ensure_equals(wktwriter.write(*g), "LINESTRING M (1 2 3, 4 5 NaN)");
 }
 
+// Test multi-part geometries with zero or more empty parts
+// https://github.com/libgeos/geos/issues/951
+template<>
+template<>
+void object::test<15>
+()
+{
+    // Run all combinations of atomic multi-geometry types and dimensions
+    std::vector<std::string> geom_types{"POINT", "LINESTRING", "POLYGON"};
+    std::vector<std::string> dim_types{"", " Z", " M", " ZM"};
+    for (const auto& geom_type : geom_types) {
+        // zero empties -- but don't check dim_types
+        // https://github.com/libgeos/geos/issues/888
+        const auto wkt0 = "MULTI" + geom_type + " EMPTY";
+        const auto g0 = wktreader.read(wkt0);
+        ensure_equals(g0->getNumGeometries(), 0u);
+        ensure_equals(wktwriter.write(*g0), wkt0);
+        for (const auto& dim_type : dim_types) {
+            // single empty
+            const auto wkt1 = "MULTI" + geom_type + dim_type + " (EMPTY)";
+            const auto g1 = wktreader.read(wkt1);
+            ensure_equals(g1->getNumGeometries(), 1u);
+            ensure_equals(wktwriter.write(*g1), wkt1);
+            // two empties
+            const auto wkt2 = "MULTI" + geom_type + " (EMPTY, EMPTY)";
+            const auto g2 = wktreader.read(wkt2);
+            ensure_equals(g2->getNumGeometries(), 2u);
+            ensure_equals(wktwriter.write(*g2), wkt2);
+        }
+    }
+
+    // Run remaining combinations with GeometryCollection
+    const auto wkt0 = "GEOMETRYCOLLECTION EMPTY";
+    const auto g0 = wktreader.read(wkt0);
+    ensure_equals(g0->getNumGeometries(), 0u);
+    ensure_equals(wktwriter.write(*g0), wkt0);
+    for (const auto& dim_type : dim_types) {
+        // single empty
+        const auto wkt1 = "GEOMETRYCOLLECTION" + dim_type +
+            " (POINT" + dim_type + " EMPTY)";
+        const auto g1 = wktreader.read(wkt1);
+        ensure_equals(g1->getNumGeometries(), 1u);
+        ensure_equals(wktwriter.write(*g1), wkt1);
+        // two empties
+        const auto wkt2 = "GEOMETRYCOLLECTION" + dim_type +
+            " (POINT" + dim_type + " EMPTY, LINESTRING" + dim_type + " EMPTY)";
+        const auto g2 = wktreader.read(wkt2);
+        ensure_equals(g2->getNumGeometries(), 2u);
+        ensure_equals(wktwriter.write(*g2), wkt2);
+    }
+
+}
+
 } // namespace tut