Skip to content
Permalink
Browse files

[pal] Remove some odd/unintelligable logic

Seems to be trimming the candidate list early and artificially
resetting all candidate costs if they are too high?

I believe this code is wrong, or has been made redundant by other
changes to the logic of the labeling engine. At this stage we have
not completely calculated the costs for label candidates, so
sorting and truncating the candidate list here is premature and potentially
removes nice candidates.
  • Loading branch information
nyalldawson committed Dec 26, 2019
1 parent ddd2c6e commit 49a7862e28e1041f1471cd9ae13dbad9a30ab15e
Showing with 28 additions and 61 deletions.
  1. +15 −50 src/core/pal/costcalculator.cpp
  2. +2 −2 src/core/pal/costcalculator.h
  3. +2 −3 src/core/pal/feature.cpp
  4. +1 −1 src/core/pal/feature.h
  5. +8 −5 src/core/pal/pal.cpp
@@ -112,27 +112,26 @@ void CostCalculator::addObstacleCostPenalty( LabelPosition *lp, FeaturePart *obs
lp->setCost( lp->cost() + obstacleCost );
}

void CostCalculator::setPolygonCandidatesCost( std::size_t nblp, std::vector< std::unique_ptr< LabelPosition > > &lPos, PalRtree<FeaturePart> *obstacles, double bbx[4], double bby[4] )
void CostCalculator::setPolygonCandidatesCost( std::vector< std::unique_ptr< LabelPosition > > &lPos, PalRtree<FeaturePart> *obstacles, double bbx[4], double bby[4] )
{
double normalizer;
// compute raw cost
for ( std::size_t i = 0; i < nblp; ++i )
setCandidateCostFromPolygon( lPos[ i ].get(), obstacles, bbx, bby );
for ( std::unique_ptr< LabelPosition > &pos : lPos )
setCandidateCostFromPolygon( pos.get(), obstacles, bbx, bby );

// lPos with big values came first (value = min distance from label to Polygon's Perimeter)
// IMPORTANT - only want to sort first nblp positions. The rest have not had the cost
// calculated so will have nonsense values
std::sort( lPos.begin(), lPos.begin() + nblp, candidateSortShrink );
std::sort( lPos.begin(), lPos.end(), candidateSortShrink );

// define the value's range
double cost_max = lPos.front()->cost();
double cost_min = lPos.back()->cost();
const double costMin = lPos.back()->cost();
const double costMax = lPos.front()->cost();
const double costRange = costMax - costMin;

cost_max -= cost_min;

if ( cost_max > EPSILON )
if ( costRange > EPSILON )
{
normalizer = 0.0020 / cost_max;
normalizer = 0.0020 / costRange;
}
else
{
@@ -141,17 +140,14 @@ void CostCalculator::setPolygonCandidatesCost( std::size_t nblp, std::vector< st

// adjust cost => the best is 0.0001, the worst is 0.0021
// others are set proportionally between best and worst
for ( std::size_t i = 0; i < nblp; ++i )
for ( std::unique_ptr< LabelPosition > &pos : lPos )
{
LabelPosition *pos = lPos[ i ].get();
//if (cost_max - cost_min < EPSILON)
if ( cost_max > EPSILON )
if ( costRange > EPSILON )
{
pos->setCost( 0.0021 - ( pos->cost() - cost_min ) * normalizer );
pos->setCost( 0.0021 - ( pos->cost() - costMin ) * normalizer );
}
else
{
//pos->cost = 0.0001 + (pos->cost - cost_min) * normalizer;
pos->setCost( 0.0001 );
}
}
@@ -203,49 +199,18 @@ void CostCalculator::setCandidateCostFromPolygon( LabelPosition *lp, PalRtree<Fe
lp->setCost( pCost.getCost() );
}

std::size_t CostCalculator::finalizeCandidatesCosts( Feats *feat, std::size_t max_p, PalRtree<FeaturePart> *obstacles, double bbx[4], double bby[4] )
void CostCalculator::finalizeCandidatesCosts( Feats *feat, PalRtree<FeaturePart> *obstacles, double bbx[4], double bby[4] )
{
// If candidates list is smaller than expected
if ( max_p == 0 || max_p > feat->candidates.size() )
max_p = feat->candidates.size();
//
// sort candidates list, best label to worst
std::sort( feat->candidates.begin(), feat->candidates.end(), candidateSortGrow );

// try to exclude all conflitual labels (good ones have cost < 1 by pruning)
double discrim = 0.0;
std::size_t stop = 0;
do
{
discrim += 1.0;
for ( stop = 0; stop < feat->candidates.size() && feat->candidates[ stop ]->cost() < discrim; stop++ )
;
}
while ( stop == 0 && discrim < feat->candidates.back()->cost() + 2.0 );

// THIS LOOKS SUSPICIOUS -- it clamps all costs to a fixed value??
if ( discrim > 1.5 )
{
for ( std::size_t k = 0; k < stop; k++ )
feat->candidates[ k ]->setCost( 0.0021 );
}

if ( max_p > stop )
max_p = stop;

// Sets costs for candidates of polygon

if ( feat->feature->getGeosType() == GEOS_POLYGON )
{
int arrangement = feat->feature->layer()->arrangement();
if ( arrangement == QgsPalLayerSettings::Free || arrangement == QgsPalLayerSettings::Horizontal )
setPolygonCandidatesCost( stop, feat->candidates, obstacles, bbx, bby );
setPolygonCandidatesCost( feat->candidates, obstacles, bbx, bby );
}

// add size penalty (small lines/polygons get higher cost)
feat->feature->addSizePenalty( max_p, feat->candidates, bbx, bby );

return max_p;
feat->feature->addSizePenalty( feat->candidates, bbx, bby );
}

PolygonCostCalculator::PolygonCostCalculator( LabelPosition *lp ) : lp( lp )
@@ -41,13 +41,13 @@ namespace pal
static void addObstacleCostPenalty( pal::LabelPosition *lp, pal::FeaturePart *obstacle, Pal *pal );

//! Calculates the costs for polygon label candidates
static void setPolygonCandidatesCost( std::size_t nblp, std::vector<std::unique_ptr<pal::LabelPosition> > &lPos, PalRtree< FeaturePart > *obstacles, double bbx[4], double bby[4] );
static void setPolygonCandidatesCost( std::vector<std::unique_ptr<pal::LabelPosition> > &lPos, PalRtree< FeaturePart > *obstacles, double bbx[4], double bby[4] );

//! Sets cost to the smallest distance between lPos's centroid and a polygon stored in geometry field
static void setCandidateCostFromPolygon( LabelPosition *lp, PalRtree< FeaturePart > *obstacles, double bbx[4], double bby[4] );

//! Sort candidates by costs, skip the worse ones, evaluate polygon candidates
static std::size_t finalizeCandidatesCosts( Feats *feat, std::size_t max_p, PalRtree< FeaturePart > *obstacles, double bbx[4], double bby[4] );
static void finalizeCandidatesCosts( Feats *feat, PalRtree< FeaturePart > *obstacles, double bbx[4], double bby[4] );

/**
* Sorts label candidates in ascending order of cost
@@ -1729,7 +1729,7 @@ std::vector< std::unique_ptr< LabelPosition > > FeaturePart::createCandidates( P
return lPos;
}

void FeaturePart::addSizePenalty( std::size_t nbp, std::vector< std::unique_ptr< LabelPosition > > &lPos, double bbx[4], double bby[4] )
void FeaturePart::addSizePenalty( std::vector< std::unique_ptr< LabelPosition > > &lPos, double bbx[4], double bby[4] )
{
if ( !mGeos )
createGeosGeom();
@@ -1780,9 +1780,8 @@ void FeaturePart::addSizePenalty( std::size_t nbp, std::vector< std::unique_ptr<
return; // no size penalty for points

// apply the penalty
for ( std::size_t i = 0; i < nbp; i++ )
for ( std::unique_ptr< LabelPosition > &pos : lPos )
{
LabelPosition *pos = lPos[ i ].get();
pos->setCost( pos->cost() + sizeCost / 100 );
}
}
@@ -324,7 +324,7 @@ namespace pal
*
* E.g. small lines or polygons get higher cost so that larger features are more likely to be labeled.
*/
void addSizePenalty( std::size_t nbp, std::vector<std::unique_ptr<LabelPosition> > &lPos, double bbx[4], double bby[4] );
void addSizePenalty( std::vector<std::unique_ptr<LabelPosition> > &lPos, double bbx[4], double bby[4] );

/**
* Calculates the priority for the feature. This will be the feature's priority if set,
@@ -316,9 +316,6 @@ std::unique_ptr<Problem> Pal::extract( const QgsRectangle &extent, const QgsGeom
break;
}

// sort candidates by cost, skip less interesting ones, calculate polygon costs (if using polygons)
max_p = CostCalculator::finalizeCandidatesCosts( feat.get(), max_p, &obstacles, bbx, bby );

if ( isCanceled() )
return nullptr;

@@ -352,10 +349,16 @@ std::unique_ptr<Problem> Pal::extract( const QgsRectangle &extent, const QgsGeom
}
}

// calculate final costs
CostCalculator::finalizeCandidatesCosts( feat.get(), &obstacles, bbx, bby );

// sort candidates list, best label to worst
std::sort( feat->candidates.begin(), feat->candidates.end(), CostCalculator::candidateSortGrow );

// only keep the 'max_p' best candidates
while ( feat->candidates.size() > max_p )
if ( feat->candidates.size() > max_p )
{
feat->candidates.pop_back();
feat->candidates.resize( max_p );
}

if ( isCanceled() )

0 comments on commit 49a7862

Please sign in to comment.
You can’t perform that action at this time.