Skip to content

Commit

Permalink
QgsDistanceArea methods should raise QgsCsException when errors occur
Browse files Browse the repository at this point in the history
Instead of just silently return "0", which is misleading and
can result in data corruption/incorrect analysis results. Better
to be safe and force callers to handle transformation errors
appropriately.

In this case we:

- raise QgsProcessingExceptions when the failed measurement is coming
from a processing tool, so that the user is forced to deal with the
issue and we aren't providing meaningless/misleading measurements
- report evaluation errors if the measurement is coming from a
QGIS expression, so the user must appropriately handle the situation
- for all other cases we currently just write a console error and
maintain the current behavior of treating the measurement length
as 0. TODO notes have been added to handle this cases better.
  • Loading branch information
nyalldawson committed Sep 21, 2024
1 parent 3f3c52b commit cde770e
Show file tree
Hide file tree
Showing 27 changed files with 533 additions and 154 deletions.
24 changes: 18 additions & 6 deletions python/PyQt6/core/auto_generated/qgsdistancearea.sip.in
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ The inverse flattening is calculated with invf = a/(a-b).
.. seealso:: :py:func:`ellipsoidSemiMinor`
%End

double measureArea( const QgsGeometry &geometry ) const;
double measureArea( const QgsGeometry &geometry ) const throw( QgsCsException );
%Docstring
Measures the area of a geometry.

Expand All @@ -162,9 +162,11 @@ Measures the area of a geometry.
.. seealso:: :py:func:`measurePerimeter`

.. seealso:: :py:func:`areaUnits`

:raises QgsCsException: if a transformation error occurs while calculating the area
%End

double measureLength( const QgsGeometry &geometry ) const;
double measureLength( const QgsGeometry &geometry ) const throw( QgsCsException );
%Docstring
Measures the length of a geometry.

Expand All @@ -178,9 +180,11 @@ Measures the length of a geometry.
.. seealso:: :py:func:`measureArea`

.. seealso:: :py:func:`measurePerimeter`

:raises QgsCsException: if a transformation error occurs while calculating the length
%End

double measurePerimeter( const QgsGeometry &geometry ) const;
double measurePerimeter( const QgsGeometry &geometry ) const throw( QgsCsException );
%Docstring
Measures the perimeter of a polygon geometry.

Expand All @@ -194,20 +198,24 @@ Measures the perimeter of a polygon geometry.
.. seealso:: :py:func:`measureArea`

.. seealso:: :py:func:`measurePerimeter`

:raises QgsCsException: if a transformation error occurs while calculating the perimeter
%End

double measureLine( const QVector<QgsPointXY> &points ) const;
double measureLine( const QVector<QgsPointXY> &points ) const throw( QgsCsException );
%Docstring
Measures the length of a line with multiple segments.

:param points: list of points in line

:return: length of line. The units for the returned length can be retrieved by calling :py:func:`~QgsDistanceArea.lengthUnits`.

:raises QgsCsException: if a transformation error occurs while calculating the length

.. seealso:: :py:func:`lengthUnits`
%End

double measureLine( const QgsPointXY &p1, const QgsPointXY &p2 ) const;
double measureLine( const QgsPointXY &p1, const QgsPointXY &p2 ) const throw( QgsCsException );
%Docstring
Measures the distance between two points.

Expand All @@ -216,6 +224,8 @@ Measures the distance between two points.

:return: distance between points. The units for the returned distance can be retrieved by calling :py:func:`~QgsDistanceArea.lengthUnits`.

:raises QgsCsException: if a transformation error occurs while calculating the length

.. seealso:: :py:func:`lengthUnits`
%End

Expand Down Expand Up @@ -255,9 +265,11 @@ Returns the units of area for areal calculations made by this object.
.. seealso:: :py:func:`lengthUnits`
%End

double measurePolygon( const QVector<QgsPointXY> &points ) const;
double measurePolygon( const QVector<QgsPointXY> &points ) const throw( QgsCsException );
%Docstring
Measures the area of the polygon described by a set of points.

:raises QgsCsException: if a transformation error occurs while calculating the area.
%End

double bearing( const QgsPointXY &p1, const QgsPointXY &p2 ) const throw( QgsCsException );
Expand Down
24 changes: 18 additions & 6 deletions python/core/auto_generated/qgsdistancearea.sip.in
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ The inverse flattening is calculated with invf = a/(a-b).
.. seealso:: :py:func:`ellipsoidSemiMinor`
%End

double measureArea( const QgsGeometry &geometry ) const;
double measureArea( const QgsGeometry &geometry ) const throw( QgsCsException );
%Docstring
Measures the area of a geometry.

Expand All @@ -162,9 +162,11 @@ Measures the area of a geometry.
.. seealso:: :py:func:`measurePerimeter`

.. seealso:: :py:func:`areaUnits`

:raises QgsCsException: if a transformation error occurs while calculating the area
%End

double measureLength( const QgsGeometry &geometry ) const;
double measureLength( const QgsGeometry &geometry ) const throw( QgsCsException );
%Docstring
Measures the length of a geometry.

Expand All @@ -178,9 +180,11 @@ Measures the length of a geometry.
.. seealso:: :py:func:`measureArea`

.. seealso:: :py:func:`measurePerimeter`

:raises QgsCsException: if a transformation error occurs while calculating the length
%End

double measurePerimeter( const QgsGeometry &geometry ) const;
double measurePerimeter( const QgsGeometry &geometry ) const throw( QgsCsException );
%Docstring
Measures the perimeter of a polygon geometry.

Expand All @@ -194,20 +198,24 @@ Measures the perimeter of a polygon geometry.
.. seealso:: :py:func:`measureArea`

.. seealso:: :py:func:`measurePerimeter`

:raises QgsCsException: if a transformation error occurs while calculating the perimeter
%End

double measureLine( const QVector<QgsPointXY> &points ) const;
double measureLine( const QVector<QgsPointXY> &points ) const throw( QgsCsException );
%Docstring
Measures the length of a line with multiple segments.

:param points: list of points in line

:return: length of line. The units for the returned length can be retrieved by calling :py:func:`~QgsDistanceArea.lengthUnits`.

:raises QgsCsException: if a transformation error occurs while calculating the length

.. seealso:: :py:func:`lengthUnits`
%End

double measureLine( const QgsPointXY &p1, const QgsPointXY &p2 ) const;
double measureLine( const QgsPointXY &p1, const QgsPointXY &p2 ) const throw( QgsCsException );
%Docstring
Measures the distance between two points.

Expand All @@ -216,6 +224,8 @@ Measures the distance between two points.

:return: distance between points. The units for the returned distance can be retrieved by calling :py:func:`~QgsDistanceArea.lengthUnits`.

:raises QgsCsException: if a transformation error occurs while calculating the length

.. seealso:: :py:func:`lengthUnits`
%End

Expand Down Expand Up @@ -255,9 +265,11 @@ Returns the units of area for areal calculations made by this object.
.. seealso:: :py:func:`lengthUnits`
%End

double measurePolygon( const QVector<QgsPointXY> &points ) const;
double measurePolygon( const QVector<QgsPointXY> &points ) const throw( QgsCsException );
%Docstring
Measures the area of the polygon described by a set of points.

:raises QgsCsException: if a transformation error occurs while calculating the area.
%End

double bearing( const QgsPointXY &p1, const QgsPointXY &p2 ) const throw( QgsCsException );
Expand Down
14 changes: 12 additions & 2 deletions src/analysis/network/qgsvectorlayerdirector.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
#include "qgsgeometry.h"
#include "qgsdistancearea.h"
#include "qgswkbtypes.h"

#include "qgslogger.h"
#include <QString>
#include <QtAlgorithms>

Expand Down Expand Up @@ -381,7 +381,17 @@ void QgsVectorLayerDirector::makeGraph( QgsGraphBuilderInterface *builder, const

if ( !isFirstPoint && arcPt1 != arcPt2 )
{
double distance = builder->distanceArea()->measureLine( arcPt1, arcPt2 );
double distance = 0;
try
{
distance = builder->distanceArea()->measureLine( arcPt1, arcPt2 );
}
catch ( QgsCsException & )
{
// TODO report errors to user
QgsDebugError( QStringLiteral( "An error occurred while calculating length" ) );
}

QVector< QVariant > prop;
prop.reserve( mStrategies.size() );
for ( QgsNetworkStrategy *strategy : mStrategies )
Expand Down
22 changes: 20 additions & 2 deletions src/analysis/processing/qgsalgorithmcalculateoverlaps.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,16 @@ QVariantMap QgsCalculateVectorOverlapsAlgorithm::processAlgorithm( const QVarian
if ( feature.hasGeometry() && !qgsDoubleNear( feature.geometry().area(), 0.0 ) )
{
const QgsGeometry inputGeom = feature.geometry();
const double inputArea = da.measureArea( inputGeom );

double inputArea = 0;
try
{
inputArea = da.measureArea( inputGeom );
}
catch ( QgsCsException & )
{
throw QgsProcessingException( QObject::tr( "An error occurred while calculating feature area" ) );
}

// prepare for lots of intersection tests (for speed)
std::unique_ptr< QgsGeometryEngine > bufferGeomEngine( QgsGeometry::createGeometryEngine( inputGeom.constGet() ) );
Expand Down Expand Up @@ -195,7 +204,16 @@ QVariantMap QgsCalculateVectorOverlapsAlgorithm::processAlgorithm( const QVarian

const QgsGeometry overlayIntersection = inputGeom.intersection( overlayDissolved, geometryParameters );

const double overlayArea = da.measureArea( overlayIntersection );
double overlayArea = 0;
try
{
overlayArea = da.measureArea( overlayIntersection );
}
catch ( QgsCsException & )
{
throw QgsProcessingException( QObject::tr( "An error occurred while calculating feature area" ) );
}

outAttributes.append( overlayArea );
outAttributes.append( 100 * overlayArea / inputArea );
}
Expand Down
22 changes: 20 additions & 2 deletions src/analysis/processing/qgsalgorithmlinedensity.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,16 @@ QVariantMap QgsLineDensityAlgorithm::processAlgorithm( const QVariantMap &parame

if ( engine->intersects( lineGeom.constGet() ) )
{
const double analysisLineLength = mDa.measureLength( QgsGeometry( engine->intersection( mIndex.geometry( id ).constGet() ) ) );
double analysisLineLength = 0;
try
{
analysisLineLength = mDa.measureLength( QgsGeometry( engine->intersection( mIndex.geometry( id ).constGet() ) ) );
}
catch ( QgsCsException & )
{
throw QgsProcessingException( QObject::tr( "An error occurred while calculating feature length" ) );
}

double weight = 1;

if ( !mWeightField.isEmpty() )
Expand All @@ -206,7 +215,16 @@ QVariantMap QgsLineDensityAlgorithm::processAlgorithm( const QVariantMap &parame
if ( absDensity > 0 )
{
//only calculate ellipsoidal area if abs density is greater 0
const double analysisSearchGeometryArea = mDa.measureArea( mSearchGeometry );
double analysisSearchGeometryArea = 0;
try
{
analysisSearchGeometryArea = mDa.measureArea( mSearchGeometry );
}
catch ( QgsCsException & )
{
throw QgsProcessingException( QObject::tr( "An error occurred while calculating feature area" ) );
}

lineDensity = absDensity / analysisSearchGeometryArea;
}
rasterDataLine->setValue( 0, col, lineDensity );
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,14 @@ QVariantMap QgsNearestNeighbourAnalysisAlgorithm::processAlgorithm( const QVaria
}

const QgsFeatureId neighbourId = spatialIndex.nearestNeighbor( f.geometry().asPoint(), 2 ).at( 1 );
sumDist += da.measureLine( spatialIndex.geometry( neighbourId ).asPoint(), f.geometry().asPoint() );
try
{
sumDist += da.measureLine( spatialIndex.geometry( neighbourId ).asPoint(), f.geometry().asPoint() );
}
catch ( QgsCsException & )
{
throw QgsProcessingException( QObject::tr( "An error occurred while calculating length" ) );
}

i++;
feedback->setProgress( i * step );
Expand Down
10 changes: 9 additions & 1 deletion src/analysis/processing/qgsalgorithmpointstopaths.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,15 @@ QVariantMap QgsPointsToPathsAlgorithm::processAlgorithm( const QVariantMap &para
for ( int i = 1; i < pathPoints.size(); ++i )
{
const double angle = pathPoints.at( i - 1 ).azimuth( pathPoints.at( i ) );
const double distance = da.measureLine( pathPoints.at( i - 1 ), pathPoints.at( i ) );
double distance = 0;
try
{
distance = da.measureLine( pathPoints.at( i - 1 ), pathPoints.at( i ) );
}
catch ( QgsCsException & )
{
throw QgsProcessingException( QObject::tr( "An error occurred while calculating length" ) );
}
out << QString( "%1;%2;90\n" ).arg( angle ).arg( distance );
}
}
Expand Down
11 changes: 10 additions & 1 deletion src/analysis/processing/qgsalgorithmserviceareafromlayer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,16 @@ QVariantMap QgsServiceAreaFromLayerAlgorithm::processAlgorithm( const QVariantMa

if ( pointDistanceThreshold >= 0 )
{
const double distancePointToNetwork = mBuilder->distanceArea()->measureLine( originalPoint, snappedPoint );
double distancePointToNetwork = 0;
try
{
distancePointToNetwork = mBuilder->distanceArea()->measureLine( originalPoint, snappedPoint );
}
catch ( QgsCsException & )
{
throw QgsProcessingException( QObject::tr( "An error occurred while calculating length" ) );
}

if ( distancePointToNetwork > pointDistanceThreshold )
{
feedback->pushWarning( QObject::tr( "Point is too far from the network layer (%1, maximum permitted is %2)" ).arg( distancePointToNetwork ).arg( pointDistanceThreshold ) );
Expand Down
12 changes: 11 additions & 1 deletion src/analysis/processing/qgsalgorithmserviceareafrompoint.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,17 @@ QVariantMap QgsServiceAreaFromPointAlgorithm::processAlgorithm( const QVariantMa
{
const double pointDistanceThreshold = parameterAsDouble( parameters, QStringLiteral( "POINT_TOLERANCE" ), context );

const double distancePointToNetwork = mBuilder->distanceArea()->measureLine( startPoint, snappedStartPoint );
double distancePointToNetwork = 0;
try
{
distancePointToNetwork = mBuilder->distanceArea()->measureLine( startPoint, snappedStartPoint );
}
catch ( QgsCsException & )
{
throw QgsProcessingException( QObject::tr( "An error occurred while calculating length" ) );
}


if ( distancePointToNetwork > pointDistanceThreshold )
{
throw QgsProcessingException( QObject::tr( "Point is too far from the network layer (%1, maximum permitted is %2)" ).arg( distancePointToNetwork ).arg( pointDistanceThreshold ) );
Expand Down
11 changes: 10 additions & 1 deletion src/analysis/processing/qgsalgorithmshortestline.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,16 @@ QVariantMap QgsShortestLineAlgorithm::processAlgorithm( const QVariantMap &param
}

const QgsGeometry shortestLine = sourceGeom.shortestLine( destinationGeom );
double dist = da.measureLength( shortestLine );
double dist = 0;
try
{
dist = da.measureLength( shortestLine );
}
catch ( QgsCsException & )
{
throw QgsProcessingException( QObject::tr( "An error occurred while calculating shortest line length" ) );
}

QgsFeature f;
QgsAttributes attrs = sourceFeature.attributes();
attrs << destinationAttributeCache.value( id ) << dist;
Expand Down
22 changes: 20 additions & 2 deletions src/analysis/processing/qgsalgorithmshortestpathlayertopoint.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,16 @@ QVariantMap QgsShortestPathLayerToPointAlgorithm::processAlgorithm( const QVaria

if ( pointDistanceThreshold >= 0 )
{
const double distanceEndPointToNetwork = mBuilder->distanceArea()->measureLine( endPoint, snappedEndPoint );
double distanceEndPointToNetwork = 0;
try
{
distanceEndPointToNetwork = mBuilder->distanceArea()->measureLine( endPoint, snappedEndPoint );
}
catch ( QgsCsException & )
{
throw QgsProcessingException( QObject::tr( "An error occurred while calculating length" ) );
}

if ( distanceEndPointToNetwork > pointDistanceThreshold )
{
throw QgsProcessingException( QObject::tr( "End point is too far from the network layer (%1, maximum permitted is %2)" ).arg( distanceEndPointToNetwork ).arg( pointDistanceThreshold ) );
Expand Down Expand Up @@ -143,7 +152,16 @@ QVariantMap QgsShortestPathLayerToPointAlgorithm::processAlgorithm( const QVaria

if ( pointDistanceThreshold >= 0 )
{
const double distancePointToNetwork = mBuilder->distanceArea()->measureLine( originalPoint, snappedPoint );
double distancePointToNetwork = 0;
try
{
distancePointToNetwork = mBuilder->distanceArea()->measureLine( originalPoint, snappedPoint );
}
catch ( QgsCsException & )
{
throw QgsProcessingException( QObject::tr( "An error occurred while calculating length" ) );
}

if ( distancePointToNetwork > pointDistanceThreshold )
{
feedback->pushWarning( QObject::tr( "Point is too far from the network layer (%1, maximum permitted is %2)" ).arg( distancePointToNetwork ).arg( pointDistanceThreshold ) );
Expand Down
Loading

0 comments on commit cde770e

Please sign in to comment.