[Geometry checker] Fix features not correctly removed from QgsFeature…

…Pool spatial index
qgis · Oct 23, 2017 · efc00c3 · efc00c3
1 parent 084660a
commit efc00c3
Show file tree

Hide file tree

Showing 7 changed files with 22 additions and 15 deletions.
diff --git a/src/analysis/vector/geometry_checker/qgsfeaturepool.cpp b/src/analysis/vector/geometry_checker/qgsfeaturepool.cpp
@@ -105,6 +105,9 @@ void QgsFeaturePool::addFeature( QgsFeature &feature )
 
 void QgsFeaturePool::updateFeature( QgsFeature &feature )
 {
+  QgsFeature origFeature;
+  get( feature.id(), origFeature );
+
   QgsGeometryMap geometryMap;
   geometryMap.insert( feature.id(), QgsGeometry( feature.geometry().geometry()->clone() ) );
   QgsChangedAttributesMap changedAttributesMap;
@@ -120,19 +123,23 @@ void QgsFeaturePool::updateFeature( QgsFeature &feature )
   mLayer->dataProvider()->changeAttributeValues( changedAttributesMap );
   mLayerMutex.unlock();
   mIndexMutex.lock();
-  mIndex.deleteFeature( feature );
+  mIndex.deleteFeature( origFeature );
   mIndex.insertFeature( feature );
   mIndexMutex.unlock();
 }
 
-void QgsFeaturePool::deleteFeature( QgsFeature &feature )
+void QgsFeaturePool::deleteFeature( const QgsFeatureId &fid )
 {
-  mIndexMutex.lock();
-  mIndex.deleteFeature( feature );
-  mIndexMutex.unlock();
+  QgsFeature origFeature;
+  if ( get( fid, origFeature ) )
+  {
+    mIndexMutex.lock();
+    mIndex.deleteFeature( origFeature );
+    mIndexMutex.unlock();
+  }
   mLayerMutex.lock();
-  mFeatureCache.remove( feature.id() );
-  mLayer->dataProvider()->deleteFeatures( QgsFeatureIds() << feature.id() );
+  mFeatureCache.remove( origFeature.id() );
+  mLayer->dataProvider()->deleteFeatures( QgsFeatureIds() << fid );
   mLayerMutex.unlock();
 }
 

diff --git a/src/analysis/vector/geometry_checker/qgsfeaturepool.h b/src/analysis/vector/geometry_checker/qgsfeaturepool.h
@@ -36,7 +36,7 @@ class ANALYSIS_EXPORT QgsFeaturePool : public QObject
     bool get( QgsFeatureId id, QgsFeature &feature );
     void addFeature( QgsFeature &feature );
     void updateFeature( QgsFeature &feature );
-    void deleteFeature( QgsFeature &feature );
+    void deleteFeature( const QgsFeatureId &fid );
     QgsFeatureIds getIntersects( const QgsRectangle &rect ) const;
     QgsVectorLayer *getLayer() const { return mLayer; }
     const QgsFeatureIds &getFeatureIds() const { return mFeatureIds; }

diff --git a/src/analysis/vector/geometry_checker/qgsgeometrycheck.cpp b/src/analysis/vector/geometry_checker/qgsgeometrycheck.cpp
@@ -185,7 +185,7 @@ void QgsGeometryCheck::deleteFeatureGeometryPart( const QString &layerId, QgsFea
     static_cast<QgsGeometryCollection *>( geom )->removeGeometry( partIdx );
     if ( static_cast<QgsGeometryCollection *>( geom )->numGeometries() == 0 )
     {
-      featurePool->deleteFeature( feature );
+      featurePool->deleteFeature( feature.id() );
       changes[layerId][feature.id()].append( Change( ChangeFeature, ChangeRemoved ) );
     }
     else
@@ -197,7 +197,7 @@ void QgsGeometryCheck::deleteFeatureGeometryPart( const QString &layerId, QgsFea
   }
   else
   {
-    featurePool->deleteFeature( feature );
+    featurePool->deleteFeature( feature.id() );
     changes[layerId][feature.id()].append( Change( ChangeFeature, ChangeRemoved ) );
   }
 }

diff --git a/src/analysis/vector/geometry_checker/qgsgeometrycontainedcheck.cpp b/src/analysis/vector/geometry_checker/qgsgeometrycontainedcheck.cpp
@@ -94,7 +94,7 @@ void QgsGeometryContainedCheck::fixError( QgsGeometryCheckError *error, int meth
   else if ( method == Delete )
   {
     changes[error->layerId()][featureA.id()].append( Change( ChangeFeature, ChangeRemoved ) );
-    featurePoolA->deleteFeature( featureA );
+    featurePoolA->deleteFeature( featureA.id() );
     error->setFixed( method );
   }
   else

diff --git a/src/analysis/vector/geometry_checker/qgsgeometryduplicatecheck.cpp b/src/analysis/vector/geometry_checker/qgsgeometryduplicatecheck.cpp
@@ -117,7 +117,7 @@ void QgsGeometryDuplicateCheck::fixError( QgsGeometryCheckError *error, int meth
         QgsAbstractGeometry *diffGeom = geomEngineA->symDifference( layerFeatureB.geometry() );
         if ( diffGeom && diffGeom->isEmpty() )
         {
-          featurePoolB->deleteFeature( featureB );
+          featurePoolB->deleteFeature( featureB.id() );
           changes[layerIdB][idB].append( Change( ChangeFeature, ChangeRemoved ) );
         }
 

diff --git a/src/analysis/vector/geometry_checker/qgsgeometrymultipartcheck.cpp b/src/analysis/vector/geometry_checker/qgsgeometrymultipartcheck.cpp
@@ -64,7 +64,7 @@ void QgsGeometryMultipartCheck::fixError( QgsGeometryCheckError *error, int meth
   }
   else if ( method == RemoveObject )
   {
-    featurePool->deleteFeature( feature );
+    featurePool->deleteFeature( feature.id() );
     error->setFixed( method );
     changes[error->layerId()][feature.id()].append( Change( ChangeFeature, ChangeRemoved ) );
   }

diff --git a/src/analysis/vector/geometry_checker/qgsgeometrytypecheck.cpp b/src/analysis/vector/geometry_checker/qgsgeometrytypecheck.cpp
@@ -132,14 +132,14 @@ void QgsGeometryTypeCheck::fixError( QgsGeometryCheckError *error, int method, c
     // Delete feature
     else
     {
-      featurePool->deleteFeature( feature );
+      featurePool->deleteFeature( feature.id() );
       changes[error->layerId()][error->featureId()].append( Change( ChangeFeature, ChangeRemoved ) );
     }
     error->setFixed( method );
   }
   else if ( method == Delete )
   {
-    featurePool->deleteFeature( feature );
+    featurePool->deleteFeature( feature.id() );
     error->setFixed( method );
     changes[error->layerId()][error->featureId()].append( Change( ChangeFeature, ChangeRemoved ) );
   }