Merge pull request #7861 from m-kuhn/featurePoolContext

Make QgsFeaturePool free of reprojection code and other improvements

Author: m-kuhn

src/analysis/vector/geometry_checker/qgsfeaturepool.cpp

@@ -26,13 +26,12 @@
 #include <QMutexLocker>
 
 
-QgsFeaturePool::QgsFeaturePool( QgsVectorLayer *layer, double layerToMapUnits, const QgsCoordinateTransform &layerToMapTransform )
+QgsFeaturePool::QgsFeaturePool( QgsVectorLayer *layer )
   : mFeatureCache( CACHE_SIZE )
   , mLayer( layer )
-  , mLayerToMapUnits( layerToMapUnits )
-  , mLayerToMapTransform( layerToMapTransform )
   , mLayerId( layer->id() )
   , mGeometryType( layer->geometryType() )
+  , mCrs( layer->crs() )
 {
 
 }
@@ -82,6 +81,11 @@ QgsVectorLayer *QgsFeaturePool::layer() const
   return mLayer.data();
 }
 
+QPointer<QgsVectorLayer> QgsFeaturePool::layerPtr() const
+{
+  return mLayer;
+}
+
 void QgsFeaturePool::insertFeature( const QgsFeature &feature )
 {
   QgsReadWriteLocker locker( mCacheLock, QgsReadWriteLocker::Write );
@@ -118,6 +122,11 @@ void QgsFeaturePool::setFeatureIds( const QgsFeatureIds &ids )
   mFeatureIds = ids;
 }
 
+QgsCoordinateReferenceSystem QgsFeaturePool::crs() const
+{
+  return mCrs;
+}
+
 QgsWkbTypes::GeometryType QgsFeaturePool::geometryType() const
 {
   return mGeometryType;

src/analysis/vector/geometry_checker/qgsfeaturepool.h

@@ -38,7 +38,7 @@ class ANALYSIS_EXPORT QgsFeaturePool : public QgsFeatureSink
 {
 
   public:
-    QgsFeaturePool( QgsVectorLayer *layer, double layerToMapUnits, const QgsCoordinateTransform &layerToMapTransform );
+    QgsFeaturePool( QgsVectorLayer *layer );
     virtual ~QgsFeaturePool() = default;
 
     /**
@@ -72,25 +72,22 @@ class ANALYSIS_EXPORT QgsFeaturePool : public QgsFeatureSink
      */
     QgsFeatureIds getIntersects( const QgsRectangle &rect ) const;
 
-    /**
-     * The factor of layer units to map units.
-     * TODO: should this be removed and determined on runtime by checks that need it?
-     */
-    double getLayerToMapUnits() const { return mLayerToMapUnits; }
-
-    /**
-     * A coordinate transform from layer to map CRS.
-     * TODO: should this be removed and determined on runtime by checks that need it?
-     */
-    const QgsCoordinateTransform &getLayerToMapTransform() const { return mLayerToMapTransform; }
-
     /**
      * Get a pointer to the underlying layer.
      * May return a ``nullptr`` if the layer has been deleted.
      * This must only be called from the main thread.
      */
     QgsVectorLayer *layer() const;
 
+    /**
+     * Get a QPointer to the underlying layer.
+     * Note that access to any methods of the object
+     * will need to be done on the main thread and
+     * the pointer will need to be checked for validity
+     * before usage.
+     */
+    QPointer<QgsVectorLayer> layerPtr() const;
+
     /**
      * The layer id of the layer.
      */
@@ -101,6 +98,11 @@ class ANALYSIS_EXPORT QgsFeaturePool : public QgsFeatureSink
      */
     QgsWkbTypes::GeometryType geometryType() const;
 
+    /**
+     * The coordinate reference system of this layer.
+     */
+    QgsCoordinateReferenceSystem crs() const;
+
   protected:
 
     /**
@@ -135,10 +137,9 @@ class ANALYSIS_EXPORT QgsFeaturePool : public QgsFeatureSink
     mutable QReadWriteLock mCacheLock;
     QgsFeatureIds mFeatureIds;
     QgsSpatialIndex mIndex;
-    double mLayerToMapUnits = 1.0;
-    QgsCoordinateTransform mLayerToMapTransform;
     QString mLayerId;
     QgsWkbTypes::GeometryType mGeometryType;
+    QgsCoordinateReferenceSystem mCrs;
 };
 
 #endif // QGS_FEATUREPOOL_H

src/analysis/vector/geometry_checker/qgsgeometryanglecheck.cpp

@@ -20,10 +20,10 @@
 void QgsGeometryAngleCheck::collectErrors( QList<QgsGeometryCheckError *> &errors, QStringList &/*messages*/, QAtomicInt *progressCounter, const QMap<QString, QgsFeatureIds> &ids ) const
 {
   QMap<QString, QgsFeatureIds> featureIds = ids.isEmpty() ? allLayerFeatureIds() : ids;
-  QgsGeometryCheckerUtils::LayerFeatures layerFeatures( mContext->featurePools, featureIds, mCompatibleGeometryTypes, progressCounter );
+  QgsGeometryCheckerUtils::LayerFeatures layerFeatures( mContext->featurePools, featureIds, mCompatibleGeometryTypes, progressCounter, context() );
   for ( const QgsGeometryCheckerUtils::LayerFeature &layerFeature : layerFeatures )
   {
-    const QgsAbstractGeometry *geom = layerFeature.geometry();
+    const QgsAbstractGeometry *geom = layerFeature.geometry().constGet();
     for ( int iPart = 0, nParts = geom->partCount(); iPart < nParts; ++iPart )
     {
       for ( int iRing = 0, nRings = geom->ringCount( iPart ); iRing < nRings; ++iRing )

src/analysis/vector/geometry_checker/qgsgeometryareacheck.cpp

@@ -21,11 +21,11 @@
 void QgsGeometryAreaCheck::collectErrors( QList<QgsGeometryCheckError *> &errors, QStringList &/*messages*/, QAtomicInt *progressCounter, const QMap<QString, QgsFeatureIds> &ids ) const
 {
   QMap<QString, QgsFeatureIds> featureIds = ids.isEmpty() ? allLayerFeatureIds() : ids;
-  QgsGeometryCheckerUtils::LayerFeatures layerFeatures( mContext->featurePools, featureIds, mCompatibleGeometryTypes, progressCounter );
+  QgsGeometryCheckerUtils::LayerFeatures layerFeatures( mContext->featurePools, featureIds, mCompatibleGeometryTypes, progressCounter, mContext );
   for ( const QgsGeometryCheckerUtils::LayerFeature &layerFeature : layerFeatures )
   {
-    double layerToMapUnits = layerFeature.layerToMapUnits();
-    const QgsAbstractGeometry *geom = layerFeature.geometry();
+    const QgsAbstractGeometry *geom = layerFeature.geometry().constGet();
+    double layerToMapUnits = mContext->layerScaleFactor( layerFeature.layer() );
     for ( int iPart = 0, nParts = geom->partCount(); iPart < nParts; ++iPart )
     {
       double value;
@@ -47,11 +47,13 @@ void QgsGeometryAreaCheck::fixError( QgsGeometryCheckError *error, int method, c
     error->setObsolete();
     return;
   }
-  double layerToMapUnits = featurePool->getLayerToMapUnits();
-  QgsGeometry g = feature.geometry();
+
+  const QgsGeometry g = feature.geometry();
   const QgsAbstractGeometry *geom = g.constGet();
   QgsVertexId vidx = error->vidx();
 
+  double layerToMapUnits = mContext->layerScaleFactor( featurePool->layer() );
+
   // Check if polygon still exists
   if ( !vidx.isValid( geom ) )
   {

src/analysis/vector/geometry_checker/qgsgeometrycheck.cpp

@@ -18,18 +18,61 @@
 #include "qgsgeometrycheck.h"
 #include "qgsfeaturepool.h"
 #include "qgsvectorlayer.h"
+#include "qgsreadwritelocker.h"
+#include "qgsthreadingutils.h"
 
-QgsGeometryCheckerContext::QgsGeometryCheckerContext( int _precision, const QgsCoordinateReferenceSystem &_mapCrs, const QMap<QString, QgsFeaturePool *> &_featurePools )
+QgsGeometryCheckerContext::QgsGeometryCheckerContext( int _precision, const QgsCoordinateReferenceSystem &_mapCrs, const QMap<QString, QgsFeaturePool *> &_featurePools, const QgsCoordinateTransformContext &transformContext )
   : tolerance( std::pow( 10, -_precision ) )
   , reducedTolerance( std::pow( 10, -_precision / 2 ) )
   , mapCrs( _mapCrs )
   , featurePools( _featurePools )
+  , transformContext( transformContext )
 {
+}
+
+const QgsCoordinateTransform &QgsGeometryCheckerContext::layerTransform( const QPointer<QgsVectorLayer> &layer )
+{
+  QgsReadWriteLocker locker( mCacheLock, QgsReadWriteLocker::Read );
+  if ( !mTransformCache.contains( layer ) )
+  {
+    QgsCoordinateTransform transform;
+    QgsThreadingUtils::runOnMainThread( [this, &transform, layer]()
+    {
+      QgsVectorLayer *lyr = layer.data();
+      if ( lyr )
+        transform = QgsCoordinateTransform( lyr->crs(), mapCrs, transformContext );
+    } );
+    locker.changeMode( QgsReadWriteLocker::Write );
+    mTransformCache[layer] = transform;
+    locker.changeMode( QgsReadWriteLocker::Read );
+  }
+
+  return mTransformCache[layer];
+}
+
+double QgsGeometryCheckerContext::layerScaleFactor( const QPointer<QgsVectorLayer> &layer )
+{
+  QgsReadWriteLocker locker( mCacheLock, QgsReadWriteLocker::Read );
+  if ( !mScaleFactorCache.contains( layer ) )
+  {
+    double scaleFactor = 1.0;
+    QgsThreadingUtils::runOnMainThread( [this, layer, &scaleFactor]()
+    {
+      QgsVectorLayer *lyr = layer.data();
+      if ( lyr )
+        scaleFactor = layerTransform( layer ).scaleFactor( lyr->extent() );
+    } );
+
+    locker.changeMode( QgsReadWriteLocker::Write );
+    mScaleFactorCache[layer] = scaleFactor;
+    locker.changeMode( QgsReadWriteLocker::Read );
+  }
 
+  return mScaleFactorCache.value( layer );
 }
 
 QgsGeometryCheckError::QgsGeometryCheckError( const QgsGeometryCheck *check, const QString &layerId,
-    QgsFeatureId featureId, QgsAbstractGeometry *geometry,
+    QgsFeatureId featureId, const QgsGeometry &geometry,
     const QgsPointXY &errorLocation,
     QgsVertexId vidx,
     const QVariant &value, ValueType valueType )
@@ -50,7 +93,7 @@ QgsGeometryCheckError::QgsGeometryCheckError( const QgsGeometryCheck *check,
     const QgsPointXY &errorLocation, QgsVertexId vidx,
     const QVariant &value, ValueType valueType )
   : mCheck( check )
-  , mLayerId( layerFeature.layer()->id() )
+  , mLayerId( layerFeature.layerId() )
   , mFeatureId( layerFeature.feature().id() )
   , mErrorLocation( errorLocation )
   , mVidx( vidx )
@@ -60,27 +103,28 @@ QgsGeometryCheckError::QgsGeometryCheckError( const QgsGeometryCheck *check,
 {
   if ( vidx.part != -1 )
   {
-    mGeometry.reset( QgsGeometryCheckerUtils::getGeomPart( layerFeature.geometry(), vidx.part )->clone() );
+    mGeometry = QgsGeometry( QgsGeometryCheckerUtils::getGeomPart( layerFeature.geometry().constGet(), vidx.part )->clone() );
   }
   else
   {
-    mGeometry.reset( layerFeature.geometry()->clone() );
+    mGeometry = layerFeature.geometry();
   }
-  if ( layerFeature.geometryCrs() != layerFeature.layerToMapTransform().destinationCrs().authid() )
+  if ( !layerFeature.useMapCrs() )
   {
-    mGeometry->transform( layerFeature.layerToMapTransform() );
-    mErrorLocation = layerFeature.layerToMapTransform().transform( mErrorLocation );
+    const QgsCoordinateTransform &transform = check->context()->layerTransform( layerFeature.layer() );
+    mGeometry.transform( transform );
+    mErrorLocation = transform.transform( mErrorLocation );
   }
 }
 
 const QgsAbstractGeometry *QgsGeometryCheckError::geometry() const
 {
-  return mGeometry.get();
+  return mGeometry.constGet();
 }
 
 QgsRectangle QgsGeometryCheckError::affectedAreaBBox() const
 {
-  return mGeometry->boundingBox();
+  return mGeometry.boundingBox();
 }
 
 bool QgsGeometryCheckError::handleChanges( const QgsGeometryCheck::Changes &changes )

src/analysis/vector/geometry_checker/qgsgeometrycheck.h

@@ -21,8 +21,11 @@
 #include <QApplication>
 #include <limits>
 #include <QStringList>
+#include <QPointer>
+
 #include "qgis_analysis.h"
 #include "qgsfeature.h"
+#include "qgsvectorlayer.h"
 #include "geometry/qgsgeometry.h"
 #include "qgsgeometrycheckerutils.h"
 
@@ -33,11 +36,19 @@ class QgsFeaturePool;
 
 struct ANALYSIS_EXPORT QgsGeometryCheckerContext
 {
-  QgsGeometryCheckerContext( int _precision, const QgsCoordinateReferenceSystem &_mapCrs, const QMap<QString, QgsFeaturePool *> &_featurePools );
-  const double tolerance;
-  const double reducedTolerance;
-  const QgsCoordinateReferenceSystem mapCrs;
-  const QMap<QString, QgsFeaturePool *> featurePools;
+    QgsGeometryCheckerContext( int _precision, const QgsCoordinateReferenceSystem &_mapCrs, const QMap<QString, QgsFeaturePool *> &_featurePools, const QgsCoordinateTransformContext &transformContext );
+    const double tolerance;
+    const double reducedTolerance;
+    const QgsCoordinateReferenceSystem mapCrs;
+    const QMap<QString, QgsFeaturePool *> featurePools;
+    const QgsCoordinateTransformContext transformContext;
+    const QgsCoordinateTransform &layerTransform( const QPointer<QgsVectorLayer> &layer );
+    double layerScaleFactor( const QPointer<QgsVectorLayer> &layer );
+
+  private:
+    QMap<QPointer<QgsVectorLayer>, QgsCoordinateTransform> mTransformCache;
+    QMap<QPointer<QgsVectorLayer>, double> mScaleFactorCache;
+    QReadWriteLock mCacheLock;
 };
 
 class ANALYSIS_EXPORT QgsGeometryCheck
@@ -177,7 +188,7 @@ class ANALYSIS_EXPORT QgsGeometryCheckError
       mErrorLocation = other->mErrorLocation;
       mVidx = other->mVidx;
       mValue = other->mValue;
-      mGeometry.reset( other->mGeometry->clone() );
+      mGeometry = other->mGeometry;
     }
 
     virtual bool handleChanges( const QgsGeometryCheck::Changes &changes );
@@ -187,7 +198,7 @@ class ANALYSIS_EXPORT QgsGeometryCheckError
     QgsGeometryCheckError( const QgsGeometryCheck *check,
                            const QString &layerId,
                            QgsFeatureId featureId,
-                           QgsAbstractGeometry *geometry,
+                           const QgsGeometry &geometry,
                            const QgsPointXY &errorLocation,
                            QgsVertexId vidx = QgsVertexId(),
                            const QVariant &value = QVariant(),
@@ -196,7 +207,7 @@ class ANALYSIS_EXPORT QgsGeometryCheckError
     const QgsGeometryCheck *mCheck = nullptr;
     QString mLayerId;
     QgsFeatureId mFeatureId;
-    std::unique_ptr<QgsAbstractGeometry> mGeometry;
+    QgsGeometry mGeometry;
     QgsPointXY mErrorLocation;
     QgsVertexId mVidx;
     QVariant mValue;

src/analysis/vector/geometry_checker/qgsgeometrychecker.cpp

@@ -291,3 +291,13 @@ void QgsGeometryChecker::runCheck( const QgsGeometryCheck *check )
     emit errorAdded( error );
   }
 }
+
+QgsGeometryChecker::RunCheckWrapper::RunCheckWrapper( QgsGeometryChecker *instance )
+  : mInstance( instance )
+{
+}
+
+void QgsGeometryChecker::RunCheckWrapper::operator()( const QgsGeometryCheck *check )
+{
+  mInstance->runCheck( check );
+}

src/analysis/vector/geometry_checker/qgsgeometrychecker.h

@@ -24,8 +24,9 @@
 #include <QMutex>
 #include <QStringList>
 #include "qgis_analysis.h"
-#include "qgsfeatureid.h"
 
+typedef qint64 QgsFeatureId;
+typedef QSet<QgsFeatureId> QgsFeatureIds;
 struct QgsGeometryCheckerContext;
 class QgsGeometryCheck;
 class QgsGeometryCheckError;
@@ -55,8 +56,8 @@ class ANALYSIS_EXPORT QgsGeometryChecker : public QObject
     class RunCheckWrapper
     {
       public:
-        explicit RunCheckWrapper( QgsGeometryChecker *instance ) : mInstance( instance ) {}
-        void operator()( const QgsGeometryCheck *check ) { mInstance->runCheck( check ); }
+        explicit RunCheckWrapper( QgsGeometryChecker *instance );
+        void operator()( const QgsGeometryCheck *check );
       private:
         QgsGeometryChecker *mInstance = nullptr;
     };