[gui] Move Diagrams properties to a panel widget, showing tabs instea…

…d of a list when docked; harmonize stacked diagram configuration with rule-based labeling; allow QgsDiagramProperties to sync to renderers and diagram layer settings, in addition to syncing to layers; when editing a subdiagram of a stacked diagram, only show widgets for diagram layer settings if the subdiagram is the first one, for the rest, hide those widgets and show a note informing users; make sure stacked diagrams handle enabled and disabled subdiagrams (i.e., don't take into account disabled subdiagrams) and add a test for it; switching from single to stacked diagram: take the single diagram definition as the first stacked diagram; fix qgis#58782 (calling twice the apply method for label rendering)

CMakeLists.txt

@@ -1101,7 +1101,7 @@ if (WITH_CORE AND WITH_BINDINGS)
  include(SIPMacros)
 
  set(SIP_INCLUDES ${PYQT_SIP_DIR} ${CMAKE_SOURCE_DIR}/python)
- set(SIP_CONCAT_PARTS 22)
+ set(SIP_CONCAT_PARTS 24)
 
  if (NOT BINDINGS_GLOBAL_INSTALL)
  set(Python_SITEARCH ${QGIS_DATA_DIR}/python)

python/PyQt6/core/auto_generated/qgsdiagramrenderer.sip.in

@@ -1017,30 +1017,24 @@ Writes stacked renderers state to a DOM element.
  virtual QList< QgsLayerTreeModelLegendNode * > legendItems( QgsLayerTreeLayer *nodeLayer ) const /Factory/;
 
 
- QList< QgsDiagramRenderer * > renderers() const;
+ QList< QgsDiagramRenderer * > renderers( bool sortByDiagramMode = false ) const;
 %Docstring
 Returns an ordered list with the renderers of the stacked renderer object.
-If the stacked diagram orientation is vertical, the list is returned backwards.
+@param sortByDiagramMode If true, the list is returned backwards for vertical orientation.
 %End
 
  void addRenderer( QgsDiagramRenderer *renderer );
 %Docstring
 Adds a renderer to the stacked renderer object.
-
-:param renderer: diagram renderer to be added to the stacked renderer
- Renderers added first will render their diagrams first, i.e., more to
- the left (horizontal mode) or more to the top (vertical mode).
+@param renderer diagram renderer to be added to the stacked renderer
+Renderers added first will render their diagrams first, i.e., more to
+the left (horizontal mode) or more to the top (vertical mode).
 %End
 
  const QgsDiagramRenderer *renderer( const int index ) const;
 %Docstring
 Returns the renderer at the given ``index``.
 @param index index of the disired renderer in the stacked renderer
-%End
-
- int rendererCount() const;
-%Docstring
-Returns the number of renderers that this stacked renderer is composed of.
 %End
 
  protected:

python/core/auto_generated/qgsdiagramrenderer.sip.in

@@ -1017,30 +1017,24 @@ Writes stacked renderers state to a DOM element.
  virtual QList< QgsLayerTreeModelLegendNode * > legendItems( QgsLayerTreeLayer *nodeLayer ) const /Factory/;
 
 
- QList< QgsDiagramRenderer * > renderers() const;
+ QList< QgsDiagramRenderer * > renderers( bool sortByDiagramMode = false ) const;
 %Docstring
 Returns an ordered list with the renderers of the stacked renderer object.
-If the stacked diagram orientation is vertical, the list is returned backwards.
+@param sortByDiagramMode If true, the list is returned backwards for vertical orientation.
 %End
 
  void addRenderer( QgsDiagramRenderer *renderer );
 %Docstring
 Adds a renderer to the stacked renderer object.
-
-:param renderer: diagram renderer to be added to the stacked renderer
- Renderers added first will render their diagrams first, i.e., more to
- the left (horizontal mode) or more to the top (vertical mode).
+@param renderer diagram renderer to be added to the stacked renderer
+Renderers added first will render their diagrams first, i.e., more to
+the left (horizontal mode) or more to the top (vertical mode).
 %End
 
  const QgsDiagramRenderer *renderer( const int index ) const;
 %Docstring
 Returns the renderer at the given ``index``.
 @param index index of the disired renderer in the stacked renderer
-%End
-
- int rendererCount() const;
-%Docstring
-Returns the number of renderers that this stacked renderer is composed of.
 %End
 
  protected:

src/app/qgisapp.cpp

@@ -8038,35 +8038,8 @@ void QgisApp::diagramProperties()
  return;
  }
 
- QDialog dlg;
- dlg.setWindowTitle( tr( "Layer Diagram Properties" ) );
- QgsStackedDiagramProperties *gui = new QgsStackedDiagramProperties( vlayer, &dlg, mMapCanvas );
- gui->layout()->setContentsMargins( 0, 0, 0, 0 );
- QVBoxLayout *layout = new QVBoxLayout( &dlg );
- layout->addWidget( gui );
-
- QDialogButtonBox *buttonBox = new QDialogButtonBox(
- QDialogButtonBox::Help | QDialogButtonBox::Ok | QDialogButtonBox::Cancel | QDialogButtonBox::Apply,
- Qt::Horizontal, &dlg );
- layout->addWidget( buttonBox );
-
- dlg.setLayout( layout );
-
- connect( buttonBox->button( QDialogButtonBox::Ok ), &QAbstractButton::clicked,
- &dlg, &QDialog::accept );
- connect( buttonBox->button( QDialogButtonBox::Cancel ), &QAbstractButton::clicked,
- &dlg, &QDialog::reject );
- connect( buttonBox->button( QDialogButtonBox::Apply ), &QAbstractButton::clicked,
- gui, &QgsStackedDiagramProperties::apply );
- connect( buttonBox->button( QDialogButtonBox::Help ), &QAbstractButton::clicked, gui, [ = ]
- {
- QgsHelp::openHelp( QStringLiteral( "working_with_vector/vector_properties.html#diagrams-properties" ) );
- } );
-
- if ( dlg.exec() )
- gui->apply();
-
- activateDeactivateLayerRelatedActions( vlayer );
+ mapStyleDock( true );
+ mMapStyleWidget->setCurrentPage( QgsLayerStylingWidget::VectorDiagram );
 }
 
 void QgisApp::createAnnotationLayer()

src/app/qgslayerstylingwidget.cpp

@@ -23,6 +23,7 @@
 #include "qgsapplication.h"
 #include "qgslabelingwidget.h"
 #include "qgsmaskingwidget.h"
+#include "qgsdiagramwidget.h"
 #include "qgslayerstylingwidget.h"
 #include "qgsrastertransparencywidget.h"
 #include "qgsrendererpropertiesdialog.h"
@@ -217,6 +218,11 @@ void QgsLayerStylingWidget::setLayer( QgsMapLayer *layer )
  symbol3DItem->setToolTip( tr( "3D View" ) );
  mOptionsListWidget->addItem( symbol3DItem );
 #endif
+
+ QListWidgetItem *diagramItem = new QListWidgetItem( QgsApplication::getThemeIcon( QStringLiteral( "/propertyicons/diagram.svg" ) ), QString() );
+ diagramItem->setData( Qt::UserRole, VectorDiagram );
+ diagramItem->setToolTip( tr( "Diagrams" ) );
+ mOptionsListWidget->addItem( diagramItem );
  break;
  }
  case Qgis::LayerType::Raster:
@@ -334,12 +340,6 @@ void QgsLayerStylingWidget::apply()
 
  bool styleWasChanged = false;
  bool triggerRepaint = false; // whether the change needs the layer to be repainted
- if ( QgsLabelingWidget *widget = qobject_cast<QgsLabelingWidget *>( current ) )
- {
- widget->apply();
- styleWasChanged = true;
- undoName = QStringLiteral( "Label Change" );
- }
  if ( QgsMaskingWidget *widget = qobject_cast<QgsMaskingWidget *>( current ) )
  {
  widget->apply();
@@ -373,8 +373,23 @@ void QgsLayerStylingWidget::apply()
  styleWasChanged = true;
  triggerRepaint = true;
  }
+ else if ( QgsLabelingWidget *widget = qobject_cast<QgsLabelingWidget *>( current ) )
+ {
+ widget->apply();
+ styleWasChanged = true;
+ undoName = QStringLiteral( "Label Change" );
+ }
+ else if ( QgsDiagramWidget *widget = qobject_cast<QgsDiagramWidget *>( current ) )
+ {
+ widget->apply();
+ styleWasChanged = true;
+ undoName = QStringLiteral( "Diagram Change" );
+ }
  else if ( QgsMapLayerConfigWidget *widget = qobject_cast<QgsMapLayerConfigWidget *>( current ) )
  {
+ // Warning: All classes inheriting from QgsMapLayerConfigWidget
+ // should come in the current if block, before this else-if
+ // clause, to avoid duplicate calls to apply()!
  widget->apply();
  styleWasChanged = true;
  triggerRepaint = widget->shouldTriggerLayerRepaint();
@@ -463,6 +478,10 @@ void QgsLayerStylingWidget::updateCurrentWidgetLayer()
  mMesh3DWidget = widget;
  }
 #endif
+ else if ( QgsDiagramWidget *widget = qobject_cast<QgsDiagramWidget *>( current ) )
+ {
+ mDiagramWidget = widget;
+ }
  }
 
  mWidgetStack->clear();
@@ -495,6 +514,11 @@ void QgsLayerStylingWidget::updateCurrentWidgetLayer()
  {
  QgsVectorLayer *vlayer = qobject_cast<QgsVectorLayer *>( mCurrentLayer );
 
+#ifdef HAVE_3D
+ const int tabShift = 1; // To move subsequent tabs
+#else
+ const int tabShift = 0;
+#endif
  switch ( row )
  {
  case 0: // Style
@@ -550,6 +574,15 @@ void QgsLayerStylingWidget::updateCurrentWidgetLayer()
  break;
  }
 #endif
+ case 3 + tabShift: // Diagrams
+ {
+ mDiagramWidget = new QgsDiagramWidget( vlayer, mMapCanvas, mWidgetStack );
+ mDiagramWidget->setDockMode( true );
+ connect( mDiagramWidget, &QgsDiagramWidget::widgetChanged, this, &QgsLayerStylingWidget::autoApply );
+ mDiagramWidget->syncToOwnLayer();
+ mWidgetStack->setMainPanel( mDiagramWidget );
+ break;
+ }
  default:
  break;
  }

src/app/qgslayerstylingwidget.h

@@ -34,6 +34,7 @@
 
 class QgsLabelingWidget;
 class QgsMaskingWidget;
+class QgsDiagramWidget;
 class QgsMapLayer;
 class QgsMapCanvas;
 class QgsRendererPropertiesDialog;
@@ -101,6 +102,7 @@ class APP_EXPORT QgsLayerStylingWidget : public QWidget, private Ui::QgsLayerSty
  History,
  Symbology3D,
  RasterAttributeTables, //!< Raster attribute tables, since QGIS 3.30
+ VectorDiagram, //!< Vector diagram, since QGIS 3.40
  };
 
  QgsLayerStylingWidget( QgsMapCanvas *canvas, QgsMessageBar *messageBar, const QList<const QgsMapLayerConfigWidgetFactory *> &pages, QWidget *parent = nullptr );
@@ -175,6 +177,7 @@ class APP_EXPORT QgsLayerStylingWidget : public QWidget, private Ui::QgsLayerSty
  QgsVectorLayer3DRendererWidget *mVector3DWidget = nullptr;
  QgsMeshLayer3DRendererWidget *mMesh3DWidget = nullptr;
 #endif
+ QgsDiagramWidget *mDiagramWidget = nullptr;
  QgsRendererRasterPropertiesWidget *mRasterStyleWidget = nullptr;
  QgsRasterAttributeTableWidget *mRasterAttributeTableWidget = nullptr;
  QgsPanelWidget *mRasterAttributeTableDisabledWidget = nullptr;

src/core/qgsdiagramrenderer.cpp

@@ -528,7 +528,7 @@ void QgsDiagramRenderer::renderDiagram( const QgsFeature &feature, QgsRenderCont
 QSizeF QgsDiagramRenderer::sizeMapUnits( const QgsFeature &feature, const QgsRenderContext &c ) const
 {
  QgsDiagramSettings s;
- if ( !diagramSettings( feature, c, s ) )
+ if ( !diagramSettings( feature, c, s ) || !s.enabled )
  {
  return QSizeF();
  }
@@ -845,16 +845,18 @@ QgsStackedDiagramRenderer *QgsStackedDiagramRenderer::clone() const
 QSizeF QgsStackedDiagramRenderer::sizeMapUnits( const QgsFeature &feature, const QgsRenderContext &c ) const
 {
  QSizeF stackedSize( 0, 0 );
+ int enabledDiagramCount = 0; // We'll add spacing only for enabled subDiagrams
 
  // Iterate renderers. For each renderer, get the diagram
  // size for the feature and add it to the total size
  // accounting for stacked diagram defined spacing
- for ( int i = 0; i < mDiagramRenderers.count(); i++ )
+ for ( const auto &subRenderer : std::as_const( mDiagramRenderers ) )
  {
- QSizeF size = mDiagramRenderers.at( i )->sizeMapUnits( feature, c );
+ QSizeF size = subRenderer->sizeMapUnits( feature, c );
 
  if ( size.isValid() )
  {
+ enabledDiagramCount++;
  switch ( mSettings.stackedDiagramMode )
  {
  case QgsDiagramSettings::Horizontal:
@@ -875,11 +877,11 @@ QSizeF QgsStackedDiagramRenderer::sizeMapUnits( const QgsFeature &feature, const
  switch ( mSettings.stackedDiagramMode )
  {
  case QgsDiagramSettings::Horizontal:
- stackedSize.scale( stackedSize.width() + spacing * ( mDiagramRenderers.count() - 1 ), stackedSize.height(), Qt::IgnoreAspectRatio );
+ stackedSize.scale( stackedSize.width() + spacing * ( enabledDiagramCount - 1 ), stackedSize.height(), Qt::IgnoreAspectRatio );
  break;
 
  case QgsDiagramSettings::Vertical:
- stackedSize.scale( stackedSize.width(), stackedSize.height() + spacing * ( mDiagramRenderers.count() - 1 ), Qt::IgnoreAspectRatio );
+ stackedSize.scale( stackedSize.width(), stackedSize.height() + spacing * ( enabledDiagramCount - 1 ), Qt::IgnoreAspectRatio );
  break;
  }
  return stackedSize;
@@ -893,19 +895,28 @@ void QgsStackedDiagramRenderer::renderDiagram( const QgsFeature &feature, QgsRen
  }
 
  QPointF newPos = pos; // Each subdiagram will have its own newPos
- QList< QgsDiagramRenderer * > stackedRenderers = renderers();
- for ( const auto &stackedRenderer : std::as_const( stackedRenderers ) )
+
+ // Get subrenderers sorted by mode (vertical diagrams are returned backwards)
+ const QList< QgsDiagramRenderer * > stackedRenderers = renderers( true );
+
+ for ( const auto &stackedRenderer : stackedRenderers )
  {
  if ( stackedRenderer->rendererName() == QStringLiteral( "Stacked" ) )
  {
+ // Nested stacked diagrams will use this recursion
  stackedRenderer->renderDiagram( feature, c, newPos, properties );
  continue;
  }
 
  QgsDiagramSettings s;
  if ( !stackedRenderer->diagramSettings( feature, c, s ) )
  {
- return;
+ continue;
+ }
+
+ if ( !s.enabled )
+ {
+ continue;
  }
 
  if ( properties.hasActiveProperties() )
@@ -963,26 +974,19 @@ QList<QString> QgsStackedDiagramRenderer::diagramAttributes() const
 QList< QgsLayerTreeModelLegendNode * > QgsStackedDiagramRenderer::legendItems( QgsLayerTreeLayer *nodeLayer ) const
 {
  QList< QgsLayerTreeModelLegendNode * > nodes;
- for ( int i = 0; i < rendererCount(); i++ )
+ for ( const auto &renderer : std::as_const( mDiagramRenderers ) )
  {
- nodes << mDiagramRenderers.at( i )->legendItems( nodeLayer );
+ nodes << renderer->legendItems( nodeLayer );
  }
 
  return nodes;
 }
 
-QList< QgsDiagramRenderer * > QgsStackedDiagramRenderer::renderers() const
+QList< QgsDiagramRenderer * > QgsStackedDiagramRenderer::renderers( bool sortByDiagramMode ) const
 {
  QList< QgsDiagramRenderer * > renderers;
 
- if ( mSettings.stackedDiagramMode == QgsDiagramSettings::Horizontal )
- {
- for ( const auto &item : std::as_const( mDiagramRenderers ) )
- {
- renderers.append( item );
- }
- }
- else
+ if ( sortByDiagramMode && mSettings.stackedDiagramMode == QgsDiagramSettings::Vertical )
  {
  // We draw vertical diagrams backwards, so
  // we return the subrenderers in reverse order
@@ -992,6 +996,10 @@ QList< QgsDiagramRenderer * > QgsStackedDiagramRenderer::renderers() const
  renderers.append( *iter );
  }
  }
+ else
+ {
+ renderers = mDiagramRenderers;
+ }
  return renderers;
 }
 
@@ -1013,11 +1021,6 @@ const QgsDiagramRenderer *QgsStackedDiagramRenderer::renderer( const int index )
  return nullptr;
 }
 
-int QgsStackedDiagramRenderer::rendererCount() const
-{
- return mDiagramRenderers.count();
-}
-
 void QgsStackedDiagramRenderer::readXml( const QDomElement &elem, const QgsReadWriteContext &context )
 {
  const QDomElement categoryElem = elem.firstChildElement( QStringLiteral( "DiagramCategory" ) );
@@ -1234,7 +1237,7 @@ QList< QgsLayerTreeModelLegendNode * > QgsDiagramRenderer::legendItems( QgsLayer
 QList< QgsLayerTreeModelLegendNode * > QgsSingleCategoryDiagramRenderer::legendItems( QgsLayerTreeLayer *nodeLayer ) const
 {
  QList< QgsLayerTreeModelLegendNode * > nodes;
- if ( mShowAttributeLegend )
+ if ( mShowAttributeLegend && mSettings.enabled )
  nodes = mSettings.legendItems( nodeLayer );
 
  return nodes;
@@ -1243,6 +1246,11 @@ QList< QgsLayerTreeModelLegendNode * > QgsSingleCategoryDiagramRenderer::legendI
 QList< QgsLayerTreeModelLegendNode * > QgsLinearlyInterpolatedDiagramRenderer::legendItems( QgsLayerTreeLayer *nodeLayer ) const
 {
  QList< QgsLayerTreeModelLegendNode * > nodes;
+ if ( !mSettings.enabled )
+ {
+ return nodes;
+ }
+
  if ( mShowAttributeLegend )
  nodes = mSettings.legendItems( nodeLayer );