Fix project unit confusion (part 1)

- Remove existing (confusing and totally useless) "canvas units"
setting. This setting was supposed to be used for a confusing number
of totally different uses, eg changing the coordinate display
format and changing the default measurement units, but it was
so broken it did none of these things except change the
coordinate display and it only did that if OTF was off.

- Add a new "coordinate display" section in project settings,
which allows choice of format for coordinate display via a
combo box (this will also make it easy to add additional custom
formats in future), and make this setting work regardless of
whether OTF is on or off. This setting applies to both the
coordinate display in the status bar and coordinates shown
via the identify tool

(refs #13209, fixes #9730)

Author: nyalldawson

src/app/qgisapp.cpp

@@ -125,6 +125,7 @@
 #include "qgsstatusbarcoordinateswidget.h"
 #include "qgsconfigureshortcutsdialog.h"
 #include "qgscoordinatetransform.h"
+#include "qgscoordinateutils.h"
 #include "qgscredentialdialog.h"
 #include "qgscursors.h"
 #include "qgscustomization.h"
@@ -9616,31 +9617,7 @@ void QgisApp::showRotation()
 
 void QgisApp::updateMouseCoordinatePrecision()
 {
-  // Work out what mouse display precision to use. This only needs to
-  // be when the settings change or the zoom level changes. This
-  // function needs to be called every time one of the above happens.
-
-  // Get the display precision from the project settings
-  bool automatic = QgsProject::instance()->readBoolEntry( "PositionPrecision", "/Automatic" );
-  int dp = 0;
-
-  if ( automatic )
-  {
-    // Work out a suitable number of decimal places for the mouse
-    // coordinates with the aim of always having enough decimal places
-    // to show the difference in position between adjacent pixels.
-    // Also avoid taking the log of 0.
-    if ( mapCanvas()->mapUnitsPerPixel() != 0.0 )
-      dp = static_cast<int>( ceil( -1.0 * log10( mapCanvas()->mapUnitsPerPixel() ) ) );
-  }
-  else
-    dp = QgsProject::instance()->readNumEntry( "PositionPrecision", "/DecimalPlaces" );
-
-  // Keep dp sensible
-  if ( dp < 0 )
-    dp = 0;
-
-  mCoordsEdit->setMouseCoordinatesPrecision( dp );
+  mCoordsEdit->setMouseCoordinatesPrecision( QgsCoordinateUtils::calculateCoordinatePrecision( mapCanvas()->mapUnitsPerPixel(), mapCanvas()->mapSettings().destinationCrs() ) );
 }
 
 void QgisApp::showStatusMessage( const QString& theMessage )

src/app/qgsprojectproperties.cpp

@@ -80,34 +80,35 @@ QgsProjectProperties::QgsProjectProperties( QgsMapCanvas* mapCanvas, QWidget *pa
   // and connecting QDialogButtonBox's accepted/rejected signals to dialog's accept/reject slots
   initOptionsBase( false );
 
+  mCoordinateDisplayComboBox->addItem( tr( "Decimal degrees" ), DecimalDegrees );
+  mCoordinateDisplayComboBox->addItem( tr( "Degrees, minutes" ), DegreesMinutes );
+  mCoordinateDisplayComboBox->addItem( tr( "Degrees, minutes, seconds" ), DegreesMinutesSeconds );
+
   connect( buttonBox->button( QDialogButtonBox::Apply ), SIGNAL( clicked() ), this, SLOT( apply() ) );
   connect( this, SIGNAL( accepted() ), this, SLOT( apply() ) );
-  connect( projectionSelector, SIGNAL( sridSelected( QString ) ), this, SLOT( setMapUnitsToCurrentProjection() ) );
+  connect( projectionSelector, SIGNAL( sridSelected( QString ) ), this, SLOT( srIdUpdated() ) );
   connect( projectionSelector, SIGNAL( initialized() ), this, SLOT( projectionSelectorInitialized() ) );
 
   connect( cmbEllipsoid, SIGNAL( currentIndexChanged( int ) ), this, SLOT( updateEllipsoidUI( int ) ) );
 
-  connect( radMeters, SIGNAL( toggled( bool ) ), btnGrpDegreeDisplay, SLOT( setDisabled( bool ) ) );
-  connect( radFeet, SIGNAL( toggled( bool ) ), btnGrpDegreeDisplay, SLOT( setDisabled( bool ) ) );
-  connect( radNMiles, SIGNAL( toggled( bool ) ), btnGrpDegreeDisplay, SLOT( setDisabled( bool ) ) );
-  connect( radDegrees, SIGNAL( toggled( bool ) ), btnGrpDegreeDisplay, SLOT( setEnabled( bool ) ) );
-
-  connect( radAutomatic, SIGNAL( toggled( bool ) ), mPrecisionFrame, SLOT( setDisabled( bool ) ) );
-  connect( radManual, SIGNAL( toggled( bool ) ), mPrecisionFrame, SLOT( setEnabled( bool ) ) );
+  connect( radAutomatic, SIGNAL( toggled( bool ) ), spinBoxDP, SLOT( setDisabled( bool ) ) );
+  connect( radAutomatic, SIGNAL( toggled( bool ) ), labelDP, SLOT( setDisabled( bool ) ) );
+  connect( radManual, SIGNAL( toggled( bool ) ), spinBoxDP, SLOT( setEnabled( bool ) ) );
+  connect( radManual, SIGNAL( toggled( bool ) ), labelDP, SLOT( setEnabled( bool ) ) );
 
   QSettings settings;
 
   ///////////////////////////////////////////////////////////
   // Properties stored in map canvas's QgsMapRenderer
   // these ones are propagated to QgsProject by a signal
 
-  QGis::UnitType myUnit = mMapCanvas->mapSettings().mapUnits();
-  setMapUnits( myUnit );
-
   // we need to initialize it, since the on_cbxProjectionEnabled_toggled()
   // slot triggered by setChecked() might use it.
   mProjectSrsId = mMapCanvas->mapSettings().destinationCrs().srsid();
 
+  QgsCoordinateReferenceSystem srs( mProjectSrsId, QgsCoordinateReferenceSystem::InternalCrsId );
+  updateGuiForMapUnits( srs.mapUnits() );
+
   QgsDebugMsg( "Read project CRSID: " + QString::number( mProjectSrsId ) );
   projectionSelector->setSelectedCrsId( mProjectSrsId );
 
@@ -127,13 +128,17 @@ QgsProjectProperties::QgsProjectProperties( QgsMapCanvas* mapCanvas, QWidget *pa
   if ( automaticPrecision )
   {
     radAutomatic->setChecked( true );
-    mPrecisionFrame->setEnabled( false );
+    spinBoxDP->setEnabled( false );
+    labelDP->setEnabled( false );
   }
   else
   {
     radManual->setChecked( true );
-    mPrecisionFrame->setEnabled( true );
+    spinBoxDP->setEnabled( true );
+    labelDP->setEnabled( true );
   }
+  int dp = QgsProject::instance()->readNumEntry( "PositionPrecision", "/DecimalPlaces" );
+  spinBoxDP->setValue( dp );
 
   cbxAbsolutePath->setCurrentIndex( QgsProject::instance()->readBoolEntry( "Paths", "/Absolute", true ) ? 0 : 1 );
 
@@ -142,17 +147,15 @@ QgsProjectProperties::QgsProjectProperties( QgsMapCanvas* mapCanvas, QWidget *pa
   // be overridden in the meanwhile by the projection selector
   populateEllipsoidList();
 
-
-  int dp = QgsProject::instance()->readNumEntry( "PositionPrecision", "/DecimalPlaces" );
-  spinBoxDP->setValue( dp );
-
   QString format = QgsProject::instance()->readEntry( "PositionPrecision", "/DegreeFormat", "D" );
-  if ( format == "DM" )
-    radDM->setChecked( true );
+  if ( format == "MU" && mCoordinateDisplayComboBox->findData( MapUnits ) >= 0 )
+    mCoordinateDisplayComboBox->setCurrentIndex( mCoordinateDisplayComboBox->findData( MapUnits ) );
+  else if ( format == "DM" )
+    mCoordinateDisplayComboBox->setCurrentIndex( mCoordinateDisplayComboBox->findData( DegreesMinutes ) );
   else if ( format == "DMS" )
-    radDMS->setChecked( true );
+    mCoordinateDisplayComboBox->setCurrentIndex( mCoordinateDisplayComboBox->findData( DegreesMinutesSeconds ) );
   else
-    radD->setChecked( true );
+    mCoordinateDisplayComboBox->setCurrentIndex( mCoordinateDisplayComboBox->findData( DecimalDegrees ) );
 
   //get the color selections and set the button color accordingly
   int myRedInt = QgsProject::instance()->readNumEntry( "Gui", "/SelectionColorRedPart", 255 );
@@ -692,11 +695,6 @@ void QgsProjectProperties::setMapUnits( QGis::UnitType unit )
     unit = QGis::Meters;
   }
 
-  radMeters->setChecked( unit == QGis::Meters );
-  radFeet->setChecked( unit == QGis::Feet );
-  radNMiles->setChecked( unit == QGis::NauticalMiles );
-  radDegrees->setChecked( unit == QGis::Degrees );
-
   mMapCanvas->setMapUnits( unit );
 }
 
@@ -714,28 +712,6 @@ void QgsProjectProperties::title( QString const & title )
 //when user clicks apply button
 void QgsProjectProperties::apply()
 {
-  // Set the map units
-  // Note. Qt 3.2.3 and greater have a function selectedId() that
-  // can be used instead of the two part technique here
-  QGis::UnitType mapUnit;
-  if ( radDegrees->isChecked() )
-  {
-    mapUnit = QGis::Degrees;
-  }
-  else if ( radFeet->isChecked() )
-  {
-    mapUnit = QGis::Feet;
-  }
-  else if ( radNMiles->isChecked() )
-  {
-    mapUnit = QGis::NauticalMiles;
-  }
-  else
-  {
-    mapUnit = QGis::Meters;
-  }
-
-  mMapCanvas->setMapUnits( mapUnit );
   mMapCanvas->setCrsTransformEnabled( cbxProjectionEnabled->isChecked() );
 
   mMapCanvas->enableMapTileRendering( mMapTileRenderingCheckBox->isChecked() );
@@ -781,8 +757,24 @@ void QgsProjectProperties::apply()
   // can be used instead of the two part technique here
   QgsProject::instance()->writeEntry( "PositionPrecision", "/Automatic", radAutomatic->isChecked() );
   QgsProject::instance()->writeEntry( "PositionPrecision", "/DecimalPlaces", spinBoxDP->value() );
-  QgsProject::instance()->writeEntry( "PositionPrecision", "/DegreeFormat",
-                                      QString( radDM->isChecked() ? "DM" : radDMS->isChecked() ? "DMS" : "D" ) );
+  QString degreeFormat;
+  switch ( static_cast< CoordinateFormat >( mCoordinateDisplayComboBox->itemData( mCoordinateDisplayComboBox->currentIndex() ).toInt() ) )
+  {
+    case DegreesMinutes:
+      degreeFormat = "DM";
+      break;
+    case DegreesMinutesSeconds:
+      degreeFormat = "DMS";
+      break;
+    case MapUnits:
+      degreeFormat = "MU";
+      break;
+    case DecimalDegrees:
+    default:
+      degreeFormat = "D";
+      break;
+  }
+  QgsProject::instance()->writeEntry( "PositionPrecision", "/DegreeFormat", degreeFormat );
 
   // Announce that we may have a new display precision setting
   emit displayPrecisionChanged();
@@ -1148,7 +1140,6 @@ void QgsProjectProperties::showProjectionsTab()
 void QgsProjectProperties::on_cbxProjectionEnabled_toggled( bool onFlyEnabled )
 {
   QString measureOnFlyState = tr( "Measure tool (CRS transformation: %1)" );
-  QString unitsOnFlyState = tr( "Canvas units (CRS transformation: %1)" );
   if ( !onFlyEnabled )
   {
     // reset projection to default
@@ -1169,20 +1160,10 @@ void QgsProjectProperties::on_cbxProjectionEnabled_toggled( bool onFlyEnabled )
     mProjectSrsId = mLayerSrsId;
     projectionSelector->setSelectedCrsId( mLayerSrsId );
 
-    QgsCoordinateReferenceSystem srs( mLayerSrsId, QgsCoordinateReferenceSystem::InternalCrsId );
-    //set radio button to crs map unit type
-    QGis::UnitType units = srs.mapUnits();
-
-    radMeters->setChecked( units == QGis::Meters );
-    radFeet->setChecked( units == QGis::Feet );
-    radNMiles->setChecked( units == QGis::NauticalMiles );
-    radDegrees->setChecked( units == QGis::Degrees );
-
     // unset ellipsoid
     mEllipsoidIndex = 0;
 
     btnGrpMeasureEllipsoid->setTitle( measureOnFlyState.arg( tr( "OFF" ) ) );
-    btnGrpMapUnits->setTitle( unitsOnFlyState.arg( tr( "OFF" ) ) );
   }
   else
   {
@@ -1193,14 +1174,12 @@ void QgsProjectProperties::on_cbxProjectionEnabled_toggled( bool onFlyEnabled )
     projectionSelector->setSelectedCrsId( mProjectSrsId );
 
     btnGrpMeasureEllipsoid->setTitle( measureOnFlyState.arg( tr( "ON" ) ) );
-    btnGrpMapUnits->setTitle( unitsOnFlyState.arg( tr( "ON" ) ) );
   }
 
-  setMapUnitsToCurrentProjection();
+  srIdUpdated();
 
   // Enable/Disable selector and update tool-tip
   updateEllipsoidUI( mEllipsoidIndex ); // maybe already done by setMapUnitsToCurrentProjection
-
 }
 
 void QgsProjectProperties::cbxWFSPubliedStateChanged( int aIdx )
@@ -1237,7 +1216,33 @@ void QgsProjectProperties::cbxWCSPubliedStateChanged( int aIdx )
   }
 }
 
-void QgsProjectProperties::setMapUnitsToCurrentProjection()
+void QgsProjectProperties::updateGuiForMapUnits( QGis::UnitType units )
+{
+  int idx = mCoordinateDisplayComboBox->findData( MapUnits );
+  if ( units == QGis::Degrees )
+  {
+    //remove map units option from coordinate display combo
+    if ( idx >= 0 )
+    {
+      mCoordinateDisplayComboBox->removeItem( idx );
+    }
+  }
+  else
+  {
+    //make sure map units option is shown in coordinate display combo
+    QString mapUnitString = tr( "Map units (%1)" ).arg( QGis::tr( units ) );
+    if ( idx < 0 )
+    {
+      mCoordinateDisplayComboBox->insertItem( 0, mapUnitString, MapUnits );
+    }
+    else
+    {
+      mCoordinateDisplayComboBox->setItemText( idx, mapUnitString );
+    }
+  }
+}
+
+void QgsProjectProperties::srIdUpdated()
 {
   long myCRSID = projectionSelector->selectedCrsId();
   if ( !isProjected() || !myCRSID )
@@ -1247,10 +1252,7 @@ void QgsProjectProperties::setMapUnitsToCurrentProjection()
   //set radio button to crs map unit type
   QGis::UnitType units = srs.mapUnits();
 
-  radMeters->setChecked( units == QGis::Meters );
-  radFeet->setChecked( units == QGis::Feet );
-  radNMiles->setChecked( units == QGis::NauticalMiles );
-  radDegrees->setChecked( units == QGis::Degrees );
+  updateGuiForMapUnits( units );
 
   // attempt to reset the projection ellipsoid according to the srs
   int myIndex = 0;

src/app/qgsprojectproperties.h

@@ -156,7 +156,7 @@ class APP_EXPORT QgsProjectProperties : public QgsOptionsDialogBase, private Ui:
     /*!
       * If user changes the CRS, set the corresponding map units
       */
-    void setMapUnitsToCurrentProjection();
+    void srIdUpdated();
 
     /* Update ComboBox accorindg to the selected new index
      * Also sets the new selected Ellipsoid. */
@@ -180,6 +180,16 @@ class APP_EXPORT QgsProjectProperties : public QgsOptionsDialogBase, private Ui:
     void refresh();
 
   private:
+
+    //! Formats for displaying coordinates
+    enum CoordinateFormat
+    {
+      DecimalDegrees, /*!< Decimal degrees */
+      DegreesMinutes, /*!< Degrees, decimal minutes */
+      DegreesMinutesSeconds, /*!< Degrees, minutes, seconds */
+      MapUnits, /*! Show coordinates in map units */
+    };
+
     QgsRelationManagerDialog *mRelationManagerDlg;
     QgsMapCanvas* mMapCanvas;
     QgsStyleV2* mStyle;
@@ -230,4 +240,5 @@ class APP_EXPORT QgsProjectProperties : public QgsOptionsDialogBase, private Ui:
 
     static const char * GEO_NONE_DESC;
 
+    void updateGuiForMapUnits( QGis::UnitType units );
 };

src/app/qgsstatusbarcoordinateswidget.cpp

@@ -26,6 +26,7 @@
 #include "qgsapplication.h"
 #include "qgsmapcanvas.h"
 #include "qgsproject.h"
+#include "qgscoordinateutils.h"
 
 
 QgsStatusBarCoordinatesWidget::QgsStatusBarCoordinatesWidget( QWidget *parent )
@@ -225,30 +226,8 @@ void QgsStatusBarCoordinatesWidget::showMouseCoordinates( const QgsPoint & p )
     return;
   }
 
-  if ( mMapCanvas->mapUnits() == QGis::Degrees )
-  {
-    if ( !mMapCanvas->mapSettings().destinationCrs().isValid() )
-      return;
-
-    QgsPoint geo = p;
-    if ( !mMapCanvas->mapSettings().destinationCrs().geographicFlag() )
-    {
-      QgsCoordinateTransform ct( mMapCanvas->mapSettings().destinationCrs(), QgsCoordinateReferenceSystem( GEOSRID ) );
-      geo = ct.transform( p );
-    }
-    QString format = QgsProject::instance()->readEntry( "PositionPrecision", "/DegreeFormat", "D" );
-
-    if ( format == "DM" )
-      mLineEdit->setText( geo.toDegreesMinutes( mMousePrecisionDecimalPlaces ) );
-    else if ( format == "DMS" )
-      mLineEdit->setText( geo.toDegreesMinutesSeconds( mMousePrecisionDecimalPlaces ) );
-    else
-      mLineEdit->setText( geo.toString( mMousePrecisionDecimalPlaces ) );
-  }
-  else
-  {
-    mLineEdit->setText( p.toString( mMousePrecisionDecimalPlaces ) );
-  }
+  mLineEdit->setText( QgsCoordinateUtils::formatCoordinateForProject( p, mMapCanvas->mapSettings().destinationCrs(),
+                      mMousePrecisionDecimalPlaces ) );
 
   if ( mLineEdit->width() > mLineEdit->minimumWidth() )
   {

src/core/CMakeLists.txt

@@ -88,6 +88,7 @@ SET(QGIS_CORE_SRCS
   qgscontexthelp_texts.cpp
   qgscoordinatereferencesystem.cpp
   qgscoordinatetransform.cpp
+  qgscoordinateutils.cpp
   qgscredentials.cpp
   qgscrscache.cpp
   qgsdartmeasurement.cpp
@@ -585,6 +586,7 @@ SET(QGIS_CORE_HDRS
   qgscontexthelp.h
   qgsconditionalstyle.h
   qgscoordinatereferencesystem.h
+  qgscoordinateutils.h
   qgscrscache.h
   qgscsexception.h
   qgsdartmeasurement.h

src/core/qgscoordinatereferencesystem.h

@@ -325,6 +325,7 @@ class CORE_EXPORT QgsCoordinateReferenceSystem
     /** Returns whether the CRS is a geographic CRS.
      * @returns true if CRS is geographic, or false if it is a projected CRS
      */
+    //TODO QGIS 3.0 - rename to isGeographic
     bool geographicFlag() const;
 
     /** Returns whether axis is inverted (eg. for WMS 1.3) for the CRS.