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.
qgis · Oct 25, 2024 · e87a1e7 · e87a1e7
1 parent b56115d
commit e87a1e7
Show file tree

Hide file tree

Showing 12 changed files with 21 additions and 15 deletions.
diff --git 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
@@ -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
@@ -265,9 +265,9 @@ QgsGeometryCheckerUtils::LayerFeatures::iterator QgsGeometryCheckerUtils::LayerF
 
 /////////////////////////////////////////////////////////////////////////////
 
-std::unique_ptr<QgsGeometryEngine> QgsGeometryCheckerUtils::createGeomEngine( const QgsAbstractGeometry *geometry, double tolerance )
+std::unique_ptr<QgsGeometryEngine> QgsGeometryCheckerUtils::createGeomEngine( const QgsAbstractGeometry *geometry, double tolerance, Qgis::GeosCreationFlags flags )
 {
-  return std::make_unique<QgsGeos>( geometry, tolerance );
+  return std::make_unique<QgsGeos>( 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
@@ -214,7 +214,7 @@ class ANALYSIS_EXPORT QgsGeometryCheckerUtils
 
 #ifndef SIP_RUN
 
-    static std::unique_ptr<QgsGeometryEngine> createGeomEngine( const QgsAbstractGeometry *geometry, double tolerance );
+    static std::unique_ptr<QgsGeometryEngine> 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
@@ -51,7 +51,7 @@ void QgsGeometryDuplicateCheck::collectErrors( const QMap<QString, QgsFeaturePoo
 
     const QgsGeometry geomA = layerFeatureA.geometry();
     const QgsRectangle bboxA = geomA.boundingBox();
-    std::unique_ptr< QgsGeometryEngine > 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
@@ -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
@@ -3483,7 +3483,7 @@ void QgsGeometry::validateGeometry( QVector<QgsGeometry::Error> &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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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 ) )