Skip to content

Commit 149fcc0

Browse files
m-kuhnm-kuhn
committedFeb 27, 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.
·
ltr-3_40final-3_8_0
1 parent 50f8f90 commit 149fcc0

11 files changed

+165
-72
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
@@ -162,6 +162,15 @@ Will be used to update existing errors whenever they are re-checked.
162162
%End
163163

164164

165+
virtual QMap<QString, QgsFeatureIds> involvedFeatures() const;
166+
%Docstring
167+
Return a list of involved features.
168+
By default returns an empty map.
169+
The map keys are layer ids, the map value is a set of feature ids.
170+
171+
.. versionadded:: 3.8
172+
%End
173+
165174
protected:
166175

167176
QgsGeometryCheckError( const QgsGeometryCheck *check,

‎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
@@ -179,6 +179,15 @@ class ANALYSIS_EXPORT QgsGeometryCheckError
179179
*/
180180
virtual bool handleChanges( const QgsGeometryCheck::Changes &changes ) SIP_SKIP;
181181

182+
/**
183+
* Return a list of involved features.
184+
* By default returns an empty map.
185+
* The map keys are layer ids, the map value is a set of feature ids.
186+
*
187+
* \since QGIS 3.8
188+
*/
189+
virtual QMap<QString, QgsFeatureIds> involvedFeatures() const;
190+
182191
protected:
183192

184193
/**

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

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

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

Lines changed: 7 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -55,36 +55,17 @@ class ANALYSIS_EXPORT QgsGeometryGapCheckError : public QgsGeometryCheckError
5555
*/
5656
const QMap<QString, QgsFeatureIds> &neighbors() const { return mNeighbors; }
5757

58-
bool isEqual( QgsGeometryCheckError *other ) const override
59-
{
60-
QgsGeometryGapCheckError *err = dynamic_cast<QgsGeometryGapCheckError *>( other );
61-
return err && QgsGeometryCheckerUtils::pointsFuzzyEqual( err->location(), location(), mCheck->context()->reducedTolerance ) && err->neighbors() == neighbors();
62-
}
58+
bool isEqual( QgsGeometryCheckError *other ) const override;
6359

64-
bool closeMatch( QgsGeometryCheckError *other ) const override
65-
{
66-
QgsGeometryGapCheckError *err = dynamic_cast<QgsGeometryGapCheckError *>( other );
67-
return err && err->layerId() == layerId() && err->neighbors() == neighbors();
68-
}
60+
bool closeMatch( QgsGeometryCheckError *other ) const override;
6961

70-
void update( const QgsGeometryCheckError *other ) override
71-
{
72-
QgsGeometryCheckError::update( other );
73-
// Static cast since this should only get called if isEqual == true
74-
const QgsGeometryGapCheckError *err = static_cast<const QgsGeometryGapCheckError *>( other );
75-
mNeighbors = err->mNeighbors;
76-
mGapAreaBBox = err->mGapAreaBBox;
77-
}
62+
void update( const QgsGeometryCheckError *other ) override;
7863

79-
bool handleChanges( const QgsGeometryCheck::Changes & /*changes*/ ) override
80-
{
81-
return true;
82-
}
64+
bool handleChanges( const QgsGeometryCheck::Changes & /*changes*/ ) override;
8365

84-
QgsRectangle affectedAreaBBox() const override
85-
{
86-
return mGapAreaBBox;
87-
}
66+
QgsRectangle affectedAreaBBox() const override;
67+
68+
QMap<QString, QgsFeatureIds > involvedFeatures() const override;
8869

8970
private:
9071
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
}
@@ -273,3 +277,13 @@ void QgsGeometryMissingVertexCheckError::setAffectedAreaBBox( const QgsRectangle
273277
{
274278
mAffectedAreaBBox = affectedAreaBBox;
275279
}
280+
281+
QMap<QString, QgsFeatureIds> QgsGeometryMissingVertexCheckError::involvedFeatures() const
282+
{
283+
return mInvolvedFeatures;
284+
}
285+
286+
void QgsGeometryMissingVertexCheckError::setInvolvedFeatures( const QMap<QString, QgsFeatureIds> &involvedFeatures )
287+
{
288+
mInvolvedFeatures = involvedFeatures;
289+
}

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

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ class QgsCurvePolygon;
3636
class ANALYSIS_EXPORT QgsGeometryMissingVertexCheckError : public QgsGeometryCheckError
3737
{
3838
public:
39-
39+
4040
/**
4141
* Create a new missing vertex check error.
4242
*/
@@ -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
{
@@ -261,7 +262,45 @@ QgsGeometryOverlapCheckError::QgsGeometryOverlapCheckError( const QgsGeometryChe
261262

262263
}
263264

265+
bool QgsGeometryOverlapCheckError::isEqual( QgsGeometryCheckError *other ) const
266+
{
267+
QgsGeometryOverlapCheckError *err = dynamic_cast<QgsGeometryOverlapCheckError *>( other );
268+
return err &&
269+
other->layerId() == layerId() &&
270+
other->featureId() == featureId() &&
271+
err->overlappedFeature() == overlappedFeature() &&
272+
QgsGeometryCheckerUtils::pointsFuzzyEqual( location(), other->location(), mCheck->context()->reducedTolerance ) &&
273+
std::fabs( value().toDouble() - other->value().toDouble() ) < mCheck->context()->reducedTolerance;
274+
}
275+
276+
bool QgsGeometryOverlapCheckError::closeMatch( QgsGeometryCheckError *other ) const
277+
{
278+
QgsGeometryOverlapCheckError *err = dynamic_cast<QgsGeometryOverlapCheckError *>( other );
279+
return err && other->layerId() == layerId() && other->featureId() == featureId() && err->overlappedFeature() == overlappedFeature();
280+
}
281+
282+
bool QgsGeometryOverlapCheckError::handleChanges( const QgsGeometryCheck::Changes &changes )
283+
{
284+
if ( !QgsGeometryCheckError::handleChanges( changes ) )
285+
{
286+
return false;
287+
}
288+
if ( changes.value( mOverlappedFeature.layerId() ).keys().contains( mOverlappedFeature.featureId() ) )
289+
{
290+
return false;
291+
}
292+
return true;
293+
}
294+
264295
QString QgsGeometryOverlapCheckError::description() const
265296
{
266297
return QCoreApplication::translate( "QgsGeometryTypeCheckError", "Overlap with %1 at feature %2" ).arg( mOverlappedFeature.layerName(), QString::number( mOverlappedFeature.featureId() ) );
267298
}
299+
300+
QMap<QString, QgsFeatureIds> QgsGeometryOverlapCheckError::involvedFeatures() const
301+
{
302+
QMap<QString, QgsFeatureIds> features;
303+
features[layerId()].insert( featureId() );
304+
features[mOverlappedFeature.layerId()].insert( mOverlappedFeature.featureId() );
305+
return features;
306+
}

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

Lines changed: 6 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -69,38 +69,16 @@ class ANALYSIS_EXPORT QgsGeometryOverlapCheckError : public QgsGeometryCheckErro
6969
*/
7070
const OverlappedFeature &overlappedFeature() const { return mOverlappedFeature; }
7171

72-
bool isEqual( QgsGeometryCheckError *other ) const override
73-
{
74-
QgsGeometryOverlapCheckError *err = dynamic_cast<QgsGeometryOverlapCheckError *>( other );
75-
return err &&
76-
other->layerId() == layerId() &&
77-
other->featureId() == featureId() &&
78-
err->overlappedFeature() == overlappedFeature() &&
79-
QgsGeometryCheckerUtils::pointsFuzzyEqual( location(), other->location(), mCheck->context()->reducedTolerance ) &&
80-
std::fabs( value().toDouble() - other->value().toDouble() ) < mCheck->context()->reducedTolerance;
81-
}
82-
83-
bool closeMatch( QgsGeometryCheckError *other ) const override
84-
{
85-
QgsGeometryOverlapCheckError *err = dynamic_cast<QgsGeometryOverlapCheckError *>( other );
86-
return err && other->layerId() == layerId() && other->featureId() == featureId() && err->overlappedFeature() == overlappedFeature();
87-
}
72+
bool isEqual( QgsGeometryCheckError *other ) const override;
8873

89-
bool handleChanges( const QgsGeometryCheck::Changes &changes ) override
90-
{
91-
if ( !QgsGeometryCheckError::handleChanges( changes ) )
92-
{
93-
return false;
94-
}
95-
if ( changes.value( mOverlappedFeature.layerId() ).keys().contains( mOverlappedFeature.featureId() ) )
96-
{
97-
return false;
98-
}
99-
return true;
100-
}
74+
bool closeMatch( QgsGeometryCheckError *other ) const override;
75+
76+
bool handleChanges( const QgsGeometryCheck::Changes &changes ) override;
10177

10278
QString description() const override;
10379

80+
QMap<QString, QgsFeatureIds > involvedFeatures() const override;
81+
10482
private:
10583
OverlappedFeature mOverlappedFeature;
10684
};

‎src/app/qgsgeometryvalidationdock.cpp

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

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

260249
void QgsGeometryValidationDock::onCurrentLayerChanged( QgsMapLayer *layer )

‎src/app/qgsgeometryvalidationservice.cpp

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

390-
QgsFeaturePool *featurePool = mFeaturePools.value( layer->id() );
390+
const QString layerId = layer->id();
391+
QgsFeaturePool *featurePool = mFeaturePools.value( layerId );
391392
if ( !featurePool )
392393
{
393394
featurePool = new QgsVectorLayerFeaturePool( layer );
394-
mFeaturePools.insert( layer->id(), featurePool );
395+
mFeaturePools.insert( layerId, featurePool );
395396
}
396397

397398
QList<std::shared_ptr<QgsGeometryCheckError>> &allErrors = mLayerChecks[layer].topologyCheckErrors;
@@ -410,7 +411,7 @@ void QgsGeometryValidationService::triggerTopologyChecks( QgsVectorLayer *layer
410411
QgsFeatureRequest areaRequest = QgsFeatureRequest().setFilterRect( area );
411412
QgsFeatureIds checkFeatureIds = featurePool->getFeatures( areaRequest );
412413

413-
layerIds.insert( layer->id(), checkFeatureIds );
414+
layerIds.insert( layerId, checkFeatureIds );
414415
QgsGeometryCheck::LayerFeatureIds layerFeatureIds( layerIds );
415416

416417
const QList<QgsGeometryCheck *> checks = mLayerChecks[layer].topologyChecks;
@@ -421,7 +422,7 @@ void QgsGeometryValidationService::triggerTopologyChecks( QgsVectorLayer *layer
421422

422423
mLayerChecks[layer].topologyCheckFeedbacks = feedbacks.values();
423424

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

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

441463
allErrors.append( sharedErrors );

0 commit comments

Comments
 (0)
Please sign in to comment.