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.
This commit is contained in:
Matthias Kuhn 2019-02-27 16:53:30 +01:00
parent 50f8f90baa
commit 149fcc040b
No known key found for this signature in database
GPG Key ID: 7A7F1A1C90C3E6A7
11 changed files with 164 additions and 71 deletions

View File

@ -162,6 +162,15 @@ Will be used to update existing errors whenever they are re-checked.
%End %End
virtual QMap<QString, QgsFeatureIds> involvedFeatures() const;
%Docstring
Return a list of involved features.
By default returns an empty map.
The map keys are layer ids, the map value is a set of feature ids.
.. versionadded:: 3.8
%End
protected: protected:
QgsGeometryCheckError( const QgsGeometryCheck *check, QgsGeometryCheckError( const QgsGeometryCheck *check,

View File

@ -180,6 +180,11 @@ bool QgsGeometryCheckError::handleChanges( const QgsGeometryCheck::Changes &chan
return true; return true;
} }
QMap<QString, QgsFeatureIds> QgsGeometryCheckError::involvedFeatures() const
{
return QMap<QString, QSet<QgsFeatureId> >();
}
void QgsGeometryCheckError::update( const QgsGeometryCheckError *other ) void QgsGeometryCheckError::update( const QgsGeometryCheckError *other )
{ {
Q_ASSERT( mCheck == other->mCheck ); Q_ASSERT( mCheck == other->mCheck );

View File

@ -179,6 +179,15 @@ class ANALYSIS_EXPORT QgsGeometryCheckError
*/ */
virtual bool handleChanges( const QgsGeometryCheck::Changes &changes ) SIP_SKIP; virtual bool handleChanges( const QgsGeometryCheck::Changes &changes ) SIP_SKIP;
/**
* Return a list of involved features.
* By default returns an empty map.
* The map keys are layer ids, the map value is a set of feature ids.
*
* \since QGIS 3.8
*/
virtual QMap<QString, QgsFeatureIds> involvedFeatures() const;
protected: protected:
/** /**

View File

@ -287,3 +287,39 @@ QgsGeometryCheck::CheckType QgsGeometryGapCheck::factoryCheckType()
return QgsGeometryCheck::LayerCheck; return QgsGeometryCheck::LayerCheck;
} }
///@endcond private ///@endcond private
bool QgsGeometryGapCheckError::isEqual( QgsGeometryCheckError *other ) const
{
QgsGeometryGapCheckError *err = dynamic_cast<QgsGeometryGapCheckError *>( other );
return err && QgsGeometryCheckerUtils::pointsFuzzyEqual( err->location(), location(), mCheck->context()->reducedTolerance ) && err->neighbors() == neighbors();
}
bool QgsGeometryGapCheckError::closeMatch( QgsGeometryCheckError *other ) const
{
QgsGeometryGapCheckError *err = dynamic_cast<QgsGeometryGapCheckError *>( other );
return err && err->layerId() == layerId() && err->neighbors() == neighbors();
}
void QgsGeometryGapCheckError::update( const QgsGeometryCheckError *other )
{
QgsGeometryCheckError::update( other );
// Static cast since this should only get called if isEqual == true
const QgsGeometryGapCheckError *err = static_cast<const QgsGeometryGapCheckError *>( other );
mNeighbors = err->mNeighbors;
mGapAreaBBox = err->mGapAreaBBox;
}
bool QgsGeometryGapCheckError::handleChanges( const QgsGeometryCheck::Changes & )
{
return true;
}
QgsRectangle QgsGeometryGapCheckError::affectedAreaBBox() const
{
return mGapAreaBBox;
}
QMap<QString, QgsFeatureIds> QgsGeometryGapCheckError::involvedFeatures() const
{
return mNeighbors;
}

View File

@ -55,36 +55,17 @@ class ANALYSIS_EXPORT QgsGeometryGapCheckError : public QgsGeometryCheckError
*/ */
const QMap<QString, QgsFeatureIds> &neighbors() const { return mNeighbors; } const QMap<QString, QgsFeatureIds> &neighbors() const { return mNeighbors; }
bool isEqual( QgsGeometryCheckError *other ) const override bool isEqual( QgsGeometryCheckError *other ) const override;
{
QgsGeometryGapCheckError *err = dynamic_cast<QgsGeometryGapCheckError *>( other );
return err && QgsGeometryCheckerUtils::pointsFuzzyEqual( err->location(), location(), mCheck->context()->reducedTolerance ) && err->neighbors() == neighbors();
}
bool closeMatch( QgsGeometryCheckError *other ) const override bool closeMatch( QgsGeometryCheckError *other ) const override;
{
QgsGeometryGapCheckError *err = dynamic_cast<QgsGeometryGapCheckError *>( other );
return err && err->layerId() == layerId() && err->neighbors() == neighbors();
}
void update( const QgsGeometryCheckError *other ) override void update( const QgsGeometryCheckError *other ) override;
{
QgsGeometryCheckError::update( other );
// Static cast since this should only get called if isEqual == true
const QgsGeometryGapCheckError *err = static_cast<const QgsGeometryGapCheckError *>( other );
mNeighbors = err->mNeighbors;
mGapAreaBBox = err->mGapAreaBBox;
}
bool handleChanges( const QgsGeometryCheck::Changes & /*changes*/ ) override bool handleChanges( const QgsGeometryCheck::Changes & /*changes*/ ) override;
{
return true;
}
QgsRectangle affectedAreaBBox() const override QgsRectangle affectedAreaBBox() const override;
{
return mGapAreaBBox; QMap<QString, QgsFeatureIds > involvedFeatures() const override;
}
private: private:
QMap<QString, QgsFeatureIds> mNeighbors; QMap<QString, QgsFeatureIds> mNeighbors;

View File

@ -180,6 +180,10 @@ void QgsGeometryMissingVertexCheck::processPolygon( const QgsCurvePolygon *polyg
{ {
std::unique_ptr<QgsGeometryMissingVertexCheckError> error = qgis::make_unique<QgsGeometryMissingVertexCheckError>( this, layerFeature, QgsPointXY( pt ) ); std::unique_ptr<QgsGeometryMissingVertexCheckError> error = qgis::make_unique<QgsGeometryMissingVertexCheckError>( this, layerFeature, QgsPointXY( pt ) );
error->setAffectedAreaBBox( contextBoundingBox( polygon, vertexId, pt ) ); error->setAffectedAreaBBox( contextBoundingBox( polygon, vertexId, pt ) );
QMap<QString, QgsFeatureIds> involvedFeatures;
involvedFeatures[layerFeature.layerId()].insert( layerFeature.feature().id() );
involvedFeatures[featurePool->layerId()].insert( fid );
error->setInvolvedFeatures( involvedFeatures );
errors.append( error.release() ); errors.append( error.release() );
} }
@ -273,3 +277,13 @@ void QgsGeometryMissingVertexCheckError::setAffectedAreaBBox( const QgsRectangle
{ {
mAffectedAreaBBox = affectedAreaBBox; mAffectedAreaBBox = affectedAreaBBox;
} }
QMap<QString, QgsFeatureIds> QgsGeometryMissingVertexCheckError::involvedFeatures() const
{
return mInvolvedFeatures;
}
void QgsGeometryMissingVertexCheckError::setInvolvedFeatures( const QMap<QString, QgsFeatureIds> &involvedFeatures )
{
mInvolvedFeatures = involvedFeatures;
}

View File

@ -36,7 +36,7 @@ class QgsCurvePolygon;
class ANALYSIS_EXPORT QgsGeometryMissingVertexCheckError : public QgsGeometryCheckError class ANALYSIS_EXPORT QgsGeometryMissingVertexCheckError : public QgsGeometryCheckError
{ {
public: public:
/** /**
* Create a new missing vertex check error. * Create a new missing vertex check error.
*/ */
@ -56,8 +56,19 @@ class ANALYSIS_EXPORT QgsGeometryMissingVertexCheckError : public QgsGeometryChe
*/ */
void setAffectedAreaBBox( const QgsRectangle &affectedAreaBBox ); void setAffectedAreaBBox( const QgsRectangle &affectedAreaBBox );
QMap<QString, QgsFeatureIds> involvedFeatures() const override;
/**
* The two involved features, that share a common boundary but not all common
* vertices on this boundary.
*
* \since QGIS 3.8
*/
void setInvolvedFeatures( const QMap<QString, QgsFeatureIds> &involvedFeatures );
private: private:
QgsRectangle mAffectedAreaBBox; QgsRectangle mAffectedAreaBBox;
QMap<QString, QgsFeatureIds> mInvolvedFeatures;
}; };
/** /**

View File

@ -60,6 +60,7 @@ void QgsGeometryOverlapCheck::collectErrors( const QMap<QString, QgsFeaturePool
{ {
continue; continue;
} }
QString errMsg; QString errMsg;
if ( geomEngineA->overlaps( layerFeatureB.geometry().constGet(), &errMsg ) ) if ( geomEngineA->overlaps( layerFeatureB.geometry().constGet(), &errMsg ) )
{ {
@ -261,7 +262,45 @@ QgsGeometryOverlapCheckError::QgsGeometryOverlapCheckError( const QgsGeometryChe
} }
bool QgsGeometryOverlapCheckError::isEqual( QgsGeometryCheckError *other ) const
{
QgsGeometryOverlapCheckError *err = dynamic_cast<QgsGeometryOverlapCheckError *>( other );
return err &&
other->layerId() == layerId() &&
other->featureId() == featureId() &&
err->overlappedFeature() == overlappedFeature() &&
QgsGeometryCheckerUtils::pointsFuzzyEqual( location(), other->location(), mCheck->context()->reducedTolerance ) &&
std::fabs( value().toDouble() - other->value().toDouble() ) < mCheck->context()->reducedTolerance;
}
bool QgsGeometryOverlapCheckError::closeMatch( QgsGeometryCheckError *other ) const
{
QgsGeometryOverlapCheckError *err = dynamic_cast<QgsGeometryOverlapCheckError *>( other );
return err && other->layerId() == layerId() && other->featureId() == featureId() && err->overlappedFeature() == overlappedFeature();
}
bool QgsGeometryOverlapCheckError::handleChanges( const QgsGeometryCheck::Changes &changes )
{
if ( !QgsGeometryCheckError::handleChanges( changes ) )
{
return false;
}
if ( changes.value( mOverlappedFeature.layerId() ).keys().contains( mOverlappedFeature.featureId() ) )
{
return false;
}
return true;
}
QString QgsGeometryOverlapCheckError::description() const QString QgsGeometryOverlapCheckError::description() const
{ {
return QCoreApplication::translate( "QgsGeometryTypeCheckError", "Overlap with %1 at feature %2" ).arg( mOverlappedFeature.layerName(), QString::number( mOverlappedFeature.featureId() ) ); return QCoreApplication::translate( "QgsGeometryTypeCheckError", "Overlap with %1 at feature %2" ).arg( mOverlappedFeature.layerName(), QString::number( mOverlappedFeature.featureId() ) );
} }
QMap<QString, QgsFeatureIds> QgsGeometryOverlapCheckError::involvedFeatures() const
{
QMap<QString, QgsFeatureIds> features;
features[layerId()].insert( featureId() );
features[mOverlappedFeature.layerId()].insert( mOverlappedFeature.featureId() );
return features;
}

View File

@ -69,38 +69,16 @@ class ANALYSIS_EXPORT QgsGeometryOverlapCheckError : public QgsGeometryCheckErro
*/ */
const OverlappedFeature &overlappedFeature() const { return mOverlappedFeature; } const OverlappedFeature &overlappedFeature() const { return mOverlappedFeature; }
bool isEqual( QgsGeometryCheckError *other ) const override bool isEqual( QgsGeometryCheckError *other ) const override;
{
QgsGeometryOverlapCheckError *err = dynamic_cast<QgsGeometryOverlapCheckError *>( other );
return err &&
other->layerId() == layerId() &&
other->featureId() == featureId() &&
err->overlappedFeature() == overlappedFeature() &&
QgsGeometryCheckerUtils::pointsFuzzyEqual( location(), other->location(), mCheck->context()->reducedTolerance ) &&
std::fabs( value().toDouble() - other->value().toDouble() ) < mCheck->context()->reducedTolerance;
}
bool closeMatch( QgsGeometryCheckError *other ) const override bool closeMatch( QgsGeometryCheckError *other ) const override;
{
QgsGeometryOverlapCheckError *err = dynamic_cast<QgsGeometryOverlapCheckError *>( other );
return err && other->layerId() == layerId() && other->featureId() == featureId() && err->overlappedFeature() == overlappedFeature();
}
bool handleChanges( const QgsGeometryCheck::Changes &changes ) override bool handleChanges( const QgsGeometryCheck::Changes &changes ) override;
{
if ( !QgsGeometryCheckError::handleChanges( changes ) )
{
return false;
}
if ( changes.value( mOverlappedFeature.layerId() ).keys().contains( mOverlappedFeature.featureId() ) )
{
return false;
}
return true;
}
QString description() const override; QString description() const override;
QMap<QString, QgsFeatureIds > involvedFeatures() const override;
private: private:
OverlappedFeature mOverlappedFeature; OverlappedFeature mOverlappedFeature;
}; };

View File

@ -244,17 +244,6 @@ void QgsGeometryValidationDock::onCurrentErrorChanged( const QModelIndex &curren
bool hasFeature = !FID_IS_NULL( current.data( QgsGeometryValidationModel::ErrorFeatureIdRole ) ); bool hasFeature = !FID_IS_NULL( current.data( QgsGeometryValidationModel::ErrorFeatureIdRole ) );
mZoomToFeatureButton->setEnabled( hasFeature ); mZoomToFeatureButton->setEnabled( hasFeature );
switch ( mLastZoomToAction )
{
case ZoomToProblem:
zoomToProblem();
break;
case ZoomToFeature:
zoomToFeature();
break;
}
} }
void QgsGeometryValidationDock::onCurrentLayerChanged( QgsMapLayer *layer ) void QgsGeometryValidationDock::onCurrentLayerChanged( QgsMapLayer *layer )

View File

@ -387,11 +387,12 @@ void QgsGeometryValidationService::triggerTopologyChecks( QgsVectorLayer *layer
affectedFeatureIds.unite( layer->editBuffer()->addedFeatures().keys().toSet() ); affectedFeatureIds.unite( layer->editBuffer()->addedFeatures().keys().toSet() );
} }
QgsFeaturePool *featurePool = mFeaturePools.value( layer->id() ); const QString layerId = layer->id();
QgsFeaturePool *featurePool = mFeaturePools.value( layerId );
if ( !featurePool ) if ( !featurePool )
{ {
featurePool = new QgsVectorLayerFeaturePool( layer ); featurePool = new QgsVectorLayerFeaturePool( layer );
mFeaturePools.insert( layer->id(), featurePool ); mFeaturePools.insert( layerId, featurePool );
} }
QList<std::shared_ptr<QgsGeometryCheckError>> &allErrors = mLayerChecks[layer].topologyCheckErrors; QList<std::shared_ptr<QgsGeometryCheckError>> &allErrors = mLayerChecks[layer].topologyCheckErrors;
@ -410,7 +411,7 @@ void QgsGeometryValidationService::triggerTopologyChecks( QgsVectorLayer *layer
QgsFeatureRequest areaRequest = QgsFeatureRequest().setFilterRect( area ); QgsFeatureRequest areaRequest = QgsFeatureRequest().setFilterRect( area );
QgsFeatureIds checkFeatureIds = featurePool->getFeatures( areaRequest ); QgsFeatureIds checkFeatureIds = featurePool->getFeatures( areaRequest );
layerIds.insert( layer->id(), checkFeatureIds ); layerIds.insert( layerId, checkFeatureIds );
QgsGeometryCheck::LayerFeatureIds layerFeatureIds( layerIds ); QgsGeometryCheck::LayerFeatureIds layerFeatureIds( layerIds );
const QList<QgsGeometryCheck *> checks = mLayerChecks[layer].topologyChecks; const QList<QgsGeometryCheck *> checks = mLayerChecks[layer].topologyChecks;
@ -421,7 +422,7 @@ void QgsGeometryValidationService::triggerTopologyChecks( QgsVectorLayer *layer
mLayerChecks[layer].topologyCheckFeedbacks = feedbacks.values(); mLayerChecks[layer].topologyCheckFeedbacks = feedbacks.values();
QFuture<void> future = QtConcurrent::map( checks, [&allErrors, layerFeatureIds, layer, feedbacks, this]( const QgsGeometryCheck * check ) QFuture<void> future = QtConcurrent::map( checks, [&allErrors, layerFeatureIds, layer, layerId, feedbacks, affectedFeatureIds, 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 // 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. // except for using its address to report the error.
@ -433,9 +434,30 @@ void QgsGeometryValidationService::triggerTopologyChecks( QgsVectorLayer *layer
QgsReadWriteLocker errorLocker( mTopologyCheckLock, QgsReadWriteLocker::Write ); QgsReadWriteLocker errorLocker( mTopologyCheckLock, QgsReadWriteLocker::Write );
QList<std::shared_ptr<QgsGeometryCheckError> > sharedErrors; QList<std::shared_ptr<QgsGeometryCheckError> > sharedErrors;
for ( QgsGeometryCheckError *error : errors ) for ( QgsGeometryCheckError *err : errors )
{ {
sharedErrors.append( std::shared_ptr<QgsGeometryCheckError>( error ) ); std::shared_ptr<QgsGeometryCheckError> error( err );
// Check if the error happened in one of the edited / checked features
// Errors which are happen to be in the same area "by chance" are ignored.
const auto involvedFeatures = error->involvedFeatures();
bool errorAffectsEditedFeature = true;
if ( !involvedFeatures.isEmpty() )
{
errorAffectsEditedFeature = false;
const auto involvedFids = involvedFeatures.value( layerId );
for ( const QgsFeatureId id : involvedFids )
{
if ( affectedFeatureIds.contains( id ) )
{
errorAffectsEditedFeature = true;
break;
}
}
}
if ( errorAffectsEditedFeature )
sharedErrors.append( error );
} }
allErrors.append( sharedErrors ); allErrors.append( sharedErrors );