Drop the cached coordinate sequence from abstract geometries

nyalldawson · nyalldawson · commit 507a93c69b22 · 2017-11-14T21:52:09.000+11:00
This cache was added to speed up expensive QgsAbstractGeometry::coordinateSequence
calls, when we were relying on coordinateSequence() for a whole
bunch of stuff like counting the number of points in a geometry.

Now it's used almost no-where in the code, so this cache is unlikely
to get filled and just makes geometries more memory heavy than
they need to be.
diff --git a/src/core/geometry/qgscurve.cpp b/src/core/geometry/qgscurve.cpp
@@ -45,14 +45,12 @@ bool QgsCurve::isRing() const
 
 QgsCoordinateSequence QgsCurve::coordinateSequence() const
 {
-  if ( !mCoordinateSequence.isEmpty() )
-    return mCoordinateSequence;
+  QgsCoordinateSequence sequence;
+  sequence.append( QgsRingSequence() );
+  sequence.back().append( QgsPointSequence() );
+  points( sequence.back().back() );
 
-  mCoordinateSequence.append( QgsRingSequence() );
-  mCoordinateSequence.back().append( QgsPointSequence() );
-  points( mCoordinateSequence.back().back() );
-
-  return mCoordinateSequence;
+  return sequence;
 }
 
 bool QgsCurve::nextVertex( QgsVertexId &id, QgsPoint &vertex ) const
@@ -191,7 +189,6 @@ QPolygonF QgsCurve::asQPolygonF() const
 void QgsCurve::clearCache() const
 {
   mBoundingBox = QgsRectangle();
-  mCoordinateSequence.clear();
   QgsAbstractGeometry::clearCache();
 }
 
diff --git a/src/core/geometry/qgscurve.h b/src/core/geometry/qgscurve.h
@@ -203,7 +203,6 @@ class CORE_EXPORT QgsCurve: public QgsAbstractGeometry
   private:
 
     mutable QgsRectangle mBoundingBox;
-    mutable QgsCoordinateSequence mCoordinateSequence;
 };
 
 #endif // QGSCURVEV2_H
diff --git a/src/core/geometry/qgscurvepolygon.cpp b/src/core/geometry/qgscurvepolygon.cpp
@@ -724,32 +724,27 @@ void QgsCurvePolygon::transform( const QTransform &t )
 
 QgsCoordinateSequence QgsCurvePolygon::coordinateSequence() const
 {
-  if ( !mCoordinateSequence.isEmpty() )
-    return mCoordinateSequence;
-
-  mCoordinateSequence.append( QgsRingSequence() );
+  QgsCoordinateSequence sequence;
+  sequence.append( QgsRingSequence() );
 
   if ( mExteriorRing )
   {
-    mCoordinateSequence.back().append( QgsPointSequence() );
-    mExteriorRing->points( mCoordinateSequence.back().back() );
+    sequence.back().append( QgsPointSequence() );
+    mExteriorRing->points( sequence.back().back() );
   }
 
   QList<QgsCurve *>::const_iterator it = mInteriorRings.constBegin();
   for ( ; it != mInteriorRings.constEnd(); ++it )
   {
-    mCoordinateSequence.back().append( QgsPointSequence() );
-    ( *it )->points( mCoordinateSequence.back().back() );
+    sequence.back().append( QgsPointSequence() );
+    ( *it )->points( sequence.back().back() );
   }
 
-  return mCoordinateSequence;
+  return sequence;
 }
 
 int QgsCurvePolygon::nCoordinates() const
 {
-  if ( !mCoordinateSequence.isEmpty() )
-    return QgsAbstractGeometry::nCoordinates();
-
   int count = 0;
 
   if ( mExteriorRing )
diff --git a/src/core/geometry/qgsgeometrycollection.cpp b/src/core/geometry/qgsgeometrycollection.cpp
@@ -421,15 +421,12 @@ QgsRectangle QgsGeometryCollection::calculateBoundingBox() const
 void QgsGeometryCollection::clearCache() const
 {
   mBoundingBox = QgsRectangle();
-  mCoordinateSequence.clear();
   QgsAbstractGeometry::clearCache();
 }
 
 QgsCoordinateSequence QgsGeometryCollection::coordinateSequence() const
 {
-  if ( !mCoordinateSequence.isEmpty() )
-    return mCoordinateSequence;
-
+  QgsCoordinateSequence sequence;
   QVector< QgsAbstractGeometry * >::const_iterator geomIt = mGeometries.constBegin();
   for ( ; geomIt != mGeometries.constEnd(); ++geomIt )
   {
@@ -438,18 +435,15 @@ QgsCoordinateSequence QgsGeometryCollection::coordinateSequence() const
     QgsCoordinateSequence::const_iterator cIt = geomCoords.constBegin();
     for ( ; cIt != geomCoords.constEnd(); ++cIt )
     {
-      mCoordinateSequence.push_back( *cIt );
+      sequence.push_back( *cIt );
     }
   }
 
-  return mCoordinateSequence;
+  return sequence;
 }
 
 int QgsGeometryCollection::nCoordinates() const
 {
-  if ( !mCoordinateSequence.isEmpty() )
-    return QgsAbstractGeometry::nCoordinates();
-
   int count = 0;
 
   QVector< QgsAbstractGeometry * >::const_iterator geomIt = mGeometries.constBegin();
diff --git a/src/core/geometry/qgsgeometrycollection.h b/src/core/geometry/qgsgeometrycollection.h
@@ -185,7 +185,6 @@ class CORE_EXPORT QgsGeometryCollection: public QgsAbstractGeometry
   private:
 
     mutable QgsRectangle mBoundingBox;
-    mutable QgsCoordinateSequence mCoordinateSequence;
 };
 
 // clazy:excludeall=qstring-allocations
diff --git a/src/core/geometry/qgssurface.h b/src/core/geometry/qgssurface.h
@@ -75,9 +75,8 @@ class CORE_EXPORT QgsSurface: public QgsAbstractGeometry
 #endif
   protected:
 
-    virtual void clearCache() const override { mBoundingBox = QgsRectangle(); mCoordinateSequence.clear(); QgsAbstractGeometry::clearCache(); }
+    virtual void clearCache() const override { mBoundingBox = QgsRectangle(); QgsAbstractGeometry::clearCache(); }
 
-    mutable QgsCoordinateSequence mCoordinateSequence;
     mutable QgsRectangle mBoundingBox;
 };
 

Original file line number	Diff line number	Diff line change
`@@ -45,14 +45,12 @@ bool QgsCurve::isRing() const`
`45`	`45`
`46`	`46`	`QgsCoordinateSequence QgsCurve::coordinateSequence() const`
`47`	`47`	`{`
`48`		`- if ( !mCoordinateSequence.isEmpty() )`
`49`		`- return mCoordinateSequence;`
	`48`	`+ QgsCoordinateSequence sequence;`
	`49`	`+ sequence.append( QgsRingSequence() );`
	`50`	`+ sequence.back().append( QgsPointSequence() );`
	`51`	`+ points( sequence.back().back() );`
`50`	`52`
`51`		`- mCoordinateSequence.append( QgsRingSequence() );`
`52`		`- mCoordinateSequence.back().append( QgsPointSequence() );`
`53`		`- points( mCoordinateSequence.back().back() );`
`54`		`-`
`55`		`- return mCoordinateSequence;`
	`53`	`+ return sequence;`
`56`	`54`	`}`
`57`	`55`
`58`	`56`	`bool QgsCurve::nextVertex( QgsVertexId &id, QgsPoint &vertex ) const`
`@@ -191,7 +189,6 @@ QPolygonF QgsCurve::asQPolygonF() const`
`191`	`189`	`void QgsCurve::clearCache() const`
`192`	`190`	`{`
`193`	`191`	`mBoundingBox = QgsRectangle();`
`194`		`- mCoordinateSequence.clear();`
`195`	`192`	`QgsAbstractGeometry::clearCache();`
`196`	`193`	`}`
`197`	`194`
Original file line number	Diff line number	Diff line change
`@@ -724,32 +724,27 @@ void QgsCurvePolygon::transform( const QTransform &t )`
`724`	`724`
`725`	`725`	`QgsCoordinateSequence QgsCurvePolygon::coordinateSequence() const`
`726`	`726`	`{`
`727`		`- if ( !mCoordinateSequence.isEmpty() )`
`728`		`- return mCoordinateSequence;`
`729`		`-`
`730`		`- mCoordinateSequence.append( QgsRingSequence() );`
	`727`	`+ QgsCoordinateSequence sequence;`
	`728`	`+ sequence.append( QgsRingSequence() );`
`731`	`729`
`732`	`730`	`if ( mExteriorRing )`
`733`	`731`	`{`
`734`		`- mCoordinateSequence.back().append( QgsPointSequence() );`
`735`		`- mExteriorRing->points( mCoordinateSequence.back().back() );`
	`732`	`+ sequence.back().append( QgsPointSequence() );`
	`733`	`+ mExteriorRing->points( sequence.back().back() );`
`736`	`734`	`}`
`737`	`735`
`738`	`736`	`QList<QgsCurve *>::const_iterator it = mInteriorRings.constBegin();`
`739`	`737`	`for ( ; it != mInteriorRings.constEnd(); ++it )`
`740`	`738`	`{`
`741`		`- mCoordinateSequence.back().append( QgsPointSequence() );`
`742`		`- ( *it )->points( mCoordinateSequence.back().back() );`
	`739`	`+ sequence.back().append( QgsPointSequence() );`
	`740`	`+ ( *it )->points( sequence.back().back() );`
`743`	`741`	`}`
`744`	`742`
`745`		`- return mCoordinateSequence;`
	`743`	`+ return sequence;`
`746`	`744`	`}`
`747`	`745`
`748`	`746`	`int QgsCurvePolygon::nCoordinates() const`
`749`	`747`	`{`
`750`		`- if ( !mCoordinateSequence.isEmpty() )`
`751`		`- return QgsAbstractGeometry::nCoordinates();`
`752`		`-`
`753`	`748`	`int count = 0;`
`754`	`749`
`755`	`750`	`if ( mExteriorRing )`
Original file line number	Diff line number	Diff line change
`@@ -421,15 +421,12 @@ QgsRectangle QgsGeometryCollection::calculateBoundingBox() const`
`421`	`421`	`void QgsGeometryCollection::clearCache() const`
`422`	`422`	`{`
`423`	`423`	`mBoundingBox = QgsRectangle();`
`424`		`- mCoordinateSequence.clear();`
`425`	`424`	`QgsAbstractGeometry::clearCache();`
`426`	`425`	`}`
`427`	`426`
`428`	`427`	`QgsCoordinateSequence QgsGeometryCollection::coordinateSequence() const`
`429`	`428`	`{`
`430`		`- if ( !mCoordinateSequence.isEmpty() )`
`431`		`- return mCoordinateSequence;`
`432`		`-`
	`429`	`+ QgsCoordinateSequence sequence;`
`433`	`430`	`QVector< QgsAbstractGeometry * >::const_iterator geomIt = mGeometries.constBegin();`
`434`	`431`	`for ( ; geomIt != mGeometries.constEnd(); ++geomIt )`
`435`	`432`	`{`
`@@ -438,18 +435,15 @@ QgsCoordinateSequence QgsGeometryCollection::coordinateSequence() const`
`438`	`435`	`QgsCoordinateSequence::const_iterator cIt = geomCoords.constBegin();`
`439`	`436`	`for ( ; cIt != geomCoords.constEnd(); ++cIt )`
`440`	`437`	`{`
`441`		`- mCoordinateSequence.push_back( *cIt );`
	`438`	`+ sequence.push_back( *cIt );`
`442`	`439`	`}`
`443`	`440`	`}`
`444`	`441`
`445`		`- return mCoordinateSequence;`
	`442`	`+ return sequence;`
`446`	`443`	`}`
`447`	`444`
`448`	`445`	`int QgsGeometryCollection::nCoordinates() const`
`449`	`446`	`{`
`450`		`- if ( !mCoordinateSequence.isEmpty() )`
`451`		`- return QgsAbstractGeometry::nCoordinates();`
`452`		`-`
`453`	`447`	`int count = 0;`
`454`	`448`
`455`	`449`	`QVector< QgsAbstractGeometry * >::const_iterator geomIt = mGeometries.constBegin();`