Add an explicit argument to set QgsGeometryValidator method

nyalldawson · nyalldawson · commit 64a86e3d6cf9 · 2017-06-07T11:36:14.000+10:00
Previously this was always read from settings when using
QgsGeometryValidator. It's now an explicit argument
when constructing QgsGeometryValidator or calling the
static validation methods, allowing choice of internal/GEOS
validation methods.

Helps remove more QgsSettings use from core.
diff --git a/python/core/geometry/qgsgeometry.sip b/python/core/geometry/qgsgeometry.sip
@@ -1077,9 +1077,16 @@ Ring 0 is outer ring and can't be deleted.
 %End
     };
 
-    void validateGeometry( QList<QgsGeometry::Error> &errors /Out/ );
+    enum ValidationMethod
+    {
+      ValidatorQgisInternal,
+      ValidatorGeos,
+    };
+
+    void validateGeometry( QList<QgsGeometry::Error> &errors /Out/, ValidationMethod method = ValidatorQgisInternal );
 %Docstring
- Validate geometry and produce a list of geometry errors
+ Validate geometry and produce a list of geometry errors.
+ The ``method`` argument dictates which validator to utilize.
 .. versionadded:: 1.5
 .. note::
 
diff --git a/python/core/qgsgeometryvalidator.sip b/python/core/qgsgeometryvalidator.sip
@@ -15,17 +15,18 @@ class QgsGeometryValidator : QThread
 #include "qgsgeometryvalidator.h"
 %End
   public:
-    QgsGeometryValidator( const QgsGeometry *g, QList<QgsGeometry::Error> *errors = 0 );
+
+    QgsGeometryValidator( const QgsGeometry *g, QList<QgsGeometry::Error> *errors = 0, QgsGeometry::ValidationMethod method = QgsGeometry::ValidatorQgisInternal );
 %Docstring
-Constructor
+ Constructor for QgsGeometryValidator.
 %End
     ~QgsGeometryValidator();
 
     virtual void run();
 
     void stop();
 
-    static void validateGeometry( const QgsGeometry *g, QList<QgsGeometry::Error> &errors /Out/ );
+    static void validateGeometry( const QgsGeometry *g, QList<QgsGeometry::Error> &errors /Out/, QgsGeometry::ValidationMethod method = QgsGeometry::ValidatorQgisInternal );
 %Docstring
 Validate geometry and produce a list of geometry errors
 %End
@@ -36,7 +37,7 @@ Validate geometry and produce a list of geometry errors
   public slots:
     void addError( const QgsGeometry::Error & );
 
