Avoid some unnecessary geometry clones during map to pixel

nyalldawson · nyalldawson · commit 959d1e930b43 · 2018-05-22T10:05:58.000+10:00
simplification
diff --git a/src/core/qgsmaptopixelgeometrysimplifier.cpp b/src/core/qgsmaptopixelgeometrysimplifier.cpp
@@ -68,7 +68,7 @@ bool QgsMapToPixelSimplifier::equalSnapToGrid( double x1, double y1, double x2,
 //////////////////////////////////////////////////////////////////////////////////////////////
 
 //! Generalize the WKB-geometry using the BBOX of the original geometry
-static QgsGeometry generalizeWkbGeometryByBoundingBox(
+static std::unique_ptr< QgsAbstractGeometry > generalizeWkbGeometryByBoundingBox(
   QgsWkbTypes::Type wkbType,
   const QgsAbstractGeometry &geometry,
   const QgsRectangle &envelope,
@@ -81,7 +81,7 @@ static QgsGeometry generalizeWkbGeometryByBoundingBox(
 
   if ( geometry.nCoordinates() <= minimumSize )
   {
-    return QgsGeometry( geometry.clone() );
+    return std::unique_ptr< QgsAbstractGeometry >( geometry.clone() );
   }
 
   const double x1 = envelope.xMinimum();
@@ -92,7 +92,7 @@ static QgsGeometry generalizeWkbGeometryByBoundingBox(
   // Write the generalized geometry
   if ( geometryType == QgsWkbTypes::LineString && !isRing )
   {
-    return QgsGeometry( qgis::make_unique< QgsLineString >( QVector<double>() << x1 << x2, QVector<double>() << y1 << y2 ) );
+    return qgis::make_unique< QgsLineString >( QVector<double>() << x1 << x2, QVector<double>() << y1 << y2 );
   }
   else
   {
@@ -108,17 +108,17 @@ static QgsGeometry generalizeWkbGeometryByBoundingBox(
         << y2
         << y1 );
     if ( geometryType == QgsWkbTypes::LineString )
-      return QgsGeometry( std::move( ext ) );
+      return ext;
     else
     {
       std::unique_ptr< QgsPolygon > polygon = qgis::make_unique< QgsPolygon >();
       polygon->setExteriorRing( ext.release() );
-      return QgsGeometry( std::move( polygon ) );
+      return polygon;
     }
   }
 }
 
-QgsGeometry QgsMapToPixelSimplifier::simplifyGeometry( int simplifyFlags,
+std::unique_ptr< QgsAbstractGeometry > QgsMapToPixelSimplifier::simplifyGeometry( int simplifyFlags,
     SimplifyAlgorithm simplifyAlgorithm,
     const QgsAbstractGeometry &geometry, double map2pixelTol,
     bool isaLinearRing )
@@ -292,7 +292,7 @@ QgsGeometry QgsMapToPixelSimplifier::simplifyGeometry( int simplifyFlags,
         // Bad luck! The simplified geometry is invalid and approximation by bounding box
         // would create artifacts due to long segments.
         // We will return the original geometry
-        return QgsGeometry( geometry.clone() );
+        return std::unique_ptr< QgsAbstractGeometry >( geometry.clone() );
       }
     }
 
@@ -305,20 +305,21 @@ QgsGeometry QgsMapToPixelSimplifier::simplifyGeometry( int simplifyFlags,
       }
     }
 
-    return QgsGeometry( output.release() );
+    return output;
   }
   else if ( flatType == QgsWkbTypes::Polygon )
   {
     const QgsPolygon &srcPolygon = dynamic_cast<const QgsPolygon &>( geometry );
     std::unique_ptr<QgsPolygon> polygon( new QgsPolygon() );
-    polygon->setExteriorRing( qgsgeometry_cast<QgsCurve *>( simplifyGeometry( simplifyFlags, simplifyAlgorithm, *srcPolygon.exteriorRing(), map2pixelTol, true ).constGet()->clone() ) );
+    std::unique_ptr<QgsAbstractGeometry> extRing = simplifyGeometry( simplifyFlags, simplifyAlgorithm, *srcPolygon.exteriorRing(), map2pixelTol, true );
+    polygon->setExteriorRing( qgsgeometry_cast<QgsCurve *>( extRing.release() ) );
     for ( int i = 0; i < srcPolygon.numInteriorRings(); ++i )
     {
       const QgsCurve *sub = srcPolygon.interiorRing( i );
-      std::unique_ptr< QgsCurve > ring( qgsgeometry_cast<QgsCurve *>( simplifyGeometry( simplifyFlags, simplifyAlgorithm, *sub, map2pixelTol, true ).constGet()->clone() ) );
-      polygon->addInteriorRing( ring.release() );
+      std::unique_ptr< QgsAbstractGeometry > ring = simplifyGeometry( simplifyFlags, simplifyAlgorithm, *sub, map2pixelTol, true );
+      polygon->addInteriorRing( qgsgeometry_cast<QgsCurve *>( ring.release() ) );
     }
-    return QgsGeometry( polygon.release() );
+    return polygon;
   }
   else if ( QgsWkbTypes::isMultiType( flatType ) )
   {
@@ -328,11 +329,12 @@ QgsGeometry QgsMapToPixelSimplifier::simplifyGeometry( int simplifyFlags,
     for ( int i = 0; i < numGeoms; ++i )
     {
       const QgsAbstractGeometry *sub = srcCollection.geometryN( i );
-      collection->addGeometry( simplifyGeometry( simplifyFlags, simplifyAlgorithm, *sub, map2pixelTol, false ).constGet()->clone() );
+      std::unique_ptr< QgsAbstractGeometry > part = simplifyGeometry( simplifyFlags, simplifyAlgorithm, *sub, map2pixelTol, false );
+      collection->addGeometry( part.release() );
     }
-    return QgsGeometry( collection.release() );
+    return collection;
   }
-  return QgsGeometry( geometry.clone() );
+  return std::unique_ptr< QgsAbstractGeometry >( geometry.clone() );
 }
 
 //////////////////////////////////////////////////////////////////////////////////////////////
@@ -378,5 +380,5 @@ QgsGeometry QgsMapToPixelSimplifier::simplify( const QgsGeometry &geometry ) con
     return geometry;
   }
 
-  return simplifyGeometry( mSimplifyFlags, mSimplifyAlgorithm, *geometry.constGet(), mTolerance, false );
+  return QgsGeometry( simplifyGeometry( mSimplifyFlags, mSimplifyAlgorithm, *geometry.constGet(), mTolerance, false ) );
 }
diff --git a/src/core/qgsmaptopixelgeometrysimplifier.h b/src/core/qgsmaptopixelgeometrysimplifier.h
@@ -58,7 +58,7 @@ class CORE_EXPORT QgsMapToPixelSimplifier : public QgsAbstractGeometrySimplifier
 
   private:
     //! Simplify the geometry using the specified tolerance
-    static QgsGeometry simplifyGeometry( int simplifyFlags, SimplifyAlgorithm simplifyAlgorithm, const QgsAbstractGeometry &geometry, double map2pixelTol, bool isaLinearRing );
+    static std::unique_ptr<QgsAbstractGeometry> simplifyGeometry( int simplifyFlags, SimplifyAlgorithm simplifyAlgorithm, const QgsAbstractGeometry &geometry, double map2pixelTol, bool isaLinearRing );
 
   protected:
     //! Current simplification flags

Original file line number	Diff line number	Diff line change
`@@ -68,7 +68,7 @@ bool QgsMapToPixelSimplifier::equalSnapToGrid( double x1, double y1, double x2,`
`68`	`68`	`//////////////////////////////////////////////////////////////////////////////////////////////`
`69`	`69`
`70`	`70`	`//! Generalize the WKB-geometry using the BBOX of the original geometry`
`71`		`-static QgsGeometry generalizeWkbGeometryByBoundingBox(`
	`71`	`+static std::unique_ptr< QgsAbstractGeometry > generalizeWkbGeometryByBoundingBox(`
`72`	`72`	`QgsWkbTypes::Type wkbType,`
`73`	`73`	`const QgsAbstractGeometry &geometry,`
`74`	`74`	`const QgsRectangle &envelope,`
`@@ -81,7 +81,7 @@ static QgsGeometry generalizeWkbGeometryByBoundingBox(`
`81`	`81`
`82`	`82`	`if ( geometry.nCoordinates() <= minimumSize )`
`83`	`83`	`{`
`84`		`- return QgsGeometry( geometry.clone() );`
	`84`	`+ return std::unique_ptr< QgsAbstractGeometry >( geometry.clone() );`
`85`	`85`	`}`
`86`	`86`
`87`	`87`	`const double x1 = envelope.xMinimum();`
`@@ -92,7 +92,7 @@ static QgsGeometry generalizeWkbGeometryByBoundingBox(`
`92`	`92`	`// Write the generalized geometry`
`93`	`93`	`if ( geometryType == QgsWkbTypes::LineString && !isRing )`
`94`	`94`	`{`
`95`		`- return QgsGeometry( qgis::make_unique< QgsLineString >( QVector<double>() << x1 << x2, QVector<double>() << y1 << y2 ) );`
	`95`	`+ return qgis::make_unique< QgsLineString >( QVector<double>() << x1 << x2, QVector<double>() << y1 << y2 );`
`96`	`96`	`}`
`97`	`97`	`else`
`98`	`98`	`{`
`@@ -108,17 +108,17 @@ static QgsGeometry generalizeWkbGeometryByBoundingBox(`
`108`	`108`	`<< y2`
`109`	`109`	`<< y1 );`
`110`	`110`	`if ( geometryType == QgsWkbTypes::LineString )`
`111`		`- return QgsGeometry( std::move( ext ) );`
	`111`	`+ return ext;`
`112`	`112`	`else`
`113`	`113`	`{`
`114`	`114`	`std::unique_ptr< QgsPolygon > polygon = qgis::make_unique< QgsPolygon >();`
`115`	`115`	`polygon->setExteriorRing( ext.release() );`
`116`		`- return QgsGeometry( std::move( polygon ) );`
	`116`	`+ return polygon;`
`117`	`117`	`}`
`118`	`118`	`}`
`119`	`119`	`}`
`120`	`120`
`121`		`-QgsGeometry QgsMapToPixelSimplifier::simplifyGeometry( int simplifyFlags,`
	`121`	`+std::unique_ptr< QgsAbstractGeometry > QgsMapToPixelSimplifier::simplifyGeometry( int simplifyFlags,`
`122`	`122`	`SimplifyAlgorithm simplifyAlgorithm,`
`123`	`123`	`const QgsAbstractGeometry &geometry, double map2pixelTol,`
`124`	`124`	`bool isaLinearRing )`
`@@ -292,7 +292,7 @@ QgsGeometry QgsMapToPixelSimplifier::simplifyGeometry( int simplifyFlags,`
`292`	`292`	`// Bad luck! The simplified geometry is invalid and approximation by bounding box`
`293`	`293`	`// would create artifacts due to long segments.`
`294`	`294`	`// We will return the original geometry`
`295`		`- return QgsGeometry( geometry.clone() );`
	`295`	`+ return std::unique_ptr< QgsAbstractGeometry >( geometry.clone() );`
`296`	`296`	`}`
`297`	`297`	`}`
`298`	`298`
`@@ -305,20 +305,21 @@ QgsGeometry QgsMapToPixelSimplifier::simplifyGeometry( int simplifyFlags,`
`305`	`305`	`}`
`306`	`306`	`}`
`307`	`307`
`308`		`- return QgsGeometry( output.release() );`
	`308`	`+ return output;`
`309`	`309`	`}`
`310`	`310`	`else if ( flatType == QgsWkbTypes::Polygon )`
`311`	`311`	`{`
`312`	`312`	`const QgsPolygon &srcPolygon = dynamic_cast<const QgsPolygon &>( geometry );`
`313`	`313`	`std::unique_ptr<QgsPolygon> polygon( new QgsPolygon() );`
`314`		`- polygon->setExteriorRing( qgsgeometry_cast<QgsCurve >( simplifyGeometry( simplifyFlags, simplifyAlgorithm, srcPolygon.exteriorRing(), map2pixelTol, true ).constGet()->clone() ) );`
	`314`	`+ std::unique_ptr<QgsAbstractGeometry> extRing = simplifyGeometry( simplifyFlags, simplifyAlgorithm, *srcPolygon.exteriorRing(), map2pixelTol, true );`
	`315`	`+ polygon->setExteriorRing( qgsgeometry_cast<QgsCurve *>( extRing.release() ) );`
`315`	`316`	`for ( int i = 0; i < srcPolygon.numInteriorRings(); ++i )`
`316`	`317`	`{`
`317`	`318`	`const QgsCurve *sub = srcPolygon.interiorRing( i );`
`318`		`- std::unique_ptr< QgsCurve > ring( qgsgeometry_cast<QgsCurve >( simplifyGeometry( simplifyFlags, simplifyAlgorithm, sub, map2pixelTol, true ).constGet()->clone() ) );`
`319`		`- polygon->addInteriorRing( ring.release() );`
	`319`	`+ std::unique_ptr< QgsAbstractGeometry > ring = simplifyGeometry( simplifyFlags, simplifyAlgorithm, *sub, map2pixelTol, true );`
	`320`	`+ polygon->addInteriorRing( qgsgeometry_cast<QgsCurve *>( ring.release() ) );`
`320`	`321`	`}`
`321`		`- return QgsGeometry( polygon.release() );`
	`322`	`+ return polygon;`
`322`	`323`	`}`
`323`	`324`	`else if ( QgsWkbTypes::isMultiType( flatType ) )`
`324`	`325`	`{`
`@@ -328,11 +329,12 @@ QgsGeometry QgsMapToPixelSimplifier::simplifyGeometry( int simplifyFlags,`
`328`	`329`	`for ( int i = 0; i < numGeoms; ++i )`
`329`	`330`	`{`
`330`	`331`	`const QgsAbstractGeometry *sub = srcCollection.geometryN( i );`
`331`		`- collection->addGeometry( simplifyGeometry( simplifyFlags, simplifyAlgorithm, *sub, map2pixelTol, false ).constGet()->clone() );`
	`332`	`+ std::unique_ptr< QgsAbstractGeometry > part = simplifyGeometry( simplifyFlags, simplifyAlgorithm, *sub, map2pixelTol, false );`
	`333`	`+ collection->addGeometry( part.release() );`
`332`	`334`	`}`
`333`		`- return QgsGeometry( collection.release() );`
	`335`	`+ return collection;`
`334`	`336`	`}`
`335`		`- return QgsGeometry( geometry.clone() );`
	`337`	`+ return std::unique_ptr< QgsAbstractGeometry >( geometry.clone() );`
`336`	`338`	`}`
`337`	`339`
`338`	`340`	`//////////////////////////////////////////////////////////////////////////////////////////////`
`@@ -378,5 +380,5 @@ QgsGeometry QgsMapToPixelSimplifier::simplify( const QgsGeometry &geometry ) con`
`378`	`380`	`return geometry;`
`379`	`381`	`}`
`380`	`382`
`381`		`- return simplifyGeometry( mSimplifyFlags, mSimplifyAlgorithm, *geometry.constGet(), mTolerance, false );`
	`383`	`+ return QgsGeometry( simplifyGeometry( mSimplifyFlags, mSimplifyAlgorithm, *geometry.constGet(), mTolerance, false ) );`
`382`	`384`	`}`