From 23affe4fae37efc394bedd5eb864019ad490095f Mon Sep 17 00:00:00 2001
From: Sandro Mani <manisandro@gmail.com>
Date: Tue, 4 Apr 2017 16:48:34 +0200
Subject: [PATCH] [Geometry checker] Store error values in map units, perform
 topology checks in map CRS

---
 .../checks/qgsgeometryareacheck.cpp           |  5 +-
 .../checks/qgsgeometrycheck.cpp               | 11 ++--
 .../checks/qgsgeometrycheck.h                 |  6 +-
 .../checks/qgsgeometrygapcheck.cpp            |  2 +-
 .../checks/qgsgeometrysegmentlengthcheck.cpp  |  2 +-
 .../geometry_checker/qgsgeometrychecker.cpp   |  4 +-
 .../ui/qgsgeometrycheckerfixsummarydialog.cpp | 16 +-----
 .../ui/qgsgeometrycheckerresulttab.cpp        | 34 ++----------
 .../ui/qgsgeometrycheckersetuptab.cpp         | 10 +---
 .../ui/qgsgeometrycheckersetuptab.ui          | 55 +++++++++----------
 10 files changed, 49 insertions(+), 96 deletions(-)

diff --git a/src/plugins/geometry_checker/checks/qgsgeometryareacheck.cpp b/src/plugins/geometry_checker/checks/qgsgeometryareacheck.cpp
index 9166c4d3694..eaf27b7745b 100644
--- a/src/plugins/geometry_checker/checks/qgsgeometryareacheck.cpp
+++ b/src/plugins/geometry_checker/checks/qgsgeometryareacheck.cpp
@@ -24,6 +24,7 @@ void QgsGeometryAreaCheck::collectErrors( QList<QgsGeometryCheckError *> &errors
   for ( const QString &layerId : featureIds.keys() )
   {
     QgsFeaturePool *featurePool = mContext->featurePools[ layerId ];
+    double mapToLayerUnits = featurePool->getMapToLayerUnits();
     if ( !getCompatibility( featurePool->getLayer()->geometryType() ) )
     {
       continue;
@@ -47,7 +48,7 @@ void QgsGeometryAreaCheck::collectErrors( QList<QgsGeometryCheckError *> &errors
           double value;
           if ( checkThreshold( featurePool->getMapToLayerUnits(), multiGeom->geometryN( i ), value ) )
           {
-            errors.append( new QgsGeometryCheckError( this, layerId, featureid, multiGeom->geometryN( i )->centroid(), QgsVertexId( i ), value, QgsGeometryCheckError::ValueArea ) );
+            errors.append( new QgsGeometryCheckError( this, layerId, featureid, multiGeom->geometryN( i )->centroid(), QgsVertexId( i ), value / ( mapToLayerUnits * mapToLayerUnits ), QgsGeometryCheckError::ValueArea ) );
           }
         }
       }
@@ -56,7 +57,7 @@ void QgsGeometryAreaCheck::collectErrors( QList<QgsGeometryCheckError *> &errors
         double value;
         if ( checkThreshold( featurePool->getMapToLayerUnits(), geom, value ) )
         {
-          errors.append( new QgsGeometryCheckError( this, layerId, featureid, geom->centroid(), QgsVertexId( 0 ), value, QgsGeometryCheckError::ValueArea ) );
+          errors.append( new QgsGeometryCheckError( this, layerId, featureid, geom->centroid(), QgsVertexId( 0 ), value / ( mapToLayerUnits * mapToLayerUnits ), QgsGeometryCheckError::ValueArea ) );
         }
       }
     }
diff --git a/src/plugins/geometry_checker/checks/qgsgeometrycheck.cpp b/src/plugins/geometry_checker/checks/qgsgeometrycheck.cpp
index 2405834ff76..5914d937015 100644
--- a/src/plugins/geometry_checker/checks/qgsgeometrycheck.cpp
+++ b/src/plugins/geometry_checker/checks/qgsgeometrycheck.cpp
@@ -19,13 +19,14 @@
 #include "qgsgeometrycheck.h"
 #include "../utils/qgsfeaturepool.h"
 
-
-QgsGeometryCheckerContext::QgsGeometryCheckerContext( int _precision, const QString &_crs, const QMap<QString, QgsFeaturePool *> &_featurePools )
+QgsGeometryCheckerContext::QgsGeometryCheckerContext( int _precision, const QString &_mapCrs, const QMap<QString, QgsFeaturePool *> &_featurePools )
   : tolerance( qPow( 10, -_precision ) )
   , reducedTolerance( qPow( 10, -_precision / 2 ) )
-  , crs( _crs )
+  , mapCrs( _mapCrs )
   , featurePools( _featurePools )
-{}
+{
+
+}
 
 QgsGeometryCheckError::QgsGeometryCheckError( const QgsGeometryCheck *check, const QString &layerId,
     QgsFeatureId featureId,
@@ -62,7 +63,7 @@ QgsRectangle QgsGeometryCheckError::affectedAreaBBox() const
     return QgsRectangle();
   }
   QString srcCrs = mCheck->getContext()->featurePools[ layerId() ]->getLayer()->crs().authid();
-  QgsCoordinateTransform t = QgsCoordinateTransformCache::instance()->transform( srcCrs, mCheck->getContext()->crs );
+  QgsCoordinateTransform t = QgsCoordinateTransformCache::instance()->transform( srcCrs, mCheck->getContext()->mapCrs );
   QgsRectangle rect = t.transformBoundingBox( geom->boundingBox() );
   delete geom;
   return rect;
diff --git a/src/plugins/geometry_checker/checks/qgsgeometrycheck.h b/src/plugins/geometry_checker/checks/qgsgeometrycheck.h
index 84ca20d299c..ac53eeea005 100644
--- a/src/plugins/geometry_checker/checks/qgsgeometrycheck.h
+++ b/src/plugins/geometry_checker/checks/qgsgeometrycheck.h
@@ -32,10 +32,10 @@ class QgsFeaturePool;
 
 struct QgsGeometryCheckerContext
 {
-  QgsGeometryCheckerContext( int _precision, const QString &_crs, const QMap<QString, QgsFeaturePool *> &_featurePools );
+  QgsGeometryCheckerContext( int _precision, const QString &_mapCrs, const QMap<QString, QgsFeaturePool *> &_featurePools );
   const double tolerance;
   const double reducedTolerance;
-  const QString crs;
+  const QString mapCrs;
   const QMap<QString, QgsFeaturePool *> featurePools;
 };
 
@@ -113,9 +113,11 @@ class QgsGeometryCheckError
     const QString &layerId() const { return mLayerId; }
     QgsFeatureId featureId() const { return mFeatureId; }
     virtual QgsAbstractGeometry *geometry() const;
+    // 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; }
diff --git a/src/plugins/geometry_checker/checks/qgsgeometrygapcheck.cpp b/src/plugins/geometry_checker/checks/qgsgeometrygapcheck.cpp
index 7d7a53241e0..e6d27553504 100644
--- a/src/plugins/geometry_checker/checks/qgsgeometrygapcheck.cpp
+++ b/src/plugins/geometry_checker/checks/qgsgeometrygapcheck.cpp
@@ -29,7 +29,7 @@ void QgsGeometryGapCheck::collectErrors( QList<QgsGeometryCheckError *> &errors,
   for ( const QString &layerId : featureIds.keys() )
   {
     QgsFeaturePool *featurePool = mContext->featurePools[ layerId ];
-    QgsCoordinateTransform t = QgsCoordinateTransformCache::instance()->transform( featurePool->getLayer()->crs().authid(), mContext->crs );
+    QgsCoordinateTransform t = QgsCoordinateTransformCache::instance()->transform( featurePool->getLayer()->crs().authid(), mContext->mapCrs );
     if ( !getCompatibility( featurePool->getLayer()->geometryType() ) )
     {
       continue;
diff --git a/src/plugins/geometry_checker/checks/qgsgeometrysegmentlengthcheck.cpp b/src/plugins/geometry_checker/checks/qgsgeometrysegmentlengthcheck.cpp
index 4a9a8320fef..383839bd619 100644
--- a/src/plugins/geometry_checker/checks/qgsgeometrysegmentlengthcheck.cpp
+++ b/src/plugins/geometry_checker/checks/qgsgeometrysegmentlengthcheck.cpp
@@ -56,7 +56,7 @@ void QgsGeometrySegmentLengthCheck::collectErrors( QList<QgsGeometryCheckError *
             double dist = qSqrt( QgsGeometryUtils::sqrDistance2D( pi, pj ) );
             if ( dist < minLength )
             {
-              errors.append( new QgsGeometryCheckError( this, layerId, featureid, QgsPoint( 0.5 * ( pi.x() + pj.x() ), 0.5 * ( pi.y() + pj.y() ) ), QgsVertexId( iPart, iRing, iVert ), dist, QgsGeometryCheckError::ValueLength ) );
+              errors.append( new QgsGeometryCheckError( this, layerId, featureid, QgsPoint( 0.5 * ( pi.x() + pj.x() ), 0.5 * ( pi.y() + pj.y() ) ), QgsVertexId( iPart, iRing, iVert ), dist / mapToLayerUnits, QgsGeometryCheckError::ValueLength ) );
             }
           }
         }
diff --git a/src/plugins/geometry_checker/qgsgeometrychecker.cpp b/src/plugins/geometry_checker/qgsgeometrychecker.cpp
index 43059f829db..4cebd36eb61 100644
--- a/src/plugins/geometry_checker/qgsgeometrychecker.cpp
+++ b/src/plugins/geometry_checker/qgsgeometrychecker.cpp
@@ -113,7 +113,7 @@ bool QgsGeometryChecker::fixError( QgsGeometryCheckError *error, int method, boo
   {
     const QMap<QgsFeatureId, QList<QgsGeometryCheck::Change>> &layerChanges = changes[layerId];
     QgsFeaturePool *featurePool = mContext->featurePools[layerId];
-    QgsCoordinateTransform t = QgsCoordinateTransformCache::instance()->transform( featurePool->getLayer()->crs().authid(), mContext->crs );
+    QgsCoordinateTransform t = QgsCoordinateTransformCache::instance()->transform( featurePool->getLayer()->crs().authid(), mContext->mapCrs );
     for ( QgsFeatureId id : layerChanges.keys() )
     {
       bool removed = false;
@@ -152,7 +152,7 @@ bool QgsGeometryChecker::fixError( QgsGeometryCheckError *error, int method, boo
   for ( const QString &layerId : mContext->featurePools.keys() )
   {
     QgsFeaturePool *featurePool = mContext->featurePools[layerId];
-    QgsCoordinateTransform t = QgsCoordinateTransformCache::instance()->transform( mContext->crs, featurePool->getLayer()->crs().authid() );
+    QgsCoordinateTransform t = QgsCoordinateTransformCache::instance()->transform( mContext->mapCrs, featurePool->getLayer()->crs().authid() );
     recheckAreaFeatures[layerId] = featurePool->getIntersects( t.transform( recheckArea ) );
     // If only selected features were checked, confine the recheck areas to the selected features
     if ( featurePool->getSelectedOnly() )
diff --git a/src/plugins/geometry_checker/ui/qgsgeometrycheckerfixsummarydialog.cpp b/src/plugins/geometry_checker/ui/qgsgeometrycheckerfixsummarydialog.cpp
index fd28ef52d36..1e0dc5a023b 100644
--- a/src/plugins/geometry_checker/ui/qgsgeometrycheckerfixsummarydialog.cpp
+++ b/src/plugins/geometry_checker/ui/qgsgeometrycheckerfixsummarydialog.cpp
@@ -67,20 +67,6 @@ void QgsGeometryCheckerFixSummaryDialog::addError( QTableWidget *table, QgsGeome
 {
   int prec = 7 - std::floor( std::max( 0., std::log10( std::max( error->location().x(), error->location().y() ) ) ) );
   QString posStr = QStringLiteral( "%1, %2" ).arg( error->location().x(), 0, 'f', prec ).arg( error->location().y(), 0, 'f', prec );
-  double layerToMap = 1. / mChecker->getContext()->featurePools[error->layerId()]->getMapToLayerUnits();
-  QVariant value;
-  if ( error->valueType() == QgsGeometryCheckError::ValueLength )
-  {
-    value = QVariant::fromValue( error->value().toDouble() * layerToMap );
-  }
-  else if ( error->valueType() == QgsGeometryCheckError::ValueArea )
-  {
-    value = QVariant::fromValue( error->value().toDouble() * layerToMap * layerToMap );
-  }
-  else
-  {
-    value = error->value();
-  }
 
   int row = table->rowCount();
   table->insertRow( row );
@@ -91,7 +77,7 @@ void QgsGeometryCheckerFixSummaryDialog::addError( QTableWidget *table, QgsGeome
   table->setItem( row, 1, new QTableWidgetItem( error->description() ) );
   table->setItem( row, 2, new QTableWidgetItem( posStr ) );
   QTableWidgetItem *valueItem = new QTableWidgetItem();
-  valueItem->setData( Qt::EditRole, value );
+  valueItem->setData( Qt::EditRole, error->value() );
   table->setItem( row, 3, valueItem );
 }
 
diff --git a/src/plugins/geometry_checker/ui/qgsgeometrycheckerresulttab.cpp b/src/plugins/geometry_checker/ui/qgsgeometrycheckerresulttab.cpp
index 971d435793b..9c2e12b2761 100644
--- a/src/plugins/geometry_checker/ui/qgsgeometrycheckerresulttab.cpp
+++ b/src/plugins/geometry_checker/ui/qgsgeometrycheckerresulttab.cpp
@@ -137,20 +137,7 @@ void QgsGeometryCheckerResultTab::addError( QgsGeometryCheckError *error )
   int row = ui.tableWidgetErrors->rowCount();
   int prec = 7 - std::floor( std::max( 0., std::log10( std::max( error->location().x(), error->location().y() ) ) ) );
   QString posStr = QStringLiteral( "%1, %2" ).arg( error->location().x(), 0, 'f', prec ).arg( error->location().y(), 0, 'f', prec );
-  double layerToMap = 1. / mChecker->getContext()->featurePools[error->layerId()]->getMapToLayerUnits();
-  QVariant value;
-  if ( error->valueType() == QgsGeometryCheckError::ValueLength )
-  {
-    value = QVariant::fromValue( error->value().toDouble() * layerToMap );
-  }
-  else if ( error->valueType() == QgsGeometryCheckError::ValueArea )
-  {
-    value = QVariant::fromValue( error->value().toDouble() * layerToMap * layerToMap );
-  }
-  else
-  {
-    value = error->value();
-  }
+
   ui.tableWidgetErrors->insertRow( row );
   QTableWidgetItem *idItem = new QTableWidgetItem();
   idItem->setData( Qt::EditRole, error->featureId() != FEATUREID_NULL ? QVariant( error->featureId() ) : QVariant() );
@@ -158,7 +145,7 @@ void QgsGeometryCheckerResultTab::addError( QgsGeometryCheckError *error )
   ui.tableWidgetErrors->setItem( row, 1, new QTableWidgetItem( error->description() ) );
   ui.tableWidgetErrors->setItem( row, 2, new QTableWidgetItem( posStr ) );
   QTableWidgetItem *valueItem = new QTableWidgetItem();
-  valueItem->setData( Qt::EditRole, value );
+  valueItem->setData( Qt::EditRole, error->value() );
   ui.tableWidgetErrors->setItem( row, 3, valueItem );
   ui.tableWidgetErrors->setItem( row, 4, new QTableWidgetItem( QLatin1String( "" ) ) );
   ui.tableWidgetErrors->item( row, 0 )->setData( Qt::UserRole, QVariant::fromValue( error ) );
@@ -185,22 +172,9 @@ void QgsGeometryCheckerResultTab::updateError( QgsGeometryCheckError *error, boo
   int row = mErrorMap.value( error ).row();
   int prec = 7 - std::floor( std::max( 0., std::log10( std::max( error->location().x(), error->location().y() ) ) ) );
   QString posStr = QStringLiteral( "%1, %2" ).arg( error->location().x(), 0, 'f', prec ).arg( error->location().y(), 0, 'f', prec );
-  double layerToMap = 1. / mChecker->getContext()->featurePools[error->layerId()]->getMapToLayerUnits();
-  QVariant value;
-  if ( error->valueType() == QgsGeometryCheckError::ValueLength )
-  {
-    value = QVariant::fromValue( error->value().toDouble() * layerToMap );
-  }
-  else if ( error->valueType() == QgsGeometryCheckError::ValueArea )
-  {
-    value = QVariant::fromValue( error->value().toDouble() * layerToMap * layerToMap );
-  }
-  else
-  {
-    value = error->value();
-  }
+
   ui.tableWidgetErrors->item( row, 2 )->setText( posStr );
-  ui.tableWidgetErrors->item( row, 3 )->setData( Qt::EditRole, value );
+  ui.tableWidgetErrors->item( row, 3 )->setData( Qt::EditRole, error->value() );
   if ( error->status() == QgsGeometryCheckError::StatusFixed )
   {
     setRowStatus( row, Qt::green, tr( "Fixed: %1" ).arg( error->resolutionMessage() ), true );
diff --git a/src/plugins/geometry_checker/ui/qgsgeometrycheckersetuptab.cpp b/src/plugins/geometry_checker/ui/qgsgeometrycheckersetuptab.cpp
index 30e30b6f0c3..4a53170e7da 100644
--- a/src/plugins/geometry_checker/ui/qgsgeometrycheckersetuptab.cpp
+++ b/src/plugins/geometry_checker/ui/qgsgeometrycheckersetuptab.cpp
@@ -183,14 +183,6 @@ void QgsGeometryCheckerSetupTab::validateInput()
       nApplicable += factory->checkApplicability( ui, nPoint, nLineString, nPolygon );
     }
   }
-  QString prevCrs = ui.comboBoxTopologyCrs->currentText();
-  ui.comboBoxTopologyCrs->clear();
-  ui.comboBoxTopologyCrs->addItems( layerCrs );
-  ui.comboBoxTopologyCrs->setCurrentIndex( ui.comboBoxTopologyCrs->findText( prevCrs ) );
-  if ( ui.comboBoxTopologyCrs->currentIndex() == -1 )
-  {
-    ui.comboBoxTopologyCrs->setCurrentIndex( 0 );
-  }
 
   bool outputOk = ui.radioButtonOutputModifyInput->isChecked() || !ui.lineEditOutputDirectory->text().isEmpty();
   mRunButton->setEnabled( !layers.isEmpty() && nApplicable > 0 && outputOk );
@@ -408,7 +400,7 @@ void QgsGeometryCheckerSetupTab::runChecks()
     featurePools.insert( layer->id(), new QgsFeaturePool( layer, mapToLayerUnits, selectedOnly ) );
   }
 
-  QgsGeometryCheckerContext *context = new QgsGeometryCheckerContext( ui.spinBoxTolerance->value(), ui.comboBoxTopologyCrs->currentText(), featurePools );
+  QgsGeometryCheckerContext *context = new QgsGeometryCheckerContext( ui.spinBoxTolerance->value(), mIface->mapCanvas()->mapSettings().destinationCrs().authid(), featurePools );
 
   QList<QgsGeometryCheck *> checks;
   for ( const QgsGeometryCheckFactory *factory : QgsGeometryCheckFactoryRegistry::getCheckFactories() )
diff --git a/src/plugins/geometry_checker/ui/qgsgeometrycheckersetuptab.ui b/src/plugins/geometry_checker/ui/qgsgeometrycheckersetuptab.ui
index a1ed53f5669..66bc5904b95 100644
--- a/src/plugins/geometry_checker/ui/qgsgeometrycheckersetuptab.ui
+++ b/src/plugins/geometry_checker/ui/qgsgeometrycheckersetuptab.ui
@@ -41,9 +41,9 @@
       <property name="geometry">
        <rect>
         <x>0</x>
-        <y>-106</y>
+        <y>0</y>
         <width>626</width>
-        <height>832</height>
+        <height>820</height>
        </rect>
       </property>
       <layout class="QGridLayout" name="gridLayout_4">
@@ -478,6 +478,13 @@
              <property name="spacing">
               <number>2</number>
              </property>
+             <item row="0" column="0">
+              <widget class="QCheckBox" name="checkBoxDuplicates">
+               <property name="text">
+                <string>Check for duplicates</string>
+               </property>
+              </widget>
+             </item>
              <item row="2" column="1">
               <widget class="QDoubleSpinBox" name="doubleSpinBoxOverlapArea">
                <property name="decimals">
@@ -488,6 +495,16 @@
                </property>
               </widget>
              </item>
+             <item row="3" column="1">
+              <widget class="QDoubleSpinBox" name="doubleSpinBoxGapArea">
+               <property name="decimals">
+                <number>6</number>
+               </property>
+               <property name="maximum">
+                <double>999999999.000000000000000</double>
+               </property>
+              </widget>
+             </item>
              <item row="2" column="0">
               <widget class="QCheckBox" name="checkBoxOverlaps">
                <property name="sizePolicy">
@@ -514,23 +531,6 @@
                </property>
               </widget>
              </item>
-             <item row="3" column="1">
-              <widget class="QDoubleSpinBox" name="doubleSpinBoxGapArea">
-               <property name="decimals">
-                <number>6</number>
-               </property>
-               <property name="maximum">
-                <double>999999999.000000000000000</double>
-               </property>
-              </widget>
-             </item>
-             <item row="0" column="0">
-              <widget class="QCheckBox" name="checkBoxDuplicates">
-               <property name="text">
-                <string>Check for duplicates</string>
-               </property>
-              </widget>
-             </item>
              <item row="1" column="0">
               <widget class="QCheckBox" name="checkBoxCovered">
                <property name="text">
@@ -538,6 +538,13 @@
                </property>
               </widget>
              </item>
+             <item row="4" column="0" colspan="2">
+              <widget class="QLabel" name="label">
+               <property name="text">
+                <string>&lt;i&gt;Note: Topology checks are performed in the current map CRS.&lt;/i&gt;</string>
+               </property>
+              </widget>
+             </item>
             </layout>
            </widget>
           </item>
@@ -585,16 +592,6 @@
               </property>
              </widget>
             </item>
-            <item row="1" column="0">
-             <widget class="QLabel" name="labelTopologyCrs">
-              <property name="text">
-               <string>CRS for topology checks:</string>
-              </property>
-             </widget>
-            </item>
-            <item row="1" column="1">
-             <widget class="QComboBox" name="comboBoxTopologyCrs"/>
-            </item>
            </layout>
           </item>
           <item>