Merge pull request #7805 from m-kuhn/vectordataproviderfeaturepool

Refactor QgsFeaturePool

Author: m-kuhn

python/core/auto_generated/qgsreadwritelocker.sip.in

@@ -0,0 +1,69 @@
+/************************************************************************
+ * This file has been generated automatically from                      *
+ *                                                                      *
+ * src/core/qgsreadwritelocker.h                                        *
+ *                                                                      *
+ * Do not edit manually ! Edit header and run scripts/sipify.pl again   *
+ ************************************************************************/
+
+
+
+
+
+class QgsReadWriteLocker
+{
+%Docstring
+The QgsReadWriteLocker class is a convenience class that simplifies locking and unlocking QReadWriteLocks.
+
+Locking and unlocking a QReadWriteLocks in complex functions and statements or in exception handling code
+is error-prone and difficult to debug.
+QgsReadWriteLocker can be used in such situations to ensure that the state of the lock is always well-defined.
+
+QgsReadWriteLocker should be created within a function where a QReadWriteLock needs to be locked.
+The lock may be locked when QgsReadWriteLocker is created or when changeMode is called.
+You can unlock and relock the lock with unlock() and changeMode().
+If locked, the lock will be unlocked when the QgsReadWriteLocker is destroyed.
+
+.. versionadded:: 3.4
+%End
+
+%TypeHeaderCode
+#include "qgsreadwritelocker.h"
+%End
+  public:
+
+    enum Mode
+    {
+      Read,
+      Write,
+      Unlocked
+    };
+
+    QgsReadWriteLocker( QReadWriteLock &lock, Mode mode );
+%Docstring
+Create a new QgsReadWriteLocker for ``lock`` and initialize in ``mode``.
+%End
+
+    void changeMode( Mode mode );
+%Docstring
+Change the mode of the lock to ``mode``.
+The lock will be unlocked and relocked as required.
+%End
+
+    void unlock();
+%Docstring
+Unlocks the lock.
+Equivalent to doing ``changeMode( QgsReadWriteLocker.Unlock );``
+%End
+
+    ~QgsReadWriteLocker();
+
+};
+
+/************************************************************************
+ * This file has been generated automatically from                      *
+ *                                                                      *
+ * src/core/qgsreadwritelocker.h                                        *
+ *                                                                      *
+ * Do not edit manually ! Edit header and run scripts/sipify.pl again   *
+ ************************************************************************/

python/core/core_auto.sip

@@ -107,6 +107,7 @@
 %Include auto_generated/qgspythonrunner.sip
 %Include auto_generated/qgsrange.sip
 %Include auto_generated/qgsreadwritecontext.sip
+%Include auto_generated/qgsreadwritelocker.sip
 %Include auto_generated/qgsrenderchecker.sip
 %Include auto_generated/qgsrendercontext.sip
 %Include auto_generated/qgsrulebasedlabeling.sip

src/analysis/CMakeLists.txt

