[Geometry checker] Introduce QgsGeometryCheckerUtils::LayerFeatures

qgis · Oct 23, 2017 · b52b2c5 · b52b2c5
1 parent 821eb40
commit b52b2c5
Show file tree

Hide file tree

Showing 27 changed files with 473 additions and 578 deletions.
diff --git a/src/plugins/geometry_checker/checks/qgsgeometryanglecheck.cpp b/src/plugins/geometry_checker/checks/qgsgeometryanglecheck.cpp
@@ -20,55 +20,42 @@
 void QgsGeometryAngleCheck::collectErrors( QList<QgsGeometryCheckError *> &errors, QStringList &/*messages*/, QAtomicInt *progressCounter, const QMap<QString, QgsFeatureIds> &ids ) const
 {
   QMap<QString, QgsFeatureIds> featureIds = ids.isEmpty() ? allLayerFeatureIds() : ids;
-  for ( const QString &layerId : featureIds.keys() )
+  QgsGeometryCheckerUtils::LayerFeatures layerFeatures( featureIds, mContext->featurePools, mCompatibleGeometryTypes, progressCounter );
+  for ( const QgsGeometryCheckerUtils::LayerFeature &layerFeature : layerFeatures )
   {
-    QgsFeaturePool *featurePool = mContext->featurePools[ layerId ];
-    if ( !getCompatibility( featurePool->getLayer()->geometryType() ) )
+    const QgsAbstractGeometry *geom = layerFeature.geometry();
+    for ( int iPart = 0, nParts = geom->partCount(); iPart < nParts; ++iPart )
     {
-      continue;
-    }
-    for ( QgsFeatureId featureid : featureIds[layerId] )
-    {
-      if ( progressCounter ) progressCounter->fetchAndAddRelaxed( 1 );
-      QgsFeature feature;
-      if ( !featurePool->get( featureid, feature ) )
+      for ( int iRing = 0, nRings = geom->ringCount( iPart ); iRing < nRings; ++iRing )
       {
-        continue;
-      }
-      QgsGeometry g = feature.geometry();
-      const QgsAbstractGeometry *geom = g.geometry();
-      for ( int iPart = 0, nParts = geom->partCount(); iPart < nParts; ++iPart )
-      {
-        for ( int iRing = 0, nRings = geom->ringCount( iPart ); iRing < nRings; ++iRing )
+        int nVerts = QgsGeometryCheckerUtils::polyLineSize( geom, iPart, iRing );
+        // Less than three points, no angles to check
+        if ( nVerts < 3 )
+        {
+          continue;
+        }
+        for ( int iVert = 0; iVert < nVerts; ++iVert )
         {
-          int nVerts = QgsGeometryCheckerUtils::polyLineSize( geom, iPart, iRing );
-          // Less than three points, no angles to check
-          if ( nVerts < 3 )
+          const QgsPoint &p1 = geom->vertexAt( QgsVertexId( iPart, iRing, ( iVert - 1 + nVerts ) % nVerts ) );
+          const QgsPoint &p2 = geom->vertexAt( QgsVertexId( iPart, iRing, iVert ) );
+          const QgsPoint &p3 = geom->vertexAt( QgsVertexId( iPart, iRing, ( iVert + 1 ) % nVerts ) );
+          QgsVector v21, v23;
+          try
           {
-            continue;
+            v21 = QgsVector( p1.x() - p2.x(), p1.y() - p2.y() ).normalized();
+            v23 = QgsVector( p3.x() - p2.x(), p3.y() - p2.y() ).normalized();
           }
-          for ( int iVert = 0; iVert < nVerts; ++iVert )
+          catch ( const QgsException & )
           {
-            const QgsPoint &p1 = geom->vertexAt( QgsVertexId( iPart, iRing, ( iVert - 1 + nVerts ) % nVerts ) );
-            const QgsPoint &p2 = geom->vertexAt( QgsVertexId( iPart, iRing, iVert ) );
-            const QgsPoint &p3 = geom->vertexAt( QgsVertexId( iPart, iRing, ( iVert + 1 ) % nVerts ) );
-            QgsVector v21, v23;
-            try
-            {
-              v21 = QgsVector( p1.x() - p2.x(), p1.y() - p2.y() ).normalized();
-              v23 = QgsVector( p3.x() - p2.x(), p3.y() - p2.y() ).normalized();
-            }
-            catch ( const QgsException & )
-            {
-              // Zero length vectors
-              continue;
-            }
+            // Zero length vectors
+            continue;
+          }
 
-            double angle = std::acos( v21 * v23 ) / M_PI * 180.0;
-            if ( angle < mMinAngle )
-            {
-              errors.append( new QgsGeometryCheckError( this, layerId, featureid, p2, QgsVertexId( iPart, iRing, iVert ), angle ) );
-            }
+          double angle = std::acos( v21 * v23 ) / M_PI * 180.0;
+          if ( angle < mMinAngle )
+          {
+            QgsAbstractGeometry *part = QgsGeometryCheckerUtils::getGeomPart( geom, iPart )->clone();
+            errors.append( new QgsGeometryCheckError( this, layerFeature.layer().id(), layerFeature.feature().id(), part, p2, QgsVertexId( iPart, iRing, iVert ), angle ) );
           }
         }
       }

diff --git a/src/plugins/geometry_checker/checks/qgsgeometryareacheck.cpp b/src/plugins/geometry_checker/checks/qgsgeometryareacheck.cpp
@@ -21,44 +21,30 @@
 void QgsGeometryAreaCheck::collectErrors( QList<QgsGeometryCheckError *> &errors, QStringList &/*messages*/, QAtomicInt *progressCounter, const QMap<QString, QgsFeatureIds> &ids ) const
 {
   QMap<QString, QgsFeatureIds> featureIds = ids.isEmpty() ? allLayerFeatureIds() : ids;
-  for ( const QString &layerId : featureIds.keys() )
+  QgsGeometryCheckerUtils::LayerFeatures layerFeatures( featureIds, mContext->featurePools, mCompatibleGeometryTypes, progressCounter );
+  for ( const QgsGeometryCheckerUtils::LayerFeature &layerFeature : layerFeatures )
   {
-    QgsFeaturePool *featurePool = mContext->featurePools[ layerId ];
-    double mapToLayerUnits = featurePool->getMapToLayerUnits();
-    if ( !getCompatibility( featurePool->getLayer()->geometryType() ) )
+    double mapToLayerUnits = layerFeature.mapToLayerUnits();
+    const QgsAbstractGeometry *geom = layerFeature.geometry();
+    if ( dynamic_cast<const QgsGeometryCollection *>( geom ) )
     {
-      continue;
-    }
-    for ( QgsFeatureId featureid : featureIds[layerId] )
-    {
-      if ( progressCounter ) progressCounter->fetchAndAddRelaxed( 1 );
-      QgsFeature feature;
-      if ( !featurePool->get( featureid, feature ) )
-      {
-        continue;
-      }
-
-      QgsGeometry g = feature.geometry();
-      QgsAbstractGeometry *geom = g.geometry();
-      if ( dynamic_cast<QgsGeometryCollection *>( geom ) )
+      const QgsGeometryCollection *multiGeom = static_cast<const QgsGeometryCollection *>( geom );
+      for ( int i = 0, n = multiGeom->numGeometries(); i < n; ++i )
       {
-        QgsGeometryCollection *multiGeom = static_cast<QgsGeometryCollection *>( geom );
-        for ( int i = 0, n = multiGeom->numGeometries(); i < n; ++i )
+        double value;
+        if ( checkThreshold( mapToLayerUnits, multiGeom->geometryN( i ), value ) )
         {
-          double value;
-          if ( checkThreshold( featurePool->getMapToLayerUnits(), multiGeom->geometryN( i ), value ) )
-          {
-            errors.append( new QgsGeometryCheckError( this, layerId, featureid, multiGeom->geometryN( i )->centroid(), QgsVertexId( i ), value / ( mapToLayerUnits * mapToLayerUnits ), QgsGeometryCheckError::ValueArea ) );
-          }
+          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
+    }
+    else
+    {
+      double value;
+      if ( checkThreshold( mapToLayerUnits, geom, value ) )
       {
-        double value;
-        if ( checkThreshold( featurePool->getMapToLayerUnits(), geom, value ) )
-        {
-          errors.append( new QgsGeometryCheckError( this, layerId, featureid, geom->centroid(), QgsVertexId( 0 ), value / ( mapToLayerUnits * mapToLayerUnits ), QgsGeometryCheckError::ValueArea ) );
-        }
+        errors.append( new QgsGeometryCheckError( this, layerFeature.layer().id(), layerFeature.feature().id(), geom->clone(), geom->centroid(), QgsVertexId( 0 ), value / ( mapToLayerUnits * mapToLayerUnits ), QgsGeometryCheckError::ValueArea ) );
       }
     }
   }
@@ -217,9 +203,8 @@ bool QgsGeometryAreaCheck::mergeWithNeighbor( const QString &layerId, QgsFeature
   // Merge geometries
   QgsGeometry mergeFeatureGeom = mergeFeature.geometry();
   QgsAbstractGeometry *mergeGeom = mergeFeatureGeom.geometry();
-  QgsGeometryEngine *geomEngine = QgsGeometryCheckerUtils::createGeomEngine( QgsGeometryCheckerUtils::getGeomPart( mergeGeom, mergePartIdx ), mContext->tolerance );
+  QSharedPointer<QgsGeometryEngine> geomEngine = QgsGeometryCheckerUtils::createGeomEngine( QgsGeometryCheckerUtils::getGeomPart( mergeGeom, mergePartIdx ), mContext->tolerance );
   QgsAbstractGeometry *combinedGeom = geomEngine->combine( *QgsGeometryCheckerUtils::getGeomPart( geom, partIdx ), &errMsg );
-  delete geomEngine;
   if ( !combinedGeom || combinedGeom->isEmpty() )
   {
     return false;

diff --git a/src/plugins/geometry_checker/checks/qgsgeometrycheck.cpp b/src/plugins/geometry_checker/checks/qgsgeometrycheck.cpp
@@ -29,44 +29,27 @@ QgsGeometryCheckerContext::QgsGeometryCheckerContext( int _precision, const QStr
 }
 
 QgsGeometryCheckError::QgsGeometryCheckError( const QgsGeometryCheck *check, const QString &layerId,
-    QgsFeatureId featureId,
+    QgsFeatureId featureId, QgsAbstractGeometry *geometry,
     const QgsPoint &errorLocation,
     QgsVertexId vidx,
     const QVariant &value, ValueType valueType )
   : mCheck( check )
   , mLayerId( layerId )
   , mFeatureId( featureId )
+  , mGeometry( geometry )
   , mErrorLocation( errorLocation )
   , mVidx( vidx )
   , mValue( value )
   , mValueType( valueType )
   , mStatus( StatusPending )
-{}
-
-QgsAbstractGeometry *QgsGeometryCheckError::geometry() const
 {
-  QgsFeature f;
-  if ( mCheck->getContext()->featurePools[ layerId() ]->get( featureId(), f ) && f.hasGeometry() )
-  {
-    QgsGeometry featureGeom = f.geometry();
-    QgsAbstractGeometry *geom = featureGeom.geometry();
-    return mVidx.part >= 0 ? QgsGeometryCheckerUtils::getGeomPart( geom, mVidx.part )->clone() : geom->clone();
-  }
-  return nullptr;
 }
 
 QgsRectangle QgsGeometryCheckError::affectedAreaBBox() const
 {
-  QgsAbstractGeometry *geom = geometry();
-  if ( !geom )
-  {
-    return QgsRectangle();
-  }
   QString srcCrs = mCheck->getContext()->featurePools[ layerId() ]->getLayer()->crs().authid();
   QgsCoordinateTransform t = QgsCoordinateTransformCache::instance()->transform( srcCrs, mCheck->getContext()->mapCrs );
-  QgsRectangle rect = t.transformBoundingBox( geom->boundingBox() );
-  delete geom;
-  return rect;
+  return t.transformBoundingBox( mGeometry->boundingBox() );
 }
 
 bool QgsGeometryCheckError::handleChanges( const QgsGeometryCheck::Changes &changes )

diff --git a/src/plugins/geometry_checker/checks/qgsgeometrycheck.h b/src/plugins/geometry_checker/checks/qgsgeometrycheck.h
@@ -83,11 +83,8 @@ class QgsGeometryCheck : public QObject
     void deleteFeatureGeometryPart( const QString &layerId, QgsFeature &feature, int partIdx, Changes &changes ) const;
     void deleteFeatureGeometryRing( const QString &layerId, QgsFeature &feature, int partIdx, int ringIdx, Changes &changes ) const;
 
-  private:
     const CheckType mCheckType;
     QList<QgsWkbTypes::GeometryType> mCompatibleGeometryTypes;
-
-  protected:
     QgsGeometryCheckerContext *mContext;
 };
 
@@ -101,26 +98,30 @@ class QgsGeometryCheckError
     QgsGeometryCheckError( const QgsGeometryCheck *check,
                            const QString &layerId,
                            QgsFeatureId featureId,
+                           QgsAbstractGeometry *geometry,
                            const QgsPoint &errorLocation,
                            QgsVertexId vidx = QgsVertexId(),
                            const QVariant &value = QVariant(),
                            ValueType valueType = ValueOther );
-    virtual ~QgsGeometryCheckError() {}
+    virtual ~QgsGeometryCheckError()
+    {
+      delete mGeometry;
+    }
 
     const QgsGeometryCheckError &operator=( const QgsGeometryCheckError & ) = delete;
 
     const QgsGeometryCheck *check() const { return mCheck; }
     const QString &layerId() const { return mLayerId; }
     QgsFeatureId featureId() const { return mFeatureId; }
-    virtual QgsAbstractGeometry *geometry() const;
+    QgsAbstractGeometry *geometry() const { return mGeometry; }
     // In map units
     virtual QgsRectangle affectedAreaBBox() const;
     virtual QString description() const { return mCheck->errorDescription(); }
     const QgsPoint &location() const { return mErrorLocation; }
     // Lengths, areas in map units
     QVariant value() const { return mValue; }
     ValueType valueType() const { return mValueType; }
-    QgsVertexId vidx() const { return mVidx; }
+    const QgsVertexId &vidx() const { return mVidx; }
     Status status() const { return mStatus; }
     QString resolutionMessage() const { return mResolutionMessage; }
     void setFixed( int method )
@@ -147,12 +148,14 @@ class QgsGeometryCheckError
     }
     virtual void update( const QgsGeometryCheckError *other )
     {
+      delete mGeometry;
       Q_ASSERT( mCheck == other->mCheck );
       Q_ASSERT( mLayerId == other->mLayerId );
       Q_ASSERT( mFeatureId == other->mFeatureId );
       mErrorLocation = other->mErrorLocation;
       mVidx = other->mVidx;
       mValue = other->mValue;
+      mGeometry = other->mGeometry->clone();
     }
 
     virtual bool handleChanges( const QgsGeometryCheck::Changes &changes );
@@ -161,6 +164,7 @@ class QgsGeometryCheckError
     const QgsGeometryCheck *mCheck = nullptr;
     QString mLayerId;
     QgsFeatureId mFeatureId;
+    QgsAbstractGeometry *mGeometry;
     QgsPoint mErrorLocation;
     QgsVertexId mVidx;
     QVariant mValue;

diff --git a/src/plugins/geometry_checker/checks/qgsgeometrycontainedcheck.cpp b/src/plugins/geometry_checker/checks/qgsgeometrycontainedcheck.cpp
@@ -21,63 +21,23 @@
 void QgsGeometryContainedCheck::collectErrors( QList<QgsGeometryCheckError *> &errors, QStringList &messages, QAtomicInt *progressCounter, const QMap<QString, QgsFeatureIds> &ids ) const
 {
   QMap<QString, QgsFeatureIds> featureIds = ids.isEmpty() ? allLayerFeatureIds() : ids;
-  for ( const QString &layerIdA : featureIds.keys() )
+  QgsGeometryCheckerUtils::LayerFeatures layerFeaturesA( featureIds, mContext->featurePools, mCompatibleGeometryTypes, progressCounter, mContext->mapCrs );
+  for ( const QgsGeometryCheckerUtils::LayerFeature &layerFeatureA : layerFeaturesA )
   {
-    QgsFeaturePool *featurePoolA = mContext->featurePools[ layerIdA ];
-    if ( !getCompatibility( featurePoolA->getLayer()->geometryType() ) )
+    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 );
+    for ( const QgsGeometryCheckerUtils::LayerFeature &layerFeatureB : layerFeaturesB )
     {
-      continue;
-    }
-    QgsCoordinateTransform crstA = QgsCoordinateTransformCache::instance()->transform( featurePoolA->getLayer()->crs().authid(), mContext->mapCrs );
-
-    for ( QgsFeatureId featureIdA : featureIds[layerIdA] )
-    {
-      if ( progressCounter ) progressCounter->fetchAndAddRelaxed( 1 );
-      QgsFeature featureA;
-      if ( !featurePoolA->get( featureIdA, featureA ) )
+      QString errMsg;
+      if ( geomEngineA->within( *layerFeatureB.geometry(), &errMsg ) )
       {
-        continue;
+        errors.append( new QgsGeometryContainedCheckError( this, layerFeatureA.layer().id(), layerFeatureA.feature().id(), layerFeatureA.geometry()->clone(), layerFeatureA.geometry()->centroid(), qMakePair( layerFeatureB.layer().id(), layerFeatureB.feature().id() ) ) );
       }
-
-      QgsAbstractGeometry *featureGeomA = featureA.geometry().geometry()->clone();
-      featureGeomA->transform( crstA );
-      QgsGeometryEngine *geomEngineA = QgsGeometryCheckerUtils::createGeomEngine( featureGeomA, mContext->tolerance );
-      QgsRectangle bboxA = crstA.transform( featureGeomA->boundingBox() );
-
-      for ( const QString &layerIdB : featureIds.keys() )
+      else if ( !errMsg.isEmpty() )
       {
-        QgsFeaturePool *featurePoolB = mContext->featurePools[ layerIdB ];
-        if ( !getCompatibility( featurePoolB->getLayer()->geometryType() ) )
-        {
-          continue;
-        }
-        QgsCoordinateTransform crstB = QgsCoordinateTransformCache::instance()->transform( featurePoolB->getLayer()->crs().authid(), mContext->mapCrs );
-
-        QgsFeatureIds idsB = featurePoolB->getIntersects( crstB.transform( bboxA, QgsCoordinateTransform::ReverseTransform ) );
-        for ( QgsFeatureId featureIdB : idsB )
-        {
-          QgsFeature featureB;
-          if ( !featurePoolB->get( featureIdB, featureB ) )
-          {
-            continue;
-          }
-          QgsAbstractGeometry *featureGeomB = featureB.geometry().geometry()->clone();
-          featureGeomB->transform( crstB );
-
-          QString errMsg;
-          if ( geomEngineA->within( *featureGeomB, &errMsg ) )
-          {
-            errors.append( new QgsGeometryContainedCheckError( this, layerIdA, featureIdA, featureGeomA->centroid(), qMakePair( layerIdB, featureIdB ) ) );
-          }
-          else if ( !errMsg.isEmpty() )
-          {
-            messages.append( tr( "Feature %1:%2 within feature %3:%4: %5" ).arg( layerIdA ).arg( featureIdA ).arg( layerIdB ).arg( featureIdB ).arg( errMsg ) );
-          }
-          delete featureGeomB;
-        }
+        messages.append( tr( "Feature %1 within feature %2: %3" ).arg( layerFeatureA.id() ).arg( layerFeatureB.id() ).arg( errMsg ) );
       }
-      delete geomEngineA;
-      delete featureGeomA;
     }
   }
 }
@@ -102,13 +62,12 @@ void QgsGeometryContainedCheck::fixError( QgsGeometryCheckError *error, int meth
   // Check if error still applies
   QgsAbstractGeometry *featureGeomA = featureA.geometry().geometry()->clone();
   featureGeomA->transform( crstA );
-  QgsGeometryEngine *geomEngineA = QgsGeometryCheckerUtils::createGeomEngine( featureGeomA, mContext->tolerance );
+  QSharedPointer<QgsGeometryEngine> geomEngineA = QgsGeometryCheckerUtils::createGeomEngine( featureGeomA, mContext->tolerance );
 
   QgsAbstractGeometry *featureGeomB = featureB.geometry().geometry()->clone();
   featureGeomB->transform( crstB );
 
   bool within = geomEngineA->within( *featureGeomB );
-  delete geomEngineA;
   delete featureGeomA;
   delete featureGeomB;
 

diff --git a/src/plugins/geometry_checker/checks/qgsgeometrycontainedcheck.h b/src/plugins/geometry_checker/checks/qgsgeometrycontainedcheck.h
@@ -24,10 +24,11 @@ class QgsGeometryContainedCheckError : public QgsGeometryCheckError
     QgsGeometryContainedCheckError( const QgsGeometryCheck *check,
                                     const QString &layerId,
                                     QgsFeatureId featureId,
+                                    QgsAbstractGeometry *geometry,
                                     const QgsPoint &errorLocation,
                                     const QPair<QString, QgsFeatureId> &containingFeature
                                   )
-      : QgsGeometryCheckError( check, layerId, featureId, errorLocation, QgsVertexId(), QString( "%1:%2" ).arg( containingFeature.first ).arg( containingFeature.second ), ValueOther )
+      : QgsGeometryCheckError( check, layerId, featureId, geometry, errorLocation, QgsVertexId(), QString( "%1:%2" ).arg( containingFeature.first ).arg( containingFeature.second ), ValueOther )
       , mContainingFeature( containingFeature )
     { }
     const QPair<QString, QgsFeatureId> &containingFeature() const { return mContainingFeature; }