-}; // class QgsGeometryValidator
+};
 
 /************************************************************************
  * This file has been generated automatically from                      *
diff --git a/src/app/nodetool/qgsnodetool.cpp b/src/app/nodetool/qgsnodetool.cpp
@@ -1814,7 +1814,12 @@ void QgsNodeTool::GeometryValidation::start( QgsGeometry &geom, QgsNodeTool *t,
 {
   tool = t;
   layer = l;
-  validator = new QgsGeometryValidator( &geom );
+  QgsGeometry::ValidationMethod method = QgsGeometry::ValidatorQgisInternal;
+  QgsSettings settings;
+  if ( settings.value( QStringLiteral( "qgis/digitizing/validate_geometries" ), 1 ).toInt() == 2 )
+    method = QgsGeometry::ValidatorGeos;
+
+  validator = new QgsGeometryValidator( &geom, nullptr, method );
   connect( validator, &QgsGeometryValidator::errorFound, tool, &QgsNodeTool::validationErrorFound );
   connect( validator, &QThread::finished, tool, &QgsNodeTool::validationFinished );
   validator->start();
diff --git a/src/app/nodetool/qgsselectedfeature.cpp b/src/app/nodetool/qgsselectedfeature.cpp
@@ -185,7 +185,10 @@ void QgsSelectedFeature::validateGeometry( QgsGeometry *g )
     delete vm;
   }
 
-  mValidator = new QgsGeometryValidator( g );
+  QgsGeometry::ValidationMethod method = QgsGeometry::ValidatorQgisInternal;
+  if ( settings.value( QStringLiteral( "qgis/digitizing/validate_geometries" ), 1 ).toInt() == 2 )
+    method = QgsGeometry::ValidatorGeos;
+  mValidator = new QgsGeometryValidator( g, nullptr, method );
   connect( mValidator, &QgsGeometryValidator::errorFound, this, &QgsSelectedFeature::addError );
   connect( mValidator, &QThread::finished, this, &QgsSelectedFeature::validationFinished );
   mValidator->start();
diff --git a/src/core/geometry/qgsgeometry.cpp b/src/core/geometry/qgsgeometry.cpp
@@ -1964,9 +1964,9 @@ QgsGeometry QgsGeometry::makeValid()
 }
 
 
-void QgsGeometry::validateGeometry( QList<QgsGeometry::Error> &errors )
+void QgsGeometry::validateGeometry( QList<QgsGeometry::Error> &errors, ValidationMethod method )
 {
-  QgsGeometryValidator::validateGeometry( this, errors );
+  QgsGeometryValidator::validateGeometry( this, errors, method );
 }
 
 bool QgsGeometry::isGeosValid() const
diff --git a/src/core/geometry/qgsgeometry.h b/src/core/geometry/qgsgeometry.h
@@ -946,11 +946,22 @@ class CORE_EXPORT QgsGeometry
         bool hasWhere() { return hasLocation; }
     };
 
-    /** Validate geometry and produce a list of geometry errors
+    /**
+     * Available methods for validating geometries.
+     * \since QGIS 3.0
+     */
+    enum ValidationMethod
+    {
+      ValidatorQgisInternal, //!< Use internal QgsGeometryValidator method
+      ValidatorGeos, //!< Use GEOS validation methods
+    };
+
+    /** Validate geometry and produce a list of geometry errors.
+     * The \a method argument dictates which validator to utilize.
      * \since QGIS 1.5
      * \note Available in Python bindings since QGIS 1.6
      **/
