Avoid working on reference to temporary objects

m-kuhn · m-kuhn · commit d31340585124 · 2019-03-01T13:25:00.000+01:00
fixes a couple of crashes in geometry validation
diff --git a/python/analysis/auto_generated/vector/geometry_checker/qgsgeometrycheckerutils.sip.in b/python/analysis/auto_generated/vector/geometry_checker/qgsgeometrycheckerutils.sip.in
@@ -51,7 +51,7 @@ If ``useMapCrs`` is ``True``, geometries will be reprojected to the mapCrs defin
 in ``context``.
 %End
 
-        const QgsFeature &feature() const;
+        QgsFeature feature() const;
 %Docstring
 Returns the feature.
 The geometry will not be reprojected regardless of useMapCrs.
@@ -63,7 +63,7 @@ The geometry will not be reprojected regardless of useMapCrs.
 The layer id.
 %End
 
-        const QgsGeometry &geometry() const;
+        QgsGeometry geometry() const;
 %Docstring
 Returns the geometry of this feature.
 If useMapCrs was specified, it will already be reprojected into the
diff --git a/src/analysis/vector/geometry_checker/qgsgeometrycheckerutils.cpp b/src/analysis/vector/geometry_checker/qgsgeometrycheckerutils.cpp
@@ -52,7 +52,7 @@ QgsGeometryCheckerUtils::LayerFeature::LayerFeature( const QgsFeaturePool *pool,
   }
 }
 
-const QgsFeature &QgsGeometryCheckerUtils::LayerFeature::feature() const
+QgsFeature QgsGeometryCheckerUtils::LayerFeature::feature() const
 {
   return mFeature;
 }
@@ -67,7 +67,7 @@ QString QgsGeometryCheckerUtils::LayerFeature::layerId() const
   return mFeaturePool->layerId();
 }
 
-const QgsGeometry &QgsGeometryCheckerUtils::LayerFeature::geometry() const
+QgsGeometry QgsGeometryCheckerUtils::LayerFeature::geometry() const
 {
   return mGeometry;
 }
diff --git a/src/analysis/vector/geometry_checker/qgsgeometrycheckerutils.h b/src/analysis/vector/geometry_checker/qgsgeometrycheckerutils.h
@@ -64,7 +64,7 @@ class ANALYSIS_EXPORT QgsGeometryCheckerUtils
          * Returns the feature.
          * The geometry will not be reprojected regardless of useMapCrs.
          */
-        const QgsFeature &feature() const;
+        QgsFeature feature() const;
 
         /**
          * The layer.
@@ -81,7 +81,7 @@ class ANALYSIS_EXPORT QgsGeometryCheckerUtils
          * If useMapCrs was specified, it will already be reprojected into the
          * CRS specified in the context specified in the constructor.
          */
