Merge pull request #10823 from rouault/fix_10819

MakeValid(METHOD=STRUCTURE): make sure to return a MULTIPOLYGON if input is MULTIPOLYGON

autotest/ogr/ogr_geom.py

@@ -3986,21 +3986,57 @@ def test_ogr_geom_makevalid():
             g, "MULTIPOLYGON (((0 0,5 5,10 0,0 0)),((5 5,0 10,10 10,5 5)))"
         )
 
-    if (
-        ogr.GetGEOSVersionMajor() * 10000
-        + ogr.GetGEOSVersionMinor() * 100
-        + ogr.GetGEOSVersionMicro()
-        >= 31000
-    ):
-        g = ogr.CreateGeometryFromWkt(
-            "POLYGON ((0 0,0 10,10 10,10 0,0 0),(5 5,15 10,15 0,5 5))"
-        )
-        # Only since GEOS 3.10
-        g = g.MakeValid(["METHOD=STRUCTURE"])
-        if g is not None:
-            ogrtest.check_feature_geometry(
-                g, "POLYGON ((0 10,10 10,10.0 7.5,5 5,10.0 2.5,10 0,0 0,0 10))"
-            )
+
+###############################################################################
+
+
+@pytest.mark.require_geos(3, 10, 0)
+def test_ogr_geom_makevalid_structure():
+
+    g = ogr.CreateGeometryFromWkt(
+        "POLYGON ((0 0,0 10,10 10,10 0,0 0),(5 5,15 10,15 0,5 5))"
+    )
+    g = g.MakeValid(["METHOD=STRUCTURE"])
+    ogrtest.check_feature_geometry(
+        g, "POLYGON ((0 10,10 10,10.0 7.5,5 5,10.0 2.5,10 0,0 0,0 10))"
+    )
+
+    # Already valid multi-polygon made of a single-part
+    g = ogr.CreateGeometryFromWkt("MULTIPOLYGON (((0 0,1 0,1 1,0 1,0 0)))")
+    g = g.MakeValid(["METHOD=STRUCTURE"])
+    assert (
+        g.ExportToIsoWkt() == "MULTIPOLYGON (((0 0,1 0,1 1,0 1,0 0)))"
+        or g.ExportToIsoWkt() == "MULTIPOLYGON (((0 0,0 1,1 1,1 0,0 0)))"
+    )
+
+    # Already valid multi-polygon made of a single-part, with duplicated point
+    g = ogr.CreateGeometryFromWkt("MULTIPOLYGON (((0 0,1 0,1 0,1 1,0 1,0 0)))")
+    g = g.MakeValid(["METHOD=STRUCTURE"])
+    assert (
+        g.ExportToIsoWkt() == "MULTIPOLYGON (((0 0,1 0,1 1,0 1,0 0)))"
+        or g.ExportToIsoWkt() == "MULTIPOLYGON (((0 0,0 1,1 1,1 0,0 0)))"
+    )
+
+    # Already valid multi-polygon made of a single-part
+    g = ogr.CreateGeometryFromWkt(
+        "MULTIPOLYGON Z (((0 0 10,1 0 10,1 1 10,0 1 10,0 0 10)))"
+    )
+    g = g.MakeValid(["METHOD=STRUCTURE"])
+    assert (
+        g.ExportToIsoWkt() == "MULTIPOLYGON Z (((0 0 10,1 0 10,1 1 10,0 1 10,0 0 10)))"
+        or g.ExportToIsoWkt()
+        == "MULTIPOLYGON Z (((0 0 10,0 1 10,1 1 10,1 0 10,0 0 10)))"
+    )
+
+    # Already valid geometry collection
+    g = ogr.CreateGeometryFromWkt(
+        "GEOMETRYCOLLECTION (POLYGON ((0 0,1 0,1 1,0 1,0 0)))"
+    )
+    g = g.MakeValid(["METHOD=STRUCTURE"])
+    assert (
+        g.ExportToIsoWkt() == "GEOMETRYCOLLECTION (POLYGON ((0 0,1 0,1 1,0 1,0 0)))"
+        or g.ExportToIsoWkt() == "GEOMETRYCOLLECTION (POLYGON ((0 0,0 1,1 1,1 0,0 0)))"
+    )
 
 
 ###############################################################################

ogr/ogrgeometry.cpp

@@ -3862,7 +3862,12 @@ static OGRBoolean OGRGEOSBooleanPredicate(
 /**
  * \brief Attempts to make an invalid geometry valid without losing vertices.
  *
- * Already-valid geometries are cloned without further intervention.
+ * Already-valid geometries are cloned without further intervention
+ * for default MODE=LINEWORK. Already-valid geometries with MODE=STRUCTURE
+ * may be subject to non-significant transformations, such as duplicated point
+ * removal, change in ring winding order, etc. (before GDAL 3.10, single-part
+ * geometry collections could be returned a single geometry. GDAL 3.10
+ * returns the same type of geometry).
  *
  * Running OGRGeometryFactory::removeLowerDimensionSubGeoms() as a
  * post-processing step is often desired.
@@ -3973,6 +3978,20 @@ OGRGeometry *OGRGeometry::MakeValid(CSLConstList papszOptions) const
             poOGRProduct =
                 OGRGeometryRebuildCurves(this, nullptr, poOGRProduct);
             GEOSGeom_destroy_r(hGEOSCtxt, hGEOSRet);
+
+#if GEOS_VERSION_MAJOR > 3 ||                                                  \
+    (GEOS_VERSION_MAJOR == 3 && GEOS_VERSION_MINOR >= 10)
+            // METHOD=STRUCTURE is not guaranteed to return a multiple geometry
+            // if the input is a multiple geometry
+            if (poOGRProduct && bStructureMethod &&
+                OGR_GT_IsSubClassOf(getGeometryType(), wkbGeometryCollection) &&
+                !OGR_GT_IsSubClassOf(poOGRProduct->getGeometryType(),
+                                     wkbGeometryCollection))
+            {
+                poOGRProduct = OGRGeometryFactory::forceTo(poOGRProduct,
+                                                           getGeometryType());
+            }
+#endif
         }
     }
     freeGEOSContext(hGEOSCtxt);