From 747fd51907b10d9a535f11782d659db40d7cf22d Mon Sep 17 00:00:00 2001 From: Nyall Dawson Date: Fri, 25 Oct 2024 10:58:11 +1000 Subject: [PATCH] Fix crashes on newer GEOS with empty polygon rings There was a previous fix for this protecting some geos calls, but on newer GEOS versions we get crashes with other methods (eg calculating centroid) when using geos geometries with empty interior rings. Avoid this by ALWAYS defaulting to skipping empty rings when creating GEOS polygons, UNLESS explicitly asked to. Then, only explicitly ask to do this when we are using GEOS to validate a geometry. In all other cases we don't need or want empty rings. --- .../PyQt6/core/auto_generated/geometry/qgsgeometry.sip.in | 4 +++- python/core/auto_generated/geometry/qgsgeometry.sip.in | 4 +++- .../vector/geometry_checker/qgsgeometrycheckerutils.cpp | 4 ++-- .../vector/geometry_checker/qgsgeometrycheckerutils.h | 2 +- .../vector/geometry_checker/qgsgeometryduplicatecheck.cpp | 2 +- src/core/geometry/qgscurve.cpp | 2 +- src/core/geometry/qgsgeometry.cpp | 6 +++--- src/core/geometry/qgsgeometry.h | 4 +++- src/core/geometry/qgsgeos.h | 2 +- src/core/geometry/qgspolyhedralsurface.cpp | 2 +- src/core/geometry/qgssurface.cpp | 2 +- src/core/qgsgeometryvalidator.cpp | 2 +- 12 files changed, 21 insertions(+), 15 deletions(-) diff --git a/python/PyQt6/core/auto_generated/geometry/qgsgeometry.sip.in b/python/PyQt6/core/auto_generated/geometry/qgsgeometry.sip.in index 449e6c591bbb..b43a47c15025 100644 --- a/python/PyQt6/core/auto_generated/geometry/qgsgeometry.sip.in +++ b/python/PyQt6/core/auto_generated/geometry/qgsgeometry.sip.in @@ -2957,7 +2957,7 @@ geometry will retain the same dimensionality as the input geometry. :param maxAngle: maximum angle at node (0-180) at which smoothing will be applied %End - static QgsGeometryEngine *createGeometryEngine( const QgsAbstractGeometry *geometry, double precision = 0.0 ) /Factory/; + static QgsGeometryEngine *createGeometryEngine( const QgsAbstractGeometry *geometry, double precision = 0.0, Qgis::GeosCreationFlags flags = Qgis::GeosCreationFlag::SkipEmptyInteriorRings ) /Factory/; %Docstring Creates and returns a new geometry engine representing the specified ``geometry`` using ``precision`` on a grid. The ``precision`` argument was added in 3.36. @@ -2972,6 +2972,8 @@ Many methods available in the :py:class:`QgsGeometryEngine` class can benefit fr a large number of spatial relationships will be tested (such as calling :py:func:`~QgsGeometry.intersects`, :py:func:`~QgsGeometry.within`, etc) then the geometry should first be prepared by calling :py:func:`~QgsGeometry.prepareGeometry` before performing the tests. +The ``flags`` argument was added in QGIS 3.40 to allow control over the resultant GEOS geometry. + Example ------------------------------------- diff --git a/python/core/auto_generated/geometry/qgsgeometry.sip.in b/python/core/auto_generated/geometry/qgsgeometry.sip.in index 449e6c591bbb..b43a47c15025 100644 --- a/python/core/auto_generated/geometry/qgsgeometry.sip.in +++ b/python/core/auto_generated/geometry/qgsgeometry.sip.in @@ -2957,7 +2957,7 @@ geometry will retain the same dimensionality as the input geometry. :param maxAngle: maximum angle at node (0-180) at which smoothing will be applied %End - static QgsGeometryEngine *createGeometryEngine( const QgsAbstractGeometry *geometry, double precision = 0.0 ) /Factory/; + static QgsGeometryEngine *createGeometryEngine( const QgsAbstractGeometry *geometry, double precision = 0.0, Qgis::GeosCreationFlags flags = Qgis::GeosCreationFlag::SkipEmptyInteriorRings ) /Factory/; %Docstring Creates and returns a new geometry engine representing the specified ``geometry`` using ``precision`` on a grid. The ``precision`` argument was added in 3.36. @@ -2972,6 +2972,8 @@ Many methods available in the :py:class:`QgsGeometryEngine` class can benefit fr a large number of spatial relationships will be tested (such as calling :py:func:`~QgsGeometry.intersects`, :py:func:`~QgsGeometry.within`, etc) then the geometry should first be prepared by calling :py:func:`~QgsGeometry.prepareGeometry` before performing the tests. +The ``flags`` argument was added in QGIS 3.40 to allow control over the resultant GEOS geometry. + Example ------------------------------------- diff --git a/src/analysis/vector/geometry_checker/qgsgeometrycheckerutils.cpp b/src/analysis/vector/geometry_checker/qgsgeometrycheckerutils.cpp index e36541e44de7..016c967045e2 100644 --- a/src/analysis/vector/geometry_checker/qgsgeometrycheckerutils.cpp +++ b/src/analysis/vector/geometry_checker/qgsgeometrycheckerutils.cpp @@ -265,9 +265,9 @@ QgsGeometryCheckerUtils::LayerFeatures::iterator QgsGeometryCheckerUtils::LayerF ///////////////////////////////////////////////////////////////////////////// -std::unique_ptr QgsGeometryCheckerUtils::createGeomEngine( const QgsAbstractGeometry *geometry, double tolerance ) +std::unique_ptr QgsGeometryCheckerUtils::createGeomEngine( const QgsAbstractGeometry *geometry, double tolerance, Qgis::GeosCreationFlags flags ) { - return std::make_unique( geometry, tolerance ); + return std::make_unique( geometry, tolerance, flags ); } QgsAbstractGeometry *QgsGeometryCheckerUtils::getGeomPart( QgsAbstractGeometry *geom, int partIdx ) diff --git a/src/analysis/vector/geometry_checker/qgsgeometrycheckerutils.h b/src/analysis/vector/geometry_checker/qgsgeometrycheckerutils.h index 7005a5a32210..407dc2adbb98 100644 --- a/src/analysis/vector/geometry_checker/qgsgeometrycheckerutils.h +++ b/src/analysis/vector/geometry_checker/qgsgeometrycheckerutils.h @@ -214,7 +214,7 @@ class ANALYSIS_EXPORT QgsGeometryCheckerUtils #ifndef SIP_RUN - static std::unique_ptr createGeomEngine( const QgsAbstractGeometry *geometry, double tolerance ); + static std::unique_ptr createGeomEngine( const QgsAbstractGeometry *geometry, double tolerance, Qgis::GeosCreationFlags flags = Qgis::GeosCreationFlag::SkipEmptyInteriorRings ); static QgsAbstractGeometry *getGeomPart( QgsAbstractGeometry *geom, int partIdx ); static const QgsAbstractGeometry *getGeomPart( const QgsAbstractGeometry *geom, int partIdx ); diff --git a/src/analysis/vector/geometry_checker/qgsgeometryduplicatecheck.cpp b/src/analysis/vector/geometry_checker/qgsgeometryduplicatecheck.cpp index 5d7fa5466fea..f887008b454e 100644 --- a/src/analysis/vector/geometry_checker/qgsgeometryduplicatecheck.cpp +++ b/src/analysis/vector/geometry_checker/qgsgeometryduplicatecheck.cpp @@ -51,7 +51,7 @@ void QgsGeometryDuplicateCheck::collectErrors( const QMap geomEngineA = QgsGeometryCheckerUtils::createGeomEngine( geomA.constGet(), mContext->tolerance ); + std::unique_ptr< QgsGeometryEngine > geomEngineA = QgsGeometryCheckerUtils::createGeomEngine( geomA.constGet(), mContext->tolerance, Qgis::GeosCreationFlags() ); if ( !geomEngineA->isValid() ) { messages.append( tr( "Duplicate check failed for (%1): the geometry is invalid" ).arg( layerFeatureA.id() ) ); diff --git a/src/core/geometry/qgscurve.cpp b/src/core/geometry/qgscurve.cpp index e256f914993c..842877f10dd6 100644 --- a/src/core/geometry/qgscurve.cpp +++ b/src/core/geometry/qgscurve.cpp @@ -253,7 +253,7 @@ bool QgsCurve::isValid( QString &error, Qgis::GeometryValidityFlags flags ) cons return error.isEmpty(); } - const QgsGeos geos( this ); + const QgsGeos geos( this, 0, Qgis::GeosCreationFlags() ); const bool res = geos.isValid( &error, flags & Qgis::GeometryValidityFlag::AllowSelfTouchingHoles, nullptr ); if ( flags == 0 ) { diff --git a/src/core/geometry/qgsgeometry.cpp b/src/core/geometry/qgsgeometry.cpp index 768a4981ad62..ab0c310fba34 100644 --- a/src/core/geometry/qgsgeometry.cpp +++ b/src/core/geometry/qgsgeometry.cpp @@ -3483,7 +3483,7 @@ void QgsGeometry::validateGeometry( QVector &errors, const Q case Qgis::GeometryValidationEngine::Geos: { - QgsGeos geos( d->geometry.get() ); + QgsGeos geos( d->geometry.get(), 0, Qgis::GeosCreationFlags() ); QString error; QgsGeometry errorLoc; if ( !geos.isValid( &error, flags & Qgis::GeometryValidityFlag::AllowSelfTouchingHoles, &errorLoc ) ) @@ -4467,9 +4467,9 @@ QgsGeometry QgsGeometry::convertToPolygon( bool destMultipart ) const } } -QgsGeometryEngine *QgsGeometry::createGeometryEngine( const QgsAbstractGeometry *geometry, double precision ) +QgsGeometryEngine *QgsGeometry::createGeometryEngine( const QgsAbstractGeometry *geometry, double precision, Qgis::GeosCreationFlags flags ) { - return new QgsGeos( geometry, precision ); + return new QgsGeos( geometry, precision, flags ); } QDataStream &operator<<( QDataStream &out, const QgsGeometry &geometry ) diff --git a/src/core/geometry/qgsgeometry.h b/src/core/geometry/qgsgeometry.h index 4411e2a1c9a7..c02ee5171f67 100644 --- a/src/core/geometry/qgsgeometry.h +++ b/src/core/geometry/qgsgeometry.h @@ -3137,6 +3137,8 @@ class CORE_EXPORT QgsGeometry * a large number of spatial relationships will be tested (such as calling intersects(), within(), etc) then the * geometry should first be prepared by calling prepareGeometry() before performing the tests. * + * The \a flags argument was added in QGIS 3.40 to allow control over the resultant GEOS geometry. + * * ### Example * * \code{.py} @@ -3160,7 +3162,7 @@ class CORE_EXPORT QgsGeometry * * QgsGeometryEngine operations are backed by the GEOS library (https://trac.osgeo.org/geos/). */ - static QgsGeometryEngine *createGeometryEngine( const QgsAbstractGeometry *geometry, double precision = 0.0 ) SIP_FACTORY; + static QgsGeometryEngine *createGeometryEngine( const QgsAbstractGeometry *geometry, double precision = 0.0, Qgis::GeosCreationFlags flags = Qgis::GeosCreationFlag::SkipEmptyInteriorRings ) SIP_FACTORY; /** * Upgrades a point list from QgsPointXY to QgsPoint diff --git a/src/core/geometry/qgsgeos.h b/src/core/geometry/qgsgeos.h index 4adec18cc480..454ae0c44bcc 100644 --- a/src/core/geometry/qgsgeos.h +++ b/src/core/geometry/qgsgeos.h @@ -144,7 +144,7 @@ class CORE_EXPORT QgsGeos: public QgsGeometryEngine * \param flags GEOS creation flags (since QGIS 3.40) * \note The third parameter was prior to QGIS 3.40 a boolean which has been incorporated into the flag */ - QgsGeos( const QgsAbstractGeometry *geometry, double precision = 0, Qgis::GeosCreationFlags flags = Qgis::GeosCreationFlags() ); + QgsGeos( const QgsAbstractGeometry *geometry, double precision = 0, Qgis::GeosCreationFlags flags = Qgis::GeosCreationFlag::SkipEmptyInteriorRings ); /** * Creates a new QgsGeometry object, feeding in a geometry in GEOS format. diff --git a/src/core/geometry/qgspolyhedralsurface.cpp b/src/core/geometry/qgspolyhedralsurface.cpp index cf2b7b095406..ce418562e4a0 100644 --- a/src/core/geometry/qgspolyhedralsurface.cpp +++ b/src/core/geometry/qgspolyhedralsurface.cpp @@ -1013,7 +1013,7 @@ bool QgsPolyhedralSurface::isValid( QString &error, Qgis::GeometryValidityFlags // GEOS does not handle PolyhedralSurface, check the polygons one by one for ( int i = 0; i < mPatches.size(); ++i ) { - const QgsGeos geos( mPatches.at( i ) ); + const QgsGeos geos( mPatches.at( i ), 0, Qgis::GeosCreationFlags() ); const bool valid = geos.isValid( &error, flags & Qgis::GeometryValidityFlag::AllowSelfTouchingHoles, nullptr ); if ( !valid ) { diff --git a/src/core/geometry/qgssurface.cpp b/src/core/geometry/qgssurface.cpp index 44045c0bcd3e..0f91df02a92a 100644 --- a/src/core/geometry/qgssurface.cpp +++ b/src/core/geometry/qgssurface.cpp @@ -30,7 +30,7 @@ bool QgsSurface::isValid( QString &error, Qgis::GeometryValidityFlags flags ) co return error.isEmpty(); } - const QgsGeos geos( this ); + const QgsGeos geos( this, 0, Qgis::GeosCreationFlags() ); const bool res = geos.isValid( &error, flags & Qgis::GeometryValidityFlag::AllowSelfTouchingHoles, nullptr ); if ( flags == 0 ) { diff --git a/src/core/qgsgeometryvalidator.cpp b/src/core/qgsgeometryvalidator.cpp index 66f147f2ec71..92c88cffc43c 100644 --- a/src/core/qgsgeometryvalidator.cpp +++ b/src/core/qgsgeometryvalidator.cpp @@ -257,7 +257,7 @@ void QgsGeometryValidator::run() return; } - const QgsGeos geos( mGeometry.constGet() ); + const QgsGeos geos( mGeometry.constGet(), 0, Qgis::GeosCreationFlags() ); QString error; QgsGeometry errorLoc; if ( !geos.isValid( &error, true, &errorLoc ) )