-    void validateGeometry( QList<QgsGeometry::Error> &errors SIP_OUT );
+    void validateGeometry( QList<QgsGeometry::Error> &errors SIP_OUT, ValidationMethod method = ValidatorQgisInternal );
 
     /** Compute the unary union on a list of \a geometries. May be faster than an iterative union on a set of geometries.
      * The returned geometry will be fully noded, i.e. a node will be created at every common intersection of the
diff --git a/src/core/qgsgeometryvalidator.cpp b/src/core/qgsgeometryvalidator.cpp
@@ -17,13 +17,13 @@ email                : jef at norbit dot de
 #include "qgsgeometryvalidator.h"
 #include "qgsgeometry.h"
 #include "qgslogger.h"
-#include "qgssettings.h"
 
-QgsGeometryValidator::QgsGeometryValidator( const QgsGeometry *g, QList<QgsGeometry::Error> *errors )
+QgsGeometryValidator::QgsGeometryValidator( const QgsGeometry *g, QList<QgsGeometry::Error> *errors, QgsGeometry::ValidationMethod method )
   : QThread()
   , mErrors( errors )
   , mStop( false )
   , mErrorCount( 0 )
+  , mMethod( method )
 {
   Q_ASSERT( g );
   if ( g )
@@ -215,134 +215,140 @@ void QgsGeometryValidator::validatePolygon( int idx, const QgsPolygon &polygon )
 void QgsGeometryValidator::run()
 {
   mErrorCount = 0;
-  QgsSettings settings;
-  if ( settings.value( QStringLiteral( "qgis/digitizing/validate_geometries" ), 1 ).toInt() == 2 )
+  switch ( mMethod )
   {
-    char *r = nullptr;
-    GEOSGeometry *g0 = mG.exportToGeos();
-    GEOSContextHandle_t handle = QgsGeometry::getGEOSHandler();
-    if ( !g0 )
+    case QgsGeometry::ValidatorGeos:
     {
-      emit errorFound( QgsGeometry::Error( QObject::tr( "GEOS error:could not produce geometry for GEOS (check log window)" ) ) );
-    }
-    else
-    {
-      GEOSGeometry *g1 = nullptr;
-      char res = GEOSisValidDetail_r( handle, g0, GEOSVALID_ALLOW_SELFTOUCHING_RING_FORMING_HOLE, &r, &g1 );
-      GEOSGeom_destroy_r( handle, g0 );
-      if ( res != 1 )
+      char *r = nullptr;
+      GEOSGeometry *g0 = mG.exportToGeos();
+      GEOSContextHandle_t handle = QgsGeometry::getGEOSHandler();
+      if ( !g0 )
+      {
+        emit errorFound( QgsGeometry::Error( QObject::tr( "GEOS error:could not produce geometry for GEOS (check log window)" ) ) );
+      }
+      else
       {
-        if ( g1 )
+        GEOSGeometry *g1 = nullptr;
+        char res = GEOSisValidDetail_r( handle, g0, GEOSVALID_ALLOW_SELFTOUCHING_RING_FORMING_HOLE, &r, &g1 );
+        GEOSGeom_destroy_r( handle, g0 );
+        if ( res != 1 )
         {
-          const GEOSCoordSequence *cs = GEOSGeom_getCoordSeq_r( handle, g1 );
-
-          unsigned int n;
-          if ( GEOSCoordSeq_getSize_r( handle, cs, &n ) && n == 1 )
+          if ( g1 )
+          {
+            const GEOSCoordSequence *cs = GEOSGeom_getCoordSeq_r( handle, g1 );
+
+            unsigned int n;
+            if ( GEOSCoordSeq_getSize_r( handle, cs, &n ) && n == 1 )
+            {
+              double x, y;
+              GEOSCoordSeq_getX_r( handle, cs, 0, &x );
+              GEOSCoordSeq_getY_r( handle, cs, 0, &y );
+              emit errorFound( QgsGeometry::Error( QObject::tr( "GEOS error:%1" ).arg( r ), QgsPointXY( x, y ) ) );
+              mErrorCount++;
+            }
+
+            GEOSGeom_destroy_r( handle, g1 );
+          }
+          else
           {
-            double x, y;
-            GEOSCoordSeq_getX_r( handle, cs, 0, &x );
-            GEOSCoordSeq_getY_r( handle, cs, 0, &y );
-            emit errorFound( QgsGeometry::Error( QObject::tr( "GEOS error:%1" ).arg( r ), QgsPointXY( x, y ) ) );
+            emit errorFound( QgsGeometry::Error( QObject::tr( "GEOS error:%1" ).arg( r ) ) );
             mErrorCount++;
           }
 
-          GEOSGeom_destroy_r( handle, g1 );
-        }
-        else
-        {
-          emit errorFound( QgsGeometry::Error( QObject::tr( "GEOS error:%1" ).arg( r ) ) );
-          mErrorCount++;
+          GEOSFree_r( handle, r );
         }
-
-        GEOSFree_r( handle, r );
       }
-    }
-
-    return;
-  }
 
-  QgsDebugMsg( "validation thread started." );
-
-  QgsWkbTypes::Type flatType = QgsWkbTypes::flatType( mG.wkbType() );
-  //if ( flatType == QgsWkbTypes::Point || flatType == QgsWkbTypes::MultiPoint )
-  //    break;
-  if ( flatType == QgsWkbTypes::LineString )
-  {
-    validatePolyline( 0, mG.asPolyline() );
-  }
-  else if ( flatType == QgsWkbTypes::MultiLineString )
-  {
-    QgsMultiPolyline mp = mG.asMultiPolyline();
-    for ( int i = 0; !mStop && i < mp.size(); i++ )
-      validatePolyline( i, mp[i] );
-  }
-  else if ( flatType == QgsWkbTypes::Polygon )
-  {
-    validatePolygon( 0, mG.asPolygon() );
-  }
-  else if ( flatType == QgsWkbTypes::MultiPolygon )
-  {
-    QgsMultiPolygon mp = mG.asMultiPolygon();
-    for ( int i = 0; !mStop && i < mp.size(); i++ )
-    {
-      validatePolygon( i, mp[i] );
+      break;
     }
 
-    for ( int i = 0; !mStop && i < mp.size(); i++ )
+    case QgsGeometry::ValidatorQgisInternal:
     {
-      if ( mp[i].isEmpty() )
+      QgsDebugMsg( "validation thread started." );
+
+      QgsWkbTypes::Type flatType = QgsWkbTypes::flatType( mG.wkbType() );
+      //if ( flatType == QgsWkbTypes::Point || flatType == QgsWkbTypes::MultiPoint )
+      //    break;
+      if ( flatType == QgsWkbTypes::LineString )
       {
-        emit errorFound( QgsGeometry::Error( QObject::tr( "polygon %1 has no rings" ).arg( i ) ) );
-        mErrorCount++;
-        continue;
+        validatePolyline( 0, mG.asPolyline() );
       }
-
-      for ( int j = i + 1;  !mStop && j < mp.size(); j++ )
+      else if ( flatType == QgsWkbTypes::MultiLineString )
       {
-        if ( mp[j].isEmpty() )
-          continue;
-
-        if ( ringInRing( mp[i][0], mp[j][0] ) )
-        {
-          emit errorFound( QgsGeometry::Error( QObject::tr( "polygon %1 inside polygon %2" ).arg( i ).arg( j ) ) );
-          mErrorCount++;
-        }
-        else if ( ringInRing( mp[j][0], mp[i][0] ) )
+        QgsMultiPolyline mp = mG.asMultiPolyline();
+        for ( int i = 0; !mStop && i < mp.size(); i++ )
+          validatePolyline( i, mp[i] );
+      }
+      else if ( flatType == QgsWkbTypes::Polygon )
+      {
+        validatePolygon( 0, mG.asPolygon() );
+      }
+      else if ( flatType == QgsWkbTypes::MultiPolygon )
+      {
+        QgsMultiPolygon mp = mG.asMultiPolygon();
+        for ( int i = 0; !mStop && i < mp.size(); i++ )
         {
-          emit errorFound( QgsGeometry::Error( QObject::tr( "polygon %1 inside polygon %2" ).arg( j ).arg( i ) ) );
-          mErrorCount++;
+          validatePolygon( i, mp[i] );
         }
-        else
+
+        for ( int i = 0; !mStop && i < mp.size(); i++ )
         {
-          checkRingIntersections( i, 0, mp[i][0], j, 0, mp[j][0] );
+          if ( mp[i].isEmpty() )
+          {
+            emit errorFound( QgsGeometry::Error( QObject::tr( "polygon %1 has no rings" ).arg( i ) ) );
+            mErrorCount++;
+            continue;
+          }
+
+          for ( int j = i + 1;  !mStop && j < mp.size(); j++ )
+          {
+            if ( mp[j].isEmpty() )
+              continue;
+
+            if ( ringInRing( mp[i][0], mp[j][0] ) )
+            {
+              emit errorFound( QgsGeometry::Error( QObject::tr( "polygon %1 inside polygon %2" ).arg( i ).arg( j ) ) );
+              mErrorCount++;
+            }
+            else if ( ringInRing( mp[j][0], mp[i][0] ) )
+            {
+              emit errorFound( QgsGeometry::Error( QObject::tr( "polygon %1 inside polygon %2" ).arg( j ).arg( i ) ) );
+              mErrorCount++;
+            }
+            else
+            {
+              checkRingIntersections( i, 0, mp[i][0], j, 0, mp[j][0] );
+            }
+          }
         }
       }
-    }
-  }
 
-  else if ( flatType == QgsWkbTypes::Unknown )
-  {
-    QgsDebugMsg( QObject::tr( "Unknown geometry type" ) );
-    emit errorFound( QgsGeometry::Error( QObject::tr( "Unknown geometry type %1" ).arg( mG.wkbType() ) ) );
-    mErrorCount++;
-  }
+      else if ( flatType == QgsWkbTypes::Unknown )
+      {
+        QgsDebugMsg( QObject::tr( "Unknown geometry type" ) );
+        emit errorFound( QgsGeometry::Error( QObject::tr( "Unknown geometry type %1" ).arg( mG.wkbType() ) ) );
+        mErrorCount++;
+      }
 
-  QgsDebugMsg( "validation finished." );
+      QgsDebugMsg( "validation finished." );
 
-  if ( mStop )
-  {
-    emit errorFound( QgsGeometry::Error( QObject::tr( "Geometry validation was aborted." ) ) );
-  }
-  else if ( mErrorCount > 0 )
-  {
-    emit errorFound( QgsGeometry::Error( QObject::tr( "Geometry has %1 errors." ).arg( mErrorCount ) ) );
-  }
+      if ( mStop )
+      {
+        emit errorFound( QgsGeometry::Error( QObject::tr( "Geometry validation was aborted." ) ) );
+      }
+      else if ( mErrorCount > 0 )
+      {
+        emit errorFound( QgsGeometry::Error( QObject::tr( "Geometry has %1 errors." ).arg( mErrorCount ) ) );
+      }
 #if 0
-  else
-  {
-    emit errorFound( QgsGeometry::Error( QObject::tr( "Geometry is valid." ) ) );
-  }
+      else
+      {
+        emit errorFound( QgsGeometry::Error( QObject::tr( "Geometry is valid." ) ) );
+      }
 #endif
+      break;
+    }
+  }
 }
 
 void QgsGeometryValidator::addError( const QgsGeometry::Error &e )
@@ -351,9 +357,9 @@ void QgsGeometryValidator::addError( const QgsGeometry::Error &e )
     *mErrors << e;
 }
 
-void QgsGeometryValidator::validateGeometry( const QgsGeometry *g, QList<QgsGeometry::Error> &errors )
+void QgsGeometryValidator::validateGeometry( const QgsGeometry *g, QList<QgsGeometry::Error> &errors, QgsGeometry::ValidationMethod method )
 {
-  QgsGeometryValidator *gv = new QgsGeometryValidator( g, &errors );
+  QgsGeometryValidator *gv = new QgsGeometryValidator( g, &errors, method );
   connect( gv, &QgsGeometryValidator::errorFound, gv, &QgsGeometryValidator::addError );
   gv->run();
   gv->wait();
diff --git a/src/core/qgsgeometryvalidator.h b/src/core/qgsgeometryvalidator.h
diff --git a/src/gui/qgsmaptoolcapture.cpp b/src/gui/qgsmaptoolcapture.cpp

Original file line number	Diff line number	Diff line change
`@@ -1964,9 +1964,9 @@ QgsGeometry QgsGeometry::makeValid()`
`1964`	`1964`	`}`
`1965`	`1965`
`1966`	`1966`
`1967`		`-void QgsGeometry::validateGeometry( QList<QgsGeometry::Error> &errors )`
	`1967`	`+void QgsGeometry::validateGeometry( QList<QgsGeometry::Error> &errors, ValidationMethod method )`
`1968`	`1968`	`{`
`1969`		`- QgsGeometryValidator::validateGeometry( this, errors );`
	`1969`	`+ QgsGeometryValidator::validateGeometry( this, errors, method );`
`1970`	`1970`	`}`
`1971`	`1971`
`1972`	`1972`	`bool QgsGeometry::isGeosValid() const`
-Original file line number
+Diff line change
     Q_OBJECT
   public:
 -    //! Constructor
 -    QgsGeometryValidator( const QgsGeometry *g, QList<QgsGeometry::Error> *errors = nullptr );
++
 +    /**
 +     * Constructor for QgsGeometryValidator.
 +     */
 +    QgsGeometryValidator( const QgsGeometry *g, QList<QgsGeometry::Error> *errors = nullptr, QgsGeometry::ValidationMethod method = QgsGeometry::ValidatorQgisInternal );
     ~QgsGeometryValidator();
     void run() override;
     void stop();
     //! Validate geometry and produce a list of geometry errors
 -    static void validateGeometry( const QgsGeometry *g, QList<QgsGeometry::Error> &errors SIP_OUT );
 +    static void validateGeometry( const QgsGeometry *g, QList<QgsGeometry::Error> &errors SIP_OUT, QgsGeometry::ValidationMethod method = QgsGeometry::ValidatorQgisInternal );
   signals:
     void errorFound( const QgsGeometry::Error & );
     QList<QgsGeometry::Error> *mErrors;
     bool mStop;
     int mErrorCount;
 -}; // class QgsGeometryValidator
 +    QgsGeometry::ValidationMethod mMethod = QgsGeometry::ValidatorQgisInternal;
 +};
 #endif