Move ALL logic for prompting users for datum transform selection

AND updating the current project transform context accordingly to a single method Unifies this logic, but most importantly fixes a bug where users are NEVER prompted for a transformation when only one valid transformation exists AND this single transform is not added to the project context (in other words, QGIS could not handle transformations when only one possible path existed). Some logic here is temporary (QgsDatumTransformDialog::defaultDatumTransform()) and will be replaced with proj db logic when we update to proj v6 API Sponsored by ICSM
qgis · Mar 22, 2019 · cd01a5b · cd01a5b
1 parent 2af0d0d
commit cd01a5b
Show file tree

Hide file tree

Showing 3 changed files with 157 additions and 84 deletions.
diff --git a/src/app/qgisapp.cpp b/src/app/qgisapp.cpp
@@ -7850,15 +7850,9 @@ QString QgisApp::saveAsVectorFileGeneral( QgsVectorLayer *vlayer, bool symbology
     QgsCoordinateTransform ct;
     destCRS = QgsCoordinateReferenceSystem::fromSrsId( dialog->crs() );
 
-    if ( destCRS.isValid() && destCRS != vlayer->crs() )
+    if ( destCRS.isValid() )
     {
-      //ask user about datum transformation
-      QgsSettings settings;
-      QgsDatumTransformDialog dlg( vlayer->crs(), destCRS );
-      if ( dlg.shouldAskUserForSelection() )
-      {
-        dlg.exec();
-      }
+      QgsDatumTransformDialog::run( vlayer->crs(), destCRS, this );
       ct = QgsCoordinateTransform( vlayer->crs(), destCRS, QgsProject::instance() );
     }
 
@@ -9892,33 +9886,15 @@ void QgisApp::projectCrsChanged()
   mMapCanvas->setDestinationCrs( QgsProject::instance()->crs() );
 
   // handle datum transforms
-  QList<QgsCoordinateReferenceSystem> transformsToAskFor = QList<QgsCoordinateReferenceSystem>();
+  QList<QgsCoordinateReferenceSystem> alreadyAsked;
   QMap<QString, QgsMapLayer *> layers = QgsProject::instance()->mapLayers();
   for ( QMap<QString, QgsMapLayer *>::const_iterator it = layers.constBegin(); it != layers.constEnd(); ++it )
   {
-    if ( !transformsToAskFor.contains( it.value()->crs() ) &&
-         it.value()->crs() != QgsProject::instance()->crs() &&
-         !QgsProject::instance()->transformContext().hasTransform( it.value()->crs(), QgsProject::instance()->crs() ) &&
-         QgsDatumTransform::datumTransformations( it.value()->crs(), QgsProject::instance()->crs() ).count() > 1 )
+    if ( !alreadyAsked.contains( it.value()->crs() ) )
     {
-      transformsToAskFor.append( it.value()->crs() );
-    }
-  }
-  if ( transformsToAskFor.count() == 1 )
-  {
-    askUserForDatumTransform( transformsToAskFor.at( 0 ),
-                              QgsProject::instance()->crs() );
-  }
-  else if ( transformsToAskFor.count() > 1 )
-  {
-    // TODO - this should actually loop through and ask for each in turn
-    bool ask = QgsSettings().value( QStringLiteral( "/Projections/showDatumTransformDialog" ), false ).toBool();
-    if ( ask )
-    {
-      visibleMessageBar()->pushMessage( tr( "Datum transforms" ),
-                                        tr( "Project CRS changed and datum transforms might need to be adapted." ),
-                                        Qgis::Warning,
-                                        5 );
+      alreadyAsked.append( it.value()->crs() );
+      askUserForDatumTransform( it.value()->crs(),
+                                QgsProject::instance()->crs() );
     }
   }
 }
