Skip to content
Permalink
Browse files

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
  • Loading branch information
nyalldawson committed Oct 18, 2015
1 parent 63cea76 commit 5f1bb6cf1f5c3132b42a10fc4703fd711e6e5cc1
@@ -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;
@@ -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;
}

@@ -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
@@ -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();
@@ -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.
*/
@@ -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;
}

@@ -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();
}
@@ -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;
}

@@ -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();
@@ -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)

@@ -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+)?")

0 comments on commit 5f1bb6c

Please sign in to comment.
You can’t perform that action at this time.