Skip to content

Commit 5f1bb6c

Browse files
nyalldawsonnyalldawson
committedOct 18, 2015
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
·
ltr-3_40archive/master_2
1 parent 63cea76 commit 5f1bb6c

File tree

12 files changed

+266
-114
lines changed

12 files changed

+266
-114
lines changed
 

‎python/core/geometry/qgscurvev2.sip

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ class QgsCurveV2: public QgsAbstractGeometryV2
1717
virtual void points( QList<QgsPointV2>& pt ) const = 0;
1818
virtual int numPoints() const = 0;
1919

20-
virtual double area() const;
2120
virtual void sumUpArea( double& sum ) const = 0;
2221

2322
virtual void coordinateSequence( QList< QList< QList< QgsPointV2 > > >& coord /Out/ ) const;

‎src/core/geometry/qgscompoundcurvev2.cpp

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,23 @@ bool QgsCompoundCurveV2::fromWkt( const QString& wkt )
161161
return false;
162162
}
163163
}
164+
165+
//scan through curves and check if dimensionality of curves is different to compound curve.
166+
//if so, update the type dimensionality of the compound curve to match
167+
bool hasZ = false;
168+
bool hasM = false;
169+
Q_FOREACH ( const QgsCurveV2* curve, mCurves )
170+
{
171+
hasZ = hasZ || curve->is3D();
172+
hasM = hasM || curve->isMeasure();
173+
if ( hasZ && hasM )
174+
break;
175+
}
176+
if ( hasZ )
177+
addZValue( 0 );
178+
if ( hasM )
179+
addMValue( 0 );
180+
164181
return true;
165182
}
166183

‎src/core/geometry/qgscurvepolygonv2.cpp

Lines changed: 37 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,27 @@ bool QgsCurvePolygonV2::fromWkt( const QString& wkt )
176176
mExteriorRing = mInteriorRings.first();
177177
mInteriorRings.removeFirst();
178178

179+
//scan through rings and check if dimensionality of rings is different to CurvePolygon.
180+
//if so, update the type dimensionality of the CurvePolygon to match
181+
bool hasZ = false;
182+
bool hasM = false;
183+
if ( mExteriorRing )
184+
{
185+
hasZ = hasZ || mExteriorRing->is3D();
186+
hasM = hasM || mExteriorRing->isMeasure();
187+
}
188+
Q_FOREACH ( const QgsCurveV2* curve, mInteriorRings )
189+
{
190+
hasZ = hasZ || curve->is3D();
191+
hasM = hasM || curve->isMeasure();
192+
if ( hasZ && hasM )
193+
break;
194+
}
195+
if ( hasZ )
196+
addZValue( 0 );
197+
if ( hasM )
198+
addMValue( 0 );
199+
179200
return true;
180201
}
181202

@@ -327,13 +348,26 @@ double QgsCurvePolygonV2::area() const
327348
return 0.0;
328349
}
329350

330-
double area = mExteriorRing->area();
351+
double totalArea = 0.0;
352+
353+
if ( mExteriorRing->isClosed() )
354+
{
355+
double area = 0.0;
356+
mExteriorRing->sumUpArea( area );
357+
totalArea += qAbs( area );
358+
}
359+
331360
QList<QgsCurveV2*>::const_iterator ringIt = mInteriorRings.constBegin();
332361
for ( ; ringIt != mInteriorRings.constEnd(); ++ringIt )
333362
{
334-
area -= ( *ringIt )->area();
363+
double area = 0.0;
364+
if (( *ringIt )->isClosed() )
365+
{
366+
( *ringIt )->sumUpArea( area );
367+
totalArea -= qAbs( area );
368+
}
335369
}
336-
return area;
370+
return totalArea;
337371
}
338372

339373
double QgsCurvePolygonV2::perimeter() const

‎src/core/geometry/qgscurvev2.cpp

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,15 @@ QgsCurveV2::~QgsCurveV2()
2626

2727
bool QgsCurveV2::isClosed() const
2828
{
29-
return ( numPoints() > 0 && ( startPoint() == endPoint() ) );
29+
if ( numPoints() == 0 )
30+
return false;
31+
32+
//don't consider M-coordinates when testing closedness
33+
QgsPointV2 start = startPoint();
34+
QgsPointV2 end = endPoint();
35+
return ( qgsDoubleNear( start.x(), end.x(), 1E-8 ) &&
36+
qgsDoubleNear( start.y(), end.y(), 1E-8 ) &&
37+
qgsDoubleNear( start.z(), end.z(), 1E-8 ) );
3038
}
3139

3240
bool QgsCurveV2::isRing() const
@@ -69,18 +77,6 @@ bool QgsCurveV2::nextVertex( QgsVertexId& id, QgsPointV2& vertex ) const
6977
return pointAt( id.vertex, vertex, id.type );
7078
}
7179

72-
double QgsCurveV2::area() const
73-
{
74-
if ( !isClosed() )
75-
{
76-
return 0.0;
77-
}
78-
79-
double area = 0.0;
80-
sumUpArea( area );
81-
return qAbs( area );
82-
}
83-
8480
QgsAbstractGeometryV2* QgsCurveV2::segmentize() const
8581
{
8682
return curveToLine();

‎src/core/geometry/qgscurvev2.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,8 +75,6 @@ class CORE_EXPORT QgsCurveV2: public QgsAbstractGeometryV2
7575
*/
7676
virtual int numPoints() const = 0;
7777

78-
virtual double area() const override;
79-
8078
/** Calculates the area of the curve. Derived classes should override this
8179
* to return the correct area of the curve.
8280
*/

‎src/core/geometry/qgsgeometrycollectionv2.cpp

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -502,6 +502,23 @@ bool QgsGeometryCollectionV2::fromCollectionWkt( const QString &wkt, const QList
502502
}
503503
}
504504
qDeleteAll( subtypes );
505+
506+
//scan through geometries and check if dimensionality of geometries is different to collection.
507+
//if so, update the type dimensionality of the collection to match
508+
bool hasZ = false;
509+
bool hasM = false;
510+
Q_FOREACH ( QgsAbstractGeometryV2* geom, mGeometries )
511+
{
512+
hasZ = hasZ || geom->is3D();
513+
hasM = hasM || geom->isMeasure();
514+
if ( hasZ && hasM )
515+
break;
516+
}
517+
if ( hasZ )
518+
addZValue( 0 );
519+
if ( hasM )
520+
addMValue( 0 );
521+
505522
return true;
506523
}
507524

‎src/core/geometry/qgsgeometryfactory.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ QgsAbstractGeometryV2* QgsGeometryFactory::geomFromWkt( const QString& text )
6161
{
6262
geom = new QgsLineStringV2();
6363
}
64-
else if ( text .startsWith( "CircularString", Qt::CaseInsensitive ) )
64+
else if ( text.startsWith( "CircularString", Qt::CaseInsensitive ) )
6565
{
6666
geom = new QgsCircularStringV2();
6767
}

‎src/core/geometry/qgsgeometryutils.cpp

Lines changed: 35 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -464,7 +464,30 @@ QList<QgsPointV2> QgsGeometryUtils::pointsFromWKT( const QString &wktCoordinateL
464464
{
465465
int dim = 2 + is3D + isMeasure;
466466
QList<QgsPointV2> points;
467-
Q_FOREACH ( const QString& pointCoordinates, wktCoordinateList.split( ",", QString::SkipEmptyParts ) )
467+
QStringList coordList = wktCoordinateList.split( ",", QString::SkipEmptyParts );
468+
469+
//first scan through for extra unexpected dimensions
470+
bool foundZ = false;
471+
bool foundM = false;
472+
Q_FOREACH ( const QString& pointCoordinates, coordList )
473+
{
474+
QStringList coordinates = pointCoordinates.split( " ", QString::SkipEmptyParts );
475+
if ( coordinates.size() == 3 && !foundZ && !foundM && !is3D && !isMeasure )
476+
{
477+
// 3 dimensional coordinates, but not specifically marked as such. We allow this
478+
// anyway and upgrade geometry to have Z dimension
479+
foundZ = true;
480+
}
481+
else if ( coordinates.size() >= 4 && ( !( is3D || foundZ ) || !( isMeasure || foundM ) ) )
482+
{
483+
// 4 (or more) dimensional coordinates, but not specifically marked as such. We allow this
484+
// anyway and upgrade geometry to have Z&M dimensions
485+
foundZ = true;
486+
foundM = true;
487+
}
488+
}
489+
490+
Q_FOREACH ( const QString& pointCoordinates, coordList )
468491
{
469492
QStringList coordinates = pointCoordinates.split( " ", QString::SkipEmptyParts );
470493
if ( coordinates.size() < dim )
@@ -474,27 +497,33 @@ QList<QgsPointV2> QgsGeometryUtils::pointsFromWKT( const QString &wktCoordinateL
474497
double x = coordinates[idx++].toDouble();
475498
double y = coordinates[idx++].toDouble();
476499

477-
double z = is3D ? coordinates[idx++].toDouble() : 0.;
478-
double m = isMeasure ? coordinates[idx++].toDouble() : 0.;
500+
double z = 0;
501+
if (( is3D || foundZ ) && coordinates.length() >= idx )
502+
z = coordinates[idx++].toDouble();
503+
504+
double m = 0;
505+
if (( isMeasure || foundM ) && coordinates.length() >= idx )
506+
m = coordinates[idx++].toDouble();
479507

480508
QgsWKBTypes::Type t = QgsWKBTypes::Point;
481-
if ( is3D )
509+
if ( is3D || foundZ )
482510
{
483-
if ( isMeasure )
511+
if ( isMeasure || foundM )
484512
t = QgsWKBTypes::PointZM;
485513
else
486514
t = QgsWKBTypes::PointZ;
487515
}
488516
else
489517
{
490-
if ( isMeasure )
518+
if ( isMeasure || foundM )
491519
t = QgsWKBTypes::PointM;
492520
else
493521
t = QgsWKBTypes::Point;
494522
}
495523

496524
points.append( QgsPointV2( t, x, y, z, m ) );
497525
}
526+
498527
return points;
499528
}
500529

‎src/core/geometry/qgspointv2.cpp

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,19 @@ bool QgsPointV2::fromWkt( const QString& wkt )
9494
clear();
9595
return false;
9696
}
97+
else if ( coordinates.size() == 3 && !is3D() && !isMeasure() )
98+
{
99+
// 3 dimensional coordinates, but not specifically marked as such. We allow this
100+
// anyway and upgrade geometry to have Z dimension
101+
mWkbType = QgsWKBTypes::addZ( mWkbType );
102+
}
103+
else if ( coordinates.size() >= 4 && ( !is3D() || !isMeasure() ) )
104+
{
105+
// 4 (or more) dimensional coordinates, but not specifically marked as such. We allow this
106+
// anyway and upgrade geometry to have Z&M dimensions
107+
mWkbType = QgsWKBTypes::addZ( mWkbType );
108+
mWkbType = QgsWKBTypes::addM( mWkbType );
109+
}
97110

98111
int idx = 0;
99112
mX = coordinates[idx++].toDouble();

‎tests/src/python/test_qgsgeometry.py

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ def testReferenceGeometry(self):
134134
#test exporting to WKT results in expected string
135135
result = geom.exportToWkt()
136136
exp = row['valid_wkt']
137-
assert compareWkt(result, exp), "WKT conversion {}: mismatch Expected:\n{}\nGot:\n{}\n".format(i + 1, exp, result)
137+
assert compareWkt(result, exp, 0.000001), "WKT conversion {}: mismatch Expected:\n{}\nGot:\n{}\n".format(i + 1, exp, result)
138138

139139
#test num points in geometry
140140
exp_nodes = int(row['num_points'])
@@ -149,13 +149,20 @@ def testReferenceGeometry(self):
149149
assert exp_geometries <= 1, "Geometry count {}: Expected:\n{} geometries but could not call numGeometries()\n".format(i + 1, exp_geometries)
150150

151151
#test count of rings
152-
if row['num_rings']:
153-
exp_rings = int(row['num_rings'])
154-
try:
155-
assert geom.geometry().numInteriorRings() == exp_geometries, "Ring count {}: mismatch Expected:\n{}\nGot:\n{}\n".format(i + 1, exp_geometries, geom.geometry().numInteriorRings())
156-
except:
157-
#some geometry types don't have numInteriorRings()
158-
assert exp_geometries <= 1, "Ring count {}: Expected:\n{} rings but could not call numInteriorRings()\n".format(i + 1, exp_geometries)
152+
exp_rings = int(row['num_rings'])
153+
try:
154+
assert geom.geometry().numInteriorRings() == exp_rings, "Ring count {}: mismatch Expected:\n{}\nGot:\n{}\n".format(i + 1, exp_rings, geom.geometry().numInteriorRings())
155+
except:
156+
#some geometry types don't have numInteriorRings()
157+
assert exp_rings <= 1, "Ring count {}: Expected:\n{} rings but could not call numInteriorRings()\n{}".format(i + 1, exp_rings, geom.geometry())
158+
159+
#test isClosed
160+
exp = (row['is_closed'] == '1')
161+
try:
162+
assert geom.geometry().isClosed() == exp, "isClosed {}: mismatch Expected:\n{}\nGot:\n{}\n".format(i + 1, True, geom.geometry().isClosed())
163+
except:
164+
#some geometry types don't have isClosed()
165+
assert not exp, "isClosed {}: Expected:\n isClosed() but could not call isClosed()\n".format(i + 1)
159166

160167
#test geometry centroid
161168
exp = row['centroid']
@@ -186,7 +193,7 @@ def testReferenceGeometry(self):
186193
exp = float(row['length'])
187194
result = geom.geometry().length()
188195
assert doubleNear(result, exp, 0.00001), "Length {}: mismatch Expected:\n{}\nGot:\n{}\n".format(i + 1, exp, result)
189-
196+
190197
#test perimeter calculation
191198
exp = float(row['perimeter'])
192199
result = geom.geometry().perimeter()
@@ -1512,7 +1519,7 @@ def testRegression13055(self):
15121519
p = QgsGeometry.fromWkt('Polygon((0 0 0, 0 1 0, 1 1 0, 0 0 0 ))')
15131520
assert p is not None
15141521

1515-
expWkt = 'Polygon ((0 0, 0 1, 1 1, 0 0))'
1522+
expWkt = 'PolygonZ ((0 0 0, 0 1 0, 1 1 0, 0 0 0 ))'
15161523
wkt = p.exportToWkt()
15171524
assert compareWkt(expWkt, wkt), "testRegression13055 failed: mismatch Expected:\n%s\nGot:\n%s\n" % (expWkt, wkt)
15181525

‎tests/src/python/utilities.py

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -226,16 +226,14 @@ def compareWkt(a, b, tol=0.000001):
226226
a0 = r.sub(r'\1', a0)
227227
b0 = r.sub(r'\1', b0)
228228

229-
#ignore the z/m flag on GeometryCollections
230-
#NOTE - I'm not sure about this, possibly the flag is required and there's a bug in QGIS omitting this
231-
r = re.compile("geometrycollection\s*[zm]*")
232-
a0 = r.sub('geometrycollection', a0)
233-
b0 = r.sub('geometrycollection', b0)
234-
235229
#spaces before brackets are optional
236-
r = re.compile("\s*\(")
230+
r = re.compile("\s*\(\s*")
237231
a0 = r.sub('(', a0)
238232
b0 = r.sub('(', b0)
233+
#spaces after brackets are optional
234+
r = re.compile("\s*\)\s*")
235+
a0 = r.sub(')', a0)
236+
b0 = r.sub(')', b0)
239237

240238
# compare the structure
241239
r0 = re.compile("-?\d+(?:\.\d+)?(?:[eE]\d+)?")

‎tests/testdata/geom_data.csv

Lines changed: 115 additions & 71 deletions
Large diffs are not rendered by default.

0 commit comments

Comments
 (0)
Please sign in to comment.