Skip to content

Commit

Permalink
[labeling] Fix incorrect bounding box of labels used when
Browse files Browse the repository at this point in the history
map is rotated

Fixes #24680

(cherry picked from commit 1bc716f)
  • Loading branch information
nyalldawson committed Jun 10, 2019
1 parent 75328ee commit 2e9fe0b
Show file tree
Hide file tree
Showing 7 changed files with 123 additions and 56 deletions.
1 change: 1 addition & 0 deletions python/core/auto_generated/qgslabelsearchtree.sip.in
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@




class QgsLabelSearchTree
{
%Docstring
Expand Down
22 changes: 18 additions & 4 deletions python/core/auto_generated/qgspallabeling.sip.in
Original file line number Diff line number Diff line change
Expand Up @@ -19,25 +19,39 @@ class QgsLabelPosition
#include "qgspallabeling.h"
%End
public:
QgsLabelPosition( int id, double r, const QVector< QgsPointXY > &corners, const QgsRectangle &rect, double w, double h, const QString &layer, const QString &labeltext, const QFont &labelfont, bool upside_down, bool diagram = false, bool pinned = false, const QString &providerId = QString() );
QgsLabelPosition( QgsFeatureId id, double r, const QVector< QgsPointXY > &corners, const QgsRectangle &rect, double w, double h, const QString &layer, const QString &labeltext, const QFont &labelfont, bool upside_down, bool diagram = false, bool pinned = false, const QString &providerId = QString(),
const QgsGeometry &labelGeometry = QgsGeometry() );

QgsLabelPosition();
%Docstring
Constructor for QgsLabelPosition
%End

int featureId;
QgsFeatureId featureId;

double rotation;

QVector< QgsPointXY > cornerPoints;
QgsRectangle labelRect;

QgsGeometry labelGeometry;

double width;

double height;

QString layerID;

QString labelText;

QFont labelFont;

bool upsideDown;

bool isDiagram;

bool isPinned;

QString providerID;
};

Expand Down Expand Up @@ -553,7 +567,7 @@ Prepares a geometry for registration with PAL. Handles reprojection, rotation, c
:param context: render context
:param ct: coordinate transform, or invalid transform if no transformation required
:param clipGeometry: geometry to clip features to, if applicable
:param mergeLines: ``True`` if touching lines from this layer will be merged and treated as single features during labeling
:param mergeLines: TRUE if touching lines from this layer will be merged and treated as single features during labeling

:return: prepared geometry

Expand All @@ -568,7 +582,7 @@ Checks whether a geometry requires preparation before registration with PAL
:param context: render context
:param ct: coordinate transform, or invalid transform if no transformation required
:param clipGeometry: geometry to clip features to, if applicable
:param mergeLines: ``True`` if touching lines from this layer will be merged and treated as single features during labeling
:param mergeLines: TRUE if touching lines from this layer will be merged and treated as single features during labeling

:return: true if geometry requires preparation

Expand Down
11 changes: 5 additions & 6 deletions src/app/qgsmaptoollabel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -68,13 +68,12 @@ void QgsMapToolLabel::createRubberBands()
delete mFeatureRubberBand;

