Report topology errors

qgis · Oct 15, 2018 · d60727f · d60727f
1 parent 91d54bc
commit d60727f
Show file tree

Hide file tree

Showing 5 changed files with 188 additions and 35 deletions.
diff --git a/src/app/qgisapp.cpp b/src/app/qgisapp.cpp
@@ -928,6 +928,10 @@ QgisApp::QgisApp( QSplashScreen *splash, bool restorePlugins, bool skipVersionCh
   QgsAnalysis::instance()->geometryCheckRegistry()->initialize();
 
   mGeometryValidationService = qgis::make_unique<QgsGeometryValidationService>( QgsProject::instance() );
+  connect( mGeometryValidationService.get(), &QgsGeometryValidationService::warning, this, [this]( const QString & message )
+  {
+    mInfoBar->pushWarning( tr( "Geometry Validation" ), message );
+  } );
   mGeometryValidationDock = new QgsGeometryValidationDock( tr( "Geometry Validation" ) );
   mGeometryValidationModel = new QgsGeometryValidationModel( mGeometryValidationService.get(), mGeometryValidationDock );
   connect( this, &QgisApp::activeLayerChanged, mGeometryValidationModel, [this]( QgsMapLayer * layer )

diff --git a/src/app/qgsgeometryvalidationmodel.cpp b/src/app/qgsgeometryvalidationmodel.cpp
@@ -11,6 +11,7 @@ QgsGeometryValidationModel::QgsGeometryValidationModel( QgsGeometryValidationSer
 {
   connect( mGeometryValidationService, &QgsGeometryValidationService::geometryCheckCompleted, this, &QgsGeometryValidationModel::onGeometryCheckCompleted );
   connect( mGeometryValidationService, &QgsGeometryValidationService::geometryCheckStarted, this, &QgsGeometryValidationModel::onGeometryCheckStarted );
+  connect( mGeometryValidationService, &QgsGeometryValidationService::topologyChecksUpdated, this, &QgsGeometryValidationModel::onTopologyChecksUpdated, Qt::QueuedConnection );
 }
 
 QModelIndex QgsGeometryValidationModel::index( int row, int column, const QModelIndex &parent ) const
@@ -28,7 +29,7 @@ QModelIndex QgsGeometryValidationModel::parent( const QModelIndex &child ) const
 int QgsGeometryValidationModel::rowCount( const QModelIndex &parent ) const
 {
   Q_UNUSED( parent )
-  return mErrorStorage.value( mCurrentLayer ).size();
+  return mErrorStorage.value( mCurrentLayer ).size() + mTopologyErrorStorage.value( mCurrentLayer ).size();
 }
 
 int QgsGeometryValidationModel::columnCount( const QModelIndex &parent ) const
@@ -41,38 +42,64 @@ QVariant QgsGeometryValidationModel::data( const QModelIndex &index, int role )
 {
   const auto &layerErrors = mErrorStorage.value( mCurrentLayer );
 
-  const auto &featureItem = layerErrors.at( index.row() );
-
-  switch ( role )
+  if ( index.row() >= layerErrors.size() )
   {
-    case Qt::DisplayRole:
+    // Topology error
+    const auto &topologyErrors = mTopologyErrorStorage.value( mCurrentLayer );
+    auto topologyError = topologyErrors.at( index.row() - layerErrors.size() );
+
+    switch ( role )
     {
-      QgsFeature feature = mCurrentLayer->getFeature( featureItem.fid ); // TODO: this should be cached!
-      mExpressionContext.setFeature( feature );
-      QString featureTitle = mDisplayExpression.evaluate( &mExpressionContext ).toString();
-      if ( featureTitle.isEmpty() )
-        featureTitle = featureItem.fid;
-
-      if ( featureItem.errors.count() > 1 )
-        return tr( "%1: %n Errors", "", featureItem.errors.count() ).arg( featureTitle );
-      else if ( featureItem.errors.count() == 1 )
-        return tr( "%1: %2" ).arg( featureTitle, featureItem.errors.at( 0 )->description() );
-#if 0
-      else
-        return tr( "%1: No Errors" ).arg( featureTitle );
-#endif
+      case Qt::DisplayRole:
+      {
+        const QgsFeatureId fid = topologyError->featureId();
+        const QgsFeature feature = mCurrentLayer->getFeature( fid ); // TODO: this should be cached!
+        mExpressionContext.setFeature( feature );
+        QString featureTitle = mDisplayExpression.evaluate( &mExpressionContext ).toString();
+        if ( featureTitle.isEmpty() )
+          featureTitle = fid;
+
+        return tr( "%1: %2" ).arg( featureTitle, topologyError->description() );
+      }
     }
+  }
+  else
+  {
+    // Geometry error
+    const auto &featureItem = layerErrors.at( index.row() );
 
-
-    case Qt::DecorationRole:
+    switch ( role )
     {
-      if ( mGeometryValidationService->validationActive( mCurrentLayer, featureItem.fid ) )
-        return QgsApplication::getThemeIcon( "/mActionTracing.svg" );
-      else
+      case Qt::DisplayRole:
+      {
+        QgsFeature feature = mCurrentLayer->getFeature( featureItem.fid ); // TODO: this should be cached!
+        mExpressionContext.setFeature( feature );
+        QString featureTitle = mDisplayExpression.evaluate( &mExpressionContext ).toString();
+        if ( featureTitle.isEmpty() )
+          featureTitle = featureItem.fid;
+
+        if ( featureItem.errors.count() > 1 )
+          return tr( "%1: %n Errors", "", featureItem.errors.count() ).arg( featureTitle );
+        else if ( featureItem.errors.count() == 1 )
+          return tr( "%1: %2" ).arg( featureTitle, featureItem.errors.at( 0 )->description() );
+        else
+          return QVariant();
+      }
+
+      case Qt::DecorationRole:
+      {
+        if ( mGeometryValidationService->validationActive( mCurrentLayer, featureItem.fid ) )
+          return QgsApplication::getThemeIcon( "/mActionTracing.svg" );
+        else
+          return QVariant();
+      }
+
+      default:
         return QVariant();
     }
   }
 
+
   return QVariant();
 }
 
@@ -162,6 +189,24 @@ void QgsGeometryValidationModel::onGeometryCheckStarted( QgsVectorLayer *layer,
   }
 }
 
+void QgsGeometryValidationModel::onTopologyChecksUpdated( QgsVectorLayer *layer, const QList<std::shared_ptr<QgsGeometryCheckError> > &errors )
+{
+  auto &topologyLayerErrors = mTopologyErrorStorage[layer];
+
+  if ( layer == currentLayer() )
+  {
+    const int oldRowCount = rowCount( QModelIndex() );
+    beginInsertRows( QModelIndex(), oldRowCount, oldRowCount + errors.size() );
+  }
+
+  topologyLayerErrors.append( errors );
+
+  if ( layer == currentLayer() )
+  {
+    endInsertRows();
+  }
+}
+
 int QgsGeometryValidationModel::errorsForFeature( QgsVectorLayer *layer, QgsFeatureId fid )
 {
   const auto &layerErrors = mErrorStorage[layer];

diff --git a/src/app/qgsgeometryvalidationmodel.h b/src/app/qgsgeometryvalidationmodel.h
@@ -27,6 +27,7 @@ class QgsGeometryValidationModel : public QAbstractItemModel
   private slots:
     void onGeometryCheckCompleted( QgsVectorLayer *layer, QgsFeatureId fid, const QList<std::shared_ptr<QgsSingleGeometryCheckError> > &errors );
     void onGeometryCheckStarted( QgsVectorLayer *layer, QgsFeatureId fid );
+    void onTopologyChecksUpdated( QgsVectorLayer *layer, const QList<std::shared_ptr<QgsGeometryCheckError> > &errors );
 
   private:
     struct FeatureErrors
@@ -50,6 +51,7 @@ class QgsGeometryValidationModel : public QAbstractItemModel
     mutable QgsExpressionContext mExpressionContext;
 
     QMap<QgsVectorLayer *, QList< FeatureErrors > > mErrorStorage;
+    QMap<QgsVectorLayer *, QList< std::shared_ptr< QgsGeometryCheckError > > > mTopologyErrorStorage;
 };
 
 #endif // QGSGEOMETRYVALIDATIONMODEL_H
diff --git a/src/app/qgsgeometryvalidationservice.cpp b/src/app/qgsgeometryvalidationservice.cpp
@@ -21,10 +21,18 @@ email                : matthias@opengis.ch
 #include "qgsanalysis.h"
 #include "qgsgeometrycheckregistry.h"
 #include "qgsgeometrycheckfactory.h"
+#include "qgsvectorlayereditbuffer.h"
+#include "qgsvectorlayerfeaturepool.h"
+#include "qgsfeedback.h"
+#include "qgsreadwritelocker.h"
+
+#include <QtConcurrent>
+#include <QFutureWatcher>
 
 QgsGeometryValidationService::QgsGeometryValidationService( QgsProject *project )
   : mProject( project )
 {
+  qRegisterMetaType< QList<std::shared_ptr<QgsGeometryCheckError> > >( "QList<std::shared_ptr<QgsGeometryCheckError>>" );
   connect( project, &QgsProject::layersAdded, this, &QgsGeometryValidationService::onLayersAdded );
 }
 
@@ -64,11 +72,21 @@ void QgsGeometryValidationService::onLayersAdded( const QList<QgsMapLayer *> &la
 
 void QgsGeometryValidationService::onFeatureAdded( QgsVectorLayer *layer, QgsFeatureId fid )
 {
+  if ( !mLayerCheckStates[layer].topologyChecks.empty() )
+  {
+    // TODO: Cancel topology checks
+    layer->setAllowCommit( false );
+  }
   processFeature( layer, fid );
 }
 
 void QgsGeometryValidationService::onGeometryChanged( QgsVectorLayer *layer, QgsFeatureId fid, const QgsGeometry &geometry )
 {
+  if ( !mLayerCheckStates[layer].topologyChecks.empty() )
+  {
+    // TODO: Cancel topology checks
+    layer->setAllowCommit( false );
+  }
   Q_UNUSED( geometry )
 
   cancelChecks( layer, fid );
@@ -77,21 +95,32 @@ void QgsGeometryValidationService::onGeometryChanged( QgsVectorLayer *layer, Qgs
 
 void QgsGeometryValidationService::onFeatureDeleted( QgsVectorLayer *layer, QgsFeatureId fid )
 {
+  if ( !mLayerCheckStates[layer].topologyChecks.empty() )
+  {
+    // TODO: Cancel topology checks
+    layer->setAllowCommit( false );
+  }
+
   cancelChecks( layer, fid );
 }
 
 void QgsGeometryValidationService::onBeforeCommitChanges( QgsVectorLayer *layer )
 {
-  if ( !mTopologyChecksOk.value( layer ) )
+  if ( !mLayerCheckStates[layer].topologyChecks.empty() ) // TODO && topologyChecks not fulfilled
   {
+    if ( !layer->allowCommit() )
+    {
+      emit warning( tr( "Can not save yet, we'll need to run some topology checks on your dataset first..." ) );
+    }
     triggerTopologyChecks( layer );
   }
 }
 
 void QgsGeometryValidationService::enableLayerChecks( QgsVectorLayer *layer )
 {
   // TODO: finish all ongoing checks
-  qDeleteAll( mSingleFeatureChecks.value( layer ) );
+  qDeleteAll( mLayerCheckStates[layer].singleFeatureChecks );
+  qDeleteAll( mLayerCheckStates[layer].topologyChecks );
 
   // TODO: ownership and lifetime of the context!!
   auto context = new QgsGeometryCheckContext( 1, mProject->crs(), mProject->transformContext() );
@@ -120,16 +149,23 @@ void QgsGeometryValidationService::enableLayerChecks( QgsVectorLayer *layer )
     singleGeometryChecks.append( dynamic_cast<QgsSingleGeometryCheck *>( check ) );
   }
 
-  mSingleFeatureChecks.insert( layer, singleGeometryChecks );
+  mLayerCheckStates[layer].singleFeatureChecks = singleGeometryChecks;
 
-#if 0
+  // Topology checks
+  QList<QgsGeometryCheck *> topologyChecks;
   const QList<QgsGeometryCheckFactory *> topologyCheckFactories = checkRegistry->geometryCheckFactories( layer, QgsGeometryCheck::SingleLayerTopologyCheck );
 
-  for ( const QString &check : activeChecks )
+  for ( QgsGeometryCheckFactory *factory : topologyCheckFactories )
   {
-    checkRegistry->geometryCheckFactories( layer, QgsGeometryCheck::SingleLayerTopologyCheck );
+    const QString checkId = factory->id();
+    if ( activeChecks.contains( checkId ) )
+    {
+      const QVariantMap checkConfiguration = layer->geometryOptions()->checkConfiguration( checkId );
+      topologyChecks.append( factory->createGeometryCheck( context, checkConfiguration ) );
+    }
   }
-#endif
+
+  mLayerCheckStates[layer].topologyChecks = topologyChecks;
 }
 
 void QgsGeometryValidationService::cancelChecks( QgsVectorLayer *layer, QgsFeatureId fid )
@@ -143,7 +179,7 @@ void QgsGeometryValidationService::processFeature( QgsVectorLayer *layer, QgsFea
 
   QgsFeature feature = layer->getFeature( fid );
 
-  const auto &checks = mSingleFeatureChecks.value( layer );
+  const auto &checks = mLayerCheckStates[layer].singleFeatureChecks;
 
   // The errors are going to be sent out via a signal. We cannot keep ownership in here (or can we?)
   // nor can we be sure that a single slot is connected to the signal. So make it a shared_ptr.
@@ -161,5 +197,58 @@ void QgsGeometryValidationService::processFeature( QgsVectorLayer *layer, QgsFea
 
 void QgsGeometryValidationService::triggerTopologyChecks( QgsVectorLayer *layer )
 {
+  QFutureWatcher<void> *futureWatcher = mLayerCheckStates[layer].topologyCheckFutureWatcher;
+  if ( futureWatcher )
+  {
+    // TODO: kill!!
+    delete futureWatcher;
+  }
+
+  QgsFeatureIds checkFeatureIds = layer->editBuffer()->changedGeometries().keys().toSet();
+  checkFeatureIds.unite( layer->editBuffer()->addedFeatures().keys().toSet() );
+
+  // TODO: ownership of these objects...
+  QgsVectorLayerFeaturePool *featurePool = new QgsVectorLayerFeaturePool( layer );
+  QList<QgsGeometryCheckError *> &allErrors = mLayerCheckStates[layer].topologyCheckErrors;
+  QgsFeedback *feedback = new QgsFeedback();
+  QMap<QString, QgsFeatureIds> layerIds;
+  layerIds.insert( layer->id(), checkFeatureIds );
+  QgsGeometryCheck::LayerFeatureIds layerFeatureIds( layerIds );
+
+  QMap<QString, QgsFeaturePool *> featurePools;
+  featurePools.insert( layer->id(), featurePool );
+
+  const QList<QgsGeometryCheck *> checks = mLayerCheckStates[layer].topologyChecks;
+
+  QFuture<void> future = QtConcurrent::map( checks, [featurePools, &allErrors, feedback, layerFeatureIds, layer, this]( const QgsGeometryCheck * check )
+  {
+    // Watch out with the layer pointer in here. We are running in a thread, so we do not want to actually use it
+    // except for using its address to report the error.
+    QList<QgsGeometryCheckError *> errors;
+    QStringList messages; // Do we really need these?
+
+    check->collectErrors( featurePools, errors, messages, feedback, layerFeatureIds );
+    QgsReadWriteLocker errorLocker( mTopologyCheckLock, QgsReadWriteLocker::Write );
+    allErrors.append( errors );
+
+    QList<std::shared_ptr<QgsGeometryCheckError> > sharedErrors;
+    for ( QgsGeometryCheckError *error : errors )
+    {
+      sharedErrors.append( std::shared_ptr<QgsGeometryCheckError>( error ) );
+    }
+    emit topologyChecksUpdated( layer, sharedErrors );
+    errorLocker.unlock();
+  } );
+
+  futureWatcher = new QFutureWatcher<void>();
+  futureWatcher->setFuture( future );
+
+  connect( futureWatcher, &QFutureWatcherBase::finished, this, [&allErrors, layer, this]()
+  {
+    QgsReadWriteLocker errorLocker( mTopologyCheckLock, QgsReadWriteLocker::Read );
+    layer->setAllowCommit( allErrors.empty() );
+    errorLocker.unlock();
+  } );
 
+  mLayerCheckStates[layer].topologyCheckFutureWatcher = futureWatcher;
 }
diff --git a/src/app/qgsgeometryvalidationservice.h b/src/app/qgsgeometryvalidationservice.h
@@ -18,6 +18,8 @@ email                : matthias@opengis.ch
 
 #include <QObject>
 #include <QMap>
+#include <QFuture>
+#include <QReadWriteLock>
 
 #include "qgsfeature.h"
 
@@ -27,7 +29,7 @@ class QgsVectorLayer;
 class QgsGeometryCheck;
 class QgsSingleGeometryCheck;
 class QgsSingleGeometryCheckError;
-
+class QgsGeometryCheckError;
 
 
 /**
@@ -65,6 +67,9 @@ class QgsGeometryValidationService : public QObject
   signals:
     void geometryCheckStarted( QgsVectorLayer *layer, QgsFeatureId fid );
     void geometryCheckCompleted( QgsVectorLayer *layer, QgsFeatureId fid, const QList<std::shared_ptr<QgsSingleGeometryCheckError>> &errors );
+    void topologyChecksUpdated( QgsVectorLayer *layer, const QList<std::shared_ptr<QgsGeometryCheckError> > &errors );
+
+    void warning( const QString &message );
 
   private slots:
     void onLayersAdded( const QList<QgsMapLayer *> &layers );
@@ -84,8 +89,16 @@ class QgsGeometryValidationService : public QObject
 
     QgsProject *mProject = nullptr;
 
-    QHash<QgsVectorLayer *, QList< QgsSingleGeometryCheck * > > mSingleFeatureChecks;
-    QHash<QgsVectorLayer *, bool > mTopologyChecksOk;
+    struct VectorCheckState
+    {
+      QList< QgsSingleGeometryCheck * > singleFeatureChecks;
+      QList< QgsGeometryCheck * > topologyChecks;
+      QFutureWatcher<void> *topologyCheckFutureWatcher = nullptr;
+      QList<QgsGeometryCheckError *> topologyCheckErrors;
+    };
+
+    QReadWriteLock mTopologyCheckLock;
+    QHash<QgsVectorLayer *, VectorCheckState> mLayerCheckStates;
 };
 
 #endif // QGSGEOMETRYVALIDATIONSERVICE_H