Allow resolving errors

qgis · Oct 15, 2018 · 4607930 · 4607930
1 parent dc2c78f
commit 4607930
Show file tree

Hide file tree

Showing 15 changed files with 199 additions and 17 deletions.
diff --git a/python/analysis/auto_generated/vector/geometry_checker/qgsfeaturepool.sip.in b/python/analysis/auto_generated/vector/geometry_checker/qgsfeaturepool.sip.in
@@ -31,6 +31,7 @@ It will be retrieved from the cache or from the underlying layer if unavailable.
 If the feature is neither available from the cache nor from the layer it will return false.
 %End
 
+
     virtual void updateFeature( QgsFeature &feature ) = 0;
 %Docstring
 Updates a feature in this pool.

diff --git a/src/analysis/vector/geometry_checker/qgsfeaturepool.cpp b/src/analysis/vector/geometry_checker/qgsfeaturepool.cpp
@@ -62,6 +62,23 @@ bool QgsFeaturePool::getFeature( QgsFeatureId id, QgsFeature &feature )
   return true;
 }
 
+QgsFeatureIds QgsFeaturePool::getFeatures( const QgsFeatureRequest &request )
+{
+  QgsFeatureIds fids;
+
+  std::unique_ptr<QgsVectorLayerFeatureSource> source = QgsVectorLayerUtils::getFeatureSource( mLayer );
+
+  QgsFeatureIterator it = source->getFeatures( request );
+  QgsFeature feature;
+  while ( it.nextFeature( feature ) )
+  {
+    insertFeature( feature );
+    fids << feature.id();
+  }
+
+  return fids;
+}
+
 QgsFeatureIds QgsFeaturePool::allFeatureIds() const
 {
   return mFeatureIds;

diff --git a/src/analysis/vector/geometry_checker/qgsfeaturepool.h b/src/analysis/vector/geometry_checker/qgsfeaturepool.h
@@ -46,6 +46,12 @@ class ANALYSIS_EXPORT QgsFeaturePool : public QgsFeatureSink SIP_ABSTRACT
      */
     bool getFeature( QgsFeatureId id, QgsFeature &feature );
 
+    /**
+     * Warm the cache ...
+     * TODO write more docs
+     */
+    QgsFeatureIds getFeatures( const QgsFeatureRequest &request ) SIP_SKIP;
+
     /**
      * Updates a feature in this pool.
      * Implementations will update the feature on the layer or on the data provider.

diff --git a/src/analysis/vector/geometry_checker/qgsgeometrygapcheck.cpp b/src/analysis/vector/geometry_checker/qgsgeometrygapcheck.cpp
@@ -97,7 +97,7 @@ void QgsGeometryGapCheck::collectErrors( const QMap<QString, QgsFeaturePool *> &
     }
 
     // Skip gaps above threshold
-    if ( gapGeom->area() > mGapThresholdMapUnits || gapGeom->area() < mContext->reducedTolerance )
+    if ( ( mGapThresholdMapUnits > 0 && gapGeom->area() > mGapThresholdMapUnits ) || gapGeom->area() < mContext->reducedTolerance )
     {
       continue;
     }

diff --git a/src/analysis/vector/geometry_checker/qgsgeometrygapcheck.h b/src/analysis/vector/geometry_checker/qgsgeometrygapcheck.h
@@ -78,6 +78,12 @@ class ANALYSIS_EXPORT QgsGeometryGapCheck : public QgsGeometryCheck
   public:
     enum ResolutionMethod { MergeLongestEdge, NoChange };
 
+    /**
+     * The \a configuration accepts a "gapThreshold" key which specifies
+     * the maximum gap size in squared map units. Any gaps which are larger
+     * than this area are accepted. If "gapThreshold" is set to 0, the check
+     * is disabled.
+     */
     explicit QgsGeometryGapCheck( const QgsGeometryCheckContext *context, const QVariantMap &configuration );
 
     QList<QgsWkbTypes::GeometryType> compatibleGeometryTypes() const override { return factoryCompatibleGeometryTypes(); }

diff --git a/src/analysis/vector/geometry_checker/qgsgeometrymissingvertexcheck.cpp b/src/analysis/vector/geometry_checker/qgsgeometrymissingvertexcheck.cpp
@@ -71,11 +71,32 @@ void QgsGeometryMissingVertexCheck::fixError( const QMap<QString, QgsFeaturePool
   {
     error->setFixed( method );
   }
+  if ( method == AddMissingVertex )
+  {
+    QgsFeaturePool *featurePool = featurePools[ error->layerId() ];
+
+    QgsFeature feature;
+    featurePool->getFeature( error->featureId(), feature );
+
+    QgsPointXY pointOnSegment; // Should be equal to location
+    int vertexIndex;
+    QgsGeometry geometry = feature.geometry();
+    geometry.closestSegmentWithContext( error->location(), pointOnSegment, vertexIndex );
+    geometry.insertVertex( QgsPoint( error->location() ), vertexIndex );
+    feature.setGeometry( geometry );
+
+    featurePool->updateFeature( feature );
+    // TODO update "changes" structure
+
+    error->setFixed( method );
+  }
 }
 
 QStringList QgsGeometryMissingVertexCheck::resolutionMethods() const
 {
-  static QStringList methods = QStringList() << tr( "No action" );
+  static QStringList methods = QStringList()
+                               << tr( "No action" )
+                               << tr( "Add missing vertex" );
   return methods;
 }
 

diff --git a/src/analysis/vector/geometry_checker/qgsgeometrymissingvertexcheck.h b/src/analysis/vector/geometry_checker/qgsgeometrymissingvertexcheck.h
@@ -37,7 +37,8 @@ class ANALYSIS_EXPORT QgsGeometryMissingVertexCheck : public QgsGeometryCheck
   public:
     enum ResolutionMethod
     {
-      NoChange
+      NoChange,
+      AddMissingVertex
     };
 
     explicit QgsGeometryMissingVertexCheck( const QgsGeometryCheckContext *context, const QVariantMap &geometryCheckConfiguration );

diff --git a/src/app/qgisapp.cpp b/src/app/qgisapp.cpp
@@ -939,6 +939,7 @@ QgisApp::QgisApp( QSplashScreen *splash, bool restorePlugins, bool skipVersionCh
     mGeometryValidationModel->setCurrentLayer( qobject_cast<QgsVectorLayer *>( layer ) );
   } );
   mGeometryValidationDock->setGeometryValidationModel( mGeometryValidationModel );
