Report errors in labeling geometry generator expression

qgis · Mar 10, 2019 · a761447 · a761447
1 parent c452821
commit a761447
Show file tree

Hide file tree

Showing 6 changed files with 114 additions and 21 deletions.
diff --git a/src/app/qgslabelinggui.cpp b/src/app/qgslabelinggui.cpp
@@ -82,6 +82,9 @@ QgsLabelingGui::QgsLabelingGui( QgsVectorLayer *layer, QgsMapCanvas *mapCanvas,
   connect( mGeometryGeneratorGroupBox, &QGroupBox::toggled, this, &QgsLabelingGui::updateGeometryTypeBasedWidgets );
   connect( mGeometryGeneratorType, qgis::overload<int>::of( &QComboBox::currentIndexChanged ), this, &QgsLabelingGui::updateGeometryTypeBasedWidgets );
   connect( mGeometryGeneratorExpressionButton, &QToolButton::clicked, this, &QgsLabelingGui::showGeometryGeneratorExpressionBuilder );
+  connect( mGeometryGeneratorGroupBox, &QGroupBox::toggled, this, &QgsLabelingGui::validateGeometryGeneratorExpression );
+  connect( mGeometryGenerator, &QgsCodeEditorExpression::textChanged, this, &QgsLabelingGui::validateGeometryGeneratorExpression );
+  connect( mGeometryGeneratorType, qgis::overload<int>::of( &QComboBox::currentIndexChanged ), this, &QgsLabelingGui::validateGeometryGeneratorExpression );
 
   mFieldExpressionWidget->registerExpressionContextGenerator( this );
 
@@ -90,11 +93,15 @@ QgsLabelingGui::QgsLabelingGui( QgsVectorLayer *layer, QgsMapCanvas *mapCanvas,
   mMaxScaleWidget->setMapCanvas( mapCanvas );
   mMaxScaleWidget->setShowCurrentScaleButton( true );
 
+  mGeometryGeneratorWarningLabel->setStyleSheet( QStringLiteral( "color: #FFC107;" ) );
+
   setLayer( layer );
 }
 
 void QgsLabelingGui::setLayer( QgsMapLayer *mapLayer )
 {
+  mPreviewFeature = QgsFeature();
+
   if ( !mapLayer || mapLayer->type() != QgsMapLayer::VectorLayer )
   {
     setEnabled( false );
@@ -216,7 +223,7 @@ void QgsLabelingGui::setLayer( QgsMapLayer *mapLayer )
   wrapCharacterEdit->setText( lyr.wrapChar );
   mAutoWrapLengthSpinBox->setValue( lyr.autoWrapLength );
   mAutoWrapTypeComboBox->setCurrentIndex( lyr.useMaxLineLengthForAutoWrap ? 0 : 1 );
-  mFontMultiLineAlignComboBox->setCurrentIndex( ( unsigned int ) lyr.multilineAlign );
+  mFontMultiLineAlignComboBox->setCurrentIndex( lyr.multilineAlign );
   chkPreserveRotation->setChecked( lyr.preserveRotation );
 
   mPreviewBackgroundBtn->setColor( lyr.previewBkgrdColor );
@@ -698,9 +705,54 @@ void QgsLabelingGui::showGeometryGeneratorExpressionBuilder()
   expressionBuilder.setExpressionText( mGeometryGenerator->text() );
   expressionBuilder.setExpressionContext( createExpressionContext() );
 
+  QgsDistanceArea da;
+  da.setSourceCrs( mLayer->crs(), QgsProject::instance()->transformContext() );
+  da.setEllipsoid( QgsProject::instance()->ellipsoid() );
+  expressionBuilder.setGeomCalculator( da );
+
   if ( expressionBuilder.exec() )
   {
     mGeometryGenerator->setText( expressionBuilder.expressionText() );
   }
+}
+
+void QgsLabelingGui::validateGeometryGeneratorExpression()
+{
+  bool valid = true;
+
+  if ( mGeometryGeneratorGroupBox->isChecked() )
+  {
+    if ( !mPreviewFeature.isValid() && mLayer )
+      mLayer->getFeatures( QgsFeatureRequest().setLimit( 1 ) ).nextFeature( mPreviewFeature );
+
+    QgsExpression expression( mGeometryGenerator->text() );
+    QgsExpressionContext context = createExpressionContext();
+    context.setFeature( mPreviewFeature );
+
+    expression.prepare( &context );
+
+    if ( expression.hasParserError() )
+    {
+      mGeometryGeneratorWarningLabel->setText( expression.parserErrorString() );
+      valid = false;
+    }
+    else
+    {
+      const QVariant result = expression.evaluate( &context );
+      const QgsGeometry geometry = result.value<QgsGeometry>();
+      QgsWkbTypes::GeometryType configuredGeometryType = mGeometryGeneratorType->currentData().value<QgsWkbTypes::GeometryType>();
+      if ( geometry.isNull() )
+      {
+        mGeometryGeneratorWarningLabel->setText( tr( "Result of the expression is not a geometry" ) );
+        valid = false;
+      }
+      else if ( geometry.type() != configuredGeometryType )
+      {
+        mGeometryGeneratorWarningLabel->setText( tr( "Result of the expression does not match configured geometry type. Result is %1, Configured %2." ).arg( QgsWkbTypes::geometryDisplayString( geometry.type() ), QgsWkbTypes::geometryDisplayString( configuredGeometryType ) ) );
+        valid = false;
+      }
+    }
+  }
 
+  mGeometryGeneratorWarningLabel->setVisible( !valid );
 }
diff --git a/src/app/qgslabelinggui.h b/src/app/qgslabelinggui.h
@@ -68,14 +68,22 @@ class APP_EXPORT QgsLabelingGui : public QgsTextFormatWidget, private QgsExpress
     void syncDefinedCheckboxFrame( QgsPropertyOverrideButton *ddBtn, QCheckBox *chkBx, QFrame *f );
 
   private slots:
+
+    /**
+     * Called when the geometry type is changed and
+     * configuration options which only work with a specific
+     * geometry type should be updated.
+     */
     void updateGeometryTypeBasedWidgets();
     void showGeometryGeneratorExpressionBuilder();
+    void validateGeometryGeneratorExpression();
 
   private:
     QgsVectorLayer *mLayer = nullptr;
     const QgsPalLayerSettings &mSettings;
     QgsPropertyCollection mDataDefinedProperties;
     LabelMode mMode;
+    QgsFeature mPreviewFeature;
 
     QgsExpressionContext createExpressionContext() const override;
 

diff --git a/src/core/qgspallabeling.cpp b/src/core/qgspallabeling.cpp
@@ -61,6 +61,25 @@
 #include "qgscurvepolygon.h"
 #include <QMessageBox>
 
+// TODO: Move to qgis.h?
+
+/**
+ * Converts a string representation \a key of an enum into the value.
+ * If it cannot be converted, the \a defaultValue will be returned.
+ *
+ */
+template <class T>
+T enumValueToKey( const QString &key, T defaultValue )
+{
+  const QMetaEnum metaEnum( QMetaEnum::fromType<T>() );
+  bool ok;
+  T result = static_cast<QgsWkbTypes::GeometryType>( metaEnum.keyToValue( key.toUtf8().constData(), &ok ) );
+  if ( !ok )
+    result = defaultValue;
+
+  return result;
+}
+
 
 using namespace pal;
 
@@ -682,12 +701,6 @@ void QgsPalLayerSettings::readFromLayerCustomProperties( QgsVectorLayer *layer )
   obstacleType = static_cast< ObstacleType >( layer->customProperty( QStringLiteral( "labeling/obstacleType" ), QVariant( PolygonInterior ) ).toUInt() );
   zIndex = layer->customProperty( QStringLiteral( "labeling/zIndex" ), QVariant( 0.0 ) ).toDouble();
 
-  geometryGenerator = layer->customProperty( QStringLiteral( "labeling/geometryGenerator" ), QString() ).toString();
-  geometryGeneratorEnabled = layer->customProperty( QStringLiteral( "labeling/geometryGeneratorEnabled" ) ).toBool();
-
-  const QMetaEnum metaEnum( QMetaEnum::fromType<QgsWkbTypes::GeometryType>() );
-  geometryGeneratorType = static_cast<QgsWkbTypes::GeometryType>( metaEnum.keyToValue( layer->customProperty( QStringLiteral( "labeling/geometryGeneratorType" ) ).toString().toUtf8().constData() ) );
-
   mDataDefinedProperties.clear();
   if ( layer->customProperty( QStringLiteral( "labeling/ddProperties" ) ).isValid() )
   {
@@ -879,10 +892,15 @@ void QgsPalLayerSettings::readXml( QDomElement &elem, const QgsReadWriteContext
 
   geometryGenerator = placementElem.attribute( QStringLiteral( "geometryGenerator" ) );
   geometryGeneratorEnabled = placementElem.attribute( QStringLiteral( "geometryGeneratorEnabled" ) ).toInt();
+  geometryGeneratorType = enumValueToKey( placementElem.attribute( QStringLiteral( "geometryGeneratorType" ) ), QgsWkbTypes::PointGeometry );
 
+#if 0
   const QMetaEnum metaEnum( QMetaEnum::fromType<QgsWkbTypes::GeometryType>() );
-  geometryGeneratorType = static_cast<QgsWkbTypes::GeometryType>( metaEnum.keyToValue( placementElem.attribute( QStringLiteral( "geometryGeneratorType" ) ).toUtf8().constData() ) );
-
+  bool ok;
+  geometryGeneratorType = static_cast<QgsWkbTypes::GeometryType>( metaEnum.keyToValue( placementElem.attribute( QStringLiteral( "geometryGeneratorType" ) ).toUtf8().constData(), &ok ) );
+  if ( !ok )
+    geometryGeneratorType = QgsWkbTypes::GeometryType::PointGeometry;
+#endif
 
   // rendering
   QDomElement renderingElem = elem.firstChildElement( QStringLiteral( "rendering" ) );

diff --git a/src/core/qgspallabeling.h b/src/core/qgspallabeling.h
@@ -758,7 +758,7 @@ class CORE_EXPORT QgsPalLayerSettings
     QString geometryGenerator;
 
     //! The type of the result geometry of the geometry generator.
-    QgsWkbTypes::GeometryType geometryGeneratorType = QgsWkbTypes::GeometryType::UnknownGeometry;
+    QgsWkbTypes::GeometryType geometryGeneratorType = QgsWkbTypes::GeometryType::PointGeometry;
 
     //! Defines if the geometry generator is enabled or not. If disabled, the standard geometry will be taken.
     bool geometryGeneratorEnabled = false;

diff --git a/src/gui/qgstextformatwidget.cpp b/src/gui/qgstextformatwidget.cpp
@@ -568,7 +568,8 @@ void QgsTextFormatWidget::setWidgetMode( QgsTextFormatWidget::Mode mode )
 
 void QgsTextFormatWidget::toggleDDButtons( bool visible )
 {
-  Q_FOREACH ( QgsPropertyOverrideButton *button, findChildren< QgsPropertyOverrideButton * >() )
+  const auto buttons = findChildren< QgsPropertyOverrideButton * >();
+  for ( QgsPropertyOverrideButton *button : buttons )
   {
     button->setVisible( visible );
   }
@@ -650,6 +651,10 @@ void QgsTextFormatWidget::connectValueChanged( const QList<QWidget *> &widgets,
     {
       connect( w, SIGNAL( toggled( bool ) ), this, slot );
     }
+    else if ( QgsCodeEditorExpression *w = qobject_cast<QgsCodeEditorExpression *>( widget ) )
+    {
+      connect( w, SIGNAL( textChanged() ), this, slot );
+    }
     else
     {
       QgsLogger::warning( QStringLiteral( "Could not create connection for widget %1" ).arg( widget->objectName() ) );

diff --git a/src/ui/qgstextformatwidgetbase.ui b/src/ui/qgstextformatwidgetbase.ui
@@ -3664,9 +3664,9 @@ font-style: italic;</string>
                      <property name="geometry">
                       <rect>
                        <x>0</x>
-                       <y>-370</y>
+                       <y>-400</y>
                        <width>772</width>
-                       <height>1187</height>
+                       <height>1211</height>
                       </rect>
                      </property>
                      <layout class="QVBoxLayout" name="verticalLayout_11">
@@ -5000,12 +5000,8 @@ font-style: italic;</string>
                          <bool>true</bool>
                         </property>
                         <layout class="QGridLayout" name="gridLayout_41">
-                         <item row="1" column="1">
-                          <widget class="QToolButton" name="mGeometryGeneratorExpressionButton">
-                           <property name="text">
-                            <string>...</string>
-                           </property>
-                          </widget>
+                         <item row="1" column="0" rowspan="2">
+                          <widget class="QgsCodeEditorExpression" name="mGeometryGenerator" native="true"/>
                          </item>
                          <item row="3" column="0" colspan="2">
                           <widget class="QComboBox" name="mGeometryGeneratorType">
@@ -5030,8 +5026,22 @@ font-style: italic;</string>
                            </property>
                           </spacer>
                          </item>
-                         <item row="1" column="0" rowspan="2">
-                          <widget class="QgsCodeEditorExpression" name="mGeometryGenerator" native="true"/>
+                         <item row="1" column="1">
+                          <widget class="QToolButton" name="mGeometryGeneratorExpressionButton">
+                           <property name="text">
+                            <string>...</string>
+                           </property>
+                          </widget>
+                         </item>
+                         <item row="4" column="0">
+                          <widget class="QLabel" name="mGeometryGeneratorWarningLabel">
+                           <property name="text">
+                            <string/>
+                           </property>
+                           <property name="wordWrap">
+                            <bool>true</bool>
+                           </property>
+                          </widget>
                          </item>
                         </layout>
                        </widget>