Geometry fixes:

- when creating geometry from WKT, upgrade dimensionality of geometry if coordinates are 3/4 dimensional - match dimensionality of collections to child dimensionality - fix area of curves was non-zero if curve is closed - don't consider m values when testing for curve closedness - add unit tests for closedness - add unit tests for CircularStrings, CompoundCurves, CurvePolygon, tests with geometries with Z/M values
qgis · Oct 18, 2015 · 5f1bb6c · 5f1bb6c
1 parent 63cea76
commit 5f1bb6c
Show file tree

Hide file tree

Showing 12 changed files with 266 additions and 114 deletions.
diff --git a/python/core/geometry/qgscurvev2.sip b/python/core/geometry/qgscurvev2.sip
@@ -17,7 +17,6 @@ class QgsCurveV2: public QgsAbstractGeometryV2
     virtual void points( QList<QgsPointV2>& pt ) const = 0;
     virtual int numPoints() const = 0;
 
-    virtual double area() const;
     virtual void sumUpArea( double& sum ) const = 0;
 
     virtual void coordinateSequence( QList< QList< QList< QgsPointV2 > > >& coord /Out/ ) const;

diff --git a/src/core/geometry/qgscompoundcurvev2.cpp b/src/core/geometry/qgscompoundcurvev2.cpp
@@ -161,6 +161,23 @@ bool QgsCompoundCurveV2::fromWkt( const QString& wkt )
       return false;
     }
   }
+
+  //scan through curves and check if dimensionality of curves is different to compound curve.
+  //if so, update the type dimensionality of the compound curve to match
+  bool hasZ = false;
+  bool hasM = false;
+  Q_FOREACH ( const QgsCurveV2* curve, mCurves )
+  {
+    hasZ = hasZ || curve->is3D();
+    hasM = hasM || curve->isMeasure();
+    if ( hasZ && hasM )
+      break;
+  }
+  if ( hasZ )
+    addZValue( 0 );
+  if ( hasM )
+    addMValue( 0 );
+
   return true;
 }
 

diff --git a/src/core/geometry/qgscurvepolygonv2.cpp b/src/core/geometry/qgscurvepolygonv2.cpp
@@ -176,6 +176,27 @@ bool QgsCurvePolygonV2::fromWkt( const QString& wkt )
   mExteriorRing = mInteriorRings.first();
   mInteriorRings.removeFirst();
 
+  //scan through rings and check if dimensionality of rings is different to CurvePolygon.
+  //if so, update the type dimensionality of the CurvePolygon to match
+  bool hasZ = false;
+  bool hasM = false;
+  if ( mExteriorRing )
+  {
+    hasZ = hasZ || mExteriorRing->is3D();
+    hasM = hasM || mExteriorRing->isMeasure();
+  }
+  Q_FOREACH ( const QgsCurveV2* curve, mInteriorRings )
+  {
+    hasZ = hasZ || curve->is3D();
+    hasM = hasM || curve->isMeasure();
+    if ( hasZ && hasM )
+      break;
+  }
+  if ( hasZ )
+    addZValue( 0 );
+  if ( hasM )
+    addMValue( 0 );
+
   return true;
 }
 
@@ -327,13 +348,26 @@ double QgsCurvePolygonV2::area() const
     return 0.0;
   }
 
-  double area = mExteriorRing->area();
+  double totalArea = 0.0;
+
+  if ( mExteriorRing->isClosed() )
+  {
+    double area = 0.0;
+    mExteriorRing->sumUpArea( area );
+    totalArea += qAbs( area );
+  }
+
   QList<QgsCurveV2*>::const_iterator ringIt = mInteriorRings.constBegin();
   for ( ; ringIt != mInteriorRings.constEnd(); ++ringIt )
   {
-    area -= ( *ringIt )->area();
+    double area = 0.0;
+    if (( *ringIt )->isClosed() )
+    {
+      ( *ringIt )->sumUpArea( area );
+      totalArea -= qAbs( area );
+    }
   }
-  return area;
+  return totalArea;
 }
 
 double QgsCurvePolygonV2::perimeter() const

diff --git a/src/core/geometry/qgscurvev2.cpp b/src/core/geometry/qgscurvev2.cpp
@@ -26,7 +26,15 @@ QgsCurveV2::~QgsCurveV2()
 
 bool QgsCurveV2::isClosed() const
 {
-  return ( numPoints() > 0 && ( startPoint() == endPoint() ) );
+  if ( numPoints() == 0 )
+    return false;
+
+  //don't consider M-coordinates when testing closedness
+  QgsPointV2 start = startPoint();
+  QgsPointV2 end = endPoint();
+  return ( qgsDoubleNear( start.x(), end.x(), 1E-8 ) &&
+           qgsDoubleNear( start.y(), end.y(), 1E-8 ) &&
+           qgsDoubleNear( start.z(), end.z(), 1E-8 ) );
 }
 
 bool QgsCurveV2::isRing() const
@@ -69,18 +77,6 @@ bool QgsCurveV2::nextVertex( QgsVertexId& id, QgsPointV2& vertex ) const
   return pointAt( id.vertex, vertex, id.type );
 }
 
-double QgsCurveV2::area() const
-{
-  if ( !isClosed() )
-  {
-    return 0.0;
-  }
-
-  double area = 0.0;
-  sumUpArea( area );
-  return qAbs( area );
-}
-
 QgsAbstractGeometryV2* QgsCurveV2::segmentize() const
 {
   return curveToLine();

diff --git a/src/core/geometry/qgscurvev2.h b/src/core/geometry/qgscurvev2.h
@@ -75,8 +75,6 @@ class CORE_EXPORT QgsCurveV2: public QgsAbstractGeometryV2
      */
     virtual int numPoints() const = 0;
 
-    virtual double area() const override;
-
     /** Calculates the area of the curve. Derived classes should override this
      * to return the correct area of the curve.
      */

diff --git a/src/core/geometry/qgsgeometrycollectionv2.cpp b/src/core/geometry/qgsgeometrycollectionv2.cpp
@@ -502,6 +502,23 @@ bool QgsGeometryCollectionV2::fromCollectionWkt( const QString &wkt, const QList
     }
   }
   qDeleteAll( subtypes );
+
+  //scan through geometries and check if dimensionality of geometries is different to collection.
+  //if so, update the type dimensionality of the collection to match
+  bool hasZ = false;
+  bool hasM = false;
+  Q_FOREACH ( QgsAbstractGeometryV2* geom, mGeometries )
+  {
+    hasZ = hasZ || geom->is3D();
+    hasM = hasM || geom->isMeasure();
+    if ( hasZ && hasM )
+      break;
+  }
+  if ( hasZ )
+    addZValue( 0 );
+  if ( hasM )
+    addMValue( 0 );
+
   return true;
 }
 

diff --git a/src/core/geometry/qgsgeometryfactory.cpp b/src/core/geometry/qgsgeometryfactory.cpp
@@ -61,7 +61,7 @@ QgsAbstractGeometryV2* QgsGeometryFactory::geomFromWkt( const QString& text )
   {
     geom = new QgsLineStringV2();
   }
-  else if ( text .startsWith( "CircularString", Qt::CaseInsensitive ) )
+  else if ( text.startsWith( "CircularString", Qt::CaseInsensitive ) )
   {
     geom = new QgsCircularStringV2();
   }

diff --git a/src/core/geometry/qgsgeometryutils.cpp b/src/core/geometry/qgsgeometryutils.cpp
@@ -464,7 +464,30 @@ QList<QgsPointV2> QgsGeometryUtils::pointsFromWKT( const QString &wktCoordinateL
 {
   int dim = 2 + is3D + isMeasure;
   QList<QgsPointV2> points;
-  Q_FOREACH ( const QString& pointCoordinates, wktCoordinateList.split( ",", QString::SkipEmptyParts ) )
+  QStringList coordList = wktCoordinateList.split( ",", QString::SkipEmptyParts );
+
+  //first scan through for extra unexpected dimensions
+  bool foundZ = false;
+  bool foundM = false;
+  Q_FOREACH ( const QString& pointCoordinates, coordList )
+  {
+    QStringList coordinates = pointCoordinates.split( " ", QString::SkipEmptyParts );
+    if ( coordinates.size() == 3 && !foundZ && !foundM && !is3D && !isMeasure )
+    {
+      // 3 dimensional coordinates, but not specifically marked as such. We allow this
+      // anyway and upgrade geometry to have Z dimension
+      foundZ = true;
+    }
+    else if ( coordinates.size() >= 4 && ( !( is3D || foundZ ) || !( isMeasure || foundM ) ) )
+    {
+      // 4 (or more) dimensional coordinates, but not specifically marked as such. We allow this
+      // anyway and upgrade geometry to have Z&M dimensions
+      foundZ = true;
+      foundM = true;
+    }
+  }
+
+  Q_FOREACH ( const QString& pointCoordinates, coordList )
   {
     QStringList coordinates = pointCoordinates.split( " ", QString::SkipEmptyParts );
     if ( coordinates.size() < dim )
@@ -474,27 +497,33 @@ QList<QgsPointV2> QgsGeometryUtils::pointsFromWKT( const QString &wktCoordinateL
     double x = coordinates[idx++].toDouble();
     double y = coordinates[idx++].toDouble();
 
-    double z = is3D ? coordinates[idx++].toDouble() : 0.;
-    double m = isMeasure ? coordinates[idx++].toDouble() : 0.;
+    double z = 0;
+    if (( is3D || foundZ ) && coordinates.length() >= idx )
+      z = coordinates[idx++].toDouble();
+
+    double m = 0;
+    if (( isMeasure || foundM ) && coordinates.length() >= idx )
+      m = coordinates[idx++].toDouble();
 
     QgsWKBTypes::Type t = QgsWKBTypes::Point;
-    if ( is3D )
+    if ( is3D || foundZ )
     {
-      if ( isMeasure )
+      if ( isMeasure || foundM )
         t = QgsWKBTypes::PointZM;
       else
         t = QgsWKBTypes::PointZ;
     }
     else
     {
-      if ( isMeasure )
+      if ( isMeasure || foundM )
         t = QgsWKBTypes::PointM;
       else
         t = QgsWKBTypes::Point;
     }
 
     points.append( QgsPointV2( t, x, y, z, m ) );
   }
+
   return points;
 }
 

diff --git a/src/core/geometry/qgspointv2.cpp b/src/core/geometry/qgspointv2.cpp
@@ -94,6 +94,19 @@ bool QgsPointV2::fromWkt( const QString& wkt )
     clear();
     return false;
   }
+  else if ( coordinates.size() == 3 && !is3D() && !isMeasure() )
+  {
+    // 3 dimensional coordinates, but not specifically marked as such. We allow this
+    // anyway and upgrade geometry to have Z dimension
+    mWkbType = QgsWKBTypes::addZ( mWkbType );
+  }
+  else if ( coordinates.size() >= 4 && ( !is3D() || !isMeasure() ) )
+  {
+    // 4 (or more) dimensional coordinates, but not specifically marked as such. We allow this
+    // anyway and upgrade geometry to have Z&M dimensions
+    mWkbType = QgsWKBTypes::addZ( mWkbType );
+    mWkbType = QgsWKBTypes::addM( mWkbType );
+  }
 
   int idx = 0;
   mX = coordinates[idx++].toDouble();

diff --git a/tests/src/python/test_qgsgeometry.py b/tests/src/python/test_qgsgeometry.py
@@ -134,7 +134,7 @@ def testReferenceGeometry(self):
                 #test exporting to WKT results in expected string
                 result = geom.exportToWkt()
                 exp = row['valid_wkt']
-                assert compareWkt(result, exp), "WKT conversion {}: mismatch Expected:\n{}\nGot:\n{}\n".format(i + 1, exp, result)
+                assert compareWkt(result, exp, 0.000001), "WKT conversion {}: mismatch Expected:\n{}\nGot:\n{}\n".format(i + 1, exp, result)
 
                 #test num points in geometry
                 exp_nodes = int(row['num_points'])
@@ -149,13 +149,20 @@ def testReferenceGeometry(self):
                     assert exp_geometries <= 1, "Geometry count {}:  Expected:\n{} geometries but could not call numGeometries()\n".format(i + 1, exp_geometries)
 
                 #test count of rings
-                if row['num_rings']:
-                    exp_rings = int(row['num_rings'])
-                    try:
-                        assert geom.geometry().numInteriorRings() == exp_geometries, "Ring count {}: mismatch Expected:\n{}\nGot:\n{}\n".format(i + 1, exp_geometries, geom.geometry().numInteriorRings())
-                    except:
-                        #some geometry types don't have numInteriorRings()
-                        assert exp_geometries <= 1, "Ring count {}:  Expected:\n{} rings but could not call numInteriorRings()\n".format(i + 1, exp_geometries)
+                exp_rings = int(row['num_rings'])
+                try:
+                    assert geom.geometry().numInteriorRings() == exp_rings, "Ring count {}: mismatch Expected:\n{}\nGot:\n{}\n".format(i + 1, exp_rings, geom.geometry().numInteriorRings())
+                except:
+                    #some geometry types don't have numInteriorRings()
+                    assert exp_rings <= 1, "Ring count {}:  Expected:\n{} rings but could not call numInteriorRings()\n{}".format(i + 1, exp_rings, geom.geometry())
+
+                #test isClosed
+                exp = (row['is_closed'] == '1')
+                try:
+                    assert geom.geometry().isClosed() == exp, "isClosed {}: mismatch Expected:\n{}\nGot:\n{}\n".format(i + 1, True, geom.geometry().isClosed())
+                except:
+                    #some geometry types don't have isClosed()
+                    assert not exp, "isClosed {}:  Expected:\n isClosed() but could not call isClosed()\n".format(i + 1)
 
                 #test geometry centroid
                 exp = row['centroid']
@@ -186,7 +193,7 @@ def testReferenceGeometry(self):
                 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()
@@ -1512,7 +1519,7 @@ def testRegression13055(self):
         p = QgsGeometry.fromWkt('Polygon((0 0 0, 0 1 0, 1 1 0, 0 0 0 ))')
         assert p is not None
 
-        expWkt = 'Polygon ((0 0, 0 1, 1 1, 0 0))'
+        expWkt = 'PolygonZ ((0 0 0, 0 1 0, 1 1 0, 0 0 0 ))'
         wkt = p.exportToWkt()
         assert compareWkt(expWkt, wkt), "testRegression13055 failed: mismatch Expected:\n%s\nGot:\n%s\n" % (expWkt, wkt)
 

diff --git a/tests/src/python/utilities.py b/tests/src/python/utilities.py
@@ -226,16 +226,14 @@ def compareWkt(a, b, tol=0.000001):
     a0 = r.sub(r'\1', a0)
     b0 = r.sub(r'\1', b0)
 
-    #ignore the z/m flag on GeometryCollections
-    #NOTE - I'm not sure about this, possibly the flag is required and there's a bug in QGIS omitting this
-    r = re.compile("geometrycollection\s*[zm]*")
-    a0 = r.sub('geometrycollection', a0)
-    b0 = r.sub('geometrycollection', b0)
-
     #spaces before brackets are optional
-    r = re.compile("\s*\(")
+    r = re.compile("\s*\(\s*")
     a0 = r.sub('(', a0)
     b0 = r.sub('(', b0)
+    #spaces after brackets are optional
+    r = re.compile("\s*\)\s*")
+    a0 = r.sub(')', a0)
+    b0 = r.sub(')', b0)
 
     # compare the structure
     r0 = re.compile("-?\d+(?:\.\d+)?(?:[eE]\d+)?")