+  mGeometryValidationDock->setGeometryValidationService( mGeometryValidationService.get() );
   addDockWidget( Qt::RightDockWidgetArea, mGeometryValidationDock );
   endProfile();
 

diff --git a/src/app/qgsgeometryvalidationdock.cpp b/src/app/qgsgeometryvalidationdock.cpp
@@ -15,7 +15,12 @@ email                : matthias@opengis.ch
 
 #include "qgsgeometryvalidationdock.h"
 #include "qgsgeometryvalidationmodel.h"
+#include "qgsgeometryvalidationservice.h"
 #include "qgsmapcanvas.h"
+#include "qgsrubberband.h"
+#include "qgsvectorlayer.h"
+#include "qgsgeometrycheck.h"
+#include "qgsgeometrycheckerror.h"
 
 #include <QButtonGroup>
 
@@ -32,8 +37,23 @@ QgsGeometryValidationDock::QgsGeometryValidationDock( const QString &title, QgsM
   connect( mMapCanvas, &QgsMapCanvas::currentLayerChanged, this, &QgsGeometryValidationDock::updateLayerTransform );
   connect( mMapCanvas, &QgsMapCanvas::destinationCrsChanged, this, &QgsGeometryValidationDock::updateLayerTransform );
   connect( mMapCanvas, &QgsMapCanvas::transformContextChanged, this, &QgsGeometryValidationDock::updateLayerTransform );
+
+  mFeatureRubberband = new QgsRubberBand( mMapCanvas );
+  mErrorRubberband = new QgsRubberBand( mMapCanvas );
+  mErrorLocationRubberband = new QgsRubberBand( mMapCanvas );
+
+  double scaleFactor = mMapCanvas->fontMetrics().xHeight() * .2;
+
+  mFeatureRubberband->setColor( QColor( 250, 180, 180, 100 ) );
+  mFeatureRubberband->setWidth( scaleFactor );
+  mErrorRubberband->setColor( QColor( 180, 250, 180, 100 ) );
+  mErrorRubberband->setWidth( scaleFactor );
+  mErrorLocationRubberband->setIcon( QgsRubberBand::ICON_X );
+  mErrorLocationRubberband->setWidth( scaleFactor * 3 );
+  mErrorLocationRubberband->setColor( QColor( 180, 180, 250, 100 ) );
 }
 