//label rubber band
QgsRectangle rect = mCurrentLabel.pos.labelRect;
mLabelRubberBand = new QgsRubberBand( mCanvas, QgsWkbTypes::LineGeometry );
mLabelRubberBand->addPoint( QgsPointXY( rect.xMinimum(), rect.yMinimum() ) );
mLabelRubberBand->addPoint( QgsPointXY( rect.xMinimum(), rect.yMaximum() ) );
mLabelRubberBand->addPoint( QgsPointXY( rect.xMaximum(), rect.yMaximum() ) );
mLabelRubberBand->addPoint( QgsPointXY( rect.xMaximum(), rect.yMinimum() ) );
mLabelRubberBand->addPoint( QgsPointXY( rect.xMinimum(), rect.yMinimum() ) );
mLabelRubberBand->addPoint( mCurrentLabel.pos.cornerPoints.at( 0 ) );
mLabelRubberBand->addPoint( mCurrentLabel.pos.cornerPoints.at( 1 ) );
mLabelRubberBand->addPoint( mCurrentLabel.pos.cornerPoints.at( 2 ) );
mLabelRubberBand->addPoint( mCurrentLabel.pos.cornerPoints.at( 3 ) );
mLabelRubberBand->addPoint( mCurrentLabel.pos.cornerPoints.at( 0 ) );
mLabelRubberBand->setColor( QColor( 0, 255, 0, 65 ) );
mLabelRubberBand->setWidth( 3 );
mLabelRubberBand->show();
Expand Down
11 changes: 5 additions & 6 deletions src/app/qgsmaptoolpinlabels.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -140,13 +140,12 @@ void QgsMapToolPinLabels::highlightLabel( const QgsLabelPosition &labelpos,
const QString &id,
const QColor &color )
{
QgsRectangle rect = labelpos.labelRect;
QgsRubberBand *rb = new QgsRubberBand( mCanvas, QgsWkbTypes::PolygonGeometry );
rb->addPoint( QgsPointXY( rect.xMinimum(), rect.yMinimum() ) );
rb->addPoint( QgsPointXY( rect.xMinimum(), rect.yMaximum() ) );
rb->addPoint( QgsPointXY( rect.xMaximum(), rect.yMaximum() ) );
rb->addPoint( QgsPointXY( rect.xMaximum(), rect.yMinimum() ) );
rb->addPoint( QgsPointXY( rect.xMinimum(), rect.yMinimum() ) );
rb->addPoint( labelpos.cornerPoints.at( 0 ) );
rb->addPoint( labelpos.cornerPoints.at( 1 ) );
rb->addPoint( labelpos.cornerPoints.at( 2 ) );
rb->addPoint( labelpos.cornerPoints.at( 3 ) );
rb->addPoint( labelpos.cornerPoints.at( 0 ) );
rb->setColor( color );
rb->setWidth( 0 );
rb->show();
Expand Down
39 changes: 22 additions & 17 deletions src/core/qgslabelsearchtree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ void QgsLabelSearchTree::label( const QgsPointXY &point, QList<QgsLabelPosition
QList<QgsLabelPosition *>::const_iterator resultIt = searchResults.constBegin();
for ( ; resultIt != searchResults.constEnd(); ++resultIt )
{
if ( ( *resultIt )->labelRect.contains( p ) )
if ( ( *resultIt )->labelGeometry.contains( &p ) )
{
posList.push_back( *resultIt );
}
Expand All @@ -68,7 +68,10 @@ void QgsLabelSearchTree::labelsInRect( const QgsRectangle &r, QList<QgsLabelPosi
QList<QgsLabelPosition *>::const_iterator resultIt = searchResults.constBegin();
for ( ; resultIt != searchResults.constEnd(); ++resultIt )
{
posList.push_back( *resultIt );
if ( ( *resultIt )->labelGeometry.intersects( r ) )
{
posList.push_back( *resultIt );
}
}
}

Expand All @@ -79,30 +82,32 @@ bool QgsLabelSearchTree::insertLabel( pal::LabelPosition *labelPos, int featureI
return false;
}

double c_min[2];
double c_max[2];
labelPos->getBoundingBox( c_min, c_max );

// we have to transform the bounding box to convert pre-rotated label positions back to real world locations
double x1, y1;
double x2, y2;
mTransform.map( c_min[0], c_min[1], &x1, &y1 );
mTransform.map( c_max[0], c_max[1], &x2, &y2 );
c_min[0] = std::min( x1, x2 );
c_min[1] = std::min( y1, y2 );
c_max[0] = std::max( x1, x2 );
c_max[1] = std::max( y1, y2 );

QVector<QgsPointXY> cornerPoints;
cornerPoints.reserve( 4 );
double xMin = std::numeric_limits< double >::max();
double yMin = std::numeric_limits< double >::max();
double xMax = std::numeric_limits< double >::lowest();
double yMax = std::numeric_limits< double >::lowest();
for ( int i = 0; i < 4; ++i )
{
// we have to transform the bounding box to convert pre-rotated label positions back to real world locations
QPointF res = mTransform.map( QPointF( labelPos->getX( i ), labelPos->getY( i ) ) );
cornerPoints.push_back( QgsPointXY( res ) );
xMin = std::min( xMin, res.x() );
xMax = std::max( xMax, res.x() );
yMin = std::min( yMin, res.y() );
yMax = std::max( yMax, res.y() );
}
double c_min[2];
double c_max[2];
c_min[0] = xMin;
c_min[1] = yMin;
c_max[0] = xMax;
c_max[1] = yMax;

QgsGeometry labelGeometry( QgsGeometry::fromPolygonXY( QVector<QgsPolylineXY>() << cornerPoints ) );
std::unique_ptr< QgsLabelPosition > newEntry = qgis::make_unique< QgsLabelPosition >( featureId, labelPos->getAlpha() + mMapSettings.rotation(), cornerPoints, QgsRectangle( c_min[0], c_min[1], c_max[0], c_max[1] ),
labelPos->getWidth(), labelPos->getHeight(), layerName, labeltext, labelfont, labelPos->getUpsideDown(), diagram, pinned, providerId );
labelPos->getWidth(), labelPos->getHeight(), layerName, labeltext, labelfont, labelPos->getUpsideDown(), diagram, pinned, providerId, labelGeometry );
mSpatialIndex.Insert( c_min, c_max, newEntry.get() );
mOwnedPositions.emplace_back( std::move( newEntry ) );
return true;
Expand Down
58 changes: 55 additions & 3 deletions src/core/qgspallabeling.h
Original file line number Diff line number Diff line change
Expand Up @@ -80,11 +80,13 @@ class QgsExpressionContext;
class CORE_EXPORT QgsLabelPosition
{
public:
QgsLabelPosition( int id, double r, const QVector< QgsPointXY > &corners, const QgsRectangle &rect, double w, double h, const QString &layer, const QString &labeltext, const QFont &labelfont, bool upside_down, bool diagram = false, bool pinned = false, const QString &providerId = QString() )
QgsLabelPosition( QgsFeatureId id, double r, const QVector< QgsPointXY > &corners, const QgsRectangle &rect, double w, double h, const QString &layer, const QString &labeltext, const QFont &labelfont, bool upside_down, bool diagram = false, bool pinned = false, const QString &providerId = QString(),
const QgsGeometry &labelGeometry = QgsGeometry() )
: featureId( id )
, rotation( r )
, cornerPoints( corners )
, labelRect( rect )
, labelGeometry( labelGeometry )
, width( w )
, height( h )
, layerID( layer )
Expand All @@ -99,19 +101,69 @@ class CORE_EXPORT QgsLabelPosition
//! Constructor for QgsLabelPosition
QgsLabelPosition() = default;

int featureId = -1;
/**
* ID of feature associated with this label.
*/
QgsFeatureId featureId = FID_NULL;

/**
* Rotation of label, in degrees clockwise.
*/
double rotation = 0;

QVector< QgsPointXY > cornerPoints;
QgsRectangle labelRect;

/**
* A polygon geometry representing the label's bounds in map coordinates.
* \since QGIS 3.4.9
*/
QgsGeometry labelGeometry;

/**
* Width of label bounding box, in map units.
*/
double width = 0;

/**
* Heeght of label bounding box, in map units.
*/
double height = 0;

/**
* ID of associated map layer.
*/
QString layerID;

/**
* String shown in label.
*/
QString labelText;

/**
* Font which the label is rendered using.
*/
QFont labelFont;

/**
* True if label is upside down.
*/
bool upsideDown = false;

/**
* True if label is a diagram.
*/
bool isDiagram = false;

/**
* True if label position has been pinned.
*/
bool isPinned = false;
//! \since QGIS 2.14

/**
* ID of the associated label provider.
* \since QGIS 2.14
*/
QString providerID;
};

Expand Down
37 changes: 17 additions & 20 deletions tests/src/core/testqgslabelingengine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1331,18 +1331,19 @@ void TestQgsLabelingEngine::labelingResults()
QCOMPARE( labels.at( 0 ).labelText, QStringLiteral( "1" ) );
QGSCOMPARENEAR( labels.at( 0 ).width, 167961, 500 ); // tolerance will probably need tweaking, to account for cross-platform font diffs
QGSCOMPARENEAR( labels.at( 0 ).height, 295119, 500 );
#if 0 // TODO
QGSCOMPARENEAR( labels.at( 0 ).labelRect.xMinimum(), -779822, 500 );
QGSCOMPARENEAR( labels.at( 0 ).labelRect.xMaximum(), -611861, 500 );
QGSCOMPARENEAR( labels.at( 0 ).labelRect.yMinimum(), 6897647, 500 );
QGSCOMPARENEAR( labels.at( 0 ).labelRect.yMaximum(), 7192767, 500 );
#endif
QGSCOMPARENEAR( labels.at( 0 ).labelRect.xMinimum(), -865622, 500 );
QGSCOMPARENEAR( labels.at( 0 ).labelRect.xMaximum(), -526060, 500 );
QGSCOMPARENEAR( labels.at( 0 ).labelRect.yMinimum(), 6898697, 500 );
QGSCOMPARENEAR( labels.at( 0 ).labelRect.yMaximum(), 7191716, 500 );
QCOMPARE( labels.at( 0 ).rotation, 60 );

// should fall outside of rotated bounding box!
labels = results->labelsAtPosition( QgsPointXY( -769822, 6927647 ) );
QCOMPARE( labels.count(), 1 );
QCOMPARE( labels.at( 0 ).featureId, 1 );
QCOMPARE( labels.count(), 0 );
labels = results->labelsAtPosition( QgsPointXY( -615861, 7132767 ) );
QCOMPARE( labels.count(), 0 );
// just on corner, should only work if rotation of label's bounding box is handled correctly
labels = results->labelsAtPosition( QgsPointXY( -610000, 6898800 ) );
QCOMPARE( labels.count(), 1 );
QCOMPARE( labels.at( 0 ).featureId, 1 );

Expand All @@ -1352,25 +1353,21 @@ void TestQgsLabelingEngine::labelingResults()
QCOMPARE( labels.at( 0 ).labelText, QStringLiteral( "8888" ) );
QGSCOMPARENEAR( labels.at( 0 ).width, 671844, 500 ); // tolerance will probably need tweaking, to account for cross-platform font diffs
QGSCOMPARENEAR( labels.at( 0 ).height, 295119, 500 );
#if 0 // TODO
QGSCOMPARENEAR( labels.at( 0 ).labelRect.xMinimum(), -2779386, 500 );
QGSCOMPARENEAR( labels.at( 0 ).labelRect.xMaximum(), -2107542, 500 );
QGSCOMPARENEAR( labels.at( 0 ).labelRect.yMinimum(), 9240403, 500 );
QGSCOMPARENEAR( labels.at( 0 ).labelRect.yMaximum(), 9535523, 500 );
#endif
QGSCOMPARENEAR( labels.at( 0 ).labelRect.xMinimum(), -2739216, 500 );
QGSCOMPARENEAR( labels.at( 0 ).labelRect.xMaximum(), -2147712, 500 );
QGSCOMPARENEAR( labels.at( 0 ).labelRect.yMinimum(), 9023266, 500 );
QGSCOMPARENEAR( labels.at( 0 ).labelRect.yMaximum(), 9752660, 500 );
QCOMPARE( labels.at( 0 ).rotation, 60 );
labels = results->labelsAtPosition( QgsPointXY( -1383, 6708478 ) );
QCOMPARE( labels.count(), 1 );
QCOMPARE( labels.at( 0 ).featureId, 3 );
QCOMPARE( labels.at( 0 ).labelText, QStringLiteral( "33333" ) );
QGSCOMPARENEAR( labels.at( 0 ).width, 839805, 500 ); // tolerance will probably need tweaking, to account for cross-platform font diffs
QGSCOMPARENEAR( labels.at( 0 ).height, 295119, 500 );
#if 0 // TODO
QGSCOMPARENEAR( labels.at( 0 ).labelRect.xMinimum(), -433112, 500 );
QGSCOMPARENEAR( labels.at( 0 ).labelRect.xMaximum(), 406692, 500 );
QGSCOMPARENEAR( labels.at( 0 ).labelRect.yMinimum(), 6563006, 500 );
QGSCOMPARENEAR( labels.at( 0 ).labelRect.yMaximum(), 6858125, 500 );
#endif
QGSCOMPARENEAR( labels.at( 0 ).labelRect.xMinimum(), -350952, 500 );
QGSCOMPARENEAR( labels.at( 0 ).labelRect.xMaximum(), 324531, 500 );
QGSCOMPARENEAR( labels.at( 0 ).labelRect.yMinimum(), 6273139, 500 );
QGSCOMPARENEAR( labels.at( 0 ).labelRect.yMaximum(), 7147992, 500 );
QCOMPARE( labels.at( 0 ).rotation, 60 );
labels = results->labelsAtPosition( QgsPointXY( -2463392, 6708478 ) );
QCOMPARE( labels.count(), 0 );
Expand Down

0 comments on commit 2e9fe0b

Please sign in to comment.