Skip to content

Commit

Permalink
[pal] More memory management, renames
Browse files Browse the repository at this point in the history
  • Loading branch information
@nyalldawson
nyalldawson committed Nov 28, 2019
1 parent 6d6d11b commit 634a815
Show file tree
Hide file tree
Showing 9 changed files with 120 additions and 154 deletions.
8 changes: 4 additions & 4 deletions src/core/pal/labelposition.cpp
View file
Expand Up @@ -422,20 +422,20 @@ bool LabelPosition::polygonObstacleCallback( FeaturePart *obstacle, void *ctx )
return true;
}

void LabelPosition::removeFromIndex( RTree<LabelPosition *, double, 2, double> *index )
void LabelPosition::removeFromIndex( RTree<LabelPosition *, double, 2, double> &index )
{
double amin[2];
double amax[2];
getBoundingBox( amin, amax );
index->Remove( amin, amax, this );
index.Remove( amin, amax, this );
}

void LabelPosition::insertIntoIndex( RTree<LabelPosition *, double, 2, double> *index )
void LabelPosition::insertIntoIndex( RTree<LabelPosition *, double, 2, double> &index )
{
double amin[2];
double amax[2];
getBoundingBox( amin, amax );
index->Insert( amin, amax, this );
index.Insert( amin, amax, this );
}

bool LabelPosition::pruneCallback( LabelPosition *candidatePosition, void *ctx )
Expand Down
4 changes: 2 additions & 2 deletions src/core/pal/labelposition.h
View file
Expand Up @@ -256,8 +256,8 @@ namespace pal
//! Returns the number of upside down characters for this label position
int upsideDownCharCount() const { return mUpsideDownCharCount; }

void removeFromIndex( RTree<LabelPosition *, double, 2, double> *index );
void insertIntoIndex( RTree<LabelPosition *, double, 2, double> *index );
void removeFromIndex( RTree<LabelPosition *, double, 2, double> &index );
void insertIntoIndex( RTree<LabelPosition *, double, 2, double> &index );

