diff --git a/autotest/ogr/ogr_geom.py b/autotest/ogr/ogr_geom.py index 935176d7c4f2..67911355b88c 100755 --- a/autotest/ogr/ogr_geom.py +++ b/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)))" + ) ############################################################################### diff --git a/ogr/ogrgeometry.cpp b/ogr/ogrgeometry.cpp index 287f20370bff..46dd0bd1264c 100644 --- a/ogr/ogrgeometry.cpp +++ b/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);