Some safer memory management for geos

qgis · Oct 13, 2017 · c3fdaa9 · c3fdaa9
1 parent 88e1360
commit c3fdaa9
Show file tree

Hide file tree

Showing 6 changed files with 61 additions and 29 deletions.
diff --git a/doc/api_break.dox b/doc/api_break.dox
@@ -1288,6 +1288,7 @@ QgsGeometryEngine        {#qgis_api_break_3_0_QgsGeometryEngine}
 - `QgsAbstractGeometry&` parameters have been changed to `QgsAbstractGeometry*` (Affects C++ only)
 - `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
 
 
 QgsGeometrySimplifier        {#qgis_api_break_3_0_QgsGeometrySimplifier}

diff --git a/python/core/geometry/qgsgeometryengine.sip b/python/core/geometry/qgsgeometryengine.sip
@@ -37,7 +37,21 @@ class QgsGeometryEngine
     virtual ~QgsGeometryEngine();
 
     virtual void geometryChanged() = 0;
+%Docstring
+ Should be called whenever the geometry associated with the engine
+ has been modified and the engine must be updated to suit.
+%End
+
     virtual void prepareGeometry() = 0;
+%Docstring
+ Prepares the geometry, so that subsequent calls to spatial relation methods
+ are much faster.
+
+ This should be called for any geometry which is used for multiple relation
+ tests against other geometries.
+
+.. seealso:: geometryChanged()
+%End
 
     virtual QgsAbstractGeometry *intersection( const QgsAbstractGeometry *geom, QString *errorMsg = 0 ) const = 0 /Factory/;
 %Docstring
@@ -244,10 +258,17 @@ class QgsGeometryEngine
 %End
 
     virtual QgsGeometryEngine::EngineOperationResult splitGeometry( const QgsLineString &splitLine,
-        QList<QgsAbstractGeometry *> &newGeometries,
+        QList<QgsGeometry > &newGeometries /Out/,
         bool topological,
         QgsPointSequence &topologyTestPoints, QString *errorMsg = 0 ) const;
 %Docstring
+ Splits this geometry according to a given line.
+ \param splitLine the line that splits the geometry
+ \param[out] newGeometries list of new geometries that have been created with the split
+ \param topological true if topological editing is enabled
+ \param[out] topologyTestPoints points that need to be tested for topological completeness in the dataset
+ \param[out] errorMsg error messages emitted, if any
+ :return: 0 in case of success, 1 if geometry has not been split, error else
  :rtype: QgsGeometryEngine.EngineOperationResult
 %End
 

diff --git a/src/core/geometry/qgsgeometry.cpp b/src/core/geometry/qgsgeometry.cpp
@@ -797,7 +797,7 @@ QgsGeometry::OperationResult QgsGeometry::splitGeometry( const QList<QgsPointXY>
     return InvalidBaseGeometry;
   }
 
-  QList<QgsAbstractGeometry *> newGeoms;
+  QList<QgsGeometry > newGeoms;
   QgsLineString splitLineString( splitLine );
   QgsPointSequence tp;
 
@@ -807,13 +807,9 @@ QgsGeometry::OperationResult QgsGeometry::splitGeometry( const QList<QgsPointXY>
 
   if ( result == QgsGeometryEngine::Success )
   {
-    reset( std::unique_ptr< QgsAbstractGeometry >( newGeoms.takeFirst() ) );
+    *this = newGeoms.takeAt( 0 );
 
-    newGeometries.clear();
-    for ( QgsAbstractGeometry *part : qgis::as_const( newGeoms ) )
-    {
-      newGeometries.push_back( QgsGeometry( part ) );
-    }
+    newGeometries = newGeoms;
   }
 
   convertPointList( tp, topologyTestPoints );
@@ -2252,7 +2248,7 @@ QgsGeometry QgsGeometry::polygonize( const QList<QgsGeometry> &geometryList )
 {
   QgsGeos geos( nullptr );
 
-  QList<QgsAbstractGeometry *> geomV2List;
+  QList<const QgsAbstractGeometry *> geomV2List;
   for ( const QgsGeometry &g : geometryList )
   {
     if ( !( g.isNull() ) )

diff --git a/src/core/geometry/qgsgeometryengine.h b/src/core/geometry/qgsgeometryengine.h
@@ -53,7 +53,21 @@ class CORE_EXPORT QgsGeometryEngine
 
     virtual ~QgsGeometryEngine() = default;
 
+    /**
+     * Should be called whenever the geometry associated with the engine
+     * has been modified and the engine must be updated to suit.
+     */
     virtual void geometryChanged() = 0;
+
+    /**
+     * Prepares the geometry, so that subsequent calls to spatial relation methods
+     * are much faster.
+     *
+     * This should be called for any geometry which is used for multiple relation
+     * tests against other geometries.
+     *
+     * \see geometryChanged()
+     */
     virtual void prepareGeometry() = 0;
 
     /**
@@ -213,8 +227,17 @@ class CORE_EXPORT QgsGeometryEngine
      */
     virtual bool isSimple( QString *errorMsg = nullptr ) const = 0;
 
+    /**
+     * Splits this geometry according to a given line.
+     * \param splitLine the line that splits the geometry
+     * \param[out] newGeometries list of new geometries that have been created with the split
+     * \param topological true if topological editing is enabled
+     * \param[out] topologyTestPoints points that need to be tested for topological completeness in the dataset
+     * \param[out] errorMsg error messages emitted, if any
+     * \returns 0 in case of success, 1 if geometry has not been split, error else
+    */
     virtual QgsGeometryEngine::EngineOperationResult splitGeometry( const QgsLineString &splitLine,
-        QList<QgsAbstractGeometry *> &newGeometries,
+        QList<QgsGeometry > &newGeometries SIP_OUT,
         bool topological,
         QgsPointSequence &topologyTestPoints, QString *errorMsg = nullptr ) const
     {

diff --git a/src/core/geometry/qgsgeos.cpp b/src/core/geometry/qgsgeos.cpp
@@ -563,7 +563,7 @@ double QgsGeos::length( QString *errorMsg ) const
 }
 
 QgsGeometryEngine::EngineOperationResult QgsGeos::splitGeometry( const QgsLineString &splitLine,
-    QList<QgsAbstractGeometry *> &newGeometries,
+    QList<QgsGeometry> &newGeometries,
     bool topological,
     QgsPointSequence &topologyTestPoints,
     QString *errorMsg ) const
@@ -767,7 +767,7 @@ GEOSGeometry *QgsGeos::linePointDifference( GEOSGeometry *GEOSsplitPoint ) const
   return asGeos( &lines, mPrecision );
 }
 
-QgsGeometryEngine::EngineOperationResult QgsGeos::splitLinearGeometry( GEOSGeometry *splitLine, QList<QgsAbstractGeometry *> &newGeometries ) const
+QgsGeometryEngine::EngineOperationResult QgsGeos::splitLinearGeometry( GEOSGeometry *splitLine, QList<QgsGeometry> &newGeometries ) const
 {
   if ( !splitLine )
     return InvalidInput;
@@ -815,15 +815,15 @@ QgsGeometryEngine::EngineOperationResult QgsGeos::splitLinearGeometry( GEOSGeome
 
   for ( int i = 0; i < lineGeoms.size(); ++i )
   {
-    newGeometries << fromGeos( lineGeoms[i] ).release();
+    newGeometries << QgsGeometry( fromGeos( lineGeoms[i] ) );
     GEOSGeom_destroy_r( geosinit.ctxt, lineGeoms[i] );
   }
 
   GEOSGeom_destroy_r( geosinit.ctxt, splitGeom );
   return Success;
 }
 
-QgsGeometryEngine::EngineOperationResult QgsGeos::splitPolygonGeometry( GEOSGeometry *splitLine, QList<QgsAbstractGeometry *> &newGeometries ) const
+QgsGeometryEngine::EngineOperationResult QgsGeos::splitPolygonGeometry( GEOSGeometry *splitLine, QList<QgsGeometry> &newGeometries ) const
 {
   if ( !splitLine )
     return InvalidInput;
@@ -917,7 +917,7 @@ QgsGeometryEngine::EngineOperationResult QgsGeos::splitPolygonGeometry( GEOSGeom
   }
 
   for ( i = 0; i < testedGeometries.size(); ++i )
-    newGeometries << fromGeos( testedGeometries[i] ).release();
+    newGeometries << QgsGeometry( fromGeos( testedGeometries[i] ) );
 
   return Success;
 }
@@ -2172,7 +2172,7 @@ double QgsGeos::lineLocatePoint( const QgsPoint &point, QString *errorMsg ) cons
   return distance;
 }
 
-QgsGeometry QgsGeos::polygonize( const QList<QgsAbstractGeometry *> &geometries, QString *errorMsg )
+QgsGeometry QgsGeos::polygonize( const QList<const QgsAbstractGeometry *> &geometries, QString *errorMsg )
 {
   GEOSGeometry **const lineGeosGeometries = new GEOSGeometry*[ geometries.size()];
   int validLines = 0;

diff --git a/src/core/geometry/qgsgeos.h b/src/core/geometry/qgsgeos.h
@@ -45,7 +45,6 @@ class CORE_EXPORT QgsGeos: public QgsGeometryEngine
     QgsGeos( const QgsAbstractGeometry *geometry, double precision = 0 );
     ~QgsGeos();
 
-    //! Removes caches
     void geometryChanged() override;
     void prepareGeometry() override;
 
@@ -138,16 +137,8 @@ class CORE_EXPORT QgsGeos: public QgsGeometryEngine
     bool isEmpty( QString *errorMsg = nullptr ) const override;
     bool isSimple( QString *errorMsg = nullptr ) const override;
 
-    /**
-     * Splits this geometry according to a given line.
-    \param splitLine the line that splits the geometry
-    \param[out] newGeometries list of new geometries that have been created with the split
-    \param topological true if topological editing is enabled
-    \param[out] topologyTestPoints points that need to be tested for topological completeness in the dataset
-    \param[out] errorMsg error messages emitted, if any
-    \returns 0 in case of success, 1 if geometry has not been split, error else*/
     EngineOperationResult splitGeometry( const QgsLineString &splitLine,
-                                         QList<QgsAbstractGeometry *> &newGeometries,
+                                         QList< QgsGeometry > &newGeometries,
                                          bool topological,
                                          QgsPointSequence &topologyTestPoints,
                                          QString *errorMsg = nullptr ) const override;
@@ -226,7 +217,7 @@ class CORE_EXPORT QgsGeos: public QgsGeometryEngine
      * An empty geometry will be returned in the case of errors.
      * \since QGIS 3.0
      */
-    static QgsGeometry polygonize( const QList<QgsAbstractGeometry *> &geometries, QString *errorMsg = nullptr );
+    static QgsGeometry polygonize( const QList< const QgsAbstractGeometry *> &geometries, QString *errorMsg = nullptr );
 
     /**
      * Creates a Voronoi diagram for the nodes contained within the geometry.
@@ -315,8 +306,8 @@ class CORE_EXPORT QgsGeos: public QgsGeometryEngine
     //utils for geometry split
     bool topologicalTestPointsSplit( const GEOSGeometry *splitLine, QgsPointSequence &testPoints, QString *errorMsg = nullptr ) const;
     GEOSGeometry *linePointDifference( GEOSGeometry *GEOSsplitPoint ) const;
-    EngineOperationResult splitLinearGeometry( GEOSGeometry *splitLine, QList<QgsAbstractGeometry *> &newGeometries ) const;
-    EngineOperationResult splitPolygonGeometry( GEOSGeometry *splitLine, QList<QgsAbstractGeometry *> &newGeometries ) const;
+    EngineOperationResult splitLinearGeometry( GEOSGeometry *splitLine, QList<QgsGeometry > &newGeometries ) const;
+    EngineOperationResult splitPolygonGeometry( GEOSGeometry *splitLine, QList<QgsGeometry > &newGeometries ) const;
 
     //utils for reshape
     static GEOSGeometry *reshapeLine( const GEOSGeometry *line, const GEOSGeometry *reshapeLineGeos, double precision );