Skip to content
Permalink
Browse files

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.
  • Loading branch information
m-kuhn committed Feb 27, 2019
1 parent 50f8f90 commit 149fcc040b22c272e1963b5bf49cf757d323847c
@@ -162,6 +162,15 @@ Will be used to update existing errors whenever they are re-checked.
%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:

QgsGeometryCheckError( const QgsGeometryCheck *check,
@@ -180,6 +180,11 @@ bool QgsGeometryCheckError::handleChanges( const QgsGeometryCheck::Changes &chan
return true;
}

QMap<QString, QgsFeatureIds> QgsGeometryCheckError::involvedFeatures() const
{
return QMap<QString, QSet<QgsFeatureId> >();
}

void QgsGeometryCheckError::update( const QgsGeometryCheckError *other )
{
Q_ASSERT( mCheck == other->mCheck );
@@ -179,6 +179,15 @@ class ANALYSIS_EXPORT QgsGeometryCheckError
*/
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:

/**
@@ -287,3 +287,39 @@ QgsGeometryCheck::CheckType QgsGeometryGapCheck::factoryCheckType()
return QgsGeometryCheck::LayerCheck;
}
///@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;
}
@@ -55,36 +55,17 @@ class ANALYSIS_EXPORT QgsGeometryGapCheckError : public QgsGeometryCheckError
*/
const QMap<QString, QgsFeatureIds> &neighbors() const { return mNeighbors; }

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 isEqual( QgsGeometryCheckError *other ) const override;

bool closeMatch( QgsGeometryCheckError *other ) const override
{
QgsGeometryGapCheckError *err = dynamic_cast<QgsGeometryGapCheckError *>( other );
return err && err->layerId() == layerId() && err->neighbors() == neighbors();
}
bool closeMatch( QgsGeometryCheckError *other ) const 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;
}
void update( const QgsGeometryCheckError *other ) override;

bool handleChanges( const QgsGeometryCheck::Changes & /*changes*/ ) override
{
return true;
}
bool handleChanges( const QgsGeometryCheck::Changes & /*changes*/ ) override;

QgsRectangle affectedAreaBBox() const override
{
return mGapAreaBBox;
}
QgsRectangle affectedAreaBBox() const override;

QMap<QString, QgsFeatureIds > involvedFeatures() const override;

private:
QMap<QString, QgsFeatureIds> mNeighbors;
@@ -180,6 +180,10 @@ void QgsGeometryMissingVertexCheck::processPolygon( const QgsCurvePolygon *polyg
{
std::unique_ptr<QgsGeometryMissingVertexCheckError> error = qgis::make_unique<QgsGeometryMissingVertexCheckError>( this, layerFeature, QgsPointXY( 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() );
}
@@ -273,3 +277,13 @@ void QgsGeometryMissingVertexCheckError::setAffectedAreaBBox( const QgsRectangle
{
mAffectedAreaBBox = affectedAreaBBox;
}

QMap<QString, QgsFeatureIds> QgsGeometryMissingVertexCheckError::involvedFeatures() const
{
return mInvolvedFeatures;
}

void QgsGeometryMissingVertexCheckError::setInvolvedFeatures( const QMap<QString, QgsFeatureIds> &involvedFeatures )
{
mInvolvedFeatures = involvedFeatures;
}
@@ -36,7 +36,7 @@ class QgsCurvePolygon;
class ANALYSIS_EXPORT QgsGeometryMissingVertexCheckError : public QgsGeometryCheckError
{
public:

/**
* Create a new missing vertex check error.
*/
@@ -56,8 +56,19 @@ class ANALYSIS_EXPORT QgsGeometryMissingVertexCheckError : public QgsGeometryChe
*/
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:
QgsRectangle mAffectedAreaBBox;
QMap<QString, QgsFeatureIds> mInvolvedFeatures;
};

/**
@@ -60,6 +60,7 @@ void QgsGeometryOverlapCheck::collectErrors( const QMap<QString, QgsFeaturePool
{
continue;
}

QString 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
{
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;
}
@@ -69,38 +69,16 @@ class ANALYSIS_EXPORT QgsGeometryOverlapCheckError : public QgsGeometryCheckErro
*/
const OverlappedFeature &overlappedFeature() const { return mOverlappedFeature; }

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
{
QgsGeometryOverlapCheckError *err = dynamic_cast<QgsGeometryOverlapCheckError *>( other );
return err && other->layerId() == layerId() && other->featureId() == featureId() && err->overlappedFeature() == overlappedFeature();
}
bool isEqual( QgsGeometryCheckError *other ) const 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;
}
bool closeMatch( QgsGeometryCheckError *other ) const override;

bool handleChanges( const QgsGeometryCheck::Changes &changes ) override;

QString description() const override;

QMap<QString, QgsFeatureIds > involvedFeatures() const override;

private:
OverlappedFeature mOverlappedFeature;
};
@@ -244,17 +244,6 @@ void QgsGeometryValidationDock::onCurrentErrorChanged( const QModelIndex &curren

bool hasFeature = !FID_IS_NULL( current.data( QgsGeometryValidationModel::ErrorFeatureIdRole ) );
mZoomToFeatureButton->setEnabled( hasFeature );

switch ( mLastZoomToAction )
{
case ZoomToProblem:
zoomToProblem();
break;

case ZoomToFeature:
zoomToFeature();
break;
}
}

void QgsGeometryValidationDock::onCurrentLayerChanged( QgsMapLayer *layer )
@@ -387,11 +387,12 @@ void QgsGeometryValidationService::triggerTopologyChecks( QgsVectorLayer *layer
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 )
{
featurePool = new QgsVectorLayerFeaturePool( layer );
mFeaturePools.insert( layer->id(), featurePool );
mFeaturePools.insert( layerId, featurePool );
}

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

layerIds.insert( layer->id(), checkFeatureIds );
layerIds.insert( layerId, checkFeatureIds );
QgsGeometryCheck::LayerFeatureIds layerFeatureIds( layerIds );

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

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
// except for using its address to report the error.
@@ -433,9 +434,30 @@ void QgsGeometryValidationService::triggerTopologyChecks( QgsVectorLayer *layer
QgsReadWriteLocker errorLocker( mTopologyCheckLock, QgsReadWriteLocker::Write );

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 );

0 comments on commit 149fcc0

Please sign in to comment.
You can’t perform that action at this time.