Fix WKTWriter to emit EMPTY elements in multi-geometries (libgeos#952)

cfl0122 · Sep 5, 2023 · f9cfee8 · f9cfee8
1 parent 15185e6
commit f9cfee8
Show file tree

Hide file tree

Showing 4 changed files with 76 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 WKTWriter to emit EMPTY elements in multi-geometries (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,63 @@ 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>
+()
+{
+    // zero empties -- but don't check dim types
+    // https://github.com/libgeos/geos/issues/888
+    std::vector<std::string> variants0{
+        "MULTIPOINT EMPTY", "MULTILINESTRING EMPTY",
+        "MULTIPOLYGON EMPTY", "GEOMETRYCOLLECTION EMPTY"
+    };
+    for (const auto& wkt : variants0) {
+        const auto g = wktreader.read(wkt);
+        ensure_equals(wktwriter.write(*g), wkt);
+        ensure_equals(g->getNumGeometries(), 0u);
+    }
+
+    // single empty
+    std::vector<std::string> variants1{
+        "MULTIPOINT (EMPTY)", "MULTIPOINT Z (EMPTY)",
+        "MULTIPOINT M (EMPTY)", "MULTIPOINT ZM (EMPTY)",
+        "MULTILINESTRING (EMPTY)", "MULTILINESTRING Z (EMPTY)",
+        "MULTILINESTRING M (EMPTY)", "MULTILINESTRING ZM (EMPTY)",
+        "MULTIPOLYGON (EMPTY)", "MULTIPOLYGON Z (EMPTY)",
+        "MULTIPOLYGON M (EMPTY)", "MULTIPOLYGON ZM (EMPTY)",
+        "GEOMETRYCOLLECTION (MULTIPOINT EMPTY)",
+        "GEOMETRYCOLLECTION Z (POINT Z EMPTY)",
+        "GEOMETRYCOLLECTION M (LINESTRING M EMPTY)",
+        "GEOMETRYCOLLECTION ZM (POLYGON ZM EMPTY)"
+    };
+    for (const auto& wkt : variants1) {
+        const auto g = wktreader.read(wkt);
+        ensure_equals(wktwriter.write(*g), wkt);
+        ensure_equals(g->getNumGeometries(), 1u);
+    }
+
+    // two empties
+    std::vector<std::string> variants2{
+        "MULTIPOINT (EMPTY, EMPTY)", "MULTIPOINT Z (EMPTY, EMPTY)",
+        "MULTIPOINT M (EMPTY, EMPTY)", "MULTIPOINT ZM (EMPTY, EMPTY)",
+        "MULTILINESTRING (EMPTY, EMPTY)", "MULTILINESTRING Z (EMPTY, EMPTY)",
+        "MULTILINESTRING M (EMPTY, EMPTY)", "MULTILINESTRING ZM (EMPTY, EMPTY)",
+        "MULTIPOLYGON (EMPTY, EMPTY)", "MULTIPOLYGON Z (EMPTY, EMPTY)",
+        "MULTIPOLYGON M (EMPTY, EMPTY)", "MULTIPOLYGON ZM (EMPTY, EMPTY)",
+        "GEOMETRYCOLLECTION (POLYGON EMPTY, LINESTRING EMPTY)",
+        "GEOMETRYCOLLECTION Z (LINESTRING Z EMPTY, POINT Z EMPTY)",
+        "GEOMETRYCOLLECTION M (POINT M EMPTY, LINESTRING M EMPTY)",
+        "GEOMETRYCOLLECTION ZM (POINT ZM EMPTY, LINESTRING ZM EMPTY)"
+    };
+    for (const auto& wkt : variants2) {
+        const auto g = wktreader.read(wkt);
+        ensure_equals(wktwriter.write(*g), wkt);
+        ensure_equals(g->getNumGeometries(), 2u);
+    }
+
+}
+
 } // namespace tut