-        const QgsGeometry &geometry() const;
+        QgsGeometry geometry() const;
 
         /**
          * Returns a combination of the layerId and the feature id.
diff --git a/src/analysis/vector/geometry_checker/qgsgeometrygapcheck.cpp b/src/analysis/vector/geometry_checker/qgsgeometrygapcheck.cpp
@@ -115,10 +115,11 @@ void QgsGeometryGapCheck::collectErrors( const QMap<QString, QgsFeaturePool *> &
     const QgsGeometryCheckerUtils::LayerFeatures layerFeatures( featurePools, featureIds.keys(), gapAreaBBox, compatibleGeometryTypes(), mContext );
     for ( const QgsGeometryCheckerUtils::LayerFeature &layerFeature : layerFeatures )
     {
-      if ( QgsGeometryCheckerUtils::sharedEdgeLength( gapGeom.get(), layerFeature.geometry().constGet(), mContext->reducedTolerance ) > 0 )
+      const QgsGeometry geom = layerFeature.geometry();
+      if ( QgsGeometryCheckerUtils::sharedEdgeLength( gapGeom.get(), geom.constGet(), mContext->reducedTolerance ) > 0 )
       {
         neighboringIds[layerFeature.layer()->id()].insert( layerFeature.feature().id() );
-        gapAreaBBox.combineExtentWith( layerFeature.geometry().constGet()->boundingBox() );
+        gapAreaBBox.combineExtentWith( layerFeature.geometry().boundingBox() );
       }
     }
 
@@ -193,7 +194,7 @@ bool QgsGeometryGapCheck::mergeWithNeighbor( const QMap<QString, QgsFeaturePool
       {
         continue;
       }
-      QgsGeometry featureGeom = testFeature.geometry();
+      const QgsGeometry featureGeom = testFeature.geometry();
       const QgsAbstractGeometry *testGeom = featureGeom.constGet();
       for ( int iPart = 0, nParts = testGeom->partCount(); iPart < nParts; ++iPart )
       {
@@ -219,7 +220,7 @@ bool QgsGeometryGapCheck::mergeWithNeighbor( const QMap<QString, QgsFeaturePool
   std::unique_ptr<QgsAbstractGeometry> errLayerGeom( errGeometry->clone() );
   QgsCoordinateTransform ct( featurePool->crs(), mContext->mapCrs, mContext->transformContext );
   errLayerGeom->transform( ct, QgsCoordinateTransform::ReverseTransform );
-  QgsGeometry mergeFeatureGeom = mergeFeature.geometry();
+  const QgsGeometry mergeFeatureGeom = mergeFeature.geometry();
   const QgsAbstractGeometry *mergeGeom = mergeFeatureGeom.constGet();
   std::unique_ptr< QgsGeometryEngine > geomEngine = QgsGeometryCheckerUtils::createGeomEngine( errLayerGeom.get(), mContext->reducedTolerance );
   std::unique_ptr<QgsAbstractGeometry> combinedGeom( geomEngine->combine( QgsGeometryCheckerUtils::getGeomPart( mergeGeom, mergePartIdx ), &errMsg ) );
diff --git a/src/analysis/vector/geometry_checker/qgsgeometrylinelayerintersectioncheck.cpp b/src/analysis/vector/geometry_checker/qgsgeometrylinelayerintersectioncheck.cpp
@@ -57,7 +57,7 @@ void QgsGeometryLineLayerIntersectionCheck::collectErrors( const QMap<QString, Q
           }
           else if ( const QgsPolygon *polygon = dynamic_cast<const QgsPolygon *>( part ) )
           {
-            QList< const QgsLineString * > rings = QgsGeometryCheckerUtils::polygonRings( polygon );
+            const QList< const QgsLineString * > rings = QgsGeometryCheckerUtils::polygonRings( polygon );
             for ( const QgsLineString *ring : rings )
             {
               const QList< QgsPoint > intersections = QgsGeometryCheckerUtils::lineIntersections( line, ring, mContext->tolerance );
diff --git a/src/analysis/vector/geometry_checker/qgsgeometrymissingvertexcheck.cpp b/src/analysis/vector/geometry_checker/qgsgeometrymissingvertexcheck.cpp
@@ -48,7 +48,8 @@ void QgsGeometryMissingVertexCheck::collectErrors( const QMap<QString, QgsFeatur
       break;
     }
 
-    const QgsAbstractGeometry *geom = layerFeature.geometry().constGet();
+    const QgsGeometry geometry = layerFeature.geometry();
+    const QgsAbstractGeometry *geom = geometry.constGet();
 
     if ( QgsCurvePolygon *polygon = qgsgeometry_cast<QgsCurvePolygon *>( geom ) )
     {
@@ -129,7 +130,7 @@ void QgsGeometryMissingVertexCheck::processPolygon( const QgsCurvePolygon *polyg
   const QgsFeature &currentFeature = layerFeature.feature();
   std::unique_ptr<QgsMultiPolygon> boundaries = qgis::make_unique<QgsMultiPolygon>();
 
-  std::unique_ptr< QgsGeometryEngine > geomEngine = QgsGeometryCheckerUtils::createGeomEngine( polygon->exteriorRing(), mContext->tolerance );
+  std::unique_ptr< QgsGeometryEngine > geomEngine = QgsGeometryCheckerUtils::createGeomEngine( polygon->exteriorRing()->clone(), mContext->tolerance );
   boundaries->addGeometry( geomEngine->buffer( mContext->tolerance, 5 ) );
 
   const int numRings = polygon->numInteriorRings();
@@ -155,7 +156,8 @@ void QgsGeometryMissingVertexCheck::processPolygon( const QgsCurvePolygon *polyg
       if ( feedback && feedback->isCanceled() )
         break;
 
-      QgsVertexIterator vertexIterator = compareFeature.geometry().vertices();
+      const QgsGeometry compareGeometry = compareFeature.geometry();
+      QgsVertexIterator vertexIterator = compareGeometry.vertices();
       while ( vertexIterator.hasNext() )
       {
         const QgsPoint &pt = vertexIterator.next();
diff --git a/src/analysis/vector/geometry_checker/qgsgeometryoverlapcheck.cpp b/src/analysis/vector/geometry_checker/qgsgeometryoverlapcheck.cpp
@@ -41,8 +41,10 @@ void QgsGeometryOverlapCheck::collectErrors( const QMap<QString, QgsFeaturePool
     // Ensure each pair of layers only gets compared once: remove the current layer from the layerIds, but add it to the layerList for layerFeaturesB
     layerIds.removeOne( layerFeatureA.layer()->id() );
 
-    QgsRectangle bboxA = layerFeatureA.geometry().constGet()->boundingBox();
-    std::unique_ptr< QgsGeometryEngine > geomEngineA = QgsGeometryCheckerUtils::createGeomEngine( layerFeatureA.geometry().constGet(), mContext->tolerance );
+    const QgsGeometry geomA = layerFeatureA.geometry();
+    QgsRectangle bboxA = geomA.boundingBox();
+    std::unique_ptr< QgsGeometryEngine > geomEngineA = QgsGeometryCheckerUtils::createGeomEngine( geomA.constGet(), mContext->tolerance );
+    geomEngineA->prepareGeometry();
     if ( !geomEngineA->isValid() )
     {
       messages.append( tr( "Overlap check failed for (%1): the geometry is invalid" ).arg( layerFeatureA.id() ) );
@@ -56,14 +58,16 @@ void QgsGeometryOverlapCheck::collectErrors( const QMap<QString, QgsFeaturePool
         break;
 
       // > : only report overlaps within same layer once
-      if ( layerFeatureA.layer()->id() == layerFeatureB.layer()->id() && layerFeatureB.feature().id() >= layerFeatureA.feature().id() )
+      if ( layerFeatureA.layerId() == layerFeatureB.layerId() && layerFeatureB.feature().id() >= layerFeatureA.feature().id() )
       {
         continue;
       }
       QString errMsg;
-      if ( geomEngineA->overlaps( layerFeatureB.geometry().constGet(), &errMsg ) )
+      const QgsGeometry geometryB = layerFeatureB.geometry();
+      const QgsAbstractGeometry *geomB = geometryB.constGet();
+      if ( geomEngineA->overlaps( geomB, &errMsg ) )
       {
-        std::unique_ptr<QgsAbstractGeometry> interGeom( geomEngineA->intersection( layerFeatureB.geometry().constGet() ) );
+        std::unique_ptr<QgsAbstractGeometry> interGeom( geomEngineA->intersection( geomB ) );
         if ( interGeom && !interGeom->isEmpty() )
         {
           QgsGeometryCheckerUtils::filter1DTypes( interGeom.get() );
@@ -105,14 +109,16 @@ void QgsGeometryOverlapCheck::fixError( const QMap<QString, QgsFeaturePool *> &f
   // Check if error still applies
   QgsGeometryCheckerUtils::LayerFeature layerFeatureA( featurePoolA, featureA, mContext, true );
   QgsGeometryCheckerUtils::LayerFeature layerFeatureB( featurePoolB, featureB, mContext, true );
-  std::unique_ptr< QgsGeometryEngine > geomEngineA = QgsGeometryCheckerUtils::createGeomEngine( layerFeatureA.geometry().constGet(), mContext->reducedTolerance );
+  const QgsGeometry geometryA = layerFeatureA.geometry();
+  std::unique_ptr< QgsGeometryEngine > geomEngineA = QgsGeometryCheckerUtils::createGeomEngine( geometryA.constGet(), mContext->reducedTolerance );
 
-  if ( !geomEngineA->overlaps( layerFeatureB.geometry().constGet() ) )
+  const QgsGeometry geometryB = layerFeatureB.geometry();
+  if ( !geomEngineA->overlaps( geometryB.constGet() ) )
   {
     error->setObsolete();
     return;
   }
-  std::unique_ptr< QgsAbstractGeometry > interGeom( geomEngineA->intersection( layerFeatureB.geometry().constGet(), &errMsg ) );
+  std::unique_ptr< QgsAbstractGeometry > interGeom( geomEngineA->intersection( geometryB.constGet(), &errMsg ) );
   if ( !interGeom )
   {
     error->setFixFailed( tr( "Failed to compute intersection between overlapping features: %1" ).arg( errMsg ) );
@@ -153,7 +159,7 @@ void QgsGeometryOverlapCheck::fixError( const QMap<QString, QgsFeaturePool *> &f
     {
       QgsGeometryCheckerUtils::filter1DTypes( diff1.get() );
     }
-    std::unique_ptr< QgsGeometryEngine > geomEngineB = QgsGeometryCheckerUtils::createGeomEngine( layerFeatureB.geometry().constGet(), mContext->reducedTolerance );
+    std::unique_ptr< QgsGeometryEngine > geomEngineB = QgsGeometryCheckerUtils::createGeomEngine( geometryB.constGet(), mContext->reducedTolerance );
     std::unique_ptr< QgsAbstractGeometry > diff2( geomEngineB->difference( interPart, &errMsg ) );
     if ( !diff2 || diff2->isEmpty() )
     {

Original file line number	Diff line number	Diff line change
`@@ -52,7 +52,7 @@ QgsGeometryCheckerUtils::LayerFeature::LayerFeature( const QgsFeaturePool *pool,`
`52`	`52`	`}`
`53`	`53`	`}`
`54`	`54`
`55`		`-const QgsFeature &QgsGeometryCheckerUtils::LayerFeature::feature() const`
	`55`	`+QgsFeature QgsGeometryCheckerUtils::LayerFeature::feature() const`
`56`	`56`	`{`
`57`	`57`	`return mFeature;`
`58`	`58`	`}`
`@@ -67,7 +67,7 @@ QString QgsGeometryCheckerUtils::LayerFeature::layerId() const`
`67`	`67`	`return mFeaturePool->layerId();`
`68`	`68`	`}`
`69`	`69`
`70`		`-const QgsGeometry &QgsGeometryCheckerUtils::LayerFeature::geometry() const`
	`70`	`+QgsGeometry QgsGeometryCheckerUtils::LayerFeature::geometry() const`
`71`	`71`	`{`
`72`	`72`	`return mGeometry;`
`73`	`73`	`}`
Original file line number	Diff line number	Diff line change
`@@ -115,10 +115,11 @@ void QgsGeometryGapCheck::collectErrors( const QMap<QString, QgsFeaturePool *> &`
`115`	`115`	`const QgsGeometryCheckerUtils::LayerFeatures layerFeatures( featurePools, featureIds.keys(), gapAreaBBox, compatibleGeometryTypes(), mContext );`
`116`	`116`	`for ( const QgsGeometryCheckerUtils::LayerFeature &layerFeature : layerFeatures )`
`117`	`117`	`{`
`118`		`- if ( QgsGeometryCheckerUtils::sharedEdgeLength( gapGeom.get(), layerFeature.geometry().constGet(), mContext->reducedTolerance ) > 0 )`
	`118`	`+ const QgsGeometry geom = layerFeature.geometry();`
	`119`	`+ if ( QgsGeometryCheckerUtils::sharedEdgeLength( gapGeom.get(), geom.constGet(), mContext->reducedTolerance ) > 0 )`
`119`	`120`	`{`
`120`	`121`	`neighboringIds[layerFeature.layer()->id()].insert( layerFeature.feature().id() );`
`121`		`- gapAreaBBox.combineExtentWith( layerFeature.geometry().constGet()->boundingBox() );`
	`122`	`+ gapAreaBBox.combineExtentWith( layerFeature.geometry().boundingBox() );`
`122`	`123`	`}`
`123`	`124`	`}`
`124`	`125`
`@@ -193,7 +194,7 @@ bool QgsGeometryGapCheck::mergeWithNeighbor( const QMap<QString, QgsFeaturePool`
`193`	`194`	`{`
`194`	`195`	`continue;`
`195`	`196`	`}`
`196`		`- QgsGeometry featureGeom = testFeature.geometry();`
	`197`	`+ const QgsGeometry featureGeom = testFeature.geometry();`
`197`	`198`	`const QgsAbstractGeometry *testGeom = featureGeom.constGet();`
`198`	`199`	`for ( int iPart = 0, nParts = testGeom->partCount(); iPart < nParts; ++iPart )`
`199`	`200`	`{`
`@@ -219,7 +220,7 @@ bool QgsGeometryGapCheck::mergeWithNeighbor( const QMap<QString, QgsFeaturePool`
`219`	`220`	`std::unique_ptr<QgsAbstractGeometry> errLayerGeom( errGeometry->clone() );`
`220`	`221`	`QgsCoordinateTransform ct( featurePool->crs(), mContext->mapCrs, mContext->transformContext );`
`221`	`222`	`errLayerGeom->transform( ct, QgsCoordinateTransform::ReverseTransform );`
`222`		`- QgsGeometry mergeFeatureGeom = mergeFeature.geometry();`
	`223`	`+ const QgsGeometry mergeFeatureGeom = mergeFeature.geometry();`
`223`	`224`	`const QgsAbstractGeometry *mergeGeom = mergeFeatureGeom.constGet();`
`224`	`225`	`std::unique_ptr< QgsGeometryEngine > geomEngine = QgsGeometryCheckerUtils::createGeomEngine( errLayerGeom.get(), mContext->reducedTolerance );`
`225`	`226`	`std::unique_ptr<QgsAbstractGeometry> combinedGeom( geomEngine->combine( QgsGeometryCheckerUtils::getGeomPart( mergeGeom, mergePartIdx ), &errMsg ) );`
Original file line number	Diff line number	Diff line change
`@@ -57,7 +57,7 @@ void QgsGeometryLineLayerIntersectionCheck::collectErrors( const QMap<QString, Q`
`57`	`57`	`}`
`58`	`58`	`else if ( const QgsPolygon polygon = dynamic_cast<const QgsPolygon >( part ) )`
`59`	`59`	`{`
`60`		`- QList< const QgsLineString * > rings = QgsGeometryCheckerUtils::polygonRings( polygon );`
	`60`	`+ const QList< const QgsLineString * > rings = QgsGeometryCheckerUtils::polygonRings( polygon );`
`61`	`61`	`for ( const QgsLineString *ring : rings )`
`62`	`62`	`{`
`63`	`63`	`const QList< QgsPoint > intersections = QgsGeometryCheckerUtils::lineIntersections( line, ring, mContext->tolerance );`
Original file line number	Diff line number	Diff line change
`@@ -41,8 +41,10 @@ void QgsGeometryOverlapCheck::collectErrors( const QMap<QString, QgsFeaturePool`
`41`	`41`	`// Ensure each pair of layers only gets compared once: remove the current layer from the layerIds, but add it to the layerList for layerFeaturesB`
`42`	`42`	`layerIds.removeOne( layerFeatureA.layer()->id() );`
`43`	`43`
`44`		`- QgsRectangle bboxA = layerFeatureA.geometry().constGet()->boundingBox();`
`45`		`- std::unique_ptr< QgsGeometryEngine > geomEngineA = QgsGeometryCheckerUtils::createGeomEngine( layerFeatureA.geometry().constGet(), mContext->tolerance );`
	`44`	`+ const QgsGeometry geomA = layerFeatureA.geometry();`
	`45`	`+ QgsRectangle bboxA = geomA.boundingBox();`
	`46`	`+ std::unique_ptr< QgsGeometryEngine > geomEngineA = QgsGeometryCheckerUtils::createGeomEngine( geomA.constGet(), mContext->tolerance );`
	`47`	`+ geomEngineA->prepareGeometry();`
`46`	`48`	`if ( !geomEngineA->isValid() )`
`47`	`49`	`{`
`48`	`50`	`messages.append( tr( "Overlap check failed for (%1): the geometry is invalid" ).arg( layerFeatureA.id() ) );`
`@@ -56,14 +58,16 @@ void QgsGeometryOverlapCheck::collectErrors( const QMap<QString, QgsFeaturePool`
`56`	`58`	`break;`
`57`	`59`
`58`	`60`	`// > : only report overlaps within same layer once`
`59`		`- if ( layerFeatureA.layer()->id() == layerFeatureB.layer()->id() && layerFeatureB.feature().id() >= layerFeatureA.feature().id() )`
	`61`	`+ if ( layerFeatureA.layerId() == layerFeatureB.layerId() && layerFeatureB.feature().id() >= layerFeatureA.feature().id() )`
`60`	`62`	`{`
`61`	`63`	`continue;`
`62`	`64`	`}`
`63`	`65`	`QString errMsg;`
`64`		`- if ( geomEngineA->overlaps( layerFeatureB.geometry().constGet(), &errMsg ) )`
	`66`	`+ const QgsGeometry geometryB = layerFeatureB.geometry();`
	`67`	`+ const QgsAbstractGeometry *geomB = geometryB.constGet();`
	`68`	`+ if ( geomEngineA->overlaps( geomB, &errMsg ) )`
`65`	`69`	`{`
`66`		`- std::unique_ptr<QgsAbstractGeometry> interGeom( geomEngineA->intersection( layerFeatureB.geometry().constGet() ) );`
	`70`	`+ std::unique_ptr<QgsAbstractGeometry> interGeom( geomEngineA->intersection( geomB ) );`
`67`	`71`	`if ( interGeom && !interGeom->isEmpty() )`
`68`	`72`	`{`
`69`	`73`	`QgsGeometryCheckerUtils::filter1DTypes( interGeom.get() );`
`@@ -105,14 +109,16 @@ void QgsGeometryOverlapCheck::fixError( const QMap<QString, QgsFeaturePool *> &f`
`105`	`109`	`// Check if error still applies`
`106`	`110`	`QgsGeometryCheckerUtils::LayerFeature layerFeatureA( featurePoolA, featureA, mContext, true );`
`107`	`111`	`QgsGeometryCheckerUtils::LayerFeature layerFeatureB( featurePoolB, featureB, mContext, true );`
`108`		`- std::unique_ptr< QgsGeometryEngine > geomEngineA = QgsGeometryCheckerUtils::createGeomEngine( layerFeatureA.geometry().constGet(), mContext->reducedTolerance );`
	`112`	`+ const QgsGeometry geometryA = layerFeatureA.geometry();`
	`113`	`+ std::unique_ptr< QgsGeometryEngine > geomEngineA = QgsGeometryCheckerUtils::createGeomEngine( geometryA.constGet(), mContext->reducedTolerance );`
`109`	`114`
`110`		`- if ( !geomEngineA->overlaps( layerFeatureB.geometry().constGet() ) )`
	`115`	`+ const QgsGeometry geometryB = layerFeatureB.geometry();`
	`116`	`+ if ( !geomEngineA->overlaps( geometryB.constGet() ) )`
`111`	`117`	`{`
`112`	`118`	`error->setObsolete();`
`113`	`119`	`return;`
`114`	`120`	`}`
`115`		`- std::unique_ptr< QgsAbstractGeometry > interGeom( geomEngineA->intersection( layerFeatureB.geometry().constGet(), &errMsg ) );`
	`121`	`+ std::unique_ptr< QgsAbstractGeometry > interGeom( geomEngineA->intersection( geometryB.constGet(), &errMsg ) );`
`116`	`122`	`if ( !interGeom )`
`117`	`123`	`{`
`118`	`124`	`error->setFixFailed( tr( "Failed to compute intersection between overlapping features: %1" ).arg( errMsg ) );`
`@@ -153,7 +159,7 @@ void QgsGeometryOverlapCheck::fixError( const QMap<QString, QgsFeaturePool *> &f`
`153`	`159`	`{`
`154`	`160`	`QgsGeometryCheckerUtils::filter1DTypes( diff1.get() );`
`155`	`161`	`}`
`156`		`- std::unique_ptr< QgsGeometryEngine > geomEngineB = QgsGeometryCheckerUtils::createGeomEngine( layerFeatureB.geometry().constGet(), mContext->reducedTolerance );`
	`162`	`+ std::unique_ptr< QgsGeometryEngine > geomEngineB = QgsGeometryCheckerUtils::createGeomEngine( geometryB.constGet(), mContext->reducedTolerance );`
`157`	`163`	`std::unique_ptr< QgsAbstractGeometry > diff2( geomEngineB->difference( interPart, &errMsg ) );`
`158`	`164`	`if ( !diff2 \|\| diff2->isEmpty() )`
`159`	`165`	`{`