@@ -13723,38 +13699,7 @@ bool QgisApp::askUserForDatumTransform( const QgsCoordinateReferenceSystem &sour
 {
   Q_ASSERT( qApp->thread() == QThread::currentThread() );
 
-  bool ok = false;
-
-  QgsCoordinateTransformContext context = QgsProject::instance()->transformContext();
-  if ( context.hasTransform( sourceCrs, destinationCrs ) )
-  {
-    ok = true;
-  }
-  else
-  {
-    //if several possibilities:  present dialog
-    QgsDatumTransformDialog dlg( sourceCrs, destinationCrs );
-    if ( dlg.shouldAskUserForSelection() )
-    {
-      if ( dlg.exec() )
-      {
-        QPair< QPair<QgsCoordinateReferenceSystem, int>, QPair<QgsCoordinateReferenceSystem, int > > dt = dlg.selectedDatumTransforms();
-        QgsCoordinateTransformContext context = QgsProject::instance()->transformContext();
-        context.addSourceDestinationDatumTransform( dt.first.first, dt.second.first, dt.first.second, dt.second.second );
-        QgsProject::instance()->setTransformContext( context );
-        ok = true;
-      }
-      else
-      {
-        ok = false;
-      }
-    }
-    else
-    {
-      ok = true;
-    }
-  }
-  return ok;
+  return QgsDatumTransformDialog::run( sourceCrs, destinationCrs, this );
 }
 
 void QgisApp::readDockWidgetSettings( QDockWidget *dockWidget, const QDomElement &elem )

diff --git a/src/gui/qgsdatumtransformdialog.cpp b/src/gui/qgsdatumtransformdialog.cpp
@@ -27,6 +27,40 @@
 #include <QDir>
 #include <QPushButton>
 
+bool QgsDatumTransformDialog::run( const QgsCoordinateReferenceSystem &sourceCrs, const QgsCoordinateReferenceSystem &destinationCrs, QWidget *parent )
+{
+  if ( sourceCrs == destinationCrs )
+    return true;
+
+  QgsCoordinateTransformContext context = QgsProject::instance()->transformContext();
+  if ( context.hasTransform( sourceCrs, destinationCrs ) )
+  {
+    return true;
+  }
+
+  QgsDatumTransformDialog dlg( sourceCrs, destinationCrs, false, qMakePair( -1, -1 ), parent );
+  if ( dlg.shouldAskUserForSelection() )
+  {
+    if ( dlg.exec() )
+    {
+      QPair< QPair<QgsCoordinateReferenceSystem, int>, QPair<QgsCoordinateReferenceSystem, int > > dt = dlg.selectedDatumTransforms();
+      QgsCoordinateTransformContext context = QgsProject::instance()->transformContext();
+      context.addSourceDestinationDatumTransform( dt.first.first, dt.second.first, dt.first.second, dt.second.second );
+      QgsProject::instance()->setTransformContext( context );
+      return true;
+    }
+    else
+    {
+      return false;
+    }
+  }
+  else
+  {
+    dlg.applyDefaultTransform();
+    return true;
+  }
+}
+
 QgsDatumTransformDialog::QgsDatumTransformDialog( const QgsCoordinateReferenceSystem &sourceCrs,
     const QgsCoordinateReferenceSystem &destinationCrs, const bool allowCrsChanges,
     QPair<int, int> selectedDatumTransforms,
@@ -154,8 +188,10 @@ void QgsDatumTransformDialog::load( QPair<int, int> selectedDatumTransforms )
       }
     }
 
-    if ( transform.sourceTransformId == selectedDatumTransforms.first &&
-         transform.destinationTransformId == selectedDatumTransforms.second )
+    if ( ( transform.sourceTransformId == selectedDatumTransforms.first &&
+           transform.destinationTransformId == selectedDatumTransforms.second ) ||
+         ( transform.sourceTransformId == selectedDatumTransforms.second &&
+           transform.destinationTransformId == selectedDatumTransforms.first ) )
     {
       mDatumTransformTableWidget->selectRow( row );
     }
@@ -188,21 +224,91 @@ QgsDatumTransformDialog::~QgsDatumTransformDialog()
   }
 }
 
