Skip to content

Commit

Permalink
Safer memory management for some pal routines
Browse files Browse the repository at this point in the history
  • Loading branch information
nyalldawson committed Jan 18, 2018
1 parent 623fd6f commit dded4ab
Show file tree
Hide file tree
Showing 5 changed files with 19 additions and 36 deletions.
13 changes: 5 additions & 8 deletions src/core/pal/pal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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();

Expand All @@ -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 );
Expand Down
2 changes: 1 addition & 1 deletion src/core/pal/pal.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 5 additions & 6 deletions src/core/pal/problem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 )
{
Expand All @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion src/core/pal/problem.h
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ namespace pal
*/
void chain_search();

QList<LabelPosition *> *getSolution( bool returnInactive );
QList<LabelPosition *> getSolution( bool returnInactive );

PalStat *getStats();

Expand Down
27 changes: 7 additions & 20 deletions src/core/qgslabelingengine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 );
Expand All @@ -271,10 +268,8 @@ void QgsLabelingEngine::run( QgsRenderContext &context )
return;
}


if ( context.renderingStopped() )
{
delete problem;
return; // it has been canceled
}

Expand Down Expand Up @@ -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()
Expand Down

0 comments on commit dded4ab

Please sign in to comment.