From 54019e11115ae9a5d0f8892088836d0d66546606 Mon Sep 17 00:00:00 2001
From: Sandro Mani <manisandro@gmail.com>
Date: Wed, 27 Sep 2017 18:18:23 +0200
Subject: [PATCH] [Geometry checker] Make contained check work with all
 geometry types

---
 .../qgsgeometrycontainedcheck.cpp             | 23 +++++++++++++++----
 .../qgsgeometrycontainedcheck.h               |  2 +-
 2 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/src/analysis/vector/geometry_checker/qgsgeometrycontainedcheck.cpp b/src/analysis/vector/geometry_checker/qgsgeometrycontainedcheck.cpp
index b6fcdd3524f..a67c7da29f7 100644
--- a/src/analysis/vector/geometry_checker/qgsgeometrycontainedcheck.cpp
+++ b/src/analysis/vector/geometry_checker/qgsgeometrycontainedcheck.cpp
@@ -26,6 +26,11 @@ void QgsGeometryContainedCheck::collectErrors( QList<QgsGeometryCheckError *> &e
   {
     QgsRectangle bboxA = layerFeatureA.geometry()->boundingBox();
     QSharedPointer<QgsGeometryEngine> geomEngineA = QgsGeometryCheckerUtils::createGeomEngine( layerFeatureA.geometry(), mContext->tolerance );
+    if ( !geomEngineA->isValid() )
+    {
+      messages.append( tr( "Contained check failed for (%1): the geometry is invalid" ).arg( layerFeatureA.id() ) );
+      continue;
+    }
     QgsGeometryCheckerUtils::LayerFeatures layerFeaturesB( mContext->featurePools, featureIds.keys(), bboxA, mCompatibleGeometryTypes );
     for ( const QgsGeometryCheckerUtils::LayerFeature &layerFeatureB : layerFeaturesB )
     {
@@ -33,14 +38,21 @@ void QgsGeometryContainedCheck::collectErrors( QList<QgsGeometryCheckError *> &e
       {
         continue;
       }
-      QString errMsg;
-      if ( geomEngineA->within( layerFeatureB.geometry(), &errMsg ) )
+      QSharedPointer<QgsGeometryEngine> geomEngineB = QgsGeometryCheckerUtils::createGeomEngine( layerFeatureB.geometry(), mContext->tolerance );
+      if ( !geomEngineB->isValid() )
       {
-        errors.append( new QgsGeometryContainedCheckError( this, layerFeatureA, layerFeatureA.geometry()->centroid(), layerFeatureB ) );
+        messages.append( tr( "Contained check failed for (%1): the geometry is invalid" ).arg( layerFeatureB.id() ) );
+        continue;
+      }
+      QString errMsg;
+      // If A contains B and B contains A, it would mean that the geometries are identical, which is covered by the duplicate check
+      if ( geomEngineA->contains( layerFeatureB.geometry(), &errMsg ) && !geomEngineB->contains( layerFeatureA.geometry(), &errMsg ) && errMsg.isEmpty() )
+      {
+        errors.append( new QgsGeometryContainedCheckError( this, layerFeatureB, layerFeatureB.geometry()->centroid(), layerFeatureA ) );
       }
       else if ( !errMsg.isEmpty() )
       {
-        messages.append( tr( "Feature %1 within feature %2: %3" ).arg( layerFeatureA.id() ).arg( layerFeatureB.id() ).arg( errMsg ) );
+        messages.append( tr( "Contained check failed for (%1, %2): %3" ).arg( layerFeatureB.id() ).arg( layerFeatureA.id() ).arg( errMsg ) );
       }
     }
   }
@@ -66,8 +78,9 @@ void QgsGeometryContainedCheck::fixError( QgsGeometryCheckError *error, int meth
   QgsGeometryCheckerUtils::LayerFeature layerFeatureB( featurePoolB, featureB, true );
 
   QSharedPointer<QgsGeometryEngine> geomEngineA = QgsGeometryCheckerUtils::createGeomEngine( layerFeatureA.geometry(), mContext->tolerance );
+  QSharedPointer<QgsGeometryEngine> geomEngineB = QgsGeometryCheckerUtils::createGeomEngine( layerFeatureB.geometry(), mContext->tolerance );
 
-  if ( !geomEngineA->within( layerFeatureB.geometry() ) )
+  if ( !( geomEngineA->contains( layerFeatureB.geometry() ) && !geomEngineB->contains( layerFeatureA.geometry() ) ) )
   {
     error->setObsolete();
     return;
diff --git a/src/analysis/vector/geometry_checker/qgsgeometrycontainedcheck.h b/src/analysis/vector/geometry_checker/qgsgeometrycontainedcheck.h
index 642e725fd65..7a54e8e27d6 100644
--- a/src/analysis/vector/geometry_checker/qgsgeometrycontainedcheck.h
+++ b/src/analysis/vector/geometry_checker/qgsgeometrycontainedcheck.h
@@ -50,7 +50,7 @@ class ANALYSIS_EXPORT QgsGeometryContainedCheck : public QgsGeometryCheck
 
   public:
     explicit QgsGeometryContainedCheck( QgsGeometryCheckerContext *context )
-      : QgsGeometryCheck( FeatureCheck, {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;