[expression] Avoid paying the price of a QgsCoordinateReferenceSystem destruction if we didn't need one

nirvn · nyalldawson · commit 9d7bdea58be7 · 2021-01-16T02:24:47.000+10:00
(cherry picked from commit ff3bb1c)
diff --git a/src/core/expression/qgsexpression.cpp b/src/core/expression/qgsexpression.cpp
@@ -278,8 +278,8 @@ void QgsExpression::initGeomCalculator( const QgsExpressionContext *context )
     // actually don't do it right away, cos it's expensive to create and only a very small number of expression
     // functions actually require it. Let's lazily construct it when needed
     d->mDaEllipsoid = context->variable( QStringLiteral( "project_ellipsoid" ) ).toString();
-    d->mDaCrs = context->variable( QStringLiteral( "_layer_crs" ) ).value<QgsCoordinateReferenceSystem>();
-    d->mDaTransformContext = context->variable( QStringLiteral( "_project_transform_context" ) ).value<QgsCoordinateTransformContext>();
+    d->mDaCrs = qgis::make_unique<QgsCoordinateReferenceSystem>( context->variable( QStringLiteral( "_layer_crs" ) ).value<QgsCoordinateReferenceSystem>() );
+    d->mDaTransformContext = qgis::make_unique<QgsCoordinateTransformContext>( context->variable( QStringLiteral( "_project_transform_context" ) ).value<QgsCoordinateTransformContext>() );
   }
 
   // Set the distance units from the context if it has not been set by setDistanceUnits()
@@ -396,12 +396,12 @@ QString QgsExpression::dump() const
 
 QgsDistanceArea *QgsExpression::geomCalculator()
 {
-  if ( !d->mCalc && d->mDaCrs.isValid() )
+  if ( !d->mCalc && d->mDaCrs && d->mDaCrs->isValid() && d->mDaTransformContext )
   {
     // calculator IS required, so initialize it now...
     d->mCalc = std::shared_ptr<QgsDistanceArea>( new QgsDistanceArea() );
     d->mCalc->setEllipsoid( d->mDaEllipsoid.isEmpty() ? geoNone() : d->mDaEllipsoid );
-    d->mCalc->setSourceCrs( d->mDaCrs, d->mDaTransformContext );
+    d->mCalc->setSourceCrs( *d->mDaCrs.get(), *d->mDaTransformContext.get() );
   }
 
   return d->mCalc.get();
diff --git a/src/core/expression/qgsexpression_p.h b/src/core/expression/qgsexpression_p.h
@@ -47,12 +47,15 @@ class QgsExpressionPrivate
       , mParserErrors( other.mParserErrors )
       , mExp( other.mExp )
       , mDaEllipsoid( other.mDaEllipsoid )
-      , mDaCrs( other.mDaCrs )
-      , mDaTransformContext( other.mDaTransformContext )
       , mCalc( other.mCalc )
       , mDistanceUnit( other.mDistanceUnit )
       , mAreaUnit( other.mAreaUnit )
-    {}
+    {
+      if ( other.mDaCrs )
+        mDaCrs = qgis::make_unique<QgsCoordinateReferenceSystem>( *other.mDaCrs.get() );
+      if ( other.mDaTransformContext )
+        mDaTransformContext = qgis::make_unique<QgsCoordinateTransformContext>( *other.mDaTransformContext.get() );
+    }
 
     ~QgsExpressionPrivate()
     {
@@ -71,8 +74,8 @@ class QgsExpressionPrivate
     QString mExp;
 
     QString mDaEllipsoid;
-    QgsCoordinateReferenceSystem mDaCrs;
-    QgsCoordinateTransformContext mDaTransformContext;
+    std::unique_ptr<QgsCoordinateReferenceSystem> mDaCrs;
+    std::unique_ptr<QgsCoordinateTransformContext> mDaTransformContext;
 
     std::shared_ptr<QgsDistanceArea> mCalc;
     QgsUnitTypes::DistanceUnit mDistanceUnit = QgsUnitTypes::DistanceUnknownUnit;

Original file line number	Diff line number	Diff line change
`@@ -278,8 +278,8 @@ void QgsExpression::initGeomCalculator( const QgsExpressionContext *context )`
`278`	`278`	`// actually don't do it right away, cos it's expensive to create and only a very small number of expression`
`279`	`279`	`// functions actually require it. Let's lazily construct it when needed`
`280`	`280`	`d->mDaEllipsoid = context->variable( QStringLiteral( "project_ellipsoid" ) ).toString();`
`281`		`- d->mDaCrs = context->variable( QStringLiteral( "_layer_crs" ) ).value<QgsCoordinateReferenceSystem>();`
`282`		`- d->mDaTransformContext = context->variable( QStringLiteral( "_project_transform_context" ) ).value<QgsCoordinateTransformContext>();`
	`281`	`+ d->mDaCrs = qgis::make_unique<QgsCoordinateReferenceSystem>( context->variable( QStringLiteral( "_layer_crs" ) ).value<QgsCoordinateReferenceSystem>() );`
	`282`	`+ d->mDaTransformContext = qgis::make_unique<QgsCoordinateTransformContext>( context->variable( QStringLiteral( "_project_transform_context" ) ).value<QgsCoordinateTransformContext>() );`
`283`	`283`	`}`
`284`	`284`
`285`	`285`	`// Set the distance units from the context if it has not been set by setDistanceUnits()`
`@@ -396,12 +396,12 @@ QString QgsExpression::dump() const`
`396`	`396`
`397`	`397`	`QgsDistanceArea *QgsExpression::geomCalculator()`
`398`	`398`	`{`
`399`		`- if ( !d->mCalc && d->mDaCrs.isValid() )`
	`399`	`+ if ( !d->mCalc && d->mDaCrs && d->mDaCrs->isValid() && d->mDaTransformContext )`
`400`	`400`	`{`
`401`	`401`	`// calculator IS required, so initialize it now...`
`402`	`402`	`d->mCalc = std::shared_ptr<QgsDistanceArea>( new QgsDistanceArea() );`
`403`	`403`	`d->mCalc->setEllipsoid( d->mDaEllipsoid.isEmpty() ? geoNone() : d->mDaEllipsoid );`
`404`		`- d->mCalc->setSourceCrs( d->mDaCrs, d->mDaTransformContext );`
	`404`	`+ d->mCalc->setSourceCrs( d->mDaCrs.get(), d->mDaTransformContext.get() );`
`405`	`405`	`}`
`406`	`406`
`407`	`407`	`return d->mCalc.get();`