Bug report #4454: multiline labels overlap in the labeling-ng engine - QGIS Application - QGIS Issue Tracking

Bug report #4454

multiline labels overlap in the labeling-ng engine

Added by Mathieu Pellerin - nIRV about 13 years ago. Updated about 13 years ago.

Status:	Closed
Priority:	Normal
Assignee:	Martin Dobias
Category:	Map Canvas
Affected QGIS version:		Regression?:	No
Operating System:		Easy fix?:	No
Pull Request or Patch supplied:	Yes	Resolution:	fixed
Crashes QGIS or corrupts data:		Copied to github as #:	14384

Description

The recently added expression-based label feature exposed an issue with multiline support in the labeling-ng engine.

The labeling-ng placement mechanism appears to only consider the width of a label's last line, resulting in major overlaps if the first line's length is greater than the last one. (see screenshot A map-preylang-labeloverlap.jpg)

E.G.
Line #1: Big Plantation Company
Line #2: 500 Ha

I think the above assessment must be pointing in the right direction. If I change the multiline label to have the longest string on the last line, the labeling engine doesn't render overlapping labels. (see screenshot B map-preylang-labelclean.jpg)

This issue will have a greater impact on users when the next qgis version will be released with the expression-based labeling, hope this issue' description is helpful.

map-preylang-labeloverlap.jpg - screenshot A (672 KB) Mathieu Pellerin - nIRV, 2011-10-27 08:24 PM

map-preylang-labelclean.jpg - screenshot B (667 KB) Mathieu Pellerin - nIRV, 2011-10-27 08:24 PM

History

#1 Updated by Mathieu Pellerin - nIRV about 13 years ago

I'm pretty sure I found the problem.

The multiline label's width is determined in the void QgsPalLayerSettings::calculateLabelSize() function. The relevant code is this:

QRectF labelRect = fm->boundingRect( text );
double w, h;
if ( !multiLineLabels )
{
  w = labelRect.width() / rasterCompressFactor;
  h = labelRect.height() / rasterCompressFactor;
}
else
{
  QStringList multiLineSplit = text.split( "\
" );
  h = fm->height() * multiLineSplit.size() / rasterCompressFactor;
  w = 0;
  for ( int i = 0; i < multiLineSplit.size(); ++i )
  {
    double width = fm->width( multiLineSplit.at( i ) );
    if ( width > w )
    {
      w = width;
    }
    w /= rasterCompressFactor;
  }
}

The problem is in the for loop that compares each line's width to find out the longest one. The rasterCompressFactor division applied to the width should occur after the for loop has found the longest width, no within the loop. The current code break the if (width > w) condition as it's not comparing two width but rather a width and a width / rasterCompressFactor.

So, the solution is to move w /= rasterCompressFactor; after the loop. Yay, a one line change fix! :)

#2 Updated by Mathieu Pellerin - nIRV about 13 years ago

The code should be:

QRectF labelRect = fm->boundingRect( text );
double w, h;
if ( !multiLineLabels )
{
  w = labelRect.width() / rasterCompressFactor;
  h = labelRect.height() / rasterCompressFactor;
}
else
{
  QStringList multiLineSplit = text.split( "\
" );
  h = fm->height() * multiLineSplit.size() / rasterCompressFactor;
  w = 0;
  for ( int i = 0; i < multiLineSplit.size(); ++i )
  {
    double width = fm->width( multiLineSplit.at( i ) );
    if ( width > w )
    {
      w = width;
    }
  }
  w /= rasterCompressFactor;
}

I don't have the environment set up to come up with a proper patch, hope someone can do it on my behalf.