From 4ab51b02fc8bec214fe8b120aecbd953bd5b9253 Mon Sep 17 00:00:00 2001 From: Nyall Dawson Date: Wed, 17 Jul 2019 08:08:03 +1000 Subject: [PATCH] [labeling] Fix labels 'jump' when using move label tool and alignment is set to a non-field based value --- src/app/qgsmaptoollabel.cpp | 21 ++-- src/app/qgsmaptoollabel.h | 5 + tests/src/app/testqgsmaptoollabel.cpp | 147 ++++++++++++++++++++++++++ 3 files changed, 162 insertions(+), 11 deletions(-) diff --git a/src/app/qgsmaptoollabel.cpp b/src/app/qgsmaptoollabel.cpp index ad9dae7c443..e938cf3796d 100644 --- a/src/app/qgsmaptoollabel.cpp +++ b/src/app/qgsmaptoollabel.cpp @@ -225,17 +225,8 @@ void QgsMapToolLabel::currentAlignment( QString &hali, QString &vali ) return; } - int haliIndx = dataDefinedColumnIndex( QgsPalLayerSettings::Hali, mCurrentLabel.settings, vlayer ); - if ( haliIndx != -1 ) - { - hali = f.attribute( haliIndx ).toString(); - } - - int valiIndx = dataDefinedColumnIndex( QgsPalLayerSettings::Vali, mCurrentLabel.settings, vlayer ); - if ( valiIndx != -1 ) - { - vali = f.attribute( valiIndx ).toString(); - } + hali = evaluateDataDefinedProperty( QgsPalLayerSettings::Hali, mCurrentLabel.settings, f, hali ).toString(); + vali = evaluateDataDefinedProperty( QgsPalLayerSettings::Vali, mCurrentLabel.settings, f, vali ).toString(); } bool QgsMapToolLabel::currentFeature( QgsFeature &f, bool fetchGeom ) @@ -459,6 +450,14 @@ int QgsMapToolLabel::dataDefinedColumnIndex( QgsPalLayerSettings::Property p, co return -1; } +QVariant QgsMapToolLabel::evaluateDataDefinedProperty( QgsPalLayerSettings::Property property, const QgsPalLayerSettings &labelSettings, const QgsFeature &feature, const QVariant &defaultValue ) const +{ + QgsExpressionContext context = mCanvas->mapSettings().expressionContext(); + context.setFeature( feature ); + context.setFields( feature.fields() ); + return labelSettings.dataDefinedProperties().value( property, context, defaultValue ); +} + bool QgsMapToolLabel::currentLabelDataDefinedPosition( double &x, bool &xSuccess, double &y, bool &ySuccess, int &xCol, int &yCol ) const { QgsVectorLayer *vlayer = mCurrentLabel.layer; diff --git a/src/app/qgsmaptoollabel.h b/src/app/qgsmaptoollabel.h index ed8e52691fb..6befd382148 100644 --- a/src/app/qgsmaptoollabel.h +++ b/src/app/qgsmaptoollabel.h @@ -139,6 +139,11 @@ class APP_EXPORT QgsMapToolLabel: public QgsMapTool \returns -1 if column does not exist or an expression is used instead */ int dataDefinedColumnIndex( QgsPalLayerSettings::Property p, const QgsPalLayerSettings &labelSettings, const QgsVectorLayer *vlayer ) const; + /** + * Evaluates a labeling data defined property for the specified \a feature. + */ + QVariant evaluateDataDefinedProperty( QgsPalLayerSettings::Property property, const QgsPalLayerSettings &labelSettings, const QgsFeature &feature, const QVariant &defaultValue ) const; + //! Returns whether to preserve predefined rotation data during label pin/unpin operations bool currentLabelPreserveRotation(); diff --git a/tests/src/app/testqgsmaptoollabel.cpp b/tests/src/app/testqgsmaptoollabel.cpp index 15dc9a49d2c..47c0c28ce99 100644 --- a/tests/src/app/testqgsmaptoollabel.cpp +++ b/tests/src/app/testqgsmaptoollabel.cpp @@ -224,6 +224,153 @@ class TestQgsMapToolLabel : public QObject QCOMPARE( pos.layerID, vl1->id() ); QCOMPARE( pos.labelText, QStringLiteral( "l" ) ); } + + void testAlignment() + { + QgsVectorLayer *vl1 = new QgsVectorLayer( QStringLiteral( "Point?crs=epsg:3946&field=halig:string&field=valig:string" ), QStringLiteral( "vl1" ), QStringLiteral( "memory" ) ); + QVERIFY( vl1->isValid() ); + QgsProject::instance()->addMapLayer( vl1 ); + QgsFeature f1; + f1.setAttributes( QgsAttributes() << QStringLiteral( "right" ) << QStringLiteral( "top" ) ); + f1.setGeometry( QgsGeometry::fromPointXY( QgsPointXY( 1, 1 ) ) ); + QVERIFY( vl1->dataProvider()->addFeature( f1 ) ); + f1.setGeometry( QgsGeometry::fromPointXY( QgsPointXY( 3, 3 ) ) ); + f1.setAttributes( QgsAttributes() << QStringLiteral( "center" ) << QStringLiteral( "base" ) ); + QVERIFY( vl1->dataProvider()->addFeature( f1 ) ); + + std::unique_ptr< QgsMapCanvas > canvas = qgis::make_unique< QgsMapCanvas >(); + canvas->setDestinationCrs( QgsCoordinateReferenceSystem( QStringLiteral( "EPSG:3946" ) ) ); + canvas->setLayers( QList() << vl1 ); + + QgsMapSettings mapSettings; + mapSettings.setOutputSize( QSize( 500, 500 ) ); + mapSettings.setExtent( QgsRectangle( -1, -1, 4, 4 ) ); + QVERIFY( mapSettings.hasValidSettings() ); + + mapSettings.setLayers( QList() << vl1 ); + + canvas->setFrameStyle( QFrame::NoFrame ); + canvas->resize( 500, 500 ); + canvas->setExtent( QgsRectangle( -1, -1, 4, 4 ) ); + canvas->show(); // to make the canvas resize + canvas->hide(); + QCOMPARE( canvas->mapSettings().outputSize(), QSize( 500, 500 ) ); + QCOMPARE( canvas->mapSettings().visibleExtent(), QgsRectangle( -1, -1, 4, 4 ) ); + + std::unique_ptr< QgsMapToolLabel > tool( new QgsMapToolLabel( canvas.get() ) ); + + // add some labels + QgsPalLayerSettings pls1; + pls1.fieldName = QStringLiteral( "'label'" ); + pls1.isExpression = true; + pls1.placement = QgsPalLayerSettings::OverPoint; + pls1.quadOffset = QgsPalLayerSettings::QuadrantOver; + pls1.displayAll = true; + QgsTextFormat format = pls1.format(); + format.setFont( QgsFontUtils::getStandardTestFont( QStringLiteral( "Bold" ) ) ); + format.setSize( 12 ); + pls1.setFormat( format ); + + vl1->setLabeling( new QgsVectorLayerSimpleLabeling( pls1 ) ); + vl1->setLabelsEnabled( true ); + + QEventLoop loop; + connect( canvas.get(), &QgsMapCanvas::mapCanvasRefreshed, &loop, &QEventLoop::quit ); + canvas->refreshAllLayers(); + canvas->show(); + loop.exec(); + + QVERIFY( canvas->labelingResults() ); + QgsPointXY pt; + pt = tool->canvas()->mapSettings().mapToPixel().transform( 1, 1 ); + std::unique_ptr< QMouseEvent > event( new QMouseEvent( + QEvent::MouseButtonPress, + QPoint( std::round( pt.x() ), std::round( pt.y() ) ), Qt::LeftButton, Qt::LeftButton, Qt::NoModifier + ) ); + QgsLabelPosition pos; + QVERIFY( tool->labelAtPosition( event.get(), pos ) ); + QCOMPARE( pos.layerID, vl1->id() ); + QCOMPARE( pos.labelText, QStringLiteral( "label" ) ); + tool->mCurrentLabel = QgsMapToolLabel::LabelDetails( pos ); + + // defaults to bottom left + QString hali, vali; + tool->currentAlignment( hali, vali ); + QCOMPARE( hali, QStringLiteral( "Left" ) ); + QCOMPARE( vali, QStringLiteral( "Bottom" ) ); + + // using field bound alignment + pls1.dataDefinedProperties().setProperty( QgsPalLayerSettings::Hali, QgsProperty::fromField( QStringLiteral( "halig" ) ) ); + pls1.dataDefinedProperties().setProperty( QgsPalLayerSettings::Vali, QgsProperty::fromField( QStringLiteral( "valig" ) ) ); + vl1->setLabeling( new QgsVectorLayerSimpleLabeling( pls1 ) ); + + canvas->refreshAllLayers(); + canvas->show(); + loop.exec(); + + QVERIFY( tool->labelAtPosition( event.get(), pos ) ); + QCOMPARE( pos.layerID, vl1->id() ); + QCOMPARE( pos.labelText, QStringLiteral( "label" ) ); + tool->mCurrentLabel = QgsMapToolLabel::LabelDetails( pos ); + + tool->currentAlignment( hali, vali ); + QCOMPARE( hali, QStringLiteral( "right" ) ); + QCOMPARE( vali, QStringLiteral( "top" ) ); + + pt = tool->canvas()->mapSettings().mapToPixel().transform( 3, 3 ); + event = qgis::make_unique< QMouseEvent >( + QEvent::MouseButtonPress, + QPoint( std::round( pt.x() ), std::round( pt.y() ) ), Qt::LeftButton, Qt::LeftButton, Qt::NoModifier + ); + + QVERIFY( tool->labelAtPosition( event.get(), pos ) ); + QCOMPARE( pos.layerID, vl1->id() ); + QCOMPARE( pos.labelText, QStringLiteral( "label" ) ); + tool->mCurrentLabel = QgsMapToolLabel::LabelDetails( pos ); + + tool->currentAlignment( hali, vali ); + QCOMPARE( hali, QStringLiteral( "center" ) ); + QCOMPARE( vali, QStringLiteral( "base" ) ); + + // now try with expression based alignment + pls1.dataDefinedProperties().setProperty( QgsPalLayerSettings::Hali, QgsProperty::fromExpression( QStringLiteral( "case when $id % 2 = 0 then 'right' else 'left' end" ) ) ); + pls1.dataDefinedProperties().setProperty( QgsPalLayerSettings::Vali, QgsProperty::fromExpression( QStringLiteral( "case when $id % 2 = 0 then 'half' else 'cap' end" ) ) ); + vl1->setLabeling( new QgsVectorLayerSimpleLabeling( pls1 ) ); + + canvas->refreshAllLayers(); + canvas->show(); + loop.exec(); + + pt = tool->canvas()->mapSettings().mapToPixel().transform( 1, 1 ); + event = qgis::make_unique< QMouseEvent >( + QEvent::MouseButtonPress, + QPoint( std::round( pt.x() ), std::round( pt.y() ) ), Qt::LeftButton, Qt::LeftButton, Qt::NoModifier + ); + + QVERIFY( tool->labelAtPosition( event.get(), pos ) ); + QCOMPARE( pos.layerID, vl1->id() ); + QCOMPARE( pos.labelText, QStringLiteral( "label" ) ); + tool->mCurrentLabel = QgsMapToolLabel::LabelDetails( pos ); + + tool->currentAlignment( hali, vali ); + QCOMPARE( hali, QStringLiteral( "left" ) ); + QCOMPARE( vali, QStringLiteral( "cap" ) ); + + pt = tool->canvas()->mapSettings().mapToPixel().transform( 3, 3 ); + event = qgis::make_unique< QMouseEvent >( + QEvent::MouseButtonPress, + QPoint( std::round( pt.x() ), std::round( pt.y() ) ), Qt::LeftButton, Qt::LeftButton, Qt::NoModifier + ); + + QVERIFY( tool->labelAtPosition( event.get(), pos ) ); + QCOMPARE( pos.layerID, vl1->id() ); + QCOMPARE( pos.labelText, QStringLiteral( "label" ) ); + tool->mCurrentLabel = QgsMapToolLabel::LabelDetails( pos ); + + tool->currentAlignment( hali, vali ); + QCOMPARE( hali, QStringLiteral( "right" ) ); + QCOMPARE( vali, QStringLiteral( "half" ) ); + } }; QGSTEST_MAIN( TestQgsMapToolLabel )