@@ -132,6 +132,7 @@ SET(QGIS_ANALYSIS_SRCS
   network/qgsgraphanalyzer.cpp
 
   vector/geometry_checker/qgsfeaturepool.cpp
+  vector/geometry_checker/qgsvectordataproviderfeaturepool.cpp
   vector/geometry_checker/qgsgeometrychecker.cpp
   vector/geometry_checker/qgsgeometryanglecheck.cpp
   vector/geometry_checker/qgsgeometryareacheck.cpp
@@ -232,6 +233,7 @@ SET(QGIS_ANALYSIS_HDRS
   vector/qgszonalstatistics.h
   vector/geometry_checker/qgsgeometrycheckerutils.h
   vector/geometry_checker/qgsfeaturepool.h
+  vector/geometry_checker/qgsvectordataproviderfeaturepool.h
 
   interpolation/qgsinterpolator.h
   interpolation/qgsgridfilewriter.h

src/analysis/vector/geometry_checker/qgsfeaturepool.cpp

@@ -20,44 +20,26 @@
 #include "qgsgeometry.h"
 #include "qgsvectorlayer.h"
 #include "qgsvectordataprovider.h"
+#include "qgsvectorlayerutils.h"
+#include "qgsreadwritelocker.h"
 
 #include <QMutexLocker>
 
-QgsFeaturePool::QgsFeaturePool( QgsVectorLayer *layer, double layerToMapUnits, const QgsCoordinateTransform &layerToMapTransform, bool selectedOnly )
+
+QgsFeaturePool::QgsFeaturePool( QgsVectorLayer *layer, double layerToMapUnits, const QgsCoordinateTransform &layerToMapTransform )
   : mFeatureCache( CACHE_SIZE )
   , mLayer( layer )
   , mLayerToMapUnits( layerToMapUnits )
   , mLayerToMapTransform( layerToMapTransform )
-  , mSelectedOnly( selectedOnly )
+  , mLayerId( layer->id() )
+  , mGeometryType( layer->geometryType() )
 {
-  // Build spatial index
-  QgsFeature feature;
-  QgsFeatureRequest req;
-  req.setSubsetOfAttributes( QgsAttributeList() );
-  if ( selectedOnly )
-  {
-    mFeatureIds = layer->selectedFeatureIds();
-    req.setFilterFids( mFeatureIds );
-  }
 
-  QgsFeatureIterator it = layer->getFeatures( req );
-  while ( it.nextFeature( feature ) )
-  {
-    if ( feature.geometry() )
-    {
-      mIndex.insertFeature( feature );
-      mFeatureIds.insert( feature.id() );
-    }
-    else
-    {
-      mFeatureIds.remove( feature.id() );
-    }
-  }
 }
 
 bool QgsFeaturePool::get( QgsFeatureId id, QgsFeature &feature )
 {
-  QMutexLocker lock( &mLayerMutex );
+  QgsReadWriteLocker locker( mCacheLock, QgsReadWriteLocker::Read );
   QgsFeature *cachedFeature = mFeatureCache.object( id );
   if ( cachedFeature )
   {
@@ -66,78 +48,82 @@ bool QgsFeaturePool::get( QgsFeatureId id, QgsFeature &feature )
   }
   else
   {
+    std::unique_ptr<QgsVectorLayerFeatureSource> source = QgsVectorLayerUtils::getFeatureSource( mLayer );
+
     // Feature not in cache, retrieve from layer
     // TODO: avoid always querying all attributes (attribute values are needed when merging by attribute)
-    if ( !mLayer->getFeatures( QgsFeatureRequest( id ) ).nextFeature( feature ) )
+    if ( !source->getFeatures( QgsFeatureRequest( id ) ).nextFeature( feature ) )
     {
       return false;
     }
+    locker.changeMode( QgsReadWriteLocker::Write );
     mFeatureCache.insert( id, new QgsFeature( feature ) );
+    mIndex.insertFeature( feature );
   }
   return true;
 }
 
-void QgsFeaturePool::addFeature( QgsFeature &feature )
+QgsFeatureIds QgsFeaturePool::getFeatureIds() const
 {
-  QgsFeatureList features;
-  features.append( feature );
-  mLayerMutex.lock();
-  mLayer->dataProvider()->addFeatures( features );
-  feature.setId( features.front().id() );
-  if ( mSelectedOnly )
-  {
-    QgsFeatureIds selectedFeatureIds = mLayer->selectedFeatureIds();
-    selectedFeatureIds.insert( feature.id() );
-    mLayer->selectByIds( selectedFeatureIds );
-  }
-  mLayerMutex.unlock();
-  mIndexMutex.lock();
-  mIndex.insertFeature( feature );
-  mIndexMutex.unlock();
+  return mFeatureIds;
 }
 
-void QgsFeaturePool::updateFeature( QgsFeature &feature )
+QgsFeatureIds QgsFeaturePool::getIntersects( const QgsRectangle &rect ) const
 {
-  QgsFeature origFeature;
-  get( feature.id(), origFeature );
+  QgsReadWriteLocker locker( mCacheLock, QgsReadWriteLocker::Read );
+  QgsFeatureIds ids = QgsFeatureIds::fromList( mIndex.intersects( rect ) );
+  return ids;
+}
 
-  QgsGeometryMap geometryMap;
-  geometryMap.insert( feature.id(), feature.geometry() );
-  QgsChangedAttributesMap changedAttributesMap;
-  QgsAttributeMap attribMap;
-  for ( int i = 0, n = feature.attributes().size(); i < n; ++i )
-  {
-    attribMap.insert( i, feature.attributes().at( i ) );
-  }
-  changedAttributesMap.insert( feature.id(), attribMap );
-  mLayerMutex.lock();
-  mFeatureCache.remove( feature.id() ); // Remove to force reload on next get()
-  mLayer->dataProvider()->changeGeometryValues( geometryMap );
-  mLayer->dataProvider()->changeAttributeValues( changedAttributesMap );
-  mLayerMutex.unlock();
-  mIndexMutex.lock();
-  mIndex.deleteFeature( origFeature );
+QgsVectorLayer *QgsFeaturePool::layer() const
+{
+  Q_ASSERT( QThread::currentThread() == qApp->thread() );
+
+  return mLayer.data();
+}
+
+void QgsFeaturePool::insertFeature( const QgsFeature &feature )
+{
+  QgsReadWriteLocker locker( mCacheLock, QgsReadWriteLocker::Write );
+  mFeatureCache.insert( feature.id(), new QgsFeature( feature ) );
   mIndex.insertFeature( feature );
-  mIndexMutex.unlock();
 }
 
-void QgsFeaturePool::deleteFeature( QgsFeatureId fid )
+void QgsFeaturePool::refreshCache( const QgsFeature &feature )
+{
+  QgsReadWriteLocker locker( mCacheLock, QgsReadWriteLocker::Write );
+  mFeatureCache.remove( feature.id() );
+  mIndex.deleteFeature( feature );
+  locker.unlock();
+
+  QgsFeature tempFeature;
+  get( feature.id(), tempFeature );
+}
+
+void QgsFeaturePool::removeFeature( const QgsFeatureId featureId )
 {
   QgsFeature origFeature;
-  if ( get( fid, origFeature ) )
+  QgsReadWriteLocker locker( mCacheLock, QgsReadWriteLocker::Unlocked );
+  if ( get( featureId, origFeature ) )
   {
-    mIndexMutex.lock();
+    locker.changeMode( QgsReadWriteLocker::Write );
     mIndex.deleteFeature( origFeature );
-    mIndexMutex.unlock();
   }
-  mLayerMutex.lock();
+  locker.changeMode( QgsReadWriteLocker::Write );
   mFeatureCache.remove( origFeature.id() );
-  mLayer->dataProvider()->deleteFeatures( QgsFeatureIds() << fid );
-  mLayerMutex.unlock();
 }
 
-QgsFeatureIds QgsFeaturePool::getIntersects( const QgsRectangle &rect ) const
+void QgsFeaturePool::setFeatureIds( const QgsFeatureIds &ids )
+{
+  mFeatureIds = ids;
+}
+
+QgsWkbTypes::GeometryType QgsFeaturePool::geometryType() const
+{
+  return mGeometryType;
+}
+
+QString QgsFeaturePool::layerId() const
 {
-  QMutexLocker lock( &mIndexMutex );
-  return QgsFeatureIds::fromList( mIndex.intersects( rect ) );
+  return mLayerId;
 }