-int QgsDatumTransformDialog::availableTransformationCount()
-{
-  return mDatumTransforms.count();
-}
-
 bool QgsDatumTransformDialog::shouldAskUserForSelection()
 {
-  if ( availableTransformationCount() > 1 )
+  if ( mDatumTransforms.count() > 1 )
   {
     return QgsSettings().value( QStringLiteral( "/Projections/showDatumTransformDialog" ), false ).toBool();
   }
   // TODO: show if transform grids are required, but missing
   return false;
 }
 
+QPair<QPair<QgsCoordinateReferenceSystem, int>, QPair<QgsCoordinateReferenceSystem, int> > QgsDatumTransformDialog::defaultDatumTransform()
+{
+  QPair<QPair<QgsCoordinateReferenceSystem, int>, QPair<QgsCoordinateReferenceSystem, int> > preferredNonDeprecated;
+  preferredNonDeprecated.first.first = mSourceCrs;
+  preferredNonDeprecated.second.first = mDestinationCrs;
+  bool foundPreferredNonDeprecated = false;
+  QPair<QPair<QgsCoordinateReferenceSystem, int>, QPair<QgsCoordinateReferenceSystem, int> > preferred;
+  preferred.first.first = mSourceCrs;
+  preferred.second.first = mDestinationCrs;
+  bool foundPreferred  = false;
+  QPair<QPair<QgsCoordinateReferenceSystem, int>, QPair<QgsCoordinateReferenceSystem, int> > nonDeprecated;
+  nonDeprecated.first.first = mSourceCrs;
+  nonDeprecated.second.first = mDestinationCrs;
+  bool foundNonDeprecated = false;
+  QPair<QPair<QgsCoordinateReferenceSystem, int>, QPair<QgsCoordinateReferenceSystem, int> > fallback;
+  fallback.first.first = mSourceCrs;
+  fallback.second.first = mDestinationCrs;
+  bool foundFallback = false;
+
+  for ( const QgsDatumTransform::TransformPair &transform : qgis::as_const( mDatumTransforms ) )
+  {
+    if ( transform.sourceTransformId == -1 && transform.destinationTransformId == -1 )
+      continue;
+
+    const QgsDatumTransform::TransformInfo srcInfo = QgsDatumTransform::datumTransformInfo( transform.sourceTransformId );
+    const QgsDatumTransform::TransformInfo destInfo = QgsDatumTransform::datumTransformInfo( transform.destinationTransformId );
+    if ( !foundPreferredNonDeprecated && ( ( srcInfo.preferred && !srcInfo.deprecated ) || transform.sourceTransformId == -1 )
+         && ( ( destInfo.preferred && !destInfo.deprecated ) || transform.destinationTransformId == -1 ) )
+    {
+      preferredNonDeprecated.first.second = transform.sourceTransformId;
+      preferredNonDeprecated.second.second = transform.destinationTransformId;
+      foundPreferredNonDeprecated = true;
+    }
+    else if ( !foundPreferred && ( srcInfo.preferred || transform.sourceTransformId == -1 ) &&
+              ( destInfo.preferred || transform.destinationTransformId == -1 ) )
+    {
+      preferred.first.second = transform.sourceTransformId;
+      preferred.second.second = transform.destinationTransformId;
+      foundPreferred = true;
+    }
+    else if ( !foundNonDeprecated && ( !srcInfo.deprecated || transform.sourceTransformId == -1 )
+              && ( !destInfo.deprecated || transform.destinationTransformId == -1 ) )
+    {
+      nonDeprecated.first.second = transform.sourceTransformId;
+      nonDeprecated.second.second = transform.destinationTransformId;
+      foundNonDeprecated = true;
+    }
+    else if ( !foundFallback )
+    {
+      fallback.first.second = transform.sourceTransformId;
+      fallback.second.second = transform.destinationTransformId;
+      foundFallback = true;
+    }
+  }
+  if ( foundPreferredNonDeprecated )
+    return preferredNonDeprecated;
+  else if ( foundPreferred )
+    return preferred;
+  else if ( foundNonDeprecated )
+    return nonDeprecated;
+  else
+    return fallback;
+}
+
+void QgsDatumTransformDialog::applyDefaultTransform()
+{
+  if ( mDatumTransforms.count() > 0 )
+  {
+    QgsCoordinateTransformContext context = QgsProject::instance()->transformContext();
+    const QPair<QPair<QgsCoordinateReferenceSystem, int>, QPair<QgsCoordinateReferenceSystem, int> > dt = defaultDatumTransform();
+    context.addSourceDestinationDatumTransform( dt.first.first, dt.second.first, dt.first.second, dt.second.second );
+    QgsProject::instance()->setTransformContext( context );
+  }
+}
+
 QPair<QPair<QgsCoordinateReferenceSystem, int>, QPair<QgsCoordinateReferenceSystem, int> > QgsDatumTransformDialog::selectedDatumTransforms()
 {
   int row = mDatumTransformTableWidget->currentRow();

diff --git a/src/gui/qgsdatumtransformdialog.h b/src/gui/qgsdatumtransformdialog.h
@@ -37,6 +37,21 @@ class GUI_EXPORT QgsDatumTransformDialog : public QDialog, private Ui::QgsDatumT
     Q_OBJECT
   public:
 
+    /**
+     * Runs the dialog (if required) prompting for the desired transform to use from \a sourceCrs to
+     * \a destinationCrs, updating the current project transform context as required
+     * based on the results of the run.
+     *
+     * This handles EVERYTHING, including only showing the dialog if multiple choices exist
+     * and the user has asked to be prompted, not re-adding transforms already in the current project
+     * context, etc.
+     *
+     * \since QGIS 3.8
+     */
+    static bool run( const QgsCoordinateReferenceSystem &sourceCrs = QgsCoordinateReferenceSystem(),
+                     const QgsCoordinateReferenceSystem &destinationCrs = QgsCoordinateReferenceSystem(),
+                     QWidget *parent = nullptr );
+
     /**
      * Constructor for QgsDatumTransformDialog.
      */
@@ -48,19 +63,6 @@ class GUI_EXPORT QgsDatumTransformDialog : public QDialog, private Ui::QgsDatumT
                              Qt::WindowFlags f = nullptr );
     ~QgsDatumTransformDialog() override;
 
-    /**
-     * Returns the number of possible datum transformation for currently selected source and destination CRS
-     * \since 3.0
-     */
-    int availableTransformationCount();
-
-    /**
-     * Returns true if the dialog should be shown and the user prompted to make the transformation selection.
-     *
-     * \since QGIS 3.8
-     */
-    bool shouldAskUserForSelection();
-
     /**
      * Returns the source and destination transforms, each being a pair of QgsCoordinateReferenceSystems and datum transform code
      * \since 3.0
@@ -80,6 +82,26 @@ class GUI_EXPORT QgsDatumTransformDialog : public QDialog, private Ui::QgsDatumT
     void load( QPair<int, int> selectedDatumTransforms = qMakePair( -1, -1 ) );
     void setOKButtonEnabled();
 
+    /**
+     * Returns true if the dialog should be shown and the user prompted to make the transformation selection.
+     *
+     * \see defaultDatumTransform()
+     */
+    bool shouldAskUserForSelection();
+
+    /**
+     * Returns the default transform (or only available transform). This represents the transform which
+     * should be used if the user is not being prompted to make this selection for themselves.
+     *
+     * \see shouldAskUserForSelection()
+     * \see applyDefaultTransform()
+     */
+    QPair< QPair<QgsCoordinateReferenceSystem, int>, QPair<QgsCoordinateReferenceSystem, int > > defaultDatumTransform();
+
+    /**
+     * Applies the defaultDatumTransform(), adding it to the current QgsProject instance.
+     */
+    void applyDefaultTransform();
 
     QList< QgsDatumTransform::TransformPair > mDatumTransforms;
     QgsCoordinateReferenceSystem mSourceCrs;