Skip to content

Commit 29a6273

Browse files
m-kuhnm-kuhn
committedMar 18, 2019
Only report topology errors in affected features
The geometry validation only works on the current edit session (added / edited geometries). To detect topology errors it is required to also get more features within the context, therefore, the bounding box of the edited geometries is taken to populate the list of features to check. This commit filters the found problems so only the ones which actually affect one of the edited geometries will be reported.
·
final-3_4_15final-3_4_6
1 parent 0984fc8 commit 29a6273

11 files changed

+165
-71
lines changed
 

‎python/analysis/auto_generated/vector/geometry_checker/qgsgeometrycheckerror.sip.in

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,15 @@ Will be used to update existing errors whenever they are re-checked.
7676
%End
7777

7878

79+
virtual QMap<QString, QgsFeatureIds> involvedFeatures() const;
80+
%Docstring
81+
Return a list of involved features.
82+
By default returns an empty map.
83+
The map keys are layer ids, the map value is a set of feature ids.
84+
85+
.. versionadded:: 3.8
86+
%End
87+
7988
protected:
8089
QgsGeometryCheckError( const QgsGeometryCheck *check,
8190
const QString &layerId,

‎src/analysis/vector/geometry_checker/qgsgeometrycheckerror.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,11 @@ bool QgsGeometryCheckError::handleChanges( const QgsGeometryCheck::Changes &chan
180180
return true;
181181
}
182182

183+
QMap<QString, QgsFeatureIds> QgsGeometryCheckError::involvedFeatures() const
184+
{
185+
return QMap<QString, QSet<QgsFeatureId> >();
186+
}
187+
183188
void QgsGeometryCheckError::update( const QgsGeometryCheckError *other )
184189
{
185190
Q_ASSERT( mCheck == other->mCheck );

‎src/analysis/vector/geometry_checker/qgsgeometrycheckerror.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,15 @@ class ANALYSIS_EXPORT QgsGeometryCheckError
9393
*/
9494
virtual bool handleChanges( const QgsGeometryCheck::Changes &changes ) SIP_SKIP;
9595

96+
/**
97+
* Return a list of involved features.
98+
* By default returns an empty map.
99+
* The map keys are layer ids, the map value is a set of feature ids.
100+
*
101+
* \since QGIS 3.8
102+
*/
103+
virtual QMap<QString, QgsFeatureIds> involvedFeatures() const;
104+
96105
protected:
97106
// Users of this constructor must ensure geometry and errorLocation are in map coordinates
98107
QgsGeometryCheckError( const QgsGeometryCheck *check,

‎src/analysis/vector/geometry_checker/qgsgeometrygapcheck.cpp

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -287,3 +287,40 @@ QgsGeometryCheck::CheckType QgsGeometryGapCheck::factoryCheckType()
287287
{
288288
return QgsGeometryCheck::LayerCheck;
289289
}
290+
///@endcond private
291+
292+
bool QgsGeometryGapCheckError::isEqual( QgsGeometryCheckError *other ) const
293+
{
294+
QgsGeometryGapCheckError *err = dynamic_cast<QgsGeometryGapCheckError *>( other );
295+
return err && QgsGeometryCheckerUtils::pointsFuzzyEqual( err->location(), location(), mCheck->context()->reducedTolerance ) && err->neighbors() == neighbors();
296+
}
297+
298+
bool QgsGeometryGapCheckError::closeMatch( QgsGeometryCheckError *other ) const
299+
{
300+
QgsGeometryGapCheckError *err = dynamic_cast<QgsGeometryGapCheckError *>( other );
301+
return err && err->layerId() == layerId() && err->neighbors() == neighbors();
302+
}
303+
304+
void QgsGeometryGapCheckError::update( const QgsGeometryCheckError *other )
305+
{
306+
QgsGeometryCheckError::update( other );
307+
// Static cast since this should only get called if isEqual == true
308+
const QgsGeometryGapCheckError *err = static_cast<const QgsGeometryGapCheckError *>( other );
309+
mNeighbors = err->mNeighbors;
310+
mGapAreaBBox = err->mGapAreaBBox;
311+
}
312+
313+
bool QgsGeometryGapCheckError::handleChanges( const QgsGeometryCheck::Changes & )
314+
{
315+
return true;
316+
}
317+
318+
QgsRectangle QgsGeometryGapCheckError::affectedAreaBBox() const
319+
{
320+
return mGapAreaBBox;
321+
}
322+
323+
QMap<QString, QgsFeatureIds> QgsGeometryGapCheckError::involvedFeatures() const
324+
{
325+
return mNeighbors;
326+
}

‎src/analysis/vector/geometry_checker/qgsgeometrygapcheck.h

Lines changed: 7 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -38,36 +38,17 @@ class ANALYSIS_EXPORT QgsGeometryGapCheckError : public QgsGeometryCheckError
3838
}
3939
const QMap<QString, QgsFeatureIds> &neighbors() const { return mNeighbors; }
4040

41-
bool isEqual( QgsGeometryCheckError *other ) const override
42-
{
43-
QgsGeometryGapCheckError *err = dynamic_cast<QgsGeometryGapCheckError *>( other );
44-
return err && QgsGeometryCheckerUtils::pointsFuzzyEqual( err->location(), location(), mCheck->context()->reducedTolerance ) && err->neighbors() == neighbors();
45-
}
41+
bool isEqual( QgsGeometryCheckError *other ) const override;
4642

47-
bool closeMatch( QgsGeometryCheckError *other ) const override
48-
{
49-
QgsGeometryGapCheckError *err = dynamic_cast<QgsGeometryGapCheckError *>( other );
50-
return err && err->layerId() == layerId() && err->neighbors() == neighbors();
51-
}
43+
bool closeMatch( QgsGeometryCheckError *other ) const override;
5244

53-
void update( const QgsGeometryCheckError *other ) override
54-
{
55-
QgsGeometryCheckError::update( other );
56-
// Static cast since this should only get called if isEqual == true
57-
const QgsGeometryGapCheckError *err = static_cast<const QgsGeometryGapCheckError *>( other );
58-
mNeighbors = err->mNeighbors;
59-
mGapAreaBBox = err->mGapAreaBBox;
60-
}
45+
void update( const QgsGeometryCheckError *other ) override;
6146

62-
bool handleChanges( const QgsGeometryCheck::Changes & /*changes*/ ) override
63-
{
64-
return true;
65-
}
47+
bool handleChanges( const QgsGeometryCheck::Changes & /*changes*/ ) override;
6648

67-
QgsRectangle affectedAreaBBox() const override
68-
{
69-
return mGapAreaBBox;
70-
}
49+
QgsRectangle affectedAreaBBox() const override;
50+
51+
QMap<QString, QgsFeatureIds > involvedFeatures() const override;
7152

7253
private:
7354
QMap<QString, QgsFeatureIds> mNeighbors;

‎src/analysis/vector/geometry_checker/qgsgeometrymissingvertexcheck.cpp

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,10 @@ void QgsGeometryMissingVertexCheck::processPolygon( const QgsCurvePolygon *polyg
180180
{
181181
std::unique_ptr<QgsGeometryMissingVertexCheckError> error = qgis::make_unique<QgsGeometryMissingVertexCheckError>( this, layerFeature, QgsPointXY( pt ) );
182182
error->setAffectedAreaBBox( contextBoundingBox( polygon, vertexId, pt ) );
183+
QMap<QString, QgsFeatureIds> involvedFeatures;
184+
involvedFeatures[layerFeature.layerId()].insert( layerFeature.feature().id() );
185+
involvedFeatures[featurePool->layerId()].insert( fid );
186+
error->setInvolvedFeatures( involvedFeatures );
183187

184188
errors.append( error.release() );
185189
}
@@ -272,3 +276,13 @@ void QgsGeometryMissingVertexCheckError::setAffectedAreaBBox( const QgsRectangle
272276
{
273277
mAffectedAreaBBox = affectedAreaBBox;
274278
}
279+
280+
QMap<QString, QgsFeatureIds> QgsGeometryMissingVertexCheckError::involvedFeatures() const
281+
{
282+
return mInvolvedFeatures;
283+
}
284+
285+
void QgsGeometryMissingVertexCheckError::setInvolvedFeatures( const QMap<QString, QgsFeatureIds> &involvedFeatures )
286+
{
287+
mInvolvedFeatures = involvedFeatures;
288+
}

‎src/analysis/vector/geometry_checker/qgsgeometrymissingvertexcheck.h

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,19 @@ class ANALYSIS_EXPORT QgsGeometryMissingVertexCheckError : public QgsGeometryChe
5656
*/
5757
void setAffectedAreaBBox( const QgsRectangle &affectedAreaBBox );
5858

59+
QMap<QString, QgsFeatureIds> involvedFeatures() const override;
60+
61+
/**
62+
* The two involved features, that share a common boundary but not all common
63+
* vertices on this boundary.
64+
*
65+
* \since QGIS 3.8
66+
*/
67+
void setInvolvedFeatures( const QMap<QString, QgsFeatureIds> &involvedFeatures );
68+
5969
private:
6070
QgsRectangle mAffectedAreaBBox;
71+
QMap<QString, QgsFeatureIds> mInvolvedFeatures;
6172
};
6273

6374
/**

‎src/analysis/vector/geometry_checker/qgsgeometryoverlapcheck.cpp

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ void QgsGeometryOverlapCheck::collectErrors( const QMap<QString, QgsFeaturePool
6060
{
6161
continue;
6262
}
63+
6364
QString errMsg;
6465
if ( geomEngineA->overlaps( layerFeatureB.geometry().constGet(), &errMsg ) )
6566
{
@@ -254,6 +255,36 @@ QgsGeometryOverlapCheckError::QgsGeometryOverlapCheckError( const QgsGeometryChe
254255

255256
}
256257

258+
bool QgsGeometryOverlapCheckError::isEqual( QgsGeometryCheckError *other ) const
259+
{
260+
QgsGeometryOverlapCheckError *err = dynamic_cast<QgsGeometryOverlapCheckError *>( other );
261+
return err &&
262+
other->layerId() == layerId() &&
263+
other->featureId() == featureId() &&
264+
err->overlappedFeature() == overlappedFeature() &&
265+
QgsGeometryCheckerUtils::pointsFuzzyEqual( location(), other->location(), mCheck->context()->reducedTolerance ) &&
266+
std::fabs( value().toDouble() - other->value().toDouble() ) < mCheck->context()->reducedTolerance;
267+
}
268+
269+
bool QgsGeometryOverlapCheckError::closeMatch( QgsGeometryCheckError *other ) const
270+
{
271+
QgsGeometryOverlapCheckError *err = dynamic_cast<QgsGeometryOverlapCheckError *>( other );
272+
return err && other->layerId() == layerId() && other->featureId() == featureId() && err->overlappedFeature() == overlappedFeature();
273+
}
274+
275+
bool QgsGeometryOverlapCheckError::handleChanges( const QgsGeometryCheck::Changes &changes )
276+
{
277+
if ( !QgsGeometryCheckError::handleChanges( changes ) )
278+
{
279+
return false;
280+
}
281+
if ( changes.value( mOverlappedFeature.layerId() ).keys().contains( mOverlappedFeature.featureId() ) )
282+
{
283+
return false;
284+
}
285+
return true;
286+
}
287+
257288
QString QgsGeometryOverlapCheckError::description() const
258289
{
259290
return QCoreApplication::translate( "QgsGeometryTypeCheckError", "Overlap with %1 at feature %2" ).arg( mOverlappedFeature.layerName(), QString::number( mOverlappedFeature.featureId() ) );
@@ -263,3 +294,11 @@ QgsGeometryCheck::CheckType QgsGeometryOverlapCheck::factoryCheckType()
263294
{
264295
return QgsGeometryCheck::LayerCheck;
265296
}
297+
298+
QMap<QString, QgsFeatureIds> QgsGeometryOverlapCheckError::involvedFeatures() const
299+
{
300+
QMap<QString, QgsFeatureIds> features;
301+
features[layerId()].insert( featureId() );
302+
features[mOverlappedFeature.layerId()].insert( mOverlappedFeature.featureId() );
303+
return features;
304+
}

‎src/analysis/vector/geometry_checker/qgsgeometryoverlapcheck.h

Lines changed: 6 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -54,38 +54,16 @@ class ANALYSIS_EXPORT QgsGeometryOverlapCheckError : public QgsGeometryCheckErro
5454
const QgsGeometryCheckerUtils::LayerFeature &overlappedFeature );
5555
const OverlappedFeature &overlappedFeature() const { return mOverlappedFeature; }
5656

57-
bool isEqual( QgsGeometryCheckError *other ) const override
58-
{
59-
QgsGeometryOverlapCheckError *err = dynamic_cast<QgsGeometryOverlapCheckError *>( other );
60-
return err &&
61-
other->layerId() == layerId() &&
62-
other->featureId() == featureId() &&
63-
err->overlappedFeature() == overlappedFeature() &&
64-
QgsGeometryCheckerUtils::pointsFuzzyEqual( location(), other->location(), mCheck->context()->reducedTolerance ) &&
65-
std::fabs( value().toDouble() - other->value().toDouble() ) < mCheck->context()->reducedTolerance;
66-
}
67-
68-
bool closeMatch( QgsGeometryCheckError *other ) const override
69-
{
70-
QgsGeometryOverlapCheckError *err = dynamic_cast<QgsGeometryOverlapCheckError *>( other );
71-
return err && other->layerId() == layerId() && other->featureId() == featureId() && err->overlappedFeature() == overlappedFeature();
72-
}
57+
bool isEqual( QgsGeometryCheckError *other ) const override;
7358

74-
bool handleChanges( const QgsGeometryCheck::Changes &changes ) override
75-
{
76-
if ( !QgsGeometryCheckError::handleChanges( changes ) )
77-
{
78-
return false;
79-
}
80-
if ( changes.value( mOverlappedFeature.layerId() ).keys().contains( mOverlappedFeature.featureId() ) )
81-
{
82-
return false;
83-
}
84-
return true;
85-
}
59+
bool closeMatch( QgsGeometryCheckError *other ) const override;
60+
61+
bool handleChanges( const QgsGeometryCheck::Changes &changes ) override;
8662

8763
QString description() const override;
8864

65+
QMap<QString, QgsFeatureIds > involvedFeatures() const override;
66+
8967
private:
9068
OverlappedFeature mOverlappedFeature;
9169
};

‎src/app/qgsgeometryvalidationdock.cpp

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -243,17 +243,6 @@ void QgsGeometryValidationDock::onCurrentErrorChanged( const QModelIndex &curren
243243

244244
bool hasFeature = !FID_IS_NULL( current.data( QgsGeometryValidationModel::ErrorFeatureIdRole ) );
245245
mZoomToFeatureButton->setEnabled( hasFeature );
246-
247-
switch ( mLastZoomToAction )
248-
{
249-
case ZoomToProblem:
250-
zoomToProblem();
251-
break;
252-
253-
case ZoomToFeature:
254-
zoomToFeature();
255-
break;
256-
}
257246
}
258247

259248
void QgsGeometryValidationDock::onCurrentLayerChanged( QgsMapLayer *layer )

‎src/app/qgsgeometryvalidationservice.cpp

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -393,11 +393,12 @@ void QgsGeometryValidationService::triggerTopologyChecks( QgsVectorLayer *layer
393393
affectedFeatureIds.unite( layer->editBuffer()->addedFeatures().keys().toSet() );
394394
}
395395

396-
QgsFeaturePool *featurePool = mFeaturePools.value( layer->id() );
396+
const QString layerId = layer->id();
397+
QgsFeaturePool *featurePool = mFeaturePools.value( layerId );
397398
if ( !featurePool )
398399
{
399400
featurePool = new QgsVectorLayerFeaturePool( layer );
400-
mFeaturePools.insert( layer->id(), featurePool );
401+
mFeaturePools.insert( layerId, featurePool );
401402
}
402403

403404
QList<std::shared_ptr<QgsGeometryCheckError>> &allErrors = mLayerChecks[layer].topologyCheckErrors;
@@ -416,7 +417,7 @@ void QgsGeometryValidationService::triggerTopologyChecks( QgsVectorLayer *layer
416417
QgsFeatureRequest areaRequest = QgsFeatureRequest().setFilterRect( area );
417418
QgsFeatureIds checkFeatureIds = featurePool->getFeatures( areaRequest );
418419

419-
layerIds.insert( layer->id(), checkFeatureIds );
420+
layerIds.insert( layerId, checkFeatureIds );
420421
QgsGeometryCheck::LayerFeatureIds layerFeatureIds( layerIds );
421422

422423
const QList<QgsGeometryCheck *> checks = mLayerChecks[layer].topologyChecks;
@@ -427,7 +428,7 @@ void QgsGeometryValidationService::triggerTopologyChecks( QgsVectorLayer *layer
427428

428429
mLayerChecks[layer].topologyCheckFeedbacks = feedbacks.values();
429430

430-
QFuture<void> future = QtConcurrent::map( checks, [&allErrors, layerFeatureIds, layer, feedbacks, this]( const QgsGeometryCheck * check )
431+
QFuture<void> future = QtConcurrent::map( checks, [&allErrors, layerFeatureIds, layer, layerId, feedbacks, affectedFeatureIds, this]( const QgsGeometryCheck * check )
431432
{
432433
// Watch out with the layer pointer in here. We are running in a thread, so we do not want to actually use it
433434
// except for using its address to report the error.
@@ -439,9 +440,30 @@ void QgsGeometryValidationService::triggerTopologyChecks( QgsVectorLayer *layer
439440
QgsReadWriteLocker errorLocker( mTopologyCheckLock, QgsReadWriteLocker::Write );
440441

441442
QList<std::shared_ptr<QgsGeometryCheckError> > sharedErrors;
442-
for ( QgsGeometryCheckError *error : errors )
443+
for ( QgsGeometryCheckError *err : errors )
443444
{
444-
sharedErrors.append( std::shared_ptr<QgsGeometryCheckError>( error ) );
445+
std::shared_ptr<QgsGeometryCheckError> error( err );
446+
// Check if the error happened in one of the edited / checked features
447+
// Errors which are happen to be in the same area "by chance" are ignored.
448+
const auto involvedFeatures = error->involvedFeatures();
449+
450+
bool errorAffectsEditedFeature = true;
451+
if ( !involvedFeatures.isEmpty() )
452+
{
453+
errorAffectsEditedFeature = false;
454+
const auto involvedFids = involvedFeatures.value( layerId );
455+
for ( const QgsFeatureId id : involvedFids )
456+
{
457+
if ( affectedFeatureIds.contains( id ) )
458+
{
459+
errorAffectsEditedFeature = true;
460+
break;
461+
}
462+
}
463+
}
464+
465+
if ( errorAffectsEditedFeature )
466+
sharedErrors.append( error );
445467
}
446468

447469
allErrors.append( sharedErrors );

0 commit comments

Comments
 (0)
Please sign in to comment.