Skip to content

Commit d60727f

Browse files
m-kuhnm-kuhn
committed
Report topology errors
1 parent 91d54bc commit d60727f

5 files changed

+188
-35
lines changed

‎src/app/qgisapp.cpp

+4
Original file line numberDiff line numberDiff line change
@@ -928,6 +928,10 @@ QgisApp::QgisApp( QSplashScreen *splash, bool restorePlugins, bool skipVersionCh
928928
QgsAnalysis::instance()->geometryCheckRegistry()->initialize();
929929

930930
mGeometryValidationService = qgis::make_unique<QgsGeometryValidationService>( QgsProject::instance() );
931+
connect( mGeometryValidationService.get(), &QgsGeometryValidationService::warning, this, [this]( const QString & message )
932+
{
933+
mInfoBar->pushWarning( tr( "Geometry Validation" ), message );
934+
} );
931935
mGeometryValidationDock = new QgsGeometryValidationDock( tr( "Geometry Validation" ) );
932936
mGeometryValidationModel = new QgsGeometryValidationModel( mGeometryValidationService.get(), mGeometryValidationDock );
933937
connect( this, &QgisApp::activeLayerChanged, mGeometryValidationModel, [this]( QgsMapLayer * layer )

‎src/app/qgsgeometryvalidationmodel.cpp

+69-24
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ QgsGeometryValidationModel::QgsGeometryValidationModel( QgsGeometryValidationSer
1111
{
1212
connect( mGeometryValidationService, &QgsGeometryValidationService::geometryCheckCompleted, this, &QgsGeometryValidationModel::onGeometryCheckCompleted );
1313
connect( mGeometryValidationService, &QgsGeometryValidationService::geometryCheckStarted, this, &QgsGeometryValidationModel::onGeometryCheckStarted );
14+
connect( mGeometryValidationService, &QgsGeometryValidationService::topologyChecksUpdated, this, &QgsGeometryValidationModel::onTopologyChecksUpdated, Qt::QueuedConnection );
1415
}
1516

1617
QModelIndex QgsGeometryValidationModel::index( int row, int column, const QModelIndex &parent ) const
@@ -28,7 +29,7 @@ QModelIndex QgsGeometryValidationModel::parent( const QModelIndex &child ) const
2829
int QgsGeometryValidationModel::rowCount( const QModelIndex &parent ) const
2930
{
3031
Q_UNUSED( parent )
31-
return mErrorStorage.value( mCurrentLayer ).size();
32+
return mErrorStorage.value( mCurrentLayer ).size() + mTopologyErrorStorage.value( mCurrentLayer ).size();
3233
}
3334

3435
int QgsGeometryValidationModel::columnCount( const QModelIndex &parent ) const
@@ -41,38 +42,64 @@ QVariant QgsGeometryValidationModel::data( const QModelIndex &index, int role )
4142
{
4243
const auto &layerErrors = mErrorStorage.value( mCurrentLayer );
4344

44-
const auto &featureItem = layerErrors.at( index.row() );
45-
46-
switch ( role )
45+
if ( index.row() >= layerErrors.size() )
4746
{
48-
case Qt::DisplayRole:
47+
// Topology error
48+
const auto &topologyErrors = mTopologyErrorStorage.value( mCurrentLayer );
49+
auto topologyError = topologyErrors.at( index.row() - layerErrors.size() );
50+
51+
switch ( role )
4952
{
50-
QgsFeature feature = mCurrentLayer->getFeature( featureItem.fid ); // TODO: this should be cached!
51-
mExpressionContext.setFeature( feature );
52-
QString featureTitle = mDisplayExpression.evaluate( &mExpressionContext ).toString();
53-
if ( featureTitle.isEmpty() )
54-
featureTitle = featureItem.fid;
55-
56-
if ( featureItem.errors.count() > 1 )
57-
return tr( "%1: %n Errors", "", featureItem.errors.count() ).arg( featureTitle );
58-
else if ( featureItem.errors.count() == 1 )
59-
return tr( "%1: %2" ).arg( featureTitle, featureItem.errors.at( 0 )->description() );
60-
#if 0
61-
else
62-
return tr( "%1: No Errors" ).arg( featureTitle );
63-
#endif
53+
case Qt::DisplayRole:
54+
{
55+
const QgsFeatureId fid = topologyError->featureId();
56+
const QgsFeature feature = mCurrentLayer->getFeature( fid ); // TODO: this should be cached!
57+
mExpressionContext.setFeature( feature );
58+
QString featureTitle = mDisplayExpression.evaluate( &mExpressionContext ).toString();
59+
if ( featureTitle.isEmpty() )
60+
featureTitle = fid;
61+
62+
return tr( "%1: %2" ).arg( featureTitle, topologyError->description() );
63+
}
6464
}
65+
}
66+
else
67+
{
68+
// Geometry error
69+
const auto &featureItem = layerErrors.at( index.row() );
6570

66-
67-
case Qt::DecorationRole:
71+
switch ( role )
6872
{
69-
if ( mGeometryValidationService->validationActive( mCurrentLayer, featureItem.fid ) )
70-
return QgsApplication::getThemeIcon( "/mActionTracing.svg" );
71-
else
73+
case Qt::DisplayRole:
74+
{
75+
QgsFeature feature = mCurrentLayer->getFeature( featureItem.fid ); // TODO: this should be cached!
76+
mExpressionContext.setFeature( feature );
77+
QString featureTitle = mDisplayExpression.evaluate( &mExpressionContext ).toString();
78+
if ( featureTitle.isEmpty() )
79+
featureTitle = featureItem.fid;
80+
81+
if ( featureItem.errors.count() > 1 )
82+
return tr( "%1: %n Errors", "", featureItem.errors.count() ).arg( featureTitle );
83+
else if ( featureItem.errors.count() == 1 )
84+
return tr( "%1: %2" ).arg( featureTitle, featureItem.errors.at( 0 )->description() );
85+
else
86+
return QVariant();
87+
}
88+
89+
case Qt::DecorationRole:
90+
{
91+
if ( mGeometryValidationService->validationActive( mCurrentLayer, featureItem.fid ) )
92+
return QgsApplication::getThemeIcon( "/mActionTracing.svg" );
93+
else
94+
return QVariant();
95+
}
96+
97+
default:
7298
return QVariant();
7399
}
74100
}
75101

102+
76103
return QVariant();
77104
}
78105

@@ -162,6 +189,24 @@ void QgsGeometryValidationModel::onGeometryCheckStarted( QgsVectorLayer *layer,
162189
}
163190
}
164191

192+
void QgsGeometryValidationModel::onTopologyChecksUpdated( QgsVectorLayer *layer, const QList<std::shared_ptr<QgsGeometryCheckError> > &errors )
193+
{
194+
auto &topologyLayerErrors = mTopologyErrorStorage[layer];
195+
196+
if ( layer == currentLayer() )
197+
{
198+
const int oldRowCount = rowCount( QModelIndex() );
199+
beginInsertRows( QModelIndex(), oldRowCount, oldRowCount + errors.size() );
200+
}
201+
202+
topologyLayerErrors.append( errors );
203+
204+
if ( layer == currentLayer() )
205+
{
206+
endInsertRows();
207+
}
208+
}
209+
165210
int QgsGeometryValidationModel::errorsForFeature( QgsVectorLayer *layer, QgsFeatureId fid )
166211
{
167212
const auto &layerErrors = mErrorStorage[layer];

‎src/app/qgsgeometryvalidationmodel.h

+2
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ class QgsGeometryValidationModel : public QAbstractItemModel
2727
private slots:
2828
void onGeometryCheckCompleted( QgsVectorLayer *layer, QgsFeatureId fid, const QList<std::shared_ptr<QgsSingleGeometryCheckError> > &errors );
2929
void onGeometryCheckStarted( QgsVectorLayer *layer, QgsFeatureId fid );
30+
void onTopologyChecksUpdated( QgsVectorLayer *layer, const QList<std::shared_ptr<QgsGeometryCheckError> > &errors );
3031

3132
private:
3233
struct FeatureErrors
@@ -50,6 +51,7 @@ class QgsGeometryValidationModel : public QAbstractItemModel
5051
mutable QgsExpressionContext mExpressionContext;
5152

5253
QMap<QgsVectorLayer *, QList< FeatureErrors > > mErrorStorage;
54+
QMap<QgsVectorLayer *, QList< std::shared_ptr< QgsGeometryCheckError > > > mTopologyErrorStorage;
5355
};
5456

5557
#endif // QGSGEOMETRYVALIDATIONMODEL_H

‎src/app/qgsgeometryvalidationservice.cpp

+97-8
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,18 @@ email : matthias@opengis.ch
2121
#include "qgsanalysis.h"
2222
#include "qgsgeometrycheckregistry.h"
2323
#include "qgsgeometrycheckfactory.h"
24+
#include "qgsvectorlayereditbuffer.h"
25+
#include "qgsvectorlayerfeaturepool.h"
26+
#include "qgsfeedback.h"
27+
#include "qgsreadwritelocker.h"
28+
29+
#include <QtConcurrent>
30+
#include <QFutureWatcher>
2431

2532
QgsGeometryValidationService::QgsGeometryValidationService( QgsProject *project )
2633
: mProject( project )
2734
{
35+
qRegisterMetaType< QList<std::shared_ptr<QgsGeometryCheckError> > >( "QList<std::shared_ptr<QgsGeometryCheckError>>" );
2836
connect( project, &QgsProject::layersAdded, this, &QgsGeometryValidationService::onLayersAdded );
2937
}
3038

@@ -64,11 +72,21 @@ void QgsGeometryValidationService::onLayersAdded( const QList<QgsMapLayer *> &la
6472

6573
void QgsGeometryValidationService::onFeatureAdded( QgsVectorLayer *layer, QgsFeatureId fid )
6674
{
75+
if ( !mLayerCheckStates[layer].topologyChecks.empty() )
76+
{
77+
// TODO: Cancel topology checks
78+
layer->setAllowCommit( false );
79+
}
6780
processFeature( layer, fid );
6881
}
6982

7083
void QgsGeometryValidationService::onGeometryChanged( QgsVectorLayer *layer, QgsFeatureId fid, const QgsGeometry &geometry )
7184
{
85+
if ( !mLayerCheckStates[layer].topologyChecks.empty() )
86+
{
87+
// TODO: Cancel topology checks
88+
layer->setAllowCommit( false );
89+
}
7290
Q_UNUSED( geometry )
7391

7492
cancelChecks( layer, fid );
@@ -77,21 +95,32 @@ void QgsGeometryValidationService::onGeometryChanged( QgsVectorLayer *layer, Qgs
7795

7896
void QgsGeometryValidationService::onFeatureDeleted( QgsVectorLayer *layer, QgsFeatureId fid )
7997
{
98+
if ( !mLayerCheckStates[layer].topologyChecks.empty() )
99+
{
100+
// TODO: Cancel topology checks
101+
layer->setAllowCommit( false );
102+
}
103+
80104
cancelChecks( layer, fid );
81105
}
82106

83107
void QgsGeometryValidationService::onBeforeCommitChanges( QgsVectorLayer *layer )
84108
{
85-
if ( !mTopologyChecksOk.value( layer ) )
109+
if ( !mLayerCheckStates[layer].topologyChecks.empty() ) // TODO && topologyChecks not fulfilled
86110
{
111+
if ( !layer->allowCommit() )
112+
{
113+
emit warning( tr( "Can not save yet, we'll need to run some topology checks on your dataset first..." ) );
114+
}
87115
triggerTopologyChecks( layer );
88116
}
89117
}
90118

91119
void QgsGeometryValidationService::enableLayerChecks( QgsVectorLayer *layer )
92120
{
93121
// TODO: finish all ongoing checks
94-
qDeleteAll( mSingleFeatureChecks.value( layer ) );
122+
qDeleteAll( mLayerCheckStates[layer].singleFeatureChecks );
123+
qDeleteAll( mLayerCheckStates[layer].topologyChecks );
95124

96125
// TODO: ownership and lifetime of the context!!
97126
auto context = new QgsGeometryCheckContext( 1, mProject->crs(), mProject->transformContext() );
@@ -120,16 +149,23 @@ void QgsGeometryValidationService::enableLayerChecks( QgsVectorLayer *layer )
120149
singleGeometryChecks.append( dynamic_cast<QgsSingleGeometryCheck *>( check ) );
121150
}
122151

123-
mSingleFeatureChecks.insert( layer, singleGeometryChecks );
152+
mLayerCheckStates[layer].singleFeatureChecks = singleGeometryChecks;
124153

125-
#if 0
154+
// Topology checks
155+
QList<QgsGeometryCheck *> topologyChecks;
126156
const QList<QgsGeometryCheckFactory *> topologyCheckFactories = checkRegistry->geometryCheckFactories( layer, QgsGeometryCheck::SingleLayerTopologyCheck );
127157

128-
for ( const QString &check : activeChecks )
158+
for ( QgsGeometryCheckFactory *factory : topologyCheckFactories )
129159
{
130-
checkRegistry->geometryCheckFactories( layer, QgsGeometryCheck::SingleLayerTopologyCheck );
160+
const QString checkId = factory->id();
161+
if ( activeChecks.contains( checkId ) )
162+
{
163+
const QVariantMap checkConfiguration = layer->geometryOptions()->checkConfiguration( checkId );
164+
topologyChecks.append( factory->createGeometryCheck( context, checkConfiguration ) );
165+
}
131166
}
132-
#endif
167+
168+
mLayerCheckStates[layer].topologyChecks = topologyChecks;
133169
}
134170

135171
void QgsGeometryValidationService::cancelChecks( QgsVectorLayer *layer, QgsFeatureId fid )
@@ -143,7 +179,7 @@ void QgsGeometryValidationService::processFeature( QgsVectorLayer *layer, QgsFea
143179

144180
QgsFeature feature = layer->getFeature( fid );
145181

146-
const auto &checks = mSingleFeatureChecks.value( layer );
182+
const auto &checks = mLayerCheckStates[layer].singleFeatureChecks;
147183

148184
// The errors are going to be sent out via a signal. We cannot keep ownership in here (or can we?)
149185
// 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
161197

162198
void QgsGeometryValidationService::triggerTopologyChecks( QgsVectorLayer *layer )
163199
{
200+
QFutureWatcher<void> *futureWatcher = mLayerCheckStates[layer].topologyCheckFutureWatcher;
201+
if ( futureWatcher )
202+
{
203+
// TODO: kill!!
204+
delete futureWatcher;
205+
}
206+
207+
QgsFeatureIds checkFeatureIds = layer->editBuffer()->changedGeometries().keys().toSet();
208+
checkFeatureIds.unite( layer->editBuffer()->addedFeatures().keys().toSet() );
209+
210+
// TODO: ownership of these objects...
211+
QgsVectorLayerFeaturePool *featurePool = new QgsVectorLayerFeaturePool( layer );
212+
QList<QgsGeometryCheckError *> &allErrors = mLayerCheckStates[layer].topologyCheckErrors;
213+
QgsFeedback *feedback = new QgsFeedback();
214+
QMap<QString, QgsFeatureIds> layerIds;
215+
layerIds.insert( layer->id(), checkFeatureIds );
216+
QgsGeometryCheck::LayerFeatureIds layerFeatureIds( layerIds );
217+
218+
QMap<QString, QgsFeaturePool *> featurePools;
219+
featurePools.insert( layer->id(), featurePool );
220+
221+
const QList<QgsGeometryCheck *> checks = mLayerCheckStates[layer].topologyChecks;
222+
223+
QFuture<void> future = QtConcurrent::map( checks, [featurePools, &allErrors, feedback, layerFeatureIds, layer, this]( const QgsGeometryCheck * check )
224+
{
225+
// Watch out with the layer pointer in here. We are running in a thread, so we do not want to actually use it
226+
// except for using its address to report the error.
227+
QList<QgsGeometryCheckError *> errors;
228+
QStringList messages; // Do we really need these?
229+
230+
check->collectErrors( featurePools, errors, messages, feedback, layerFeatureIds );
231+
QgsReadWriteLocker errorLocker( mTopologyCheckLock, QgsReadWriteLocker::Write );
232+
allErrors.append( errors );
233+
234+
QList<std::shared_ptr<QgsGeometryCheckError> > sharedErrors;
235+
for ( QgsGeometryCheckError *error : errors )
236+
{
237+
sharedErrors.append( std::shared_ptr<QgsGeometryCheckError>( error ) );
238+
}
239+
emit topologyChecksUpdated( layer, sharedErrors );
240+
errorLocker.unlock();
241+
} );
242+
243+
futureWatcher = new QFutureWatcher<void>();
244+
futureWatcher->setFuture( future );
245+
246+
connect( futureWatcher, &QFutureWatcherBase::finished, this, [&allErrors, layer, this]()
247+
{
248+
QgsReadWriteLocker errorLocker( mTopologyCheckLock, QgsReadWriteLocker::Read );
249+
layer->setAllowCommit( allErrors.empty() );
250+
errorLocker.unlock();
251+
} );
164252

253+
mLayerCheckStates[layer].topologyCheckFutureWatcher = futureWatcher;
165254
}

‎src/app/qgsgeometryvalidationservice.h

+16-3
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ email : matthias@opengis.ch
1818

1919
#include <QObject>
2020
#include <QMap>
21+
#include <QFuture>
22+
#include <QReadWriteLock>
2123

2224
#include "qgsfeature.h"
2325

@@ -27,7 +29,7 @@ class QgsVectorLayer;
2729
class QgsGeometryCheck;
2830
class QgsSingleGeometryCheck;
2931
class QgsSingleGeometryCheckError;
30-
32+
class QgsGeometryCheckError;
3133

3234

3335
/**
@@ -65,6 +67,9 @@ class QgsGeometryValidationService : public QObject
6567
signals:
6668
void geometryCheckStarted( QgsVectorLayer *layer, QgsFeatureId fid );
6769
void geometryCheckCompleted( QgsVectorLayer *layer, QgsFeatureId fid, const QList<std::shared_ptr<QgsSingleGeometryCheckError>> &errors );
70+
void topologyChecksUpdated( QgsVectorLayer *layer, const QList<std::shared_ptr<QgsGeometryCheckError> > &errors );
71+
72+
void warning( const QString &message );
6873

6974
private slots:
7075
void onLayersAdded( const QList<QgsMapLayer *> &layers );
@@ -84,8 +89,16 @@ class QgsGeometryValidationService : public QObject
8489

8590
QgsProject *mProject = nullptr;
8691

87-
QHash<QgsVectorLayer *, QList< QgsSingleGeometryCheck * > > mSingleFeatureChecks;
88-
QHash<QgsVectorLayer *, bool > mTopologyChecksOk;
92+
struct VectorCheckState
93+
{
94+
QList< QgsSingleGeometryCheck * > singleFeatureChecks;
95+
QList< QgsGeometryCheck * > topologyChecks;
96+
QFutureWatcher<void> *topologyCheckFutureWatcher = nullptr;
97+
QList<QgsGeometryCheckError *> topologyCheckErrors;
98+
};
99+
100+
QReadWriteLock mTopologyCheckLock;
101+
QHash<QgsVectorLayer *, VectorCheckState> mLayerCheckStates;
89102
};
90103

91104
#endif // QGSGEOMETRYVALIDATIONSERVICE_H

0 commit comments

Comments
 (0)