+
 QgsGeometryValidationModel *QgsGeometryValidationDock::geometryValidationModel() const
 {
   return mGeometryValidationModel;
@@ -89,6 +109,16 @@ void QgsGeometryValidationDock::updateLayerTransform()
   mLayerTransform = QgsCoordinateTransform( mMapCanvas->currentLayer()->crs(), mMapCanvas->mapSettings().destinationCrs(), mMapCanvas->mapSettings().transformContext() );
 }
 
+QgsGeometryValidationService *QgsGeometryValidationDock::geometryValidationService() const
+{
+  return mGeometryValidationService;
+}
+
+void QgsGeometryValidationDock::setGeometryValidationService( QgsGeometryValidationService *geometryValidationService )
+{
+  mGeometryValidationService = geometryValidationService;
+}
+
 QModelIndex QgsGeometryValidationDock::currentIndex() const
 {
   return mErrorListView->selectionModel()->currentIndex();
@@ -102,6 +132,38 @@ void QgsGeometryValidationDock::onCurrentErrorChanged( const QModelIndex &curren
 
   mProblemDetailWidget->setVisible( current.isValid() );
   mProblemDescriptionLabel->setText( current.data().toString() );
+  {
+    QgsGeometryCheckError *error = current.data( QgsGeometryValidationModel::GeometryCheckErrorRole ).value<QgsGeometryCheckError *>();
+    while ( QPushButton *btn =  mResolutionWidget->findChild<QPushButton *>() )
+      delete btn;
+    const QStringList resolutionMethods = error->check()->resolutionMethods();
+    int resolutionIndex = 0;
+    for ( const QString &resolutionMethod : resolutionMethods )
+    {
+      QPushButton *resolveBtn = new QPushButton( resolutionMethod );
+      connect( resolveBtn, &QPushButton::clicked, this, [resolutionIndex, error, this]()
+      {
+        mGeometryValidationService->fixError( error, resolutionIndex );
+      } );
+      mResolutionWidget->layout()->addWidget( resolveBtn );
+      resolutionIndex++;
+    }
+  }
+
+  QgsVectorLayer *vlayer = qobject_cast<QgsVectorLayer *>( mMapCanvas->currentLayer() );
+  if ( vlayer )
+  {
+    QgsGeometry featureGeometry = current.data( QgsGeometryValidationModel::FeatureGeometryRole ).value<QgsGeometry>();
+    QgsGeometry errorGeometry = current.data( QgsGeometryValidationModel::ErrorGeometryRole ).value<QgsGeometry>();
+    QgsPointXY locationGeometry = current.data( QgsGeometryValidationModel::ErrorLocationGeometryRole ).value<QgsPointXY>();
+    qDebug() << "feature geom : " << featureGeometry.asWkt();
+    qDebug() << "error   geom : " << errorGeometry.asWkt();
+    qDebug() << "locationgeom : " << QgsGeometry( new QgsPoint( locationGeometry ) ).asWkt();
+
+    mFeatureRubberband->setToGeometry( featureGeometry );
+    mErrorRubberband->setToGeometry( errorGeometry );
+    mErrorLocationRubberband->setToGeometry( QgsGeometry( new QgsPoint( locationGeometry ) ) );
+  }
 
   switch ( mLastZoomToAction )
   {

diff --git a/src/app/qgsgeometryvalidationdock.h b/src/app/qgsgeometryvalidationdock.h
@@ -22,6 +22,8 @@ email                : matthias@opengis.ch
 
 class QgsMapCanvas;
 class QgsGeometryValidationModel;
+class QgsGeometryValidationService;
+class QgsRubberBand;
 
 /**
  * @brief The QgsGeometryValidationDock class
@@ -36,6 +38,9 @@ class QgsGeometryValidationDock : public QgsDockWidget, public Ui_QgsGeometryVal
     QgsGeometryValidationModel *geometryValidationModel() const;
     void setGeometryValidationModel( QgsGeometryValidationModel *geometryValidationModel );
 
+    QgsGeometryValidationService *geometryValidationService() const;
+    void setGeometryValidationService( QgsGeometryValidationService *geometryValidationService );
+
   private slots:
     void onCurrentErrorChanged( const QModelIndex &current, const QModelIndex &previous );
     void gotoNextError();
@@ -52,10 +57,14 @@ class QgsGeometryValidationDock : public QgsDockWidget, public Ui_QgsGeometryVal
     };
     ZoomToAction mLastZoomToAction = ZoomToFeature;
     QgsGeometryValidationModel *mGeometryValidationModel = nullptr;
+    QgsGeometryValidationService *mGeometryValidationService = nullptr;
     QButtonGroup *mZoomToButtonGroup = nullptr;
     QgsMapCanvas *mMapCanvas = nullptr;
     QgsCoordinateTransform mLayerTransform;
     QModelIndex currentIndex() const;
+    QgsRubberBand *mFeatureRubberband = nullptr;
+    QgsRubberBand *mErrorRubberband = nullptr;
+    QgsRubberBand *mErrorLocationRubberband = nullptr;
 };
 
 #endif // QGSGEOMETRYVALIDATIONPANEL_H
diff --git a/src/app/qgsgeometryvalidationmodel.cpp b/src/app/qgsgeometryvalidationmodel.cpp
@@ -74,6 +74,28 @@ QVariant QgsGeometryValidationModel::data( const QModelIndex &index, int role )
       {
         return topologyError->affectedAreaBBox();
       }
+
+      case ErrorGeometryRole:
+      {
+        return topologyError->geometry();
+      }
+
+      case FeatureGeometryRole:
+      {
+        const QgsFeatureId fid = topologyError->featureId();
+        const QgsFeature feature = mCurrentLayer->getFeature( fid ); // TODO: this should be cached!
+        return feature.geometry();
+      }
+
+      case ErrorLocationGeometryRole:
+      {
+        return topologyError->location();
+      }
+
+      case GeometryCheckErrorRole:
+      {
+        return QVariant::fromValue<QgsGeometryCheckError *>( topologyError.get() );
+      }
     }
   }
   else

diff --git a/src/app/qgsgeometryvalidationmodel.h b/src/app/qgsgeometryvalidationmodel.h
@@ -16,7 +16,11 @@ class QgsGeometryValidationModel : public QAbstractItemModel
     enum Roles
     {
       FeatureExtentRole = Qt::UserRole,
-      ProblemExtentRole
+      ProblemExtentRole,
+      ErrorGeometryRole,
+      FeatureGeometryRole,
+      ErrorLocationGeometryRole,
+      GeometryCheckErrorRole
     };
 
     QgsGeometryValidationModel( QgsGeometryValidationService *geometryValidationService, QObject *parent = nullptr );

diff --git a/src/app/qgsgeometryvalidationservice.cpp b/src/app/qgsgeometryvalidationservice.cpp
@@ -40,6 +40,13 @@ bool QgsGeometryValidationService::validationActive( QgsVectorLayer *layer, QgsF
   return false;
 }
 
+void QgsGeometryValidationService::fixError( const QgsGeometryCheckError *error, int method )
+{
+  QgsGeometryCheck::Changes changes;
+  QgsGeometryCheckError *nonconsterr = const_cast<QgsGeometryCheckError *>( error );
+  error->check()->fixError( mFeaturePools, nonconsterr, method, QMap<QString, int>(), changes );
+}
+
 void QgsGeometryValidationService::onLayersAdded( const QList<QgsMapLayer *> &layers )
 {
   for ( QgsMapLayer *layer : layers )
@@ -122,7 +129,7 @@ void QgsGeometryValidationService::enableLayerChecks( QgsVectorLayer *layer )
   qDeleteAll( mLayerCheckStates[layer].topologyChecks );
 
   // TODO: ownership and lifetime of the context!!
-  auto context = new QgsGeometryCheckContext( 1, mProject->crs(), mProject->transformContext() );
+  auto context = new QgsGeometryCheckContext( 8, mProject->crs(), mProject->transformContext() );
   QList<QgsGeometryCheck *> layerChecks;
 
   QgsGeometryCheckRegistry *checkRegistry = QgsAnalysis::instance()->geometryCheckRegistry();
@@ -216,18 +223,33 @@ void QgsGeometryValidationService::triggerTopologyChecks( QgsVectorLayer *layer
     mLayerCheckStates[layer].topologyCheckFeedbacks.clear();
   }
 
-  QgsFeatureIds checkFeatureIds = layer->editBuffer()->changedGeometries().keys().toSet();
-  checkFeatureIds.unite( layer->editBuffer()->addedFeatures().keys().toSet() );
+  QgsFeatureIds affectedFeatureIds = layer->editBuffer()->changedGeometries().keys().toSet();
+  affectedFeatureIds.unite( layer->editBuffer()->addedFeatures().keys().toSet() );
 
   // TODO: ownership of these objects...
   QgsVectorLayerFeaturePool *featurePool = new QgsVectorLayerFeaturePool( layer );
   QList<QgsGeometryCheckError *> &allErrors = mLayerCheckStates[layer].topologyCheckErrors;
   QMap<QString, QgsFeatureIds> layerIds;
+
+  QgsFeatureRequest request = QgsFeatureRequest( affectedFeatureIds ).setSubsetOfAttributes( QgsAttributeList() );
+  QgsFeatureIterator it = layer->getFeatures( request );
+  QgsFeature feature;
+  QgsRectangle area;
+  while ( it.nextFeature( feature ) )
+  {
+    area.combineExtentWith( feature.geometry().boundingBox() );
+  }
+
+  QgsFeatureRequest areaRequest = QgsFeatureRequest().setFilterRect( area );
+  QgsFeatureIds checkFeatureIds = featurePool->getFeatures( areaRequest );
+
   layerIds.insert( layer->id(), checkFeatureIds );
   QgsGeometryCheck::LayerFeatureIds layerFeatureIds( layerIds );
 
-  QMap<QString, QgsFeaturePool *> featurePools;
-  featurePools.insert( layer->id(), featurePool );
+  if ( !mFeaturePools.contains( layer->id() ) )
+  {
+    mFeaturePools.insert( layer->id(), featurePool );
+  }
 
   const QList<QgsGeometryCheck *> checks = mLayerCheckStates[layer].topologyChecks;
 
@@ -237,15 +259,15 @@ void QgsGeometryValidationService::triggerTopologyChecks( QgsVectorLayer *layer
 
   mLayerCheckStates[layer].topologyCheckFeedbacks = feedbacks.values();
 
-  QFuture<void> future = QtConcurrent::map( checks, [featurePools, &allErrors, layerFeatureIds, layer, feedbacks, this]( const QgsGeometryCheck * check )
+  QFuture<void> future = QtConcurrent::map( checks, [&allErrors, layerFeatureIds, layer, feedbacks, 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?
     QgsFeedback *feedback = feedbacks.value( check );
 
-    check->collectErrors( featurePools, errors, messages, feedback, layerFeatureIds );
+    check->collectErrors( mFeaturePools, errors, messages, feedback, layerFeatureIds );
     QgsReadWriteLocker errorLocker( mTopologyCheckLock, QgsReadWriteLocker::Write );
     allErrors.append( errors );
 

diff --git a/src/app/qgsgeometryvalidationservice.h b/src/app/qgsgeometryvalidationservice.h
@@ -31,6 +31,7 @@ class QgsSingleGeometryCheck;
 class QgsSingleGeometryCheckError;
 class QgsGeometryCheckError;
 class QgsFeedback;
+class QgsFeaturePool;
 
 /**
  * This service connects to all layers in a project and triggers validation
@@ -64,6 +65,8 @@ class QgsGeometryValidationService : public QObject
      */
     bool validationActive( QgsVectorLayer *layer, QgsFeatureId feature ) const;
 
+    void fixError( const QgsGeometryCheckError *error, int method );
+
   signals:
     void geometryCheckStarted( QgsVectorLayer *layer, QgsFeatureId fid );
     void geometryCheckCompleted( QgsVectorLayer *layer, QgsFeatureId fid, const QList<std::shared_ptr<QgsSingleGeometryCheckError>> &errors );
@@ -101,6 +104,7 @@ class QgsGeometryValidationService : public QObject
 
     QReadWriteLock mTopologyCheckLock;
     QHash<QgsVectorLayer *, VectorCheckState> mLayerCheckStates;
+    QMap<QString, QgsFeaturePool *> mFeaturePools;
 };
 
 #endif // QGSGEOMETRYVALIDATIONSERVICE_H