Skip to content

Commit

Permalink
[FEATURE] Also show cartesian areas/perimeters in identify results
Browse files Browse the repository at this point in the history 
Because users are still getting confused with the difference
between the cartesian areas and ellipsoidal areas, show both
in the identify results dock.

The reasoning here is:

- if a user understands this concept, they will know the correct
one to use, and its good for us to inform them of the difference
here. Plus, it means they immediately see if the ellipsoid
setting is correct and the difference it is making for the
area/length calculation.

- if a user has no idea what the difference is, then we should
make them aware that there's (at least!) two different possible
measurement values. They can then either research what these mean and make
the right choice (and become better informed GIS practitioners),
OR pick a random one - which really is no different then the
previous situation, because an uniformed user is just as likely
to be working in an unsuitable projection with a poor ellipsoid
choice.

In short, we don't try to guess the right choice for users
and instead give them all the information and let them make the
call which value to use.
  • Loading branch information
@nyalldawson
nyalldawson committed May 8, 2018
1 parent 98fc858 commit f9be605
Show file tree
Hide file tree
Showing 3 changed files with 131 additions and 21 deletions.
59 changes: 47 additions & 12 deletions src/gui/qgsmaptoolidentify.cpp
View file
Expand Up @@ -323,7 +323,7 @@ bool QgsMapToolIdentify::identifyVectorLayer( QList<QgsMapToolIdentify::Identify
if ( isSingleClick )
derivedAttributes.unite( featureDerivedAttributes( feature, layer, toLayerCoordinates( layer, point ) ) );

derivedAttributes.insert( tr( "feature id" ), fid < 0 ? tr( "new feature" ) : FID_TO_STRING( fid ) );
derivedAttributes.insert( tr( "Feature ID" ), fid < 0 ? tr( "new feature" ) : FID_TO_STRING( fid ) );

results->append( IdentifyResult( qobject_cast<QgsMapLayer *>( layer ), feature, derivedAttributes ) );
}
Expand Down Expand Up @@ -425,12 +425,24 @@ QMap< QString, QString > QgsMapToolIdentify::featureDerivedAttributes( const Qgs
derivedAttributes.insert( tr( "Part number" ), str );
}

QgsUnitTypes::DistanceUnit cartesianDistanceUnits = QgsUnitTypes::unitType( layer->crs().mapUnits() ) == QgsUnitTypes::unitType( displayDistanceUnits() )
? displayDistanceUnits() : layer->crs().mapUnits();
QgsUnitTypes::AreaUnit cartesianAreaUnits = QgsUnitTypes::unitType( QgsUnitTypes::distanceToAreaUnit( layer->crs().mapUnits() ) ) == QgsUnitTypes::unitType( displayAreaUnits() )
? displayAreaUnits() : QgsUnitTypes::distanceToAreaUnit( layer->crs().mapUnits() );

if ( geometryType == QgsWkbTypes::LineGeometry )
{
double dist = calc.measureLength( feature.geometry() );
dist = calc.convertLengthMeasurement( dist, displayDistanceUnits() );
QString str = formatDistance( dist );
derivedAttributes.insert( tr( "Length" ), str );
QString str;
if ( ellipsoid != GEO_NONE )
{
str = formatDistance( dist );
derivedAttributes.insert( tr( "Length (Ellipsoidal, %1)" ).arg( ellipsoid ), str );
}
str = formatDistance( feature.geometry().constGet()->length()
* QgsUnitTypes::fromUnitToUnitFactor( layer->crs().mapUnits(), cartesianDistanceUnits ), cartesianDistanceUnits );
derivedAttributes.insert( tr( "Length (Cartesian)" ), str );

const QgsAbstractGeometry *geom = feature.geometry().constGet();
if ( geom )
Expand Down Expand Up @@ -460,13 +472,26 @@ QMap< QString, QString > QgsMapToolIdentify::featureDerivedAttributes( const Qgs
{
double area = calc.measureArea( feature.geometry() );
area = calc.convertAreaMeasurement( area, displayAreaUnits() );
QString str = formatArea( area );
derivedAttributes.insert( tr( "Area" ), str );
QString str;
if ( ellipsoid != GEO_NONE )
{
str = formatArea( area );
derivedAttributes.insert( tr( "Area (Ellipsoidal, %1)" ).arg( ellipsoid ), str );
}
str = formatArea( feature.geometry().area()
* QgsUnitTypes::fromUnitToUnitFactor( QgsUnitTypes::distanceToAreaUnit( layer->crs().mapUnits() ), cartesianAreaUnits ), cartesianAreaUnits );
derivedAttributes.insert( tr( "Area (Cartesian)" ), str );

double perimeter = calc.measurePerimeter( feature.geometry() );
perimeter = calc.convertLengthMeasurement( perimeter, displayDistanceUnits() );
str = formatDistance( perimeter );
derivedAttributes.insert( tr( "Perimeter" ), str );
if ( ellipsoid != GEO_NONE )
{
double perimeter = calc.measurePerimeter( feature.geometry() );
perimeter = calc.convertLengthMeasurement( perimeter, displayDistanceUnits() );
str = formatDistance( perimeter );
derivedAttributes.insert( tr( "Perimeter (Ellipsoidal, %1)" ).arg( ellipsoid ), str );
}
str = formatDistance( feature.geometry().constGet()->perimeter()
* QgsUnitTypes::fromUnitToUnitFactor( layer->crs().mapUnits(), cartesianDistanceUnits ), cartesianDistanceUnits );
derivedAttributes.insert( tr( "Perimeter (Cartesian)" ), str );

str = QLocale::system().toString( feature.geometry().constGet()->nCoordinates() );
derivedAttributes.insert( tr( "Vertices" ), str );
Expand Down Expand Up @@ -738,19 +763,29 @@ QgsUnitTypes::AreaUnit QgsMapToolIdentify::displayAreaUnits() const
}

QString QgsMapToolIdentify::formatDistance( double distance ) const
{
return formatDistance( distance, displayDistanceUnits() );
}

QString QgsMapToolIdentify::formatArea( double area ) const
{
return formatArea( area, displayAreaUnits() );
}

QString QgsMapToolIdentify::formatDistance( double distance, QgsUnitTypes::DistanceUnit unit ) const
{
QgsSettings settings;
bool baseUnit = settings.value( QStringLiteral( "qgis/measure/keepbaseunit" ), true ).toBool();

return QgsDistanceArea::formatDistance( distance, 3, displayDistanceUnits(), baseUnit );
return QgsDistanceArea::formatDistance( distance, 3, unit, baseUnit );
}

QString QgsMapToolIdentify::formatArea( double area ) const
QString QgsMapToolIdentify::formatArea( double area, QgsUnitTypes::AreaUnit unit ) const
{
QgsSettings settings;
bool baseUnit = settings.value( QStringLiteral( "qgis/measure/keepbaseunit" ), true ).toBool();

return QgsDistanceArea::formatArea( area, 3, displayAreaUnits(), baseUnit );
return QgsDistanceArea::formatArea( area, 3, unit, baseUnit );
}

void QgsMapToolIdentify::formatChanged( QgsRasterLayer *layer )
Expand Down
12 changes: 12 additions & 0 deletions src/gui/qgsmaptoolidentify.h
View file
Expand Up @@ -199,6 +199,18 @@ class GUI_EXPORT QgsMapToolIdentify : public QgsMapTool
*/
QString formatArea( double area ) const;

/**
* Format a distance into a suitable string for display to the user
* \see formatArea()
*/
QString formatDistance( double distance, QgsUnitTypes::DistanceUnit unit ) const;

/**
* Format a distance into a suitable string for display to the user
* \see formatDistance()
*/
QString formatArea( double area, QgsUnitTypes::AreaUnit unit ) const;

QMap< QString, QString > featureDerivedAttributes( const QgsFeature &feature, QgsMapLayer *layer, const QgsPointXY &layerPoint = QgsPointXY() );

/**
Expand Down
81 changes: 72 additions & 9 deletions tests/src/app/testqgsmaptoolidentifyaction.cpp
View file
Expand Up @@ -239,25 +239,46 @@ void TestQgsMapToolIdentifyAction::lengthCalculation()
std::unique_ptr< QgsMapToolIdentifyAction > action( new QgsMapToolIdentifyAction( canvas ) );
QList<QgsMapToolIdentify::IdentifyResult> result = action->identify( mapPoint.x(), mapPoint.y(), QList<QgsMapLayer *>() << tempLayer.get() );
QCOMPARE( result.length(), 1 );
QString derivedLength = result.at( 0 ).mDerivedAttributes[tr( "Length" )];
QString derivedLength = result.at( 0 ).mDerivedAttributes[tr( "Length (Ellipsoidal, WGS84)" )];
double length = derivedLength.remove( ',' ).split( ' ' ).at( 0 ).toDouble();
QGSCOMPARENEAR( length, 26932.2, 0.1 );
derivedLength = result.at( 0 ).mDerivedAttributes[tr( "Length (Cartesian)" )];
length = derivedLength.remove( ',' ).split( ' ' ).at( 0 ).toDouble();
QGSCOMPARENEAR( length, 26930.6, 0.1 );

//check that project units are respected
QgsProject::instance()->setDistanceUnits( QgsUnitTypes::DistanceFeet );
result = action->identify( mapPoint.x(), mapPoint.y(), QList<QgsMapLayer *>() << tempLayer.get() );
QCOMPARE( result.length(), 1 );
derivedLength = result.at( 0 ).mDerivedAttributes[tr( "Length" )];
derivedLength = result.at( 0 ).mDerivedAttributes[tr( "Length (Ellipsoidal, WGS84)" )];
length = derivedLength.remove( ',' ).split( ' ' ).at( 0 ).toDouble();
QGSCOMPARENEAR( length, 88360.1, 0.1 );
derivedLength = result.at( 0 ).mDerivedAttributes[tr( "Length (Cartesian)" )];
length = derivedLength.remove( ',' ).split( ' ' ).at( 0 ).toDouble();
QGSCOMPARENEAR( length, 88355.1, 0.1 );

//test unchecked "keep base units" setting
s.setValue( QStringLiteral( "/qgis/measure/keepbaseunit" ), false );
result = action->identify( mapPoint.x(), mapPoint.y(), QList<QgsMapLayer *>() << tempLayer.get() );
QCOMPARE( result.length(), 1 );
derivedLength = result.at( 0 ).mDerivedAttributes[tr( "Length" )];
derivedLength = result.at( 0 ).mDerivedAttributes[tr( "Length (Ellipsoidal, WGS84)" )];
length = derivedLength.remove( ',' ).split( ' ' ).at( 0 ).toDouble();
QGSCOMPARENEAR( length, 16.735, 0.001 );
derivedLength = result.at( 0 ).mDerivedAttributes[tr( "Length (Cartesian)" )];
length = derivedLength.remove( ',' ).split( ' ' ).at( 0 ).toDouble();
QGSCOMPARENEAR( length, 16.734000, 0.001 );

// no conversion of cartesian lengths between unit types
s.setValue( QStringLiteral( "/qgis/measure/keepbaseunit" ), true );
QgsProject::instance()->setDistanceUnits( QgsUnitTypes::DistanceDegrees );
result = action->identify( mapPoint.x(), mapPoint.y(), QList<QgsMapLayer *>() << tempLayer.get() );
QCOMPARE( result.length(), 1 );
derivedLength = result.at( 0 ).mDerivedAttributes[tr( "Length (Ellipsoidal, WGS84)" )];
length = derivedLength.remove( ',' ).split( ' ' ).at( 0 ).toDouble();
QGSCOMPARENEAR( length, 0.242000, 0.001 );
derivedLength = result.at( 0 ).mDerivedAttributes[tr( "Length (Cartesian)" )];
length = derivedLength.remove( ',' ).split( ' ' ).at( 0 ).toDouble();
QGSCOMPARENEAR( length, 26930.6, 0.1 );
}

void TestQgsMapToolIdentifyAction::perimeterCalculation()
Expand Down Expand Up @@ -292,25 +313,46 @@ void TestQgsMapToolIdentifyAction::perimeterCalculation()
std::unique_ptr< QgsMapToolIdentifyAction > action( new QgsMapToolIdentifyAction( canvas ) );
QList<QgsMapToolIdentify::IdentifyResult> result = action->identify( mapPoint.x(), mapPoint.y(), QList<QgsMapLayer *>() << tempLayer.get() );
QCOMPARE( result.length(), 1 );
QString derivedPerimeter = result.at( 0 ).mDerivedAttributes[tr( "Perimeter" )];
QString derivedPerimeter = result.at( 0 ).mDerivedAttributes[tr( "Perimeter (Ellipsoidal, WGS84)" )];
double perimeter = derivedPerimeter.remove( ',' ).split( ' ' ).at( 0 ).toDouble();
QCOMPARE( perimeter, 128289.074 );
derivedPerimeter = result.at( 0 ).mDerivedAttributes[tr( "Perimeter (Cartesian)" )];
perimeter = derivedPerimeter.remove( ',' ).split( ' ' ).at( 0 ).toDouble();
QCOMPARE( perimeter, 128282.086 );

//check that project units are respected
QgsProject::instance()->setDistanceUnits( QgsUnitTypes::DistanceFeet );
result = action->identify( mapPoint.x(), mapPoint.y(), QList<QgsMapLayer *>() << tempLayer.get() );
QCOMPARE( result.length(), 1 );
derivedPerimeter = result.at( 0 ).mDerivedAttributes[tr( "Perimeter" )];
derivedPerimeter = result.at( 0 ).mDerivedAttributes[tr( "Perimeter (Ellipsoidal, WGS84)" )];
perimeter = derivedPerimeter.remove( ',' ).split( ' ' ).at( 0 ).toDouble();
QGSCOMPARENEAR( perimeter, 420896.0, 0.1 );
derivedPerimeter = result.at( 0 ).mDerivedAttributes[tr( "Perimeter (Cartesian)" )];
perimeter = derivedPerimeter.remove( ',' ).split( ' ' ).at( 0 ).toDouble();
QGSCOMPARENEAR( perimeter, 420873.0, 0.1 );

//test unchecked "keep base units" setting
s.setValue( QStringLiteral( "/qgis/measure/keepbaseunit" ), false );
result = action->identify( mapPoint.x(), mapPoint.y(), QList<QgsMapLayer *>() << tempLayer.get() );
QCOMPARE( result.length(), 1 );
derivedPerimeter = result.at( 0 ).mDerivedAttributes[tr( "Perimeter" )];
derivedPerimeter = result.at( 0 ).mDerivedAttributes[tr( "Perimeter (Ellipsoidal, WGS84)" )];
perimeter = derivedPerimeter.remove( ',' ).split( ' ' ).at( 0 ).toDouble();
QGSCOMPARENEAR( perimeter, 79.715, 0.001 );
derivedPerimeter = result.at( 0 ).mDerivedAttributes[tr( "Perimeter (Cartesian)" )];
perimeter = derivedPerimeter.remove( ',' ).split( ' ' ).at( 0 ).toDouble();
QCOMPARE( perimeter, 79.711 );

// no conversion of cartesian lengths between unit types
s.setValue( QStringLiteral( "/qgis/measure/keepbaseunit" ), true );
QgsProject::instance()->setDistanceUnits( QgsUnitTypes::DistanceDegrees );
result = action->identify( mapPoint.x(), mapPoint.y(), QList<QgsMapLayer *>() << tempLayer.get() );
QCOMPARE( result.length(), 1 );
derivedPerimeter = result.at( 0 ).mDerivedAttributes[tr( "Perimeter (Ellipsoidal, WGS84)" )];
perimeter = derivedPerimeter.remove( ',' ).split( ' ' ).at( 0 ).toDouble();
QGSCOMPARENEAR( perimeter, 1.152000, 0.001 );
derivedPerimeter = result.at( 0 ).mDerivedAttributes[tr( "Perimeter (Cartesian)" )];
perimeter = derivedPerimeter.remove( ',' ).split( ' ' ).at( 0 ).toDouble();
QGSCOMPARENEAR( perimeter, 128282, 0.1 );
}

void TestQgsMapToolIdentifyAction::areaCalculation()
Expand Down Expand Up @@ -346,26 +388,47 @@ void TestQgsMapToolIdentifyAction::areaCalculation()
std::unique_ptr< QgsMapToolIdentifyAction > action( new QgsMapToolIdentifyAction( canvas ) );
QList<QgsMapToolIdentify::IdentifyResult> result = action->identify( mapPoint.x(), mapPoint.y(), QList<QgsMapLayer *>() << tempLayer.get() );
QCOMPARE( result.length(), 1 );
QString derivedArea = result.at( 0 ).mDerivedAttributes[tr( "Area" )];
QString derivedArea = result.at( 0 ).mDerivedAttributes[tr( "Area (Ellipsoidal, WGS84)" )];
double area = derivedArea.remove( ',' ).split( ' ' ).at( 0 ).toDouble();
QGSCOMPARENEAR( area, 1009089817.0, 1.0 );
derivedArea = result.at( 0 ).mDerivedAttributes[tr( "Area (Cartesian)" )];
area = derivedArea.remove( ',' ).split( ' ' ).at( 0 ).toDouble();
QGSCOMPARENEAR( area, 1005640568.0, 1.0 );

//check that project units are respected
QgsProject::instance()->setAreaUnits( QgsUnitTypes::AreaSquareMiles );
result = action->identify( mapPoint.x(), mapPoint.y(), QList<QgsMapLayer *>() << tempLayer.get() );
QCOMPARE( result.length(), 1 );
derivedArea = result.at( 0 ).mDerivedAttributes[tr( "Area" )];
derivedArea = result.at( 0 ).mDerivedAttributes[tr( "Area (Ellipsoidal, WGS84)" )];
area = derivedArea.remove( ',' ).split( ' ' ).at( 0 ).toDouble();
QGSCOMPARENEAR( area, 389.6117, 0.001 );
derivedArea = result.at( 0 ).mDerivedAttributes[tr( "Area (Cartesian)" )];
area = derivedArea.remove( ',' ).split( ' ' ).at( 0 ).toDouble();
QGSCOMPARENEAR( area, 388.280000, 0.001 );

//test unchecked "keep base units" setting
s.setValue( QStringLiteral( "/qgis/measure/keepbaseunit" ), false );
QgsProject::instance()->setAreaUnits( QgsUnitTypes::AreaSquareFeet );
result = action->identify( mapPoint.x(), mapPoint.y(), QList<QgsMapLayer *>() << tempLayer.get() );
QCOMPARE( result.length(), 1 );
derivedArea = result.at( 0 ).mDerivedAttributes[tr( "Area" )];
derivedArea = result.at( 0 ).mDerivedAttributes[tr( "Area (Ellipsoidal, WGS84)" )];
area = derivedArea.remove( ',' ).split( ' ' ).at( 0 ).toDouble();
QGSCOMPARENEAR( area, 389.6117, 0.001 );
derivedArea = result.at( 0 ).mDerivedAttributes[tr( "Area (Cartesian)" )];
area = derivedArea.remove( ',' ).split( ' ' ).at( 0 ).toDouble();
QGSCOMPARENEAR( area, 388.280000, 0.001 );

// no conversion of cartesian lengths between unit types
s.setValue( QStringLiteral( "/qgis/measure/keepbaseunit" ), true );
QgsProject::instance()->setAreaUnits( QgsUnitTypes::AreaSquareDegrees );
result = action->identify( mapPoint.x(), mapPoint.y(), QList<QgsMapLayer *>() << tempLayer.get() );
QCOMPARE( result.length(), 1 );
derivedArea = result.at( 0 ).mDerivedAttributes[tr( "Area (Ellipsoidal, WGS84)" )];
area = derivedArea.remove( ',' ).split( ' ' ).at( 0 ).toDouble();
QGSCOMPARENEAR( area, 0.081000, 0.001 );
derivedArea = result.at( 0 ).mDerivedAttributes[tr( "Area (Cartesian)" )];
area = derivedArea.remove( ',' ).split( ' ' ).at( 0 ).toDouble();
QGSCOMPARENEAR( area, 1005640568.6, 1 );
}

// private
Expand Down

0 comments on commit f9be605

Please sign in to comment.