[geometry] Fix calculation of length/perimeter for collections

Split length from perimeter calculation in geometry API, as returning perimeter for length for polygons is misleading and results in incorrect length/perimeter calculations for mixed geometry collections. Enable length & perimeter unit tests against reference geometries. Now the length & perimeter values match those calculated by PostGIS.
qgis · Oct 17, 2015 · 8c5f3f8 · 8c5f3f8
1 parent 785d995
commit 8c5f3f8
Show file tree

Hide file tree

Showing 12 changed files with 55 additions and 76 deletions.
diff --git a/python/core/geometry/qgsabstractgeometryv2.sip b/python/core/geometry/qgsabstractgeometryv2.sip
@@ -118,8 +118,22 @@ class QgsAbstractGeometryV2
     virtual bool moveVertex( const QgsVertexId& position, const QgsPointV2& newPos ) = 0;
     virtual bool deleteVertex( const QgsVertexId& position ) = 0;
 
-    /** Length for linear geometries,perimeter for area geometries*/
+    /** Returns the length of the geometry.
+     * @see area()
+     * @see perimeter()
+     */
     virtual double length() const;
+
+    /** Returns the perimeter of the geometry.
+     * @see area()
+     * @see length()
+     */
+    virtual double perimeter() const;
+
+    /** Returns the area of the geometry.
+     * @see length()
+     * @see perimeter()
+     */
     virtual double area() const;
 
     /** Returns the centroid of the geometry*/

diff --git a/python/core/geometry/qgscurvepolygonv2.sip b/python/core/geometry/qgscurvepolygonv2.sip
@@ -29,7 +29,7 @@ class QgsCurvePolygonV2: public QgsSurfaceV2
 
     //surface interface
     virtual double area() const;
-    virtual double length() const;
+    virtual double perimeter() const;
     QgsPointV2 pointOnSurface() const;
     QgsPolygonV2* surfaceToPolygon() const;
 

diff --git a/python/core/geometry/qgsgeometrycollectionv2.sip b/python/core/geometry/qgsgeometrycollectionv2.sip
@@ -58,6 +58,7 @@ class QgsGeometryCollectionV2: public QgsAbstractGeometryV2
 
     virtual double length() const;
     virtual double area() const;
+    virtual double perimeter() const;
 
     bool hasCurvedSegments() const;
 

diff --git a/src/core/geometry/qgsabstractgeometryv2.h b/src/core/geometry/qgsabstractgeometryv2.h
@@ -259,13 +259,21 @@ class CORE_EXPORT QgsAbstractGeometryV2
      */
     virtual bool deleteVertex( const QgsVertexId& position ) = 0;
 
-    /** Returns the length (or perimeter for area geometries) of the geometry.
-     * @see area
+    /** Returns the length of the geometry.
+     * @see area()
+     * @see perimeter()
      */
     virtual double length() const { return 0.0; }
 
+    /** Returns the perimeter of the geometry.
+     * @see area()
+     * @see length()
+     */
+    virtual double perimeter() const { return 0.0; }
+
     /** Returns the area of the geometry.
-     * @see length
+     * @see length()
+     * @see perimeter()
      */
     virtual double area() const { return 0.0; }
 

diff --git a/src/core/geometry/qgscurvepolygonv2.cpp b/src/core/geometry/qgscurvepolygonv2.cpp
@@ -336,16 +336,16 @@ double QgsCurvePolygonV2::area() const
   return area;
 }
 
-double QgsCurvePolygonV2::length() const
+double QgsCurvePolygonV2::perimeter() const
 {
   //sum perimeter of rings
-  double length = mExteriorRing->length();
+  double perimeter = mExteriorRing->length();
   QList<QgsCurveV2*>::const_iterator ringIt = mInteriorRings.constBegin();
   for ( ; ringIt != mInteriorRings.constEnd(); ++ringIt )
   {
-    length += ( *ringIt )->length();
+    perimeter += ( *ringIt )->length();
   }
-  return length;
+  return perimeter;
 }
 
 QgsPointV2 QgsCurvePolygonV2::pointOnSurface() const

diff --git a/src/core/geometry/qgscurvepolygonv2.h b/src/core/geometry/qgscurvepolygonv2.h
@@ -55,7 +55,7 @@ class CORE_EXPORT QgsCurvePolygonV2: public QgsSurfaceV2
 
     //surface interface
     virtual double area() const override;
-    virtual double length() const override;
+    virtual double perimeter() const override;
     QgsPointV2 pointOnSurface() const override;
     QgsPolygonV2* surfaceToPolygon() const override;
 

diff --git a/src/core/geometry/qgsgeometrycollectionv2.cpp b/src/core/geometry/qgsgeometrycollectionv2.cpp
@@ -454,6 +454,17 @@ double QgsGeometryCollectionV2::area() const
   return area;
 }
 
+double QgsGeometryCollectionV2::perimeter() const
+{
+  double perimeter = 0.0;
+  QVector< QgsAbstractGeometryV2* >::const_iterator geomIt = mGeometries.constBegin();
+  for ( ; geomIt != mGeometries.constEnd(); ++geomIt )
+  {
+    perimeter += ( *geomIt )->perimeter();
+  }
+  return perimeter;
+}
+
 bool QgsGeometryCollectionV2::fromCollectionWkt( const QString &wkt, const QList<QgsAbstractGeometryV2*>& subtypes, const QString& defaultChildWkbType )
 {
   clear();

diff --git a/src/core/geometry/qgsgeometrycollectionv2.h b/src/core/geometry/qgsgeometrycollectionv2.h
@@ -103,6 +103,7 @@ class CORE_EXPORT QgsGeometryCollectionV2: public QgsAbstractGeometryV2
 
     virtual double length() const override;
     virtual double area() const override;
+    virtual double perimeter() const override;
 
     bool hasCurvedSegments() const override;
 

diff --git a/src/core/qgsdistancearea.cpp b/src/core/qgsdistancearea.cpp
@@ -384,7 +384,7 @@ double QgsDistanceArea::measurePerimeter( const QgsGeometry* geometry ) const
 
   if ( !mEllipsoidalMode || mEllipsoid == GEO_NONE )
   {
-    return geomV2->length();
+    return geomV2->perimeter();
   }
 
   //create list with (single) surfaces

diff --git a/tests/src/python/test_qgsgeometry.py b/tests/src/python/test_qgsgeometry.py
@@ -182,41 +182,15 @@ def testReferenceGeometry(self):
                 result = geom.geometry().area()
                 assert doubleNear(result, exp), "Area {}: mismatch Expected:\n{}\nGot:\n{}\n".format(i + 1, exp, result)
 
-                #NOTE - disabled due to misleading length/perimeter calculations for geometry collections
-                #exp = float(row['length'])
-                #result = geom.geometry().length()
-                #assert doubleNear(result, exp), "Length {}: mismatch Expected:\n{}\nGot:\n{}\n".format(i + 1, exp, result)
-                #exp = float(row['perimeter'])
-                #result = geom.geometry().length()
-                #assert doubleNear(result, exp), "Length {}: mismatch Expected:\n{}\nGot:\n{}\n".format(i + 1, exp, result)
-
-    def testArea(self):
-        """ Test area calculations """
-        with open(os.path.join(TEST_DATA_DIR, 'area_data.csv'), 'r') as d:
-            for i, t in enumerate(d):
-                if not t:
-                    continue
-
-                test_data = t.strip().split('|')
-                geom = QgsGeometry.fromWkt(test_data[0])
-                assert geom, "Area {} failed: could not create geom:\n{}\n".format(i + 1, test_data[0])
-                result = geom.area()
-                exp = float(test_data[1])
-                assert abs(float(result) - float(exp)) < 0.0000001, "Area failed: mismatch Expected:\n{}\nGot:\n{}\nGeom:\n{}\n".format(i + 1, exp, result, test_data[0])
-
-    def testLength(self):
-        """ Test length/perimeter calculations """
-        with open(os.path.join(TEST_DATA_DIR, 'length_data.csv'), 'r') as d:
-            for i, t in enumerate(d):
-                if not t:
-                    continue
-
-                test_data = t.strip().split('|')
-                geom = QgsGeometry.fromWkt(test_data[0])
-                assert geom, "Length {} failed: could not create geom:\n{}\n".format(i + 1, test_data[0])
-                result = geom.length()
-                exp = float(test_data[1])
-                assert abs(float(result) - float(exp)) < 0.0000001, "Length {} failed: mismatch Expected:\n{}\nGot:\n{}\nGeom:\n{}\n".format(i + 1, exp, result, test_data[0])
+                #test length calculation
+                exp = float(row['length'])
+                result = geom.geometry().length()
+                assert doubleNear(result, exp, 0.00001), "Length {}: mismatch Expected:\n{}\nGot:\n{}\n".format(i + 1, exp, result)
+
+                #test perimeter calculation
+                exp = float(row['perimeter'])
+                result = geom.geometry().perimeter()
+                assert doubleNear(result, exp, 0.00001), "Perimeter {}: mismatch Expected:\n{}\nGot:\n{}\n".format(i + 1, exp, result)
 
     def testIntersection(self):
         myLine = QgsGeometry.fromPolyline([

diff --git a/tests/testdata/area_data.csv b/tests/testdata/area_data.csv
diff --git a/tests/testdata/length_data.csv b/tests/testdata/length_data.csv