Make interactive labeling tools correctly work with data defined

properties which aren't bound to fields, but which are still effectively representing a single column name
qgis · Jun 9, 2021 · c6bd366 · c6bd366
1 parent 0a4b9a6
commit c6bd366
Show file tree

Hide file tree

Showing 7 changed files with 180 additions and 29 deletions.
diff --git a/src/app/labeling/qgslabelpropertydialog.cpp b/src/app/labeling/qgslabelpropertydialog.cpp
@@ -30,14 +30,16 @@
 #include "qgsexpressioncontextutils.h"
 #include "qgsgui.h"
 #include "qgshelp.h"
+#include "qgsexpressionnodeimpl.h"
 
 #include <QColorDialog>
 #include <QFontDatabase>
 #include <QDialogButtonBox>
 
 
-QgsLabelPropertyDialog::QgsLabelPropertyDialog( const QString &layerId, const QString &providerId, QgsFeatureId featureId, const QFont &labelFont, const QString &labelText, bool isPinned, const QgsPalLayerSettings &layerSettings, QWidget *parent, Qt::WindowFlags f )
+QgsLabelPropertyDialog::QgsLabelPropertyDialog( const QString &layerId, const QString &providerId, QgsFeatureId featureId, const QFont &labelFont, const QString &labelText, bool isPinned, const QgsPalLayerSettings &layerSettings, QgsMapCanvas *canvas, QWidget *parent, Qt::WindowFlags f )
   : QDialog( parent, f )
+  , mCanvas( canvas )
   , mLabelFont( labelFont )
   , mIsPinned( isPinned )
 {
@@ -289,6 +291,47 @@ void QgsLabelPropertyDialog::blockElementSignals( bool block )
   mLabelAllPartsCheckBox->blockSignals( block );
 }
 
