Snapping tolerance: Fix "map units" vs "layer units" dilemma (fixes #…

…11634) Until now "map units" in snapping config dialog actually referred to layer units. From now on, "map units" refer to project's CRS units. Where appropriate, it is possible to choose "layer units" that refer to layer CRS units. Projects from older versions of QGIS should be handled seamlessly (tolerances in layer units will be still handled in layer units, not project units). In API, QgsTolerance::MapUnits is deprecated as it is unclear to what units it refers to (should be ProjectUnits, but actually it is LayerUnits)
qgis · Jan 23, 2015 · 334d885 · 334d885
1 parent 7e0f646
commit 334d885
Show file tree

Hide file tree

Showing 8 changed files with 93 additions and 38 deletions.
diff --git a/python/core/qgstolerance.sip b/python/core/qgstolerance.sip
@@ -6,13 +6,19 @@ class QgsTolerance
 %End
 
   public:
-    /**Type of unit of tolerance value from settings*/
+    /** Type of unit of tolerance value from settings.
+     * MapUnits is slightly confusing, because it actually refers to layer units (historically).
+     * For map (project) units, use ProjectUnits. Try to avoid using MapUnits value and use LayerUnits instead. */
     enum UnitType
     {
-      /**Map unit value*/
+      /**Layer unit value. @note deprecated: use LayerUnits */
       MapUnits,
+      /**Layer unit value */
+      LayerUnits,
       /**Pixels unit of tolerance*/
-      Pixels
+      Pixels,
+      /**Map (project) units. Added in 2.8 */
+      ProjectUnits
     };
 
     /**
@@ -56,12 +62,13 @@ class QgsTolerance
     /**
     * Static function to translate tolerance value into map units
     * @param tolerance tolerance value to be translated
+    * @param layer source layer necessary in case tolerance is in layer units
     * @param mapSettings settings of the map
     * @param units type of units to be translated
     * @return value of tolerance in map units
     * @note added in 2.8
     */
-    static double toleranceInMapUnits( double tolerance, const QgsMapSettings& mapSettings, QgsTolerance::UnitType units );
+    static double toleranceInProjectUnits( double tolerance, QgsMapLayer* layer, const QgsMapSettings& mapSettings, QgsTolerance::UnitType units );
 
     /**
     * Static function to translate tolerance value into layer units
@@ -71,7 +78,7 @@ class QgsTolerance
     * @param units type of units to be translated
     * @return value of tolerance in layer units
     */
-    static double toleranceInMapUnits( double tolerance, QgsMapLayer* layer, const QgsMapSettings& mapSettings, UnitType units = MapUnits );
+    static double toleranceInMapUnits( double tolerance, QgsMapLayer* layer, const QgsMapSettings& mapSettings, UnitType units = LayerUnits );
 
     /**
     * Static function to translate tolerance value into layer units
@@ -82,5 +89,5 @@ class QgsTolerance
     * @return value of tolerance in layer units
     */
     //! @deprecated since 2.4 - use the override with QgsMapSettings
-    static double toleranceInMapUnits( double tolerance, QgsMapLayer* layer, QgsMapRenderer* renderer, UnitType units = MapUnits ) /Deprecated/;
+    static double toleranceInMapUnits( double tolerance, QgsMapLayer* layer, QgsMapRenderer* renderer, UnitType units = LayerUnits ) /Deprecated/;
 };
diff --git a/src/app/qgsoptions.cpp b/src/app/qgsoptions.cpp
@@ -801,7 +801,8 @@ QgsOptions::QgsOptions( QWidget *parent, Qt::WindowFlags fl ) :
   mDefaultSnapModeComboBox->setCurrentIndex( mDefaultSnapModeComboBox->findData( defaultSnapString ) );
   mDefaultSnappingToleranceSpinBox->setValue( settings.value( "/qgis/digitizing/default_snapping_tolerance", 0 ).toDouble() );
   mSearchRadiusVertexEditSpinBox->setValue( settings.value( "/qgis/digitizing/search_radius_vertex_edit", 10 ).toDouble() );
-  if ( settings.value( "/qgis/digitizing/default_snapping_tolerance_unit", 0 ).toInt() == QgsTolerance::MapUnits )
+  int defSnapUnits = settings.value( "/qgis/digitizing/default_snapping_tolerance_unit", QgsTolerance::ProjectUnits ).toInt();
+  if ( defSnapUnits == QgsTolerance::ProjectUnits || defSnapUnits == QgsTolerance::LayerUnits )
   {
     index = mDefaultSnappingToleranceComboBox->findText( tr( "map units" ) );
   }
@@ -810,7 +811,8 @@ QgsOptions::QgsOptions( QWidget *parent, Qt::WindowFlags fl ) :
     index = mDefaultSnappingToleranceComboBox->findText( tr( "pixels" ) );
   }
   mDefaultSnappingToleranceComboBox->setCurrentIndex( index );
-  if ( settings.value( "/qgis/digitizing/search_radius_vertex_edit_unit", QgsTolerance::Pixels ).toInt() == QgsTolerance::MapUnits )
+  int defRadiusUnits = settings.value( "/qgis/digitizing/search_radius_vertex_edit_unit", QgsTolerance::Pixels ).toInt();
+  if ( defRadiusUnits == QgsTolerance::ProjectUnits || defSnapUnits == QgsTolerance::LayerUnits )
   {
     index = mSearchRadiusVertexEditComboBox->findText( tr( "map units" ) );
   }
@@ -1272,9 +1274,9 @@ void QgsOptions::saveOptions()
   settings.setValue( "/qgis/digitizing/default_snapping_tolerance", mDefaultSnappingToleranceSpinBox->value() );
   settings.setValue( "/qgis/digitizing/search_radius_vertex_edit", mSearchRadiusVertexEditSpinBox->value() );
   settings.setValue( "/qgis/digitizing/default_snapping_tolerance_unit",
-                     ( mDefaultSnappingToleranceComboBox->currentIndex() == 0 ? QgsTolerance::MapUnits : QgsTolerance::Pixels ) );
+                     ( mDefaultSnappingToleranceComboBox->currentIndex() == 0 ? QgsTolerance::ProjectUnits : QgsTolerance::Pixels ) );
   settings.setValue( "/qgis/digitizing/search_radius_vertex_edit_unit",
-                     ( mSearchRadiusVertexEditComboBox->currentIndex()  == 0 ? QgsTolerance::MapUnits : QgsTolerance::Pixels ) );
+                     ( mSearchRadiusVertexEditComboBox->currentIndex()  == 0 ? QgsTolerance::ProjectUnits : QgsTolerance::Pixels ) );
 
   settings.setValue( "/qgis/digitizing/marker_only_for_selected", mMarkersOnlyForSelectedCheckBox->isChecked() );
 

diff --git a/src/app/qgssnappingdialog.cpp b/src/app/qgssnappingdialog.cpp
@@ -137,9 +137,9 @@ void QgsSnappingDialog::reload()
   tolerance = QgsProject::instance()->readDoubleEntry( "Digitizing", "/DefaultSnapTolerance", tolerance );
   mDefaultSnappingToleranceSpinBox->setValue( tolerance );
 
-  int unit = settings.value( "/qgis/digitizing/default_snapping_tolerance_unit", 0 ).toInt();
+  int unit = settings.value( "/qgis/digitizing/default_snapping_tolerance_unit", QgsTolerance::ProjectUnits ).toInt();
   unit = QgsProject::instance()->readNumEntry( "Digitizing", "/DefaultSnapToleranceUnit", unit );
-  mDefaultSnappingToleranceComboBox->setCurrentIndex( unit );
+  mDefaultSnappingToleranceComboBox->setCurrentIndex( unit == QgsTolerance::Pixels ? 1 : 0 );
 
   mLayerTreeWidget->clear();
 
@@ -182,7 +182,7 @@ void QgsSnappingDialog::initNewProject()
   QgsProject::instance()->writeEntry( "Digitizing", "/DefaultSnapType", snapType );
   double tolerance = settings.value( "/qgis/digitizing/default_snapping_tolerance", 0 ).toDouble();
   QgsProject::instance()->writeEntry( "Digitizing", "/DefaultSnapTolerance", tolerance );
-  int unit = settings.value( "/qgis/digitizing/default_snapping_tolerance_unit", 0 ).toInt();
+  int unit = settings.value( "/qgis/digitizing/default_snapping_tolerance_unit", QgsTolerance::ProjectUnits ).toInt();
   QgsProject::instance()->writeEntry( "Digitizing", "/DefaultSnapToleranceUnit", unit );
 
   reload();
@@ -215,7 +215,7 @@ void QgsSnappingDialog::apply()
   QString snapType = mDefaultSnapToComboBox->itemData( mDefaultSnapToComboBox->currentIndex() ).toString();
   QgsProject::instance()->writeEntry( "Digitizing", "/DefaultSnapType", snapType );
   QgsProject::instance()->writeEntry( "Digitizing", "/DefaultSnapTolerance", mDefaultSnappingToleranceSpinBox->value() );
-  QgsProject::instance()->writeEntry( "Digitizing", "/DefaultSnapToleranceUnit", mDefaultSnappingToleranceComboBox->currentIndex() );
+  QgsProject::instance()->writeEntry( "Digitizing", "/DefaultSnapToleranceUnit", mDefaultSnappingToleranceComboBox->currentIndex() == 1 ? QgsTolerance::Pixels : QgsTolerance::ProjectUnits );
 
 
   QStringList layerIdList;
@@ -314,7 +314,7 @@ void QgsSnappingDialog::addLayer( QgsMapLayer *theMapLayer )
   QSettings myQsettings;
   bool myDockFlag = myQsettings.value( "/qgis/dockSnapping", false ).toBool();
   double defaultSnappingTolerance = myQsettings.value( "/qgis/digitizing/default_snapping_tolerance", 0 ).toDouble();
-  int defaultSnappingUnit = myQsettings.value( "/qgis/digitizing/default_snapping_tolerance_unit", 0 ).toInt();
+  int defaultSnappingUnit = myQsettings.value( "/qgis/digitizing/default_snapping_tolerance_unit", QgsTolerance::ProjectUnits ).toInt();
   QString defaultSnappingString = myQsettings.value( "/qgis/digitizing/default_snap_mode", "to vertex" ).toString();
 
   int defaultSnappingStringIdx = 0;
@@ -366,8 +366,9 @@ void QgsSnappingDialog::addLayer( QgsMapLayer *theMapLayer )
 
   //snap to vertex/ snap to segment
   QComboBox *cbxUnits = new QComboBox( mLayerTreeWidget );
-  cbxUnits->insertItem( 0, tr( "map units" ) );
+  cbxUnits->insertItem( 0, tr( "layer units" ) );
   cbxUnits->insertItem( 1, tr( "pixels" ) );
+  cbxUnits->insertItem( 2, tr( "map units" ) );
   cbxUnits->setCurrentIndex( defaultSnappingUnit );
   mLayerTreeWidget->setItemWidget( item, 4, cbxUnits );
 

diff --git a/src/core/qgsproject.cpp b/src/core/qgsproject.cpp
@@ -1931,7 +1931,7 @@ void QgsProject::setSnapSettingsForLayer( const QString& layerId, bool enabled,
   snapTypeList.append( typeString );
 
   //units
-  toleranceUnitList.append( unit == QgsTolerance::Pixels ? "1" : "0" );
+  toleranceUnitList.append( QString::number( unit ) );
 
   //tolerance
   toleranceList.append( QString::number( tolerance ) );
@@ -1993,9 +1993,13 @@ bool QgsProject::snapSettingsForLayer( const QString& layerId, bool& enabled, Qg
   {
     units = QgsTolerance::Pixels;
   }
+  else if ( toleranceUnitList.at( idx ) == "2" )
+  {
+    units = QgsTolerance::ProjectUnits;
+  }
   else
   {
-    units = QgsTolerance::MapUnits;
+    units = QgsTolerance::LayerUnits;
   }
 
   //tolerance

diff --git a/src/core/qgssnappingutils.cpp b/src/core/qgssnappingutils.cpp
@@ -216,7 +216,7 @@ QgsPointLocator::Match QgsSnappingUtils::snapToMap( const QgsPoint& pointMap, Qg
     prepareIndex( QList<QgsVectorLayer*>() << mCurrentLayer );
 
     // data from project
-    double tolerance = QgsTolerance::toleranceInMapUnits( mDefaultTolerance, mMapSettings, mDefaultUnit );
+    double tolerance = QgsTolerance::toleranceInProjectUnits( mDefaultTolerance, mCurrentLayer, mMapSettings, mDefaultUnit );
     int type = mDefaultType;
 
     // use ad-hoc locator
@@ -249,7 +249,7 @@ QgsPointLocator::Match QgsSnappingUtils::snapToMap( const QgsPoint& pointMap, Qg
 
     foreach ( const LayerConfig& layerConfig, mLayers )
     {
-      double tolerance = QgsTolerance::toleranceInMapUnits( layerConfig.tolerance, mMapSettings, layerConfig.unit );
+      double tolerance = QgsTolerance::toleranceInProjectUnits( layerConfig.tolerance, layerConfig.layer, mMapSettings, layerConfig.unit );
       if ( QgsPointLocator* loc = locatorForLayerUsingStrategy( layerConfig.layer, pointMap, tolerance ) )
       {
         _updateBestMatch( bestMatch, pointMap, loc, layerConfig.type, tolerance, filter );
@@ -270,7 +270,7 @@ QgsPointLocator::Match QgsSnappingUtils::snapToMap( const QgsPoint& pointMap, Qg
   else if ( mSnapToMapMode == SnapAllLayers )
   {
     // data from project
-    double tolerance = QgsTolerance::toleranceInMapUnits( mDefaultTolerance, mMapSettings, mDefaultUnit );
+    double tolerance = QgsTolerance::toleranceInProjectUnits( mDefaultTolerance, 0, mMapSettings, mDefaultUnit );
     int type = mDefaultType;
 
     QList<QgsVectorLayer*> layers;
@@ -360,6 +360,10 @@ void QgsSnappingUtils::setMapSettings( const QgsMapSettings& settings )
 
 void QgsSnappingUtils::setDefaultSettings( int type, double tolerance, QgsTolerance::UnitType unit )
 {
+  // force map units - can't use layer units for just any layer
+  if ( unit == QgsTolerance::LayerUnits )
+    unit = QgsTolerance::ProjectUnits;
+
   mDefaultType = type;
   mDefaultTolerance = tolerance;
   mDefaultUnit = unit;
@@ -394,7 +398,7 @@ void QgsSnappingUtils::readConfigFromProject()
   else if ( snapType == "to vertex" )
     type = QgsPointLocator::Vertex;
   double tolerance = QgsProject::instance()->readDoubleEntry( "Digitizing", "/DefaultSnapTolerance", 0 );
-  QgsTolerance::UnitType unit = ( QgsTolerance::UnitType ) QgsProject::instance()->readNumEntry( "Digitizing", "/DefaultSnapToleranceUnit", 0 );
+  QgsTolerance::UnitType unit = ( QgsTolerance::UnitType ) QgsProject::instance()->readNumEntry( "Digitizing", "/DefaultSnapToleranceUnit", QgsTolerance::ProjectUnits );
   setDefaultSettings( type, tolerance, unit );
 
   //snapping on intersection on?

diff --git a/src/core/qgstolerance.cpp b/src/core/qgstolerance.cpp
@@ -19,24 +19,52 @@
 #include <cmath>
 
 
-double QgsTolerance::toleranceInMapUnits( double tolerance, const QgsMapSettings& mapSettings, QgsTolerance::UnitType units )
+// return ratio [mu/lu] between map units and layer units
+// this is of course only an approximation
+double _ratioMU2LU( const QgsMapSettings& mapSettings, QgsMapLayer* layer )
+{
+  double distMU = mapSettings.mapUnitsPerPixel();
+  QgsPoint ptMapCenterMU = mapSettings.visibleExtent().center();
+  QgsPoint ptMapCenterRightMU( ptMapCenterMU.x() + distMU, ptMapCenterMU.y() );
+  QgsPoint ptMapCenterLU = mapSettings.mapToLayerCoordinates( layer, ptMapCenterMU );
+  QgsPoint ptMapCenterRightLU = mapSettings.mapToLayerCoordinates( layer, ptMapCenterRightMU );
+  double distLU = sqrt( ptMapCenterLU.sqrDist( ptMapCenterRightLU ) );
+  double ratio = distMU / distLU;
+  return ratio;
+}
+
+double QgsTolerance::toleranceInProjectUnits(double tolerance, QgsMapLayer* layer, const QgsMapSettings& mapSettings, QgsTolerance::UnitType units )
 {
   // converts to map units
-  if ( units == MapUnits )
+  if ( units == ProjectUnits )
     return tolerance;
-  else
+  else if ( units == Pixels )
     return tolerance * mapSettings.mapUnitsPerPixel();
+  else // units == LayerUnits
+  {
+    // [mu] = [lu] * [mu/lu]
+    return tolerance * _ratioMU2LU( mapSettings, layer );
+  }
 }
 
+
 double QgsTolerance::toleranceInMapUnits( double tolerance, QgsMapLayer *layer, const QgsMapSettings& mapSettings, QgsTolerance::UnitType units )
 {
   // converts to layer units
-  if ( units == MapUnits )
+  if ( units == LayerUnits )
   {
     return tolerance;
   }
-  double mapUnitsPerPixel = computeMapUnitPerPixel( layer, mapSettings );
-  return tolerance * mapUnitsPerPixel;
+  else if ( units == Pixels )
+  {
+    double layerUnitsPerPixel = computeMapUnitPerPixel( layer, mapSettings );
+    return tolerance * layerUnitsPerPixel;
+  }
+  else // ProjectUnits
+  {
+    // [lu] = [mu] / [mu/lu]
+    return tolerance / _ratioMU2LU( mapSettings, layer );
+  }
 }
 
 double QgsTolerance::toleranceInMapUnits( double tolerance, QgsMapLayer* layer, QgsMapRenderer* renderer, UnitType units )
@@ -49,7 +77,9 @@ double QgsTolerance::vertexSearchRadius( const QgsMapSettings& mapSettings )
   QSettings settings;
   double tolerance = settings.value( "/qgis/digitizing/search_radius_vertex_edit", 10 ).toDouble();
   UnitType units = ( QgsTolerance::UnitType ) settings.value( "/qgis/digitizing/search_radius_vertex_edit_unit", QgsTolerance::Pixels ).toInt();
-  return toleranceInMapUnits( tolerance, mapSettings, units );
+  if ( units == LayerUnits )
+    units = ProjectUnits;
+  return toleranceInProjectUnits( tolerance, 0, mapSettings, units );
 }
 
 double QgsTolerance::vertexSearchRadius( QgsMapLayer *layer, const QgsMapSettings &mapSettings )
@@ -69,7 +99,7 @@ double QgsTolerance::defaultTolerance( QgsMapLayer *layer, const QgsMapSettings&
 {
   QSettings settings;
   double tolerance = settings.value( "/qgis/digitizing/default_snapping_tolerance", 0 ).toDouble();
-  UnitType units = ( QgsTolerance::UnitType ) settings.value( "/qgis/digitizing/default_snapping_tolerance_unit", 0 ).toInt();
+  UnitType units = ( QgsTolerance::UnitType ) settings.value( "/qgis/digitizing/default_snapping_tolerance_unit", ProjectUnits ).toInt();
   return toleranceInMapUnits( tolerance, layer, mapSettings, units );
 }
 

diff --git a/src/core/qgstolerance.h b/src/core/qgstolerance.h
@@ -27,13 +27,19 @@ class CORE_EXPORT QgsTolerance
 {
 
   public:
-    /**Type of unit of tolerance value from settings*/
+    /** Type of unit of tolerance value from settings.
+     * MapUnits is slightly confusing, because it actually refers to layer units (historically).
+     * For map (project) units, use ProjectUnits. Try to avoid using MapUnits value and use LayerUnits instead. */
     enum UnitType
     {
-      /**Map unit value*/
+      /**Layer unit value. @note deprecated: use LayerUnits */
       MapUnits,
+      /**Layer unit value */
+      LayerUnits = MapUnits,
       /**Pixels unit of tolerance*/
-      Pixels
+      Pixels,
+      /**Map (project) units. Added in 2.8 */
+      ProjectUnits
     };
 
     /**
@@ -77,12 +83,13 @@ class CORE_EXPORT QgsTolerance
     /**
     * Static function to translate tolerance value into map units
     * @param tolerance tolerance value to be translated
+    * @param layer source layer necessary in case tolerance is in layer units
     * @param mapSettings settings of the map
     * @param units type of units to be translated
     * @return value of tolerance in map units
     * @note added in 2.8
     */
-    static double toleranceInMapUnits( double tolerance, const QgsMapSettings& mapSettings, QgsTolerance::UnitType units );
+    static double toleranceInProjectUnits( double tolerance, QgsMapLayer* layer, const QgsMapSettings& mapSettings, QgsTolerance::UnitType units );
 
     /**
     * Static function to translate tolerance value into layer units
@@ -92,7 +99,7 @@ class CORE_EXPORT QgsTolerance
     * @param units type of units to be translated
     * @return value of tolerance in layer units
     */
-    static double toleranceInMapUnits( double tolerance, QgsMapLayer* layer, const QgsMapSettings& mapSettings, UnitType units = MapUnits );
+    static double toleranceInMapUnits( double tolerance, QgsMapLayer* layer, const QgsMapSettings& mapSettings, UnitType units = LayerUnits );
 
     /**
     * Static function to translate tolerance value into layer units
@@ -103,7 +110,7 @@ class CORE_EXPORT QgsTolerance
     * @return value of tolerance in layer units
     */
     //! @deprecated since 2.4 - use the override with QgsMapSettings
-    Q_DECL_DEPRECATED static double toleranceInMapUnits( double tolerance, QgsMapLayer* layer, QgsMapRenderer* renderer, UnitType units = MapUnits );
+    Q_DECL_DEPRECATED static double toleranceInMapUnits( double tolerance, QgsMapLayer* layer, QgsMapRenderer* renderer, UnitType units = LayerUnits );
 
   private:
     static double computeMapUnitPerPixel( QgsMapLayer* layer, const QgsMapSettings& mapSettings );

diff --git a/src/gui/qgsmapcanvassnapper.cpp b/src/gui/qgsmapcanvassnapper.cpp
@@ -92,7 +92,7 @@ int QgsMapCanvasSnapper::snapToCurrentLayer( const QPoint& p, QList<QgsSnappingR
   QgsSnapper::SnapLayer snapLayer;
   snapLayer.mLayer = vlayer;
   snapLayer.mSnapTo = snap_to;
-  snapLayer.mUnitType = QgsTolerance::MapUnits;
+  snapLayer.mUnitType = QgsTolerance::LayerUnits;
 
   if ( snappingTol < 0 )
   {
@@ -250,7 +250,7 @@ int QgsMapCanvasSnapper::snapToBackgroundLayers( const QgsPoint& point, QList<Qg
 
     //default snapping tolerance (returned in map units)
     snapLayer.mTolerance = QgsTolerance::defaultTolerance( currentVectorLayer, mMapCanvas->mapSettings() );
-    snapLayer.mUnitType = QgsTolerance::MapUnits;
+    snapLayer.mUnitType = QgsTolerance::LayerUnits;
 
     snapLayers.append( snapLayer );
   }