Safer memory management for some pal routines

qgis · Jan 18, 2018 · dded4ab · dded4ab
1 parent 623fd6f
commit dded4ab
Show file tree

Hide file tree

Showing 5 changed files with 19 additions and 36 deletions.
diff --git a/src/core/pal/pal.cpp b/src/core/pal/pal.cpp
@@ -222,12 +222,12 @@ bool filteringCallback( FeaturePart *featurePart, void *ctx )
   return true;
 }
 
-Problem *Pal::extract( double lambda_min, double phi_min, double lambda_max, double phi_max )
+std::unique_ptr<Problem> Pal::extract( const QgsRectangle &extent, const QgsGeometry &mapBoundary )
 {
   // to store obstacles
   RTree<FeaturePart *, double, 2, double> *obstacles = new RTree<FeaturePart *, double, 2, double>();
 
-  Problem *prob = new Problem();
+  std::unique_ptr< Problem > prob = qgis::make_unique< Problem >();
 
   int i, j;
 
@@ -314,7 +314,6 @@ Problem *Pal::extract( double lambda_min, double phi_min, double lambda_max, dou
   if ( fFeats->isEmpty() )
   {
     delete fFeats;
-    delete prob;
     delete obstacles;
     return nullptr;
   }
@@ -345,7 +344,6 @@ Problem *Pal::extract( double lambda_min, double phi_min, double lambda_max, dou
 
     qDeleteAll( *fFeats );
     delete fFeats;
-    delete prob;
     delete obstacles;
     return nullptr;
   }
@@ -410,7 +408,6 @@ Problem *Pal::extract( double lambda_min, double phi_min, double lambda_max, dou
 
       qDeleteAll( *fFeats );
       delete fFeats;
-      delete prob;
       delete obstacles;
       return nullptr;
     }
@@ -459,10 +456,10 @@ Problem *Pal::extractProblem( double bbox[4] )
   return extract( bbox[0], bbox[1], bbox[2], bbox[3] );
 }
 
-QList<LabelPosition *> *Pal::solveProblem( Problem *prob, bool displayAll )
+QList<LabelPosition *> Pal::solveProblem( Problem *prob, bool displayAll )
 {
   if ( !prob )
-    return new QList<LabelPosition *>();
+    return QList<LabelPosition *>();
 
   prob->reduce();
 
@@ -477,7 +474,7 @@ QList<LabelPosition *> *Pal::solveProblem( Problem *prob, bool displayAll )
   }
   catch ( InternalException::Empty )
   {
-    return new QList<LabelPosition *>();
+    return QList<LabelPosition *>();
   }
 
   return prob->getSolution( displayAll );

diff --git a/src/core/pal/pal.h b/src/core/pal/pal.h
@@ -140,7 +140,7 @@ namespace pal
 
       Problem *extractProblem( double bbox[4] );
 
-      QList<LabelPosition *> *solveProblem( Problem *prob, bool displayAll );
+      QList<LabelPosition *> solveProblem( Problem *prob, bool displayAll );
 
       /**
        *\brief Set flag show partial label

diff --git a/src/core/pal/problem.cpp b/src/core/pal/problem.cpp
@@ -2228,11 +2228,10 @@ bool Problem::compareLabelArea( pal::LabelPosition *l1, pal::LabelPosition *l2 )
   return l1->getWidth() * l1->getHeight() > l2->getWidth() * l2->getHeight();
 }
 
-QList<LabelPosition *> *Problem::getSolution( bool returnInactive )
+QList<LabelPosition *> Problem::getSolution( bool returnInactive )
 {
-
   int i;
-  QList<LabelPosition *> *solList = new QList<LabelPosition *>();
+  QList<LabelPosition *> solList;
 
   if ( nbft == 0 )
   {
@@ -2243,20 +2242,20 @@ QList<LabelPosition *> *Problem::getSolution( bool returnInactive )
   {
     if ( sol->s[i] != -1 )
     {
-      solList->push_back( mLabelPositions.at( sol->s[i] ) ); // active labels
+      solList.push_back( mLabelPositions.at( sol->s[i] ) ); // active labels
     }
     else if ( returnInactive
               || mLabelPositions.at( featStartId[i] )->getFeaturePart()->layer()->displayAll()
               || mLabelPositions.at( featStartId[i] )->getFeaturePart()->alwaysShow() )
     {
-      solList->push_back( mLabelPositions.at( featStartId[i] ) ); // unplaced label
+      solList.push_back( mLabelPositions.at( featStartId[i] ) ); // unplaced label
     }
   }
 
   // if features collide, order by size, so smaller ones appear on top
   if ( returnInactive )
   {
-    std::sort( solList->begin(), solList->end(), compareLabelArea );
+    std::sort( solList.begin(), solList.end(), compareLabelArea );
   }
 
   return solList;

diff --git a/src/core/pal/problem.h b/src/core/pal/problem.h
@@ -150,7 +150,7 @@ namespace pal
        */
       void chain_search();
 
-      QList<LabelPosition *> *getSolution( bool returnInactive );
+      QList<LabelPosition *> getSolution( bool returnInactive );
 
       PalStat *getStats();
 

diff --git a/src/core/qgslabelingengine.cpp b/src/core/qgslabelingengine.cpp
@@ -256,10 +256,7 @@ void QgsLabelingEngine::run( QgsRenderContext &context )
   t.start();
 
   // do the labeling itself
-  double bbox[] = { extent.xMinimum(), extent.yMinimum(), extent.xMaximum(), extent.yMaximum() };
-
-  QList<pal::LabelPosition *> *labels;
-  pal::Problem *problem = nullptr;
+  std::unique_ptr< pal::Problem > problem;
   try
   {
     problem = p.extractProblem( bbox );
@@ -271,10 +268,8 @@ void QgsLabelingEngine::run( QgsRenderContext &context )
     return;
   }
 
-
   if ( context.renderingStopped() )
   {
-    delete problem;
     return; // it has been canceled
   }
 
@@ -308,47 +303,39 @@ void QgsLabelingEngine::run( QgsRenderContext &context )
   }
 
   // find the solution
-  labels = p.solveProblem( problem, settings.testFlag( QgsLabelingEngineSettings::UseAllLabels ) );
+  QList<pal::LabelPosition *> labels = p.solveProblem( problem.get(), settings.testFlag( QgsLabelingEngineSettings::UseAllLabels ) );
 
-  QgsDebugMsgLevel( QString( "LABELING work:  %1 ms ... labels# %2" ).arg( t.elapsed() ).arg( labels->size() ), 4 );
+  QgsDebugMsgLevel( QString( "LABELING work:  %1 ms ... labels# %2" ).arg( t.elapsed() ).arg( labels.size() ), 4 );
   t.restart();
 
   if ( context.renderingStopped() )
   {
-    delete problem;
-    delete labels;
     return;
   }
   painter->setRenderHint( QPainter::Antialiasing );
 
   // sort labels
-  std::sort( labels->begin(), labels->end(), QgsLabelSorter( mMapSettings ) );
+  std::sort( labels.begin(), labels.end(), QgsLabelSorter( mMapSettings ) );
 
   // draw the labels
-  QList<pal::LabelPosition *>::iterator it = labels->begin();
-  for ( ; it != labels->end(); ++it )
+  for ( pal::LabelPosition *label : qgis::as_const( labels ) )
   {
     if ( context.renderingStopped() )
       break;
 
-    QgsLabelFeature *lf = ( *it )->getFeaturePart()->feature();
+    QgsLabelFeature *lf = label->getFeaturePart()->feature();
     if ( !lf )
     {
       continue;
     }
 
-    lf->provider()->drawLabel( context, *it );
+    lf->provider()->drawLabel( context, label );
   }
 
   // Reset composition mode for further drawing operations
   painter->setCompositionMode( QPainter::CompositionMode_SourceOver );
 
   QgsDebugMsgLevel( QString( "LABELING draw:  %1 ms" ).arg( t.elapsed() ), 4 );
-
-  delete problem;
-  delete labels;
-
-
 }
 
 QgsLabelingResults *QgsLabelingEngine::takeResults()