Skip to content

Commit

Permalink
Snapping tolerance: Fix "map units" vs "layer units" dilemma (fixes #…
Browse files Browse the repository at this point in the history
…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)
  • Loading branch information
wonder-sk committed Jan 23, 2015
1 parent 7e0f646 commit 334d885
Show file tree
Hide file tree
Showing 8 changed files with 93 additions and 38 deletions.
19 changes: 13 additions & 6 deletions python/core/qgstolerance.sip
Original file line number Diff line number Diff line change
Expand Up @@ -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
};

/**
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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/;
};
10 changes: 6 additions & 4 deletions src/app/qgsoptions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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" ) );
}
Expand All @@ -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" ) );
}
Expand Down Expand Up @@ -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() );

Expand Down
13 changes: 7 additions & 6 deletions src/app/qgssnappingdialog.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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 );

Expand Down
8 changes: 6 additions & 2 deletions src/core/qgsproject.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 ) );
Expand Down Expand Up @@ -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
Expand Down
12 changes: 8 additions & 4 deletions src/core/qgssnappingutils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 );
Expand All @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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?
Expand Down
46 changes: 38 additions & 8 deletions src/core/qgstolerance.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 )
Expand All @@ -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 )
Expand All @@ -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 );
}

Expand Down
19 changes: 13 additions & 6 deletions src/core/qgstolerance.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
};

/**
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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 );
Expand Down
4 changes: 2 additions & 2 deletions src/gui/qgsmapcanvassnapper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 )
{
Expand Down Expand Up @@ -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 );
}
Expand Down

0 comments on commit 334d885

Please sign in to comment.