+int QgsLabelPropertyDialog::dataDefinedColumnIndex( QgsPalLayerSettings::Property p, const QgsVectorLayer *vlayer, const QgsExpressionContext &context ) const
+{
+  if ( !mDataDefinedProperties.isActive( p ) )
+    return -1;
+
+  const QgsProperty property = mDataDefinedProperties.property( p );
+
+  QString fieldName;
+  switch ( property.propertyType() )
+  {
+    case QgsProperty::InvalidProperty:
+    case QgsProperty::StaticProperty:
+      break;
+
+    case QgsProperty::FieldBasedProperty:
+      fieldName = property.field();
+      break;
+
+    case QgsProperty::ExpressionBasedProperty:
+    {
+      // an expression based property may still be a effectively a single field reference in the map canvas context.
+      // e.g. if it is a expression like '"some_field"', or 'case when @some_project_var = 'a' then "field_a" else "field_b" end'
+      QgsExpression expression( property.expressionString() );
+      if ( expression.prepare( &context ) )
+      {
+        const QgsExpressionNode *node = expression.rootNode()->effectiveNode();
+        if ( node->nodeType() == QgsExpressionNode::ntColumnRef )
+        {
+          const QgsExpressionNodeColumnRef *columnRef = qgis::down_cast<const QgsExpressionNodeColumnRef *>( node );
+          fieldName = columnRef->name();
+        }
+      }
+      break;
+    }
+  }
+
+  if ( !fieldName.isEmpty() )
+    return vlayer->fields().lookupField( fieldName );
+  return -1;
+}
+
 void QgsLabelPropertyDialog::setDataDefinedValues( QgsVectorLayer *vlayer )
 {
   //loop through data defined properties and set all the GUI widget values. We can do this
@@ -444,30 +487,24 @@ void QgsLabelPropertyDialog::setDataDefinedValues( QgsVectorLayer *vlayer )
 
 void QgsLabelPropertyDialog::enableDataDefinedWidgets( QgsVectorLayer *vlayer )
 {
+  QgsExpressionContext context = mCanvas->createExpressionContext();
+  context.appendScope( vlayer->createExpressionContextScope() );
+
   //loop through data defined properties, this time setting whether or not the widgets are enabled
   //this can only be done for properties which are assigned to fields
   const auto constPropertyKeys = mDataDefinedProperties.propertyKeys();
   for ( int key : constPropertyKeys )
   {
     QgsProperty prop = mDataDefinedProperties.property( key );
-    if ( !prop || !prop.isActive() || prop.propertyType() != QgsProperty::FieldBasedProperty )
-    {
-      continue; // can only modify attributes with an active data definition of a mapped field
-    }
-
-    QString ddField = prop.field();
-    if ( ddField.isEmpty() )
+    if ( !prop || !prop.isActive() )
     {
       continue;
     }
 
-    int ddIndx = vlayer->fields().lookupField( ddField );
-    if ( ddIndx == -1 )
-    {
-      continue;
-    }
-
-    QgsDebugMsg( QStringLiteral( "ddField: %1" ).arg( ddField ) );
+    int ddIndex = dataDefinedColumnIndex( static_cast< QgsPalLayerSettings::Property >( key ), vlayer, context );
+    mPropertyToFieldMap[ key ] = ddIndex;
+    if ( ddIndex < 0 )
+      continue; // can only modify attributes with an active data definition of a mapped field
 
     switch ( key )
     {
@@ -801,9 +838,9 @@ void QgsLabelPropertyDialog::insertChangedValue( QgsPalLayerSettings::Property p
   if ( mDataDefinedProperties.isActive( p ) )
   {
     QgsProperty prop = mDataDefinedProperties.property( p );
-    if ( prop.propertyType() == QgsProperty::FieldBasedProperty )
+    if ( int index = mPropertyToFieldMap.value( p ); index >= 0 )
     {
-      mChangedProperties.insert( mCurLabelFeat.fieldNameIndex( prop.field() ), value );
+      mChangedProperties.insert( index, value );
     }
   }
 }

diff --git a/src/app/labeling/qgslabelpropertydialog.h b/src/app/labeling/qgslabelpropertydialog.h
@@ -37,6 +37,7 @@ class APP_EXPORT QgsLabelPropertyDialog: public QDialog, private Ui::QgsLabelPro
                             const QString &labelText,
                             bool isPinned,
                             const QgsPalLayerSettings &layerSettings,
+                            QgsMapCanvas *canvas,
                             QWidget *parent = nullptr,
                             Qt::WindowFlags f = Qt::WindowFlags() );
 
@@ -89,6 +90,8 @@ class APP_EXPORT QgsLabelPropertyDialog: public QDialog, private Ui::QgsLabelPro
     //! Block / unblock all input element signals
     void blockElementSignals( bool block );
 
+    int dataDefinedColumnIndex( QgsPalLayerSettings::Property p, const QgsVectorLayer *vlayer, const QgsExpressionContext &context ) const;
+
     void setDataDefinedValues( QgsVectorLayer *vlayer );
     void enableDataDefinedWidgets( QgsVectorLayer *vlayer );
 
@@ -107,8 +110,11 @@ class APP_EXPORT QgsLabelPropertyDialog: public QDialog, private Ui::QgsLabelPro
 
     void enableWidgetsForPinnedLabels();
 
+    QgsMapCanvas *mCanvas = nullptr;
+
     QgsAttributeMap mChangedProperties;
     QgsPropertyCollection mDataDefinedProperties;
+    QMap< int, int > mPropertyToFieldMap;
     QFont mLabelFont;
 
     QFontDatabase mFontDB;

diff --git a/src/app/labeling/qgsmaptoolchangelabelproperties.cpp b/src/app/labeling/qgsmaptoolchangelabelproperties.cpp
@@ -111,6 +111,7 @@ void QgsMapToolChangeLabelProperties::canvasReleaseEvent( QgsMapMouseEvent *e )
                               labeltext,
                               mCurrentLabel.pos.isPinned,
                               mCurrentLabel.settings,
+                              mCanvas,
                               nullptr );
     d.setMapCanvas( canvas() );
 

diff --git a/src/app/labeling/qgsmaptoollabel.cpp b/src/app/labeling/qgsmaptoollabel.cpp
@@ -35,6 +35,7 @@
 #include "qgsadvanceddigitizingdockwidget.h"
 #include "qgsstatusbar.h"
 #include "qgslabelingresults.h"
+#include "qgsexpressionnodeimpl.h"
 
 #include <QMouseEvent>
 
@@ -500,21 +501,50 @@ bool QgsMapToolLabel::hasDataDefinedColumn( QgsPalLayerSettings::DataDefinedProp
 }
 #endif
 
-QString QgsMapToolLabel::dataDefinedColumnName( QgsPalLayerSettings::Property p, const QgsPalLayerSettings &labelSettings ) const
+QString QgsMapToolLabel::dataDefinedColumnName( QgsPalLayerSettings::Property p, const QgsPalLayerSettings &labelSettings, const QgsVectorLayer *layer ) const
 {
   if ( !labelSettings.dataDefinedProperties().isActive( p ) )
     return QString();
 
-  QgsProperty prop = labelSettings.dataDefinedProperties().property( p );
-  if ( prop.propertyType() != QgsProperty::FieldBasedProperty )
-    return QString();
+  const QgsProperty property = labelSettings.dataDefinedProperties().property( p );
+
+  switch ( property.propertyType() )
+  {
+    case QgsProperty::InvalidProperty:
+    case QgsProperty::StaticProperty:
+      break;
+
+    case QgsProperty::FieldBasedProperty:
+      return property.field();
 
-  return prop.field();
+    case QgsProperty::ExpressionBasedProperty:
+    {
+      // an expression based property may still be a effectively a single field reference in the map canvas context.
+      // e.g. if it is a expression like '"some_field"', or 'case when @some_project_var = 'a' then "field_a" else "field_b" end'
+
+      QgsExpressionContext context = mCanvas->createExpressionContext();
+      context.appendScope( layer->createExpressionContextScope() );
+
+      QgsExpression expression( property.expressionString() );
+      if ( expression.prepare( &context ) )
+      {
+        const QgsExpressionNode *node = expression.rootNode()->effectiveNode();
+        if ( node->nodeType() == QgsExpressionNode::ntColumnRef )
+        {
+          const QgsExpressionNodeColumnRef *columnRef = qgis::down_cast<const QgsExpressionNodeColumnRef *>( node );
+          return columnRef->name();
+        }
+      }
+      break;
+    }
+  }
+
+  return QString();
 }
 
 int QgsMapToolLabel::dataDefinedColumnIndex( QgsPalLayerSettings::Property p, const QgsPalLayerSettings &labelSettings, const QgsVectorLayer *vlayer ) const
 {
-  QString fieldname = dataDefinedColumnName( p, labelSettings );
+  QString fieldname = dataDefinedColumnName( p, labelSettings, vlayer );
   if ( !fieldname.isEmpty() )
     return vlayer->fields().lookupField( fieldname );
   return -1;
@@ -595,7 +625,7 @@ bool QgsMapToolLabel::layerIsRotatable( QgsVectorLayer *vlayer, int &rotationCol
 
 bool QgsMapToolLabel::labelIsRotatable( QgsVectorLayer *layer, const QgsPalLayerSettings &settings, int &rotationCol ) const
 {
-  QString rColName = dataDefinedColumnName( QgsPalLayerSettings::LabelRotation, settings );
+  QString rColName = dataDefinedColumnName( QgsPalLayerSettings::LabelRotation, settings, layer );
   rotationCol = layer->fields().lookupField( rColName );
   return rotationCol != -1;
 }
@@ -717,8 +747,8 @@ bool QgsMapToolLabel::labelMoveable( QgsVectorLayer *vlayer, int &xCol, int &yCo
 
 bool QgsMapToolLabel::labelMoveable( QgsVectorLayer *vlayer, const QgsPalLayerSettings &settings, int &xCol, int &yCol ) const
 {
-  QString xColName = dataDefinedColumnName( QgsPalLayerSettings::PositionX, settings );
-  QString yColName = dataDefinedColumnName( QgsPalLayerSettings::PositionY, settings );
+  QString xColName = dataDefinedColumnName( QgsPalLayerSettings::PositionX, settings, vlayer );
+  QString yColName = dataDefinedColumnName( QgsPalLayerSettings::PositionY, settings, vlayer );
   //return !xColName.isEmpty() && !yColName.isEmpty();
   xCol = vlayer->fields().lookupField( xColName );
   yCol = vlayer->fields().lookupField( yColName );
@@ -743,7 +773,7 @@ bool QgsMapToolLabel::labelCanShowHide( QgsVectorLayer *vlayer, int &showCol ) c
   for ( const QString &providerId : constSubProviders )
   {
     QString fieldname = dataDefinedColumnName( QgsPalLayerSettings::Show,
-                        vlayer->labeling()->settings( providerId ) );
+                        vlayer->labeling()->settings( providerId ), vlayer );
     showCol = vlayer->fields().lookupField( fieldname );
     if ( showCol != -1 )
       return true;

diff --git a/src/app/labeling/qgsmaptoollabel.h b/src/app/labeling/qgsmaptoollabel.h
@@ -164,7 +164,7 @@ class APP_EXPORT QgsMapToolLabel: public QgsMapToolAdvancedDigitizing
     QFont currentLabelFont();
 
     //! Returns a data defined attribute column name for particular property or empty string if not defined
-    QString dataDefinedColumnName( QgsPalLayerSettings::Property p, const QgsPalLayerSettings &labelSettings ) const;
+    QString dataDefinedColumnName( QgsPalLayerSettings::Property p, const QgsPalLayerSettings &labelSettings, const QgsVectorLayer *layer ) const;
 
     /**
      * Returns a data defined attribute column index

diff --git a/tests/src/app/testqgslabelpropertydialog.cpp b/tests/src/app/testqgslabelpropertydialog.cpp
@@ -24,6 +24,7 @@
 #include "qgsauxiliarystorage.h"
 #include "qgslabelpropertydialog.h"
 #include "qgsvectorlayerlabeling.h"
+#include "qgsmapcanvas.h"
 
 class TestQgsLabelPropertyDialog : public QObject
 {
@@ -80,8 +81,10 @@ class TestQgsLabelPropertyDialog : public QObject
       QgsFeatureId fid = 0;
       QVariant val = vl->getFeature( fid ).attribute( propName );
 
+      std::unique_ptr< QgsMapCanvas > mapCanvas = std::make_unique< QgsMapCanvas >();
+
       // init label property dialog and togle buffer draw
-      QgsLabelPropertyDialog dialog( vl->id(), QString(), fid, QFont(), QString(), false, settings );
+      QgsLabelPropertyDialog dialog( vl->id(), QString(), fid, QFont(), QString(), false, settings, mapCanvas.get() );
       dialog.bufferDrawToggled( true );
 
       // apply changes

diff --git a/tests/src/app/testqgsmaptoollabel.cpp b/tests/src/app/testqgsmaptoollabel.cpp
@@ -28,6 +28,7 @@
 #include "qgsvectorlayerlabelprovider.h"
 #include "qgsvectorlayerlabeling.h"
 #include "qgsadvanceddigitizingdockwidget.h"
+#include "qgsexpressioncontextutils.h"
 
 class TestQgsMapToolLabel : public QObject
 {
@@ -374,6 +375,79 @@ class TestQgsMapToolLabel : public QObject
       QCOMPARE( hali, QStringLiteral( "right" ) );
       QCOMPARE( vali, QStringLiteral( "half" ) );
     }
+
+    void dataDefinedColumnName()
+    {
+      QgsVectorLayer *vl1 = new QgsVectorLayer( QStringLiteral( "Point?crs=epsg:3946&field=label_x_1:string&field=label_y_1:string&field=label_x_2:string&field=label_y_2:string" ), QStringLiteral( "vl1" ), QStringLiteral( "memory" ) );
+      QVERIFY( vl1->isValid() );
+      QgsProject::instance()->addMapLayer( vl1 );
+
+      std::unique_ptr< QgsMapCanvas > canvas = std::make_unique< QgsMapCanvas >();
+      canvas->setDestinationCrs( QgsCoordinateReferenceSystem( QStringLiteral( "EPSG:3946" ) ) );
+      canvas->setLayers( QList<QgsMapLayer *>() << vl1 );
+      std::unique_ptr< QgsAdvancedDigitizingDockWidget > advancedDigitizingDockWidget = std::make_unique< QgsAdvancedDigitizingDockWidget >( canvas.get() );
+
+      std::unique_ptr< QgsMapToolLabel > tool( new QgsMapToolLabel( canvas.get(), advancedDigitizingDockWidget.get() ) );
+
+      QgsExpressionContextUtils::setProjectVariable( QgsProject::instance(), QStringLiteral( "var_1" ), QStringLiteral( "1" ) );
+
+      // add some labels
+      QgsPalLayerSettings pls1;
+      pls1.fieldName = QStringLiteral( "'label'" );
+
+      // not using a column
+      pls1.dataDefinedProperties().setProperty( QgsPalLayerSettings::PositionX, QgsProperty::fromValue( 5 ) );
+      pls1.dataDefinedProperties().setProperty( QgsPalLayerSettings::PositionY, QgsProperty::fromValue( 6 ) );
+
+      vl1->setLabeling( new QgsVectorLayerSimpleLabeling( pls1 ) );
+      vl1->setLabelsEnabled( true );
+
+      QCOMPARE( tool->dataDefinedColumnName( QgsPalLayerSettings::AlwaysShow, pls1, vl1 ), QString() );
+      QCOMPARE( tool->dataDefinedColumnName( QgsPalLayerSettings::PositionX, pls1, vl1 ), QString() );
+      QCOMPARE( tool->dataDefinedColumnName( QgsPalLayerSettings::PositionY, pls1, vl1 ), QString() );
+
+      // using direct field references
+      pls1.dataDefinedProperties().setProperty( QgsPalLayerSettings::PositionX, QgsProperty::fromField( QStringLiteral( "label_x_2" ) ) );
+      pls1.dataDefinedProperties().setProperty( QgsPalLayerSettings::PositionY, QgsProperty::fromField( QStringLiteral( "label_y_2" ) ) );
+
+      vl1->setLabeling( new QgsVectorLayerSimpleLabeling( pls1 ) );
+      vl1->setLabelsEnabled( true );
+
+      QCOMPARE( tool->dataDefinedColumnName( QgsPalLayerSettings::AlwaysShow, pls1, vl1 ), QString() );
+      QCOMPARE( tool->dataDefinedColumnName( QgsPalLayerSettings::PositionX, pls1, vl1 ), QStringLiteral( "label_x_2" ) );
+      QCOMPARE( tool->dataDefinedColumnName( QgsPalLayerSettings::PositionY, pls1, vl1 ), QStringLiteral( "label_y_2" ) );
+
+      // using expressions which are just field references, should still work
+      pls1.dataDefinedProperties().setProperty( QgsPalLayerSettings::PositionX, QgsProperty::fromExpression( QStringLiteral( "\"label_x_1\"" ) ) );
+      pls1.dataDefinedProperties().setProperty( QgsPalLayerSettings::PositionY, QgsProperty::fromExpression( QStringLiteral( "\"label_y_1\"" ) ) );
+
+      vl1->setLabeling( new QgsVectorLayerSimpleLabeling( pls1 ) );
+      vl1->setLabelsEnabled( true );
+
+      QCOMPARE( tool->dataDefinedColumnName( QgsPalLayerSettings::AlwaysShow, pls1, vl1 ), QString() );
+      QCOMPARE( tool->dataDefinedColumnName( QgsPalLayerSettings::PositionX, pls1, vl1 ), QStringLiteral( "label_x_1" ) );
+      QCOMPARE( tool->dataDefinedColumnName( QgsPalLayerSettings::PositionY, pls1, vl1 ), QStringLiteral( "label_y_1" ) );
+
+
+      // using complex expressions which change field depending on a project level variable
+
+      pls1.dataDefinedProperties().setProperty( QgsPalLayerSettings::PositionX, QgsProperty::fromExpression( QStringLiteral( "case when @var_1 = '1' then \"label_x_1\" else \"label_x_2\" end" ) ) );
+      pls1.dataDefinedProperties().setProperty( QgsPalLayerSettings::PositionY, QgsProperty::fromExpression( QStringLiteral( "case when @var_1 = '1' then \"label_y_1\" else \"label_y_2\" end" ) ) );
+      vl1->setLabeling( new QgsVectorLayerSimpleLabeling( pls1 ) );
+      vl1->setLabelsEnabled( true );
+
+      QCOMPARE( tool->dataDefinedColumnName( QgsPalLayerSettings::AlwaysShow, pls1, vl1 ), QString() );
+      QCOMPARE( tool->dataDefinedColumnName( QgsPalLayerSettings::PositionX, pls1, vl1 ), QStringLiteral( "label_x_1" ) );
+      QCOMPARE( tool->dataDefinedColumnName( QgsPalLayerSettings::PositionY, pls1, vl1 ), QStringLiteral( "label_y_1" ) );
+
+      QgsExpressionContextUtils::setProjectVariable( QgsProject::instance(), QStringLiteral( "var_1" ), QStringLiteral( "2" ) );
+
+      QCOMPARE( tool->dataDefinedColumnName( QgsPalLayerSettings::AlwaysShow, pls1, vl1 ), QString() );
+      QCOMPARE( tool->dataDefinedColumnName( QgsPalLayerSettings::PositionX, pls1, vl1 ), QStringLiteral( "label_x_2" ) );
+      QCOMPARE( tool->dataDefinedColumnName( QgsPalLayerSettings::PositionY, pls1, vl1 ), QStringLiteral( "label_y_2" ) );
+    }
+
+
 };
 
 QGSTEST_MAIN( TestQgsMapToolLabel )