Skip to content

Commit

Permalink
Merge pull request #10823 from rouault/fix_10819
Browse files Browse the repository at this point in the history
MakeValid(METHOD=STRUCTURE): make sure to return a MULTIPOLYGON if input is MULTIPOLYGON
  • Loading branch information
rouault authored Sep 18, 2024
2 parents 39c1842 + ec3f277 commit 7567141
Show file tree
Hide file tree
Showing 2 changed files with 71 additions and 16 deletions.
66 changes: 51 additions & 15 deletions autotest/ogr/ogr_geom.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)))"
)


###############################################################################
Expand Down
21 changes: 20 additions & 1 deletion ogr/ogrgeometry.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit 7567141

Please sign in to comment.