Safer memory management in geos

Also speed up avoid intersections by removing unnecessary geometry
cloning

doc/api_break.dox

@@ -1289,6 +1289,7 @@ QgsGeometryEngine        {#qgis_api_break_3_0_QgsGeometryEngine}
 - `centroid()` returns the point instead of working on a parameter. The return value is a `nullptr` when `false` has been returned in the past.
 - `pointOnSurface()` returns the point instead of working on a parameter. The return value is a `nullptr` when `false` has been returned in the past.
 - splitGeometry() now returns new geometries as QgsGeometry, instead of QgsAbstractGeometry
+- combine() now requires a list of QgsGeometry, instead of QgsAbstractGeometry
 
 
 QgsGeometrySimplifier        {#qgis_api_break_3_0_QgsGeometrySimplifier}

python/core/geometry/qgsgeometryengine.sip

@@ -77,7 +77,7 @@ class QgsGeometryEngine
  :rtype: QgsAbstractGeometry
 %End
 
-    virtual QgsAbstractGeometry *combine( const QList< QgsAbstractGeometry * > &geometries, QString *errorMsg = 0 ) const = 0 /Factory/;
+    virtual QgsAbstractGeometry *combine( const QList< QgsGeometry > &geometries, QString *errorMsg = 0 ) const = 0 /Factory/;
 %Docstring
  Calculate the combination of this and ``geometries``.
 

src/core/geometry/qgsgeometry.cpp

@@ -2228,17 +2228,8 @@ QgsGeometry QgsGeometry::unaryUnion( const QList<QgsGeometry> &geometries )
 {
   QgsGeos geos( nullptr );
 
-  QList<QgsAbstractGeometry *> geomV2List;
-  for ( const QgsGeometry &g : geometries )
-  {
-    if ( !( g.isNull() ) )
-    {
-      geomV2List.append( g.geometry() );
-    }
-  }
-
   QString error;
-  std::unique_ptr< QgsAbstractGeometry > geom( geos.combine( geomV2List, &error ) );
+  std::unique_ptr< QgsAbstractGeometry > geom( geos.combine( geometries, &error ) );
   QgsGeometry result( std::move( geom ) );
   result.mLastError = error;
   return result;

src/core/geometry/qgsgeometryeditutils.cpp

@@ -240,7 +240,7 @@ std::unique_ptr<QgsAbstractGeometry> QgsGeometryEditUtils::avoidIntersections( c
   if ( avoidIntersectionsLayers.isEmpty() )
     return nullptr; //no intersections stored in project does not mean error
 
-  QList< QgsAbstractGeometry * > nearGeometries;
+  QList< QgsGeometry > nearGeometries;
 
   //go through list, convert each layer to vector layer and call QgsVectorLayer::removePolygonIntersections for each
   for ( QgsVectorLayer *currentLayer : avoidIntersectionsLayers )
@@ -262,7 +262,7 @@ std::unique_ptr<QgsAbstractGeometry> QgsGeometryEditUtils::avoidIntersections( c
       if ( !f.hasGeometry() )
         continue;
 
-      nearGeometries << f.geometry().geometry()->clone();
+      nearGeometries << f.geometry();
     }
   }
 
@@ -271,9 +271,7 @@ std::unique_ptr<QgsAbstractGeometry> QgsGeometryEditUtils::avoidIntersections( c
     return nullptr;
   }
 
-
   std::unique_ptr< QgsAbstractGeometry > combinedGeometries( geomEngine->combine( nearGeometries ) );
-  qDeleteAll( nearGeometries );
   if ( !combinedGeometries )
   {
     return nullptr;

src/core/geometry/qgsgeometryengine.h

@@ -96,7 +96,7 @@ class CORE_EXPORT QgsGeometryEngine
      *
      * \since QGIS 3.0 \a geom is a pointer
      */
-    virtual QgsAbstractGeometry *combine( const QList< QgsAbstractGeometry * > &geometries, QString *errorMsg = nullptr ) const = 0 SIP_FACTORY;
+    virtual QgsAbstractGeometry *combine( const QList< QgsGeometry > &geometries, QString *errorMsg = nullptr ) const = 0 SIP_FACTORY;
 
     /**
      * Calculate the symmetric difference of this and \a geom.

src/core/geometry/qgsgeos.cpp

@@ -333,14 +333,17 @@ QgsAbstractGeometry *QgsGeos::combine( const QgsAbstractGeometry *geom, QString
   return overlay( geom, UNION, errorMsg ).release();
 }
 
-QgsAbstractGeometry *QgsGeos::combine( const QList<QgsAbstractGeometry *> &geomList, QString *errorMsg ) const
+QgsAbstractGeometry *QgsGeos::combine( const QList<QgsGeometry> &geomList, QString *errorMsg ) const
 {
 
   QVector< GEOSGeometry * > geosGeometries;
-  geosGeometries.resize( geomList.size() );
-  for ( int i = 0; i < geomList.size(); ++i )
+  geosGeometries.reserve( geomList.size() );
+  for ( const QgsGeometry &g : geomList )
   {
-    geosGeometries[i] = asGeos( geomList.at( i ), mPrecision );
+    if ( !g )
+      continue;
+
+    geosGeometries << asGeos( g.geometry(), mPrecision );
   }
 
   GEOSGeometry *geomUnion = nullptr;

src/core/geometry/qgsgeos.h

@@ -74,7 +74,7 @@ class CORE_EXPORT QgsGeos: public QgsGeometryEngine
     std::unique_ptr< QgsAbstractGeometry > subdivide( int maxNodes, QString *errorMsg = nullptr ) const;
 
     QgsAbstractGeometry *combine( const QgsAbstractGeometry *geom, QString *errorMsg = nullptr ) const override;
-    QgsAbstractGeometry *combine( const QList< QgsAbstractGeometry *> &, QString *errorMsg = nullptr ) const override;
+    QgsAbstractGeometry *combine( const QList< QgsGeometry > &, QString *errorMsg = nullptr ) const override;
     QgsAbstractGeometry *symDifference( const QgsAbstractGeometry *geom, QString *errorMsg = nullptr ) const override;
     QgsAbstractGeometry *buffer( double distance, int segments, QString *errorMsg = nullptr ) const override;
     QgsAbstractGeometry *buffer( double distance, int segments, int endCapStyle, int joinStyle, double miterLimit, QString *errorMsg = nullptr ) const override;

src/plugins/geometry_checker/checks/qgsgeometrygapcheck.cpp

@@ -25,14 +25,14 @@ void QgsGeometryGapCheck::collectErrors( QList<QgsGeometryCheckError *> &errors,
   if ( progressCounter ) progressCounter->fetchAndAddRelaxed( 1 );
 
   // Collect geometries, build spatial index
-  QList<QgsAbstractGeometry *> geomList;
+  QList<QgsGeometry > geomList;
   const QgsFeatureIds &featureIds = ids.isEmpty() ? mFeaturePool->getFeatureIds() : ids;
   Q_FOREACH ( QgsFeatureId id, featureIds )
   {
     QgsFeature feature;
     if ( mFeaturePool->get( id, feature ) )
     {
-      geomList.append( feature.geometry().geometry()->clone() );
+      geomList.append( feature.geometry() );
     }
   }
 
@@ -46,7 +46,6 @@ void QgsGeometryGapCheck::collectErrors( QList<QgsGeometryCheckError *> &errors,
   // Create union of geometry
   QString errMsg;
   QgsAbstractGeometry *unionGeom = geomEngine->combine( geomList, &errMsg );
-  qDeleteAll( geomList );
   delete geomEngine;
   if ( !unionGeom )
   {