Fix calculation of area/length of mixed geometry collections

Author: nyalldawson

python/core/qgsdistancearea.sip

@@ -55,8 +55,29 @@ class QgsDistanceArea
     //! returns ellipsoid's inverse flattening
     double ellipsoidInverseFlattening() const;
 
-    //! general measurement (line distance or polygon area)
-    double measure( const QgsGeometry* geometry ) const;
+    /** General measurement (line distance or polygon area)
+     * @deprecated use measureArea() or measureLength() methods instead, as this method
+     * is unpredictable for geometry collections
+     */
+    double measure( const QgsGeometry* geometry ) const /Deprecated/;
+
+    /** Measures the area of a geometry.
+     * @param geometry geometry to measure
+     * @returns area of geometry. For geometry collections, non surface geometries will be ignored
+     * @note added in QGIS 2.12
+     * @see measureLength()
+     * @see measurePerimeter()
+     */
+    double measureArea( const QgsGeometry* geometry ) const;
+
+    /** Measures the length of a geometry.
+     * @param geometry geometry to measure
+     * @returns length of geometry. For geometry collections, non curve geometries will be ignored
+     * @note added in QGIS 2.12
+     * @see measureArea()
+     * @see measurePerimeter()
+     */
+    double measureLength( const QgsGeometry* geometry ) const;
 
     //! measures perimeter of polygon
     double measurePerimeter( const QgsGeometry* geometry ) const;

src/analysis/vector/qgsgeometryanalyzer.cpp

@@ -345,7 +345,7 @@ QList<double> QgsGeometryAnalyzer::simpleMeasure( QgsGeometry* mpGeometry )
   else
   {
     QgsDistanceArea measure;
-    list.append( measure.measure( mpGeometry ) );
+    list.append( measure.measureArea( mpGeometry ) );
     if ( mpGeometry->type() == QGis::Polygon )
     {
       perim = perimeterMeasure( mpGeometry, measure );
@@ -357,34 +357,7 @@ QList<double> QgsGeometryAnalyzer::simpleMeasure( QgsGeometry* mpGeometry )
 
 double QgsGeometryAnalyzer::perimeterMeasure( QgsGeometry* geometry, QgsDistanceArea& measure )
 {
-  double value = 0.00;
-  if ( geometry->isMultipart() )
-  {
-    QgsMultiPolygon poly = geometry->asMultiPolygon();
-    QgsMultiPolygon::iterator it;
-    QgsPolygon::iterator jt;
-    for ( it = poly.begin(); it != poly.end(); ++it )
-    {
-      for ( jt = it->begin(); jt != it->end(); ++jt )
-      {
-        QgsGeometry* geom = QgsGeometry::fromPolyline( *jt );
-        value = value + measure.measure( geom );
-        delete geom;
-      }
-    }
-  }
-  else
-  {
-    QgsPolygon::iterator jt;
-    QgsPolygon poly = geometry->asPolygon();
-    for ( jt = poly.begin(); jt != poly.end(); ++jt )
-    {
-      QgsGeometry* geom = QgsGeometry::fromPolyline( *jt );
-      value = value + measure.measure( geom );
-      delete geom;
-    }
-  }
-  return value;
+  return measure.measurePerimeter( geometry );
 }
 
 bool QgsGeometryAnalyzer::convexHull( QgsVectorLayer* layer, const QString& shapefileName,

src/analysis/vector/qgstransectsample.cpp

@@ -259,7 +259,7 @@ int QgsTransectSample::createSample( QProgressDialog* pd )
       }
 
       //cancel if length of lineClipStratum is too small
-      double transectLength = distanceArea.measure( lineClipStratum );
+      double transectLength = distanceArea.measureLength( lineClipStratum );
       if ( transectLength < mMinTransectLength )
       {
         delete lineFarAwayGeom; delete lineClipStratum;

src/core/qgsdistancearea.cpp

@@ -263,7 +263,7 @@ bool  QgsDistanceArea::setEllipsoid( double semiMajor, double semiMinor )
   return true;
 }
 
-double QgsDistanceArea::measure( const QgsAbstractGeometryV2* geomV2 ) const
+double QgsDistanceArea::measure( const QgsAbstractGeometryV2* geomV2, MeasureType type ) const
 {
   if ( !geomV2 )
   {
@@ -276,10 +276,16 @@ double QgsDistanceArea::measure( const QgsAbstractGeometryV2* geomV2 ) const
     return 0.0;
   }
 
+  MeasureType measureType = type;
+  if ( measureType == Default )
+  {
+    measureType = ( geomDimension == 1 ? Length : Area );
+  }
+
   if ( !mEllipsoidalMode || mEllipsoid == GEO_NONE )
   {
     //no transform required
-    if ( geomDimension == 1 )
+    if ( measureType == Length )
     {
       return geomV2->length();
     }
@@ -297,14 +303,12 @@ double QgsDistanceArea::measure( const QgsAbstractGeometryV2* geomV2 ) const
       double sum = 0;
       for ( int i = 0; i < collection->numGeometries(); ++i )
       {
-        //hmm... this is a bit broken. What if a collection consists of both lines and polygons?
-        //the sum will be a mash of both areas and lengths
-        sum += measure( collection->geometryN( i ) );
+        sum += measure( collection->geometryN( i ), measureType );
       }
       return sum;
     }
 
-    if ( geomDimension == 1 )
+    if ( measureType == Length )
     {
       const QgsCurveV2* curve = dynamic_cast<const QgsCurveV2*>( geomV2 );
       if ( !curve )
@@ -320,6 +324,9 @@ double QgsDistanceArea::measure( const QgsAbstractGeometryV2* geomV2 ) const
     else
     {
       const QgsSurfaceV2* surface = dynamic_cast<const QgsSurfaceV2*>( geomV2 );
+      if ( !surface )
+        return 0.0;
+
       QgsPolygonV2* polygon = surface->surfaceToPolygon();
 
       double area = 0;
@@ -346,6 +353,24 @@ double QgsDistanceArea::measure( const QgsGeometry *geometry ) const
   return measure( geomV2 );
 }
 
+double QgsDistanceArea::measureArea( const QgsGeometry* geometry ) const
+{
+  if ( !geometry )
+    return 0.0;
+
+  const QgsAbstractGeometryV2* geomV2 = geometry->geometry();
+  return measure( geomV2, Area );
+}
+
+double QgsDistanceArea::measureLength( const QgsGeometry* geometry ) const
+{
+  if ( !geometry )
+    return 0.0;
+
+  const QgsAbstractGeometryV2* geomV2 = geometry->geometry();
+  return measure( geomV2, Length );
+}
+
 double QgsDistanceArea::measurePerimeter( const QgsGeometry* geometry ) const
 {
   if ( !geometry )

src/core/qgsdistancearea.h

@@ -88,8 +88,29 @@ class CORE_EXPORT QgsDistanceArea
     //! returns ellipsoid's inverse flattening
     double ellipsoidInverseFlattening() const { return mInvFlattening; }
 
-    //! general measurement (line distance or polygon area)
-    double measure( const QgsGeometry* geometry ) const;
+    /** General measurement (line distance or polygon area)
+     * @deprecated use measureArea() or measureLength() methods instead, as this method
+     * is unpredictable for geometry collections
+     */
+    Q_DECL_DEPRECATED double measure( const QgsGeometry* geometry ) const;
+
+    /** Measures the area of a geometry.
+     * @param geometry geometry to measure
+     * @returns area of geometry. For geometry collections, non surface geometries will be ignored
+     * @note added in QGIS 2.12
+     * @see measureLength()
+     * @see measurePerimeter()
+     */
+    double measureArea( const QgsGeometry* geometry ) const;
+
+    /** Measures the length of a geometry.
+     * @param geometry geometry to measure
+     * @returns length of geometry. For geometry collections, non curve geometries will be ignored
+     * @note added in QGIS 2.12
+     * @see measureArea()
+     * @see measurePerimeter()
+     */
+    double measureLength( const QgsGeometry* geometry ) const;
 
     //! measures perimeter of polygon
     double measurePerimeter( const QgsGeometry *geometry ) const;
@@ -151,6 +172,14 @@ class CORE_EXPORT QgsDistanceArea
     void computeAreaInit();
 
   private:
+
+    enum MeasureType
+    {
+      Default,
+      Area,
+      Length
+    };
+
     //! Copy helper
     void _copy( const QgsDistanceArea & origDA );
 
@@ -171,7 +200,7 @@ class CORE_EXPORT QgsDistanceArea
     double getQ( double x ) const;
     double getQbar( double x ) const;
 
-    double measure( const QgsAbstractGeometryV2* geomV2 ) const;
+    double measure( const QgsAbstractGeometryV2* geomV2, MeasureType type = Default ) const;
     double measureLine( const QgsCurveV2* curve ) const;
     double measurePolygon( const QgsCurveV2* curve ) const;
 

src/core/qgsexpression.cpp

@@ -1383,7 +1383,7 @@ static QVariant fcnGeomArea( const QVariantList&, const QgsExpressionContext* co
   FEAT_FROM_CONTEXT( context, f );
   ENSURE_GEOM_TYPE( f, g, QGis::Polygon );
   QgsDistanceArea* calc = parent->geomCalculator();
-  return QVariant( calc->measure( f.constGeometry() ) );
+  return QVariant( calc->measureArea( f.constGeometry() ) );
 }
 
 static QVariant fcnArea( const QVariantList& values, const QgsExpressionContext*, QgsExpression* parent )
@@ -1401,7 +1401,7 @@ static QVariant fcnGeomLength( const QVariantList&, const QgsExpressionContext*
   FEAT_FROM_CONTEXT( context, f );
   ENSURE_GEOM_TYPE( f, g, QGis::Line );
   QgsDistanceArea* calc = parent->geomCalculator();
-  return QVariant( calc->measure( f.constGeometry() ) );
+  return QVariant( calc->measureLength( f.constGeometry() ) );
 }
 
 static QVariant fcnGeomPerimeter( const QVariantList&, const QgsExpressionContext* context, QgsExpression* parent )

src/gui/qgsmaptoolidentify.cpp

@@ -308,7 +308,7 @@ QMap< QString, QString > QgsMapToolIdentify::featureDerivedAttributes( QgsFeatur
   if ( geometryType == QGis::Line )
   {
     const QgsPolyline &pline = feature->constGeometry()->asPolyline();
-    double dist = calc.measure( feature->constGeometry() );
+    double dist = calc.measureLength( feature->constGeometry() );
     QGis::UnitType myDisplayUnits;
     convertMeasurement( calc, dist, myDisplayUnits, false );
     QString str = calc.textUnit( dist, 3, myDisplayUnits, false );  // dist and myDisplayUnits are out params
@@ -332,7 +332,7 @@ QMap< QString, QString > QgsMapToolIdentify::featureDerivedAttributes( QgsFeatur
   }
   else if ( geometryType == QGis::Polygon )
   {
-    double area = calc.measure( feature->constGeometry() );
+    double area = calc.measureArea( feature->constGeometry() );
     double perimeter = calc.measurePerimeter( feature->constGeometry() );
     QGis::UnitType myDisplayUnits;
     convertMeasurement( calc, area, myDisplayUnits, true );  // area and myDisplayUnits are out params

tests/src/core/testqgsdistancearea.cpp

@@ -37,6 +37,7 @@ class TestQgsDistanceArea: public QObject
     void test_distances();
     void unit_conversions();
     void regression13601();
+    void collections();
 };
 
 void TestQgsDistanceArea::initTestCase()
@@ -175,8 +176,49 @@ void TestQgsDistanceArea::regression13601()
   calc.setEllipsoidalMode( true );
   calc.setEllipsoid( "NONE" );
   calc.setSourceCrs( 1108L );
-  QgsGeometry geom( QgsGeometryFactory::geomFromWkt("Polygon ((252000 1389000, 265000 1389000, 265000 1385000, 252000 1385000, 252000 1389000))") );
-  QVERIFY( qgsDoubleNear( calc.measure( &geom ), 52000000, 0.0001 ) );
+  QgsGeometry geom( QgsGeometryFactory::geomFromWkt( "Polygon ((252000 1389000, 265000 1389000, 265000 1385000, 252000 1385000, 252000 1389000))" ) );
+  QVERIFY( qgsDoubleNear( calc.measureArea( &geom ), 52000000, 0.0001 ) );
+}
+
+void TestQgsDistanceArea::collections()
+{
+  Q_NOWARN_DEPRECATED_PUSH
+  //test measuring for collections
+  QgsDistanceArea myDa;
+  myDa.setSourceAuthId( "EPSG:4030" );
+  myDa.setEllipsoidalMode( true );
+  myDa.setEllipsoid( "WGS84" );
+
+  //collection of lines, should be sum of line length
+  QgsGeometry lines( QgsGeometryFactory::geomFromWkt( "GeometryCollection( LineString(0 36.53, 5.76 -48.16), LineString(0 25.54, 24.20 36.70) )" ) );
+  double result = myDa.measure( &lines ); //should measure length
+  QVERIFY( qgsDoubleNear( result, 12006159, 1 ) );
+  result = myDa.measureLength( &lines );
+  QVERIFY( qgsDoubleNear( result, 12006159, 1 ) );
+  result = myDa.measureArea( &lines );
+  QVERIFY( qgsDoubleNear( result, 0 ) );
+
+  //collection of polygons
+  QgsGeometry polys( QgsGeometryFactory::geomFromWkt( "GeometryCollection( Polygon((0 36.53, 5.76 -48.16, 0 25.54, 0 36.53)), Polygon((10 20, 15 20, 15 10, 10 20)) )" ) );
+  result = myDa.measure( &polys ); //should meaure area
+  QVERIFY( qgsDoubleNear( result, 670434859475, 1 ) );
+  result = myDa.measureArea( &polys );
+  QVERIFY( qgsDoubleNear( result, 670434859475, 1 ) );
+  result = myDa.measureLength( &polys );
+  QVERIFY( qgsDoubleNear( result, 0 ) );
+
+  //mixed collection
+  QgsGeometry mixed( QgsGeometryFactory::geomFromWkt( "GeometryCollection( LineString(0 36.53, 5.76 -48.16), LineString(0 25.54, 24.20 36.70), Polygon((0 36.53, 5.76 -48.16, 0 25.54, 0 36.53)), Polygon((10 20, 15 20, 15 10, 10 20)) )" ) );
+  result = myDa.measure( &mixed ); //should measure area
+  QVERIFY( qgsDoubleNear( result, 670434859475, 1 ) );
+  //measure area specifically
+  result = myDa.measureArea( &mixed );
+  QVERIFY( qgsDoubleNear( result, 670434859475, 1 ) );
+  //measure length
+  result = myDa.measureLength( &mixed );
+  QVERIFY( qgsDoubleNear( result, 12006159, 1 ) );
+
+  Q_NOWARN_DEPRECATED_POP
 }
 
 QTEST_MAIN( TestQgsDistanceArea )