Skip to content
Permalink
Browse files

[Geometry checker] Use map crs for geometry and errorLocation in Chec…

…kErrors, overall code cleanup
  • Loading branch information
manisandro committed Jun 26, 2017
1 parent 1642eb1 commit 84184d453f51a3e515b31a4cb380a329e3707050
Showing with 179 additions and 237 deletions.
  1. +6 −8 src/plugins/geometry_checker/checks/qgsgeometryanglecheck.cpp
  2. +16 −43 src/plugins/geometry_checker/checks/qgsgeometryareacheck.cpp
  3. +1 −1 src/plugins/geometry_checker/checks/qgsgeometryareacheck.h
  4. +28 −0 src/plugins/geometry_checker/checks/qgsgeometrycheck.cpp
  5. +11 −3 src/plugins/geometry_checker/checks/qgsgeometrycheck.h
  6. +7 −15 src/plugins/geometry_checker/checks/qgsgeometrycontainedcheck.cpp
  7. +2 −4 src/plugins/geometry_checker/checks/qgsgeometrycontainedcheck.h
  8. +3 −5 src/plugins/geometry_checker/checks/qgsgeometrydegeneratepolygoncheck.cpp
  9. +7 −13 src/plugins/geometry_checker/checks/qgsgeometryduplicatecheck.cpp
  10. +2 −4 src/plugins/geometry_checker/checks/qgsgeometryduplicatecheck.h
  11. +2 −5 src/plugins/geometry_checker/checks/qgsgeometryduplicatenodescheck.cpp
  12. +9 −25 src/plugins/geometry_checker/checks/qgsgeometrygapcheck.cpp
  13. +3 −5 src/plugins/geometry_checker/checks/qgsgeometryholecheck.cpp
  14. +2 −5 src/plugins/geometry_checker/checks/qgsgeometrymultipartcheck.cpp
  15. +14 −23 src/plugins/geometry_checker/checks/qgsgeometryoverlapcheck.cpp
  16. +7 −9 src/plugins/geometry_checker/checks/qgsgeometrysegmentlengthcheck.cpp
  17. +2 −5 src/plugins/geometry_checker/checks/qgsgeometryselfcontactcheck.cpp
  18. +2 −5 src/plugins/geometry_checker/checks/qgsgeometryselfintersectioncheck.cpp
  19. +2 −4 src/plugins/geometry_checker/checks/qgsgeometryselfintersectioncheck.h
  20. +2 −2 src/plugins/geometry_checker/checks/qgsgeometrysliverpolygoncheck.cpp
  21. +1 −1 src/plugins/geometry_checker/checks/qgsgeometrysliverpolygoncheck.h
  22. +2 −5 src/plugins/geometry_checker/checks/qgsgeometrytypecheck.cpp
  23. +2 −4 src/plugins/geometry_checker/checks/qgsgeometrytypecheck.h
  24. +3 −3 src/plugins/geometry_checker/utils/qgsfeaturepool.cpp
  25. +5 −5 src/plugins/geometry_checker/utils/qgsfeaturepool.h
  26. +22 −21 src/plugins/geometry_checker/utils/qgsgeometrycheckerutils.cpp
  27. +16 −14 src/plugins/geometry_checker/utils/qgsgeometrycheckerutils.h
@@ -20,7 +20,7 @@
void QgsGeometryAngleCheck::collectErrors( QList<QgsGeometryCheckError *> &errors, QStringList &/*messages*/, QAtomicInt *progressCounter, const QMap<QString, QgsFeatureIds> &ids ) const
{
QMap<QString, QgsFeatureIds> featureIds = ids.isEmpty() ? allLayerFeatureIds() : ids;
QgsGeometryCheckerUtils::LayerFeatures layerFeatures( featureIds, mContext->featurePools, mCompatibleGeometryTypes, progressCounter );
QgsGeometryCheckerUtils::LayerFeatures layerFeatures( mContext->featurePools, featureIds, mCompatibleGeometryTypes, progressCounter );
for ( const QgsGeometryCheckerUtils::LayerFeature &layerFeature : layerFeatures )
{
const QgsAbstractGeometry *geom = layerFeature.geometry();
@@ -54,10 +54,7 @@ void QgsGeometryAngleCheck::collectErrors( QList<QgsGeometryCheckError *> &error
double angle = qAcos( v21 * v23 ) / M_PI * 180.0;
if ( angle < mMinAngle )
{
QgsAbstractGeometry *part = QgsGeometryCheckerUtils::getGeomPart( geom, iPart )->clone();
part->transform( layerFeature.mapToLayerTransform(), QgsCoordinateTransform::ReverseTransform );
QgsPointXY pos = layerFeature.mapToLayerTransform().transform( p2, QgsCoordinateTransform::ReverseTransform );
errors.append( new QgsGeometryCheckError( this, layerFeature.layer().id(), layerFeature.feature().id(), part, pos, QgsVertexId( iPart, iRing, iVert ), angle ) );
errors.append( new QgsGeometryCheckError( this, layerFeature, p2, QgsVertexId( iPart, iRing, iVert ), angle ) );
}
}
}
@@ -74,8 +71,8 @@ void QgsGeometryAngleCheck::fixError( QgsGeometryCheckError *error, int method,
error->setObsolete();
return;
}
QgsGeometry g = feature.geometry();
QgsAbstractGeometry *geometry = g.geometry();
QgsGeometry featureGeometry = feature.geometry();
QgsAbstractGeometry *geometry = featureGeometry.geometry();
QgsVertexId vidx = error->vidx();

// Check if point still exists
@@ -131,13 +128,14 @@ void QgsGeometryAngleCheck::fixError( QgsGeometryCheckError *error, int method,
else
{
changes[error->layerId()][error->featureId()].append( Change( ChangeNode, ChangeRemoved, vidx ) );
// Avoid duplicate nodes as result of deleting spike vertex
if ( QgsGeometryUtils::sqrDistance2D( p1, p3 ) < mContext->tolerance &&
QgsGeometryCheckerUtils::canDeleteVertex( geometry, vidx.part, vidx.ring ) &&
geometry->deleteVertex( error->vidx() ) ) // error->vidx points to p3 after removing p2
{
changes[error->layerId()][error->featureId()].append( Change( ChangeNode, ChangeRemoved, QgsVertexId( vidx.part, vidx.ring, ( vidx.vertex + 1 ) % n ) ) );
}
feature.setGeometry( g );
feature.setGeometry( featureGeometry );
featurePool->updateFeature( feature );
error->setFixed( method );
}
@@ -21,33 +21,18 @@
void QgsGeometryAreaCheck::collectErrors( QList<QgsGeometryCheckError *> &errors, QStringList &/*messages*/, QAtomicInt *progressCounter, const QMap<QString, QgsFeatureIds> &ids ) const
{
QMap<QString, QgsFeatureIds> featureIds = ids.isEmpty() ? allLayerFeatureIds() : ids;
QgsGeometryCheckerUtils::LayerFeatures layerFeatures( featureIds, mContext->featurePools, mCompatibleGeometryTypes, progressCounter );
QgsGeometryCheckerUtils::LayerFeatures layerFeatures( mContext->featurePools, featureIds, mCompatibleGeometryTypes, progressCounter );
for ( const QgsGeometryCheckerUtils::LayerFeature &layerFeature : layerFeatures )
{
double mapToLayerUnits = layerFeature.mapToLayerUnits();
double layerToMapUnits = layerFeature.layerToMapUnits();
const QgsAbstractGeometry *geom = layerFeature.geometry();
if ( dynamic_cast<const QgsGeometryCollection *>( geom ) )
{
const QgsGeometryCollection *multiGeom = static_cast<const QgsGeometryCollection *>( geom );
for ( int i = 0, n = multiGeom->numGeometries(); i < n; ++i )
{
double value;
if ( checkThreshold( mapToLayerUnits, multiGeom->geometryN( i ), value ) )
{
QgsAbstractGeometry *part = multiGeom->geometryN( i )->clone();
errors.append( new QgsGeometryCheckError( this, layerFeature.layer().id(), layerFeature.feature().id(), part, part->centroid(), QgsVertexId( i ), value / ( mapToLayerUnits * mapToLayerUnits ), QgsGeometryCheckError::ValueArea ) );
}
}
}
else
for ( int iPart = 0, nParts = geom->partCount(); iPart < nParts; ++iPart )
{
double value;
if ( checkThreshold( mapToLayerUnits, geom, value ) )
const QgsAbstractGeometry *part = QgsGeometryCheckerUtils::getGeomPart( geom, iPart );
if ( checkThreshold( layerToMapUnits, part, value ) )
{
QgsAbstractGeometry *g = geom->clone();
g->transform( layerFeature.mapToLayerTransform(), QgsCoordinateTransform::ReverseTransform );
QgsPoint pos = g->centroid();
errors.append( new QgsGeometryCheckError( this, layerFeature.layer().id(), layerFeature.feature().id(), g, pos, QgsVertexId( 0 ), value / ( mapToLayerUnits * mapToLayerUnits ), QgsGeometryCheckError::ValueArea ) );
errors.append( new QgsGeometryCheckError( this, layerFeature, part->centroid(), QgsVertexId( iPart ), value * layerToMapUnits * layerToMapUnits, QgsGeometryCheckError::ValueArea ) );
}
}
}
@@ -62,7 +47,7 @@ void QgsGeometryAreaCheck::fixError( QgsGeometryCheckError *error, int method, c
error->setObsolete();
return;
}
double mapToLayerUnits = featurePool->getMapToLayerUnits();
double layerToMapUnits = featurePool->getLayerToMapUnits();
QgsGeometry g = feature.geometry();
QgsAbstractGeometry *geom = g.geometry();
QgsVertexId vidx = error->vidx();
@@ -75,23 +60,11 @@ void QgsGeometryAreaCheck::fixError( QgsGeometryCheckError *error, int method, c
}

// Check if error still applies
if ( dynamic_cast<QgsGeometryCollection *>( geom ) )
{
double value;
if ( !checkThreshold( mapToLayerUnits, static_cast<QgsGeometryCollection *>( geom )->geometryN( vidx.part ), value ) )
{
error->setObsolete();
return;
}
}
else
double value;
if ( !checkThreshold( layerToMapUnits, QgsGeometryCheckerUtils::getGeomPart( geom, vidx.part ), value ) )
{
double value;
if ( !checkThreshold( mapToLayerUnits, geom, value ) )
{
error->setObsolete();
return;
}
error->setObsolete();
return;
}

// Fix with selected method
@@ -122,10 +95,10 @@ void QgsGeometryAreaCheck::fixError( QgsGeometryCheckError *error, int method, c
}
}

bool QgsGeometryAreaCheck::checkThreshold( double mapToLayerUnits, const QgsAbstractGeometry *geom, double &value ) const
bool QgsGeometryAreaCheck::checkThreshold( double layerToMapUnits, const QgsAbstractGeometry *geom, double &value ) const
{
value = geom->area();
double threshold = mThresholdMapUnits * mapToLayerUnits * mapToLayerUnits;
double threshold = mThresholdMapUnits / ( layerToMapUnits * layerToMapUnits );
return value < threshold;
}

@@ -137,11 +110,11 @@ bool QgsGeometryAreaCheck::mergeWithNeighbor( const QString &layerId, QgsFeature
QgsFeature mergeFeature;
int mergePartIdx = -1;
bool matchFound = false;
QgsGeometry g = feature.geometry();
QgsAbstractGeometry *geom = g.geometry();
QgsGeometry featureGeometry = feature.geometry();
QgsAbstractGeometry *geom = featureGeometry.geometry();

// Search for touching neighboring geometries
for ( QgsFeatureId testId : featurePool->getIntersects( g.boundingBox() ) )
for ( QgsFeatureId testId : featurePool->getIntersects( featureGeometry.boundingBox() ) )
{
QgsFeature testFeature;
if ( !featurePool->get( testId, testFeature ) )
@@ -37,7 +37,7 @@ class QgsGeometryAreaCheck : public QgsGeometryCheck
private:
enum ResolutionMethod { MergeLongestEdge, MergeLargestArea, MergeIdenticalAttribute, Delete, NoChange };

virtual bool checkThreshold( double mapToLayerUnits, const QgsAbstractGeometry *geom, double &value ) const;
virtual bool checkThreshold( double layerToMapUnits, const QgsAbstractGeometry *geom, double &value ) const;
bool mergeWithNeighbor( const QString &layerId, QgsFeature &feature, int partIdx, int method, int mergeAttributeIndex, Changes &changes, QString &errMsg ) const;

protected:
@@ -44,6 +44,34 @@ QgsGeometryCheckError::QgsGeometryCheckError( const QgsGeometryCheck *check, con
{
}

QgsGeometryCheckError::QgsGeometryCheckError( const QgsGeometryCheck *check,
const QgsGeometryCheckerUtils::LayerFeature &layerFeature,
const QgsPointXY &errorLocation, QgsVertexId vidx,
const QVariant &value, ValueType valueType )
: mCheck( check )
, mLayerId( layerFeature.layer().id() )
, mFeatureId( layerFeature.feature().id() )
, mErrorLocation( errorLocation )
, mVidx( vidx )
, mValue( value )
, mValueType( valueType )
, mStatus( StatusPending )
{
if ( vidx.part != -1 )
{
mGeometry = QgsGeometryCheckerUtils::getGeomPart( layerFeature.geometry(), vidx.part )->clone();
}
else
{
mGeometry = layerFeature.geometry()->clone();
}
if ( layerFeature.geometryCrs() != layerFeature.layerToMapTransform().destinationCrs().authid() )
{
mGeometry->transform( layerFeature.layerToMapTransform() );
mErrorLocation = layerFeature.layerToMapTransform().transform( mErrorLocation );
}
}

QgsRectangle QgsGeometryCheckError::affectedAreaBBox() const
{
return mGeometry->boundingBox();
@@ -96,9 +96,7 @@ class QgsGeometryCheckError
enum ValueType { ValueLength, ValueArea, ValueOther };

QgsGeometryCheckError( const QgsGeometryCheck *check,
const QString &layerId,
QgsFeatureId featureId,
QgsAbstractGeometry *geometry,
const QgsGeometryCheckerUtils::LayerFeature &layerFeature,
const QgsPointXY &errorLocation,
QgsVertexId vidx = QgsVertexId(),
const QVariant &value = QVariant(),
@@ -163,6 +161,16 @@ class QgsGeometryCheckError
virtual bool handleChanges( const QgsGeometryCheck::Changes &changes );

protected:
// Users of this constructor must ensure geometry and errorLocation are in map coordinates
QgsGeometryCheckError( const QgsGeometryCheck *check,
const QString &layerId,
QgsFeatureId featureId,
QgsAbstractGeometry *geometry,
const QgsPointXY &errorLocation,
QgsVertexId vidx = QgsVertexId(),
const QVariant &value = QVariant(),
ValueType valueType = ValueOther );

const QgsGeometryCheck *mCheck = nullptr;
QString mLayerId;
QgsFeatureId mFeatureId;
@@ -20,20 +20,18 @@
void QgsGeometryContainedCheck::collectErrors( QList<QgsGeometryCheckError *> &errors, QStringList &messages, QAtomicInt *progressCounter, const QMap<QString, QgsFeatureIds> &ids ) const
{
QMap<QString, QgsFeatureIds> featureIds = ids.isEmpty() ? allLayerFeatureIds() : ids;
QgsGeometryCheckerUtils::LayerFeatures layerFeaturesA( featureIds, mContext->featurePools, mCompatibleGeometryTypes, progressCounter, mContext->mapCrs );
QgsGeometryCheckerUtils::LayerFeatures layerFeaturesA( mContext->featurePools, featureIds, mCompatibleGeometryTypes, progressCounter );
for ( const QgsGeometryCheckerUtils::LayerFeature &layerFeatureA : layerFeaturesA )
{
QgsRectangle bboxA = layerFeatureA.geometry()->boundingBox();
QSharedPointer<QgsGeometryEngine> geomEngineA = QgsGeometryCheckerUtils::createGeomEngine( layerFeatureA.geometry(), mContext->tolerance );
QgsGeometryCheckerUtils::LayerFeatures layerFeaturesB( featureIds.keys(), bboxA, mContext->mapCrs, mContext->featurePools, mCompatibleGeometryTypes );
QgsGeometryCheckerUtils::LayerFeatures layerFeaturesB( mContext->featurePools, featureIds.keys(), bboxA, mCompatibleGeometryTypes );
for ( const QgsGeometryCheckerUtils::LayerFeature &layerFeatureB : layerFeaturesB )
{
QString errMsg;
if ( geomEngineA->within( *layerFeatureB.geometry(), &errMsg ) )
{
QgsAbstractGeometry *g = layerFeatureA.geometry()->clone();
QgsPoint pos = g->centroid();
errors.append( new QgsGeometryContainedCheckError( this, layerFeatureA.layer().id(), layerFeatureA.feature().id(), g, pos, qMakePair( layerFeatureB.layer().id(), layerFeatureB.feature().id() ) ) );
errors.append( new QgsGeometryContainedCheckError( this, layerFeatureA, layerFeatureA.geometry()->centroid(), qMakePair( layerFeatureB.layer().id(), layerFeatureB.feature().id() ) ) );
}
else if ( !errMsg.isEmpty() )
{
@@ -59,18 +57,12 @@ void QgsGeometryContainedCheck::fixError( QgsGeometryCheckError *error, int meth
}

// Check if error still applies
QgsAbstractGeometry *featureGeomA = featureA.geometry().geometry()->clone();
featureGeomA->transform( featurePoolA->getMapToLayerTransform(), QgsCoordinateTransform::ReverseTransform );
QSharedPointer<QgsGeometryEngine> geomEngineA = QgsGeometryCheckerUtils::createGeomEngine( featureGeomA, mContext->tolerance );
QgsGeometryCheckerUtils::LayerFeature layerFeatureA( featurePoolA, featureA, true );
QgsGeometryCheckerUtils::LayerFeature layerFeatureB( featurePoolB, featureB, true );

QgsAbstractGeometry *featureGeomB = featureB.geometry().geometry()->clone();
featureGeomB->transform( featurePoolB->getMapToLayerTransform(), QgsCoordinateTransform::ReverseTransform );
QSharedPointer<QgsGeometryEngine> geomEngineA = QgsGeometryCheckerUtils::createGeomEngine( layerFeatureA.geometry(), mContext->tolerance );

bool within = geomEngineA->within( *featureGeomB );
delete featureGeomA;
delete featureGeomB;

if ( !within )
if ( !geomEngineA->within( *layerFeatureB.geometry() ) )
{
error->setObsolete();
return;
@@ -22,13 +22,11 @@ class QgsGeometryContainedCheckError : public QgsGeometryCheckError
{
public:
QgsGeometryContainedCheckError( const QgsGeometryCheck *check,
const QString &layerId,
QgsFeatureId featureId,
QgsAbstractGeometry *geometry,
const QgsGeometryCheckerUtils::LayerFeature &layerFeature,
const QgsPointXY &errorLocation,
const QPair<QString, QgsFeatureId> &containingFeature
)
: QgsGeometryCheckError( check, layerId, featureId, geometry, errorLocation, QgsVertexId(), QString( "%1:%2" ).arg( containingFeature.first ).arg( containingFeature.second ), ValueOther )
: QgsGeometryCheckError( check, layerFeature, errorLocation, QgsVertexId(), QString( "%1:%2" ).arg( containingFeature.first ).arg( containingFeature.second ), ValueOther )
, mContainingFeature( containingFeature )
{ }
const QPair<QString, QgsFeatureId> &containingFeature() const { return mContainingFeature; }
@@ -19,7 +19,7 @@
void QgsGeometryDegeneratePolygonCheck::collectErrors( QList<QgsGeometryCheckError *> &errors, QStringList &/*messages*/, QAtomicInt *progressCounter, const QMap<QString, QgsFeatureIds> &ids ) const
{
QMap<QString, QgsFeatureIds> featureIds = ids.isEmpty() ? allLayerFeatureIds() : ids;
QgsGeometryCheckerUtils::LayerFeatures layerFeatures( featureIds, mContext->featurePools, mCompatibleGeometryTypes, progressCounter );
QgsGeometryCheckerUtils::LayerFeatures layerFeatures( mContext->featurePools, featureIds, mCompatibleGeometryTypes, progressCounter );
for ( const QgsGeometryCheckerUtils::LayerFeature &layerFeature : layerFeatures )
{
const QgsAbstractGeometry *geom = layerFeature.geometry();
@@ -29,10 +29,8 @@ void QgsGeometryDegeneratePolygonCheck::collectErrors( QList<QgsGeometryCheckErr
{
if ( QgsGeometryCheckerUtils::polyLineSize( geom, iPart, iRing ) < 3 )
{
QgsAbstractGeometry *g = geom->clone();
g->transform( layerFeature.mapToLayerTransform(), QgsCoordinateTransform::ReverseTransform );
QgsPointXY pos = layerFeature.mapToLayerTransform().transform( geom->vertexAt( QgsVertexId( iPart, iRing, 0 ) ), QgsCoordinateTransform::ReverseTransform );
errors.append( new QgsGeometryCheckError( this, layerFeature.layer().id(), layerFeature.feature().id(), g, pos, QgsVertexId( iPart, iRing ) ) );
QgsVertexId vidx( iPart, iRing );
errors.append( new QgsGeometryCheckError( this, layerFeature, geom->vertexAt( vidx ), vidx ) );
}
}
}

0 comments on commit 84184d4

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