[Geometry checker] Use symDifference for all geometry types to detect…

… duplicates
qgis · Oct 23, 2017 · 183671f · 183671f
1 parent 816273e
commit 183671f
Show file tree

Hide file tree

Showing 2 changed files with 18 additions and 40 deletions.
diff --git a/src/analysis/vector/geometry_checker/qgsgeometryduplicatecheck.cpp b/src/analysis/vector/geometry_checker/qgsgeometryduplicatecheck.cpp
@@ -48,6 +48,11 @@ void QgsGeometryDuplicateCheck::collectErrors( QList<QgsGeometryCheckError *> &e
 
     QgsRectangle bboxA = layerFeatureA.geometry()->boundingBox();
     QSharedPointer<QgsGeometryEngine> geomEngineA = QgsGeometryCheckerUtils::createGeomEngine( layerFeatureA.geometry(), mContext->tolerance );
+    if ( !geomEngineA->isValid() )
+    {
+      messages.append( tr( "Duplicate check failed for %1: the geometry is invalid" ).arg( layerFeatureA.id() ) );
+      continue;
+    }
     QMap<QString, QList<QgsFeatureId>> duplicates;
 
     QgsWkbTypes::GeometryType geomType = layerFeatureA.feature().geometry().type();
@@ -59,29 +64,17 @@ void QgsGeometryDuplicateCheck::collectErrors( QList<QgsGeometryCheckError *> &e
       {
         continue;
       }
-      if ( geomType == QgsWkbTypes::PointGeometry )
+      QString errMsg;
+      QgsAbstractGeometry *diffGeom = geomEngineA->symDifference( layerFeatureB.geometry(), &errMsg );
+      if ( errMsg.isEmpty() && diffGeom && diffGeom->isEmpty() )
       {
-        const QgsPoint *p = dynamic_cast<const QgsPoint *>( layerFeatureA.geometry() );
+        duplicates[layerFeatureB.layer().id()].append( layerFeatureB.feature().id() );
-        const QgsPoint *q = dynamic_cast<const QgsPoint *>( layerFeatureB.geometry() );
-        if ( p && q && p->distanceSquared( *q ) < mContext->tolerance * mContext->tolerance )
-        {
-          duplicates[layerFeatureB.layer().id()].append( layerFeatureB.feature().id() );
-        }
       }
-      else if ( geomType == QgsWkbTypes::PolygonGeometry )
+      else if ( !errMsg.isEmpty() )
       {
-        QString errMsg;
+        messages.append( tr( "Duplicate check failed for %1, %2: %3" ).arg( layerFeatureA.id() ).arg( layerFeatureB.id() ).arg( errMsg ) );
-        QgsAbstractGeometry *diffGeom = geomEngineA->symDifference( layerFeatureB.geometry(), &errMsg );
-        if ( diffGeom && diffGeom->area() < mContext->tolerance )
-        {
-          duplicates[layerFeatureB.layer().id()].append( layerFeatureB.feature().id() );
-        }
-        else if ( !diffGeom )
-        {
-          messages.append( tr( "Duplicate check between features %1 and %2: %3" ).arg( layerFeatureA.id() ).arg( layerFeatureB.id() ).arg( errMsg ) );
-        }
-        delete diffGeom;
       }
+      delete diffGeom;
     }
     if ( !duplicates.isEmpty() )
     {
@@ -106,8 +99,6 @@ void QgsGeometryDuplicateCheck::fixError( QgsGeometryCheckError *error, int meth
   }
   else if ( method == RemoveDuplicates )
   {
-    QgsWkbTypes::GeometryType geomType = featureA.geometry().type();
-
     QgsGeometryCheckerUtils::LayerFeature layerFeatureA( featurePoolA, featureA, true );
     QSharedPointer<QgsGeometryEngine> geomEngineA = QgsGeometryCheckerUtils::createGeomEngine( layerFeatureA.geometry(), mContext->tolerance );
 
@@ -123,27 +114,14 @@ void QgsGeometryDuplicateCheck::fixError( QgsGeometryCheckError *error, int meth
           continue;
         }
         QgsGeometryCheckerUtils::LayerFeature layerFeatureB( featurePoolB, featureB, true );
-        if ( geomType == QgsWkbTypes::PointGeometry )
+        QgsAbstractGeometry *diffGeom = geomEngineA->symDifference( layerFeatureB.geometry() );
+        if ( diffGeom && diffGeom->isEmpty() )
         {
-          const QgsPoint *p = dynamic_cast<const QgsPoint *>( layerFeatureA.geometry() );
+          featurePoolB->deleteFeature( featureB );
-          const QgsPoint *q = dynamic_cast<const QgsPoint *>( layerFeatureB.geometry() );
+          changes[layerIdB][idB].append( Change( ChangeFeature, ChangeRemoved ) );
-          if ( p && q && p->distanceSquared( *q ) < mContext->tolerance * mContext->tolerance )
-          {
-            featurePoolB->deleteFeature( featureB );
-            changes[layerIdB][idB].append( Change( ChangeFeature, ChangeRemoved ) );
-          }
         }
-        else if ( geomType == QgsWkbTypes::PolygonGeometry )
-        {
-          QgsAbstractGeometry *diffGeom = geomEngineA->symDifference( layerFeatureB.geometry() );
-          if ( diffGeom && diffGeom->area() < mContext->tolerance )
-          {
-            featurePoolB->deleteFeature( featureB );
-            changes[layerIdB][idB].append( Change( ChangeFeature, ChangeRemoved ) );
-          }
 
-          delete diffGeom;
+        delete diffGeom;
-        }
       }
     }
     error->setFixed( method );

diff --git a/src/analysis/vector/geometry_checker/qgsgeometryduplicatecheck.h b/src/analysis/vector/geometry_checker/qgsgeometryduplicatecheck.h
@@ -51,7 +51,7 @@ class ANALYSIS_EXPORT QgsGeometryDuplicateCheck : public QgsGeometryCheck
 
   public:
     explicit QgsGeometryDuplicateCheck( QgsGeometryCheckerContext *context )
-      : QgsGeometryCheck( FeatureCheck, {QgsWkbTypes::PointGeometry, QgsWkbTypes::PolygonGeometry}, context ) {}
+      : QgsGeometryCheck( FeatureCheck, {QgsWkbTypes::PointGeometry, QgsWkbTypes::LineGeometry, QgsWkbTypes::PolygonGeometry}, context ) {}
     void collectErrors( QList<QgsGeometryCheckError *> &errors, QStringList &messages, QAtomicInt *progressCounter = nullptr, const QMap<QString, QgsFeatureIds> &ids = QMap<QString, QgsFeatureIds>() ) const override;
     void fixError( QgsGeometryCheckError *error, int method, const QMap<QString, int> &mergeAttributeIndices, Changes &changes ) const override;
     QStringList getResolutionMethods() const override;