struct PruneCtx
{
Expand Down
22 changes: 8 additions & 14 deletions src/core/pal/layer.cpp
View file
Expand Up @@ -54,9 +54,6 @@ Layer::Layer( QgsAbstractLabelProvider *provider, const QString &name, QgsPalLay
, mMergeLines( false )
, mUpsidedownLabels( Upright )
{
mFeatureIndex = new RTree<FeaturePart *, double, 2, double>();
mObstacleIndex = new RTree<FeaturePart *, double, 2, double>();

if ( defaultPriority < 0.0001 )
mDefaultPriority = 0.0001;
else if ( defaultPriority > 1.0 )
Expand All @@ -72,9 +69,6 @@ Layer::~Layer()
qDeleteAll( mFeatureParts );
qDeleteAll( mObstacleParts );

delete mFeatureIndex;
delete mObstacleIndex;

mMutex.unlock();
}

Expand Down Expand Up @@ -274,7 +268,7 @@ void Layer::addFeaturePart( FeaturePart *fpart, const QString &labelText )
mFeatureParts << fpart;

// add to r-tree for fast spatial access
mFeatureIndex->Insert( bmin, bmax, fpart );
mFeatureIndex.Insert( bmin, bmax, fpart );

// add to hashtable with equally named feature parts
if ( mMergeLines && !labelText.isEmpty() )
Expand All @@ -293,7 +287,7 @@ void Layer::addObstaclePart( FeaturePart *fpart )
mObstacleParts.append( fpart );

// add to obstacle r-tree
mObstacleIndex->Insert( bmin, bmax, fpart );
mObstacleIndex.Insert( bmin, bmax, fpart );
}

static FeaturePart *_findConnectedPart( FeaturePart *partCheck, const QVector<FeaturePart *> &otherParts )
Expand Down Expand Up @@ -349,12 +343,12 @@ void Layer::joinConnectedFeatures()
if ( otherPart->mergeWithFeaturePart( partCheck ) )
{
// remove the parts we are joining from the index
mFeatureIndex->Remove( checkpartBMin, checkpartBMax, partCheck );
mFeatureIndex->Remove( otherPartBMin, otherPartBMax, otherPart );
mFeatureIndex.Remove( checkpartBMin, checkpartBMax, partCheck );
mFeatureIndex.Remove( otherPartBMin, otherPartBMax, otherPart );

// reinsert merged line to r-tree (probably not needed)
otherPart->getBoundingBox( otherPartBMin, otherPartBMax );
mFeatureIndex->Insert( otherPartBMin, otherPartBMax, otherPart );
mFeatureIndex.Insert( otherPartBMin, otherPartBMax, otherPart );

mConnectedFeaturesIds.insert( partCheck->featureId(), connectedFeaturesId );
mConnectedFeaturesIds.insert( otherPart->featureId(), connectedFeaturesId );
Expand Down Expand Up @@ -423,7 +417,7 @@ void Layer::chopFeaturesAtRepeatDistance()
{
double bmin[2], bmax[2];
fpart->getBoundingBox( bmin, bmax );
mFeatureIndex->Remove( bmin, bmax, fpart.get() );
mFeatureIndex.Remove( bmin, bmax, fpart.get() );

const GEOSCoordSequence *cs = GEOSGeom_getCoordSeq_r( geosctxt, geom );

Expand Down Expand Up @@ -484,7 +478,7 @@ void Layer::chopFeaturesAtRepeatDistance()
FeaturePart *newfpart = new FeaturePart( fpart->feature(), newgeom );
newFeatureParts.append( newfpart );
newfpart->getBoundingBox( bmin, bmax );
mFeatureIndex->Insert( bmin, bmax, newfpart );
mFeatureIndex.Insert( bmin, bmax, newfpart );
repeatParts.push_back( newfpart );

break;
Expand All @@ -509,7 +503,7 @@ void Layer::chopFeaturesAtRepeatDistance()
FeaturePart *newfpart = new FeaturePart( fpart->feature(), newgeom );
newFeatureParts.append( newfpart );
newfpart->getBoundingBox( bmin, bmax );
mFeatureIndex->Insert( bmin, bmax, newfpart );
mFeatureIndex.Insert( bmin, bmax, newfpart );
part.clear();
part.push_back( p );
repeatParts.push_back( newfpart );
Expand Down
34 changes: 17 additions & 17 deletions src/core/pal/layer.h
View file
Expand Up @@ -77,6 +77,21 @@ namespace pal
ShowAll // show upside down for all labels, including dynamic ones
};

/**
* \brief Create a new layer
*
* \param provider Associated provider
* \param name Name of the layer (for stats, debugging - does not need to be unique)
* \param arrangement Arrangement mode : how to place candidates
* \param defaultPriority layer's prioriry (0 is the best, 1 the worst)
* \param active is the layer is active (currently displayed)
* \param toLabel the layer will be labeled whether toLablel is TRUE
* \param pal pointer to the pal object
* \param displayAll if TRUE, all features will be labelled even though overlaps occur
*
*/
Layer( QgsAbstractLabelProvider *provider, const QString &name, QgsPalLayerSettings::Placement arrangement, double defaultPriority, bool active, bool toLabel, Pal *pal, bool displayAll = false );

virtual ~Layer();

bool displayAll() const { return mDisplayAll; }
Expand Down Expand Up @@ -332,33 +347,18 @@ namespace pal
UpsideDownLabels mUpsidedownLabels;

// indexes (spatial and id)
RTree<FeaturePart *, double, 2, double, 8, 4> *mFeatureIndex;
RTree<FeaturePart *, double, 2, double, 8, 4> mFeatureIndex;
//! Lookup table of label features (owned by the label feature provider that created them)
QHash< QgsFeatureId, QgsLabelFeature *> mHashtable;

//obstacle r-tree
RTree<FeaturePart *, double, 2, double, 8, 4> *mObstacleIndex;
RTree<FeaturePart *, double, 2, double, 8, 4> mObstacleIndex;

QHash< QString, QVector<FeaturePart *> > mConnectedHashtable;
QHash< QgsFeatureId, int > mConnectedFeaturesIds;

QMutex mMutex;

/**
* \brief Create a new layer
*
* \param provider Associated provider
* \param name Name of the layer (for stats, debugging - does not need to be unique)
* \param arrangement Arrangement mode : how to place candidates
* \param defaultPriority layer's prioriry (0 is the best, 1 the worst)
* \param active is the layer is active (currently displayed)
* \param toLabel the layer will be labeled whether toLablel is TRUE
* \param pal pointer to the pal object
* \param displayAll if TRUE, all features will be labelled even though overlaps occur
*
*/
Layer( QgsAbstractLabelProvider *provider, const QString &name, QgsPalLayerSettings::Placement arrangement, double defaultPriority, bool active, bool toLabel, Pal *pal, bool displayAll = false );

//! Add newly created feature part into r tree and to the list
void addFeaturePart( FeaturePart *fpart, const QString &labelText = QString() );

Expand Down
54 changes: 28 additions & 26 deletions src/core/pal/pal.cpp
View file
Expand Up @@ -58,20 +58,21 @@ void Pal::removeLayer( Layer *layer )
return;

mMutex.lock();
if ( QgsAbstractLabelProvider *key = mLayers.key( layer, nullptr ) )

for ( auto it = mLayers.begin(); it != mLayers.end(); ++it )
{
mLayers.remove( key );
delete layer;
if ( it->second.get() == layer )
{
mLayers.erase( it );
break;
}
}
mMutex.unlock();
}

Pal::~Pal()
{

mMutex.lock();

qDeleteAll( mLayers );
mLayers.clear();
mMutex.unlock();

Expand All @@ -83,13 +84,14 @@ Layer *Pal::addLayer( QgsAbstractLabelProvider *provider, const QString &layerNa
{
mMutex.lock();

Q_ASSERT( !mLayers.contains( provider ) );
Q_ASSERT( mLayers.find( provider ) == mLayers.end() );

Layer *layer = new Layer( provider, layerName, arrangement, defaultPriority, active, toLabel, this, displayAll );
mLayers.insert( provider, layer );
std::unique_ptr< Layer > layer = qgis::make_unique< Layer >( provider, layerName, arrangement, defaultPriority, active, toLabel, this, displayAll );
Layer *res = layer.get();
mLayers.insert( { provider, std::move( layer ) } );
mMutex.unlock();

return layer;
return res;
}

struct FeatCallBackCtx
Expand All @@ -99,8 +101,8 @@ struct FeatCallBackCtx
std::list< std::unique_ptr< Feats > > *features = nullptr;

RTree<FeaturePart *, double, 2, double> *obstacleIndex = nullptr;
RTree<LabelPosition *, double, 2, double> *candidateIndex = nullptr;
QList<LabelPosition *> *positionsWithNoCandidates = nullptr;
RTree<LabelPosition *, double, 2, double> *allCandidateIndex = nullptr;
std::vector< std::unique_ptr< LabelPosition > > *positionsWithNoCandidates = nullptr;
const GEOSPreparedGeometry *mapBoundary = nullptr;
Pal *pal = nullptr;
};
Expand Down Expand Up @@ -145,7 +147,7 @@ bool extractFeatCallback( FeaturePart *featurePart, void *ctx )
{
for ( std::unique_ptr< LabelPosition > &candidate : candidates )
{
candidate->insertIntoIndex( context->candidateIndex );
candidate->insertIntoIndex( *context->allCandidateIndex );
}

std::sort( candidates.begin(), candidates.end(), CostCalculator::candidateSortGrow );
Expand All @@ -163,7 +165,7 @@ bool extractFeatCallback( FeaturePart *featurePart, void *ctx )
// features with no candidates are recorded in the unlabeled feature list
std::unique_ptr< LabelPosition > unplacedPosition = featurePart->createCandidatePointOnSurface( featurePart );
if ( unplacedPosition )
context->positionsWithNoCandidates->append( unplacedPosition.release() );
context->positionsWithNoCandidates->emplace_back( std::move( unplacedPosition ) );
}

return true;
Expand Down Expand Up @@ -194,14 +196,13 @@ bool extractObstaclesCallback( FeaturePart *ft_ptr, void *ctx )

struct FilterContext
{
RTree<LabelPosition *, double, 2, double> *candidateIndex = nullptr;
RTree<LabelPosition *, double, 2, double> *allCandidatesIndex = nullptr;
Pal *pal = nullptr;
};

bool filteringCallback( FeaturePart *featurePart, void *ctx )
{

RTree<LabelPosition *, double, 2, double> *cdtsIndex = ( reinterpret_cast< FilterContext * >( ctx ) )->candidateIndex;
RTree<LabelPosition *, double, 2, double> *allCandidatesIndex = ( reinterpret_cast< FilterContext * >( ctx ) )->allCandidatesIndex;
Pal *pal = ( reinterpret_cast< FilterContext * >( ctx ) )->pal;

if ( pal->isCanceled() )
Expand All @@ -213,7 +214,7 @@ bool filteringCallback( FeaturePart *featurePart, void *ctx )
LabelPosition::PruneCtx pruneContext;
pruneContext.obstacle = featurePart;
pruneContext.pal = pal;
cdtsIndex->Search( amin, amax, LabelPosition::pruneCallback, static_cast< void * >( &pruneContext ) );
allCandidatesIndex->Search( amin, amax, LabelPosition::pruneCallback, static_cast< void * >( &pruneContext ) );

return true;
}
Expand Down Expand Up @@ -249,7 +250,7 @@ std::unique_ptr<Problem> Pal::extract( const QgsRectangle &extent, const QgsGeom

context.features = &features;
context.obstacleIndex = &obstacles;
context.candidateIndex = prob->mCandidatesIndex;
context.allCandidateIndex = &prob->mAllCandidatesIndex;
context.positionsWithNoCandidates = prob->positionsWithNoCandidates();
context.mapBoundary = mapBoundaryPrepared.get();
context.pal = this;
Expand All @@ -266,8 +267,9 @@ std::unique_ptr<Problem> Pal::extract( const QgsRectangle &extent, const QgsGeom
QStringList layersWithFeaturesInBBox;

mMutex.lock();
for ( Layer *layer : qgis::as_const( mLayers ) )
for ( const auto &it : mLayers )
{
Layer *layer = it.second.get();
if ( !layer )
{
// invalid layer name
Expand All @@ -288,9 +290,9 @@ std::unique_ptr<Problem> Pal::extract( const QgsRectangle &extent, const QgsGeom

// find features within bounding box and generate candidates list
context.layer = layer;
layer->mFeatureIndex->Search( amin, amax, extractFeatCallback, static_cast< void * >( &context ) );
layer->mFeatureIndex.Search( amin, amax, extractFeatCallback, static_cast< void * >( &context ) );
// find obstacles within bounding box
layer->mObstacleIndex->Search( amin, amax, extractObstaclesCallback, static_cast< void * >( &obstacleContext ) );
layer->mObstacleIndex.Search( amin, amax, extractObstaclesCallback, static_cast< void * >( &obstacleContext ) );

layer->mMutex.unlock();

Expand Down Expand Up @@ -318,7 +320,7 @@ std::unique_ptr<Problem> Pal::extract( const QgsRectangle &extent, const QgsGeom
amin[0] = amin[1] = std::numeric_limits<double>::lowest();
amax[0] = amax[1] = std::numeric_limits<double>::max();
FilterContext filterCtx;
filterCtx.candidateIndex = prob->mCandidatesIndex;
filterCtx.allCandidatesIndex = &prob->mAllCandidatesIndex;
filterCtx.pal = this;
obstacles.Search( amin, amax, filteringCallback, static_cast< void * >( &filterCtx ) );

Expand Down Expand Up @@ -356,7 +358,7 @@ std::unique_ptr<Problem> Pal::extract( const QgsRectangle &extent, const QgsGeom
while ( feat->candidates.size() > max_p )
{
// TODO remove from index
feat->candidates.back()->removeFromIndex( prob->mCandidatesIndex );
feat->candidates.back()->removeFromIndex( prob->mAllCandidatesIndex );
feat->candidates.pop_back();
}

Expand Down Expand Up @@ -399,11 +401,11 @@ std::unique_ptr<Problem> Pal::extract( const QgsRectangle &extent, const QgsGeom
lp->getBoundingBox( amin, amax );

// lookup for overlapping candidate
prob->mCandidatesIndex->Search( amin, amax, LabelPosition::countOverlapCallback, static_cast< void * >( lp.get() ) );
prob->mAllCandidatesIndex.Search( amin, amax, LabelPosition::countOverlapCallback, static_cast< void * >( lp.get() ) );

nbOverlaps += lp->getNumOverlaps();

prob->addCandidatePosition( lp.release() );
prob->addCandidatePosition( std::move( lp ) );
}
}
nbOverlaps /= 2;
Expand Down
3 changes: 2 additions & 1 deletion src/core/pal/pal.h
View file
Expand Up @@ -43,6 +43,7 @@
#include <ctime>
#include <QMutex>
#include <QStringList>
#include <unordered_map>

// TODO ${MAJOR} ${MINOR} etc instead of 0.2

Expand Down Expand Up @@ -224,7 +225,7 @@ namespace pal

private:

QHash< QgsAbstractLabelProvider *, Layer * > mLayers;
std::unordered_map< QgsAbstractLabelProvider *, std::unique_ptr< Layer > > mLayers;

QMutex mMutex;

Expand Down

0 comments on commit 634a815

Please sign in to comment.