Use a safer approach to update renderers after symbol levels are changed

Instead of directly changing the renderer in place in the symbol levels widget, we delegate responsibility for handling the changes to symbol levels to the parent QgsRendererWidget subclass. This allows us to implement different logic in the various subclasses which correctly handle how that particular widget subclass should update any internal symbol references and ultimately update the renderer. Fixes instability and crashes after editing symbol levels. Fixes #42671
qgis · May 26, 2021 · f3f4c17 · f3f4c17
1 parent c3e01e5
commit f3f4c17
Show file tree

Hide file tree

Showing 19 changed files with 263 additions and 68 deletions.
diff --git a/python/gui/auto_generated/symbology/qgscategorizedsymbolrendererwidget.sip.in b/python/gui/auto_generated/symbology/qgscategorizedsymbolrendererwidget.sip.in
@@ -88,6 +88,10 @@ from the XML file with a matching name.
 .. versionadded:: 2.9
 %End
 
+  protected:
+    virtual void setSymbolLevels( const QgsLegendSymbolList &levels, bool enabled );
+
+
   protected slots:
 
     virtual void pasteSymbolToSelection();

diff --git a/python/gui/auto_generated/symbology/qgsgraduatedsymbolrendererwidget.sip.in b/python/gui/auto_generated/symbology/qgsgraduatedsymbolrendererwidget.sip.in
@@ -71,6 +71,10 @@ Refreshes the ranges for the renderer.
 The ``reset`` argument is deprecated and has no effect.
 %End
 
+  protected:
+    virtual void setSymbolLevels( const QgsLegendSymbolList &levels, bool enabled );
+
+
   protected slots:
 
     virtual void pasteSymbolToSelection();

diff --git a/python/gui/auto_generated/symbology/qgsrendererwidget.sip.in b/python/gui/auto_generated/symbology/qgsrendererwidget.sip.in
@@ -105,6 +105,17 @@ Creates widget to setup data-defined size legend.
 Returns newly created panel - may be ``None`` if it could not be opened. Ownership is transferred to the caller.
 
 .. versionadded:: 3.0
+%End
+
+    virtual void setSymbolLevels( const QList< QgsLegendSymbolItem > &levels, bool enabled );
+%Docstring
+Sets the symbol levels for the renderer defined in the widget.
+
+The ``levels`` argument defines the updated list of symbols with rendering passes set.
+
+The ``enabled`` arguments specifies if symbol levels should be enabled for the renderer.
+
+.. versionadded:: 3.20
 %End
 
   protected slots:

diff --git a/python/gui/auto_generated/symbology/qgsrulebasedrendererwidget.sip.in b/python/gui/auto_generated/symbology/qgsrulebasedrendererwidget.sip.in
@@ -140,6 +140,9 @@ Opens the dialog for refining a rule using ranges
 %End
     void refineRuleScalesGui( const QModelIndexList &index );
 
+    virtual void setSymbolLevels( const QList< QgsLegendSymbolItem > &levels, bool enabled );
+
+
     QgsRuleBasedRenderer::Rule *currentRule();
 
     virtual QList<QgsSymbol *> selectedSymbols();

diff --git a/python/gui/auto_generated/symbology/qgssinglesymbolrendererwidget.sip.in b/python/gui/auto_generated/symbology/qgssinglesymbolrendererwidget.sip.in
@@ -36,6 +36,10 @@ widgets and not open dialogs
 :param dockMode: ``True`` to enable dock mode.
 %End
 
+  protected:
+    virtual void setSymbolLevels( const QList< QgsLegendSymbolItem > &levels, bool enabled );
+
+
 };
 
 

diff --git a/python/gui/auto_generated/symbology/qgssymbollevelsdialog.sip.in b/python/gui/auto_generated/symbology/qgssymbollevelsdialog.sip.in
@@ -23,14 +23,29 @@ A widget which allows the user to modify the rendering order of symbol layers.
 #include "qgssymbollevelsdialog.h"
 %End
   public:
+
     QgsSymbolLevelsWidget( QgsFeatureRenderer *renderer, bool usingSymbolLevels, QWidget *parent /TransferThis/ = 0 );
 %Docstring
 Constructor for QgsSymbolLevelsWidget
+%End
+
+    QgsSymbolLevelsWidget( const QgsLegendSymbolList &symbols, bool usingSymbolLevels, QWidget *parent /TransferThis/ = 0 );
+%Docstring
+Constructor for QgsSymbolLevelsWidget, which takes a list of ``symbols`` to show in the dialog.
+
+.. versionadded:: 3.20
 %End
 
     bool usingLevels() const;
 %Docstring
 Returns whether the level ordering is enabled
+%End
+
+    QgsLegendSymbolList symbolLevels() const;
+%Docstring
+Returns the current legend symbols with rendering passes set, as defined in the widget.
+
+.. versionadded:: 3.20
 %End
 
     void setForceOrderingEnabled( bool enabled );
@@ -41,21 +56,18 @@ Sets whether the level ordering is always forced on and hide the checkbox (used
 %End
 
   public slots:
-    void apply();
-%Docstring
-Apply button
-%End
-
-  protected:
-
-
 
+ void apply() /Deprecated/;
+%Docstring
+Apply button.
 
-  private:
-    QgsSymbolLevelsWidget();
+.. deprecated:: QGIS 3.20.
+   Use :py:func:`~QgsSymbolLevelsWidget.symbolLevels` and manually apply the changes to the renderer as appropriate.
+%End
 
 };
 
+
 class QgsSymbolLevelsDialog : QDialog
 {
 %Docstring(signature="appended")
@@ -76,6 +88,20 @@ Constructor for QgsSymbolLevelsDialog.
 
     void setForceOrderingEnabled( bool enabled );
 
+    bool usingLevels() const;
+%Docstring
+Returns whether the level ordering is enabled.
+
+.. versionadded:: 3.20
+%End
+
+    QgsLegendSymbolList symbolLevels() const;
+%Docstring
+Returns the current legend symbols with rendering passes set, as defined in the widget.
+
+.. versionadded:: 3.20
+%End
+
 };
 
 

diff --git a/src/gui/symbology/qgscategorizedsymbolrendererwidget.cpp b/src/gui/symbology/qgscategorizedsymbolrendererwidget.cpp
@@ -1096,6 +1096,21 @@ void QgsCategorizedSymbolRendererWidget::matchToSymbolsFromXml()
   }
 }
 
+void QgsCategorizedSymbolRendererWidget::setSymbolLevels( const QgsLegendSymbolList &levels, bool enabled )
+{
+  for ( const QgsLegendSymbolItem &legendSymbol : levels )
+  {
+    QgsSymbol *sym = legendSymbol.symbol();
+    for ( int layer = 0; layer < sym->symbolLayerCount(); layer++ )
+    {
+      mRenderer->setLegendSymbolItem( legendSymbol.ruleKey(), sym->clone() );
+    }
+  }
+  mRenderer->setUsingSymbolLevels( enabled );
+  mModel->updateSymbology();
+  emit widgetChanged();
+}
+
 void QgsCategorizedSymbolRendererWidget::pasteSymbolToSelection()
 {
   std::unique_ptr< QgsSymbol > tempSymbol( QgsSymbolLayerUtils::symbolFromMimeData( QApplication::clipboard()->mimeData() ) );

diff --git a/src/gui/symbology/qgscategorizedsymbolrendererwidget.h b/src/gui/symbology/qgscategorizedsymbolrendererwidget.h
@@ -149,6 +149,9 @@ class GUI_EXPORT QgsCategorizedSymbolRendererWidget : public QgsRendererWidget,
      */
     void matchToSymbolsFromXml();
 
+  protected:
+    void setSymbolLevels( const QgsLegendSymbolList &levels, bool enabled ) override;
+
   protected slots:
 
     void pasteSymbolToSelection() override;

diff --git a/src/gui/symbology/qgsgraduatedsymbolrendererwidget.cpp b/src/gui/symbology/qgsgraduatedsymbolrendererwidget.cpp
@@ -911,6 +911,21 @@ void QgsGraduatedSymbolRendererWidget::refreshRanges( bool )
   emit widgetChanged();
 }
 
+void QgsGraduatedSymbolRendererWidget::setSymbolLevels( const QgsLegendSymbolList &levels, bool enabled )
+{
+  for ( const QgsLegendSymbolItem &legendSymbol : levels )
+  {
+    QgsSymbol *sym = legendSymbol.symbol();
+    for ( int layer = 0; layer < sym->symbolLayerCount(); layer++ )
+    {
+      mRenderer->setLegendSymbolItem( legendSymbol.ruleKey(), sym->clone() );
+    }
+  }
+  mRenderer->setUsingSymbolLevels( enabled );
+  mModel->updateSymbology();
+  emit widgetChanged();
+}
+
 void QgsGraduatedSymbolRendererWidget::cleanUpSymbolSelector( QgsPanelWidget *container )
 {
   QgsSymbolSelectorWidget *dlg = qobject_cast<QgsSymbolSelectorWidget *>( container );

diff --git a/src/gui/symbology/qgsgraduatedsymbolrendererwidget.h b/src/gui/symbology/qgsgraduatedsymbolrendererwidget.h
@@ -133,6 +133,9 @@ class GUI_EXPORT QgsGraduatedSymbolRendererWidget : public QgsRendererWidget, pr
      */
     void refreshRanges( bool reset );
 
+  protected:
+    void setSymbolLevels( const QgsLegendSymbolList &levels, bool enabled ) override;
+
   private slots:
     void mSizeUnitWidget_changed();
     void methodComboBox_currentIndexChanged( int );

diff --git a/src/gui/symbology/qgsrendererwidget.cpp b/src/gui/symbology/qgsrendererwidget.cpp
@@ -320,19 +320,21 @@ void QgsRendererWidget::showSymbolLevelsDialog( QgsFeatureRenderer *r )
   QgsPanelWidget *panel = QgsPanelWidget::findParentPanel( this );
   if ( panel && panel->dockMode() )
   {
-    QgsSymbolLevelsWidget *widget = new QgsSymbolLevelsWidget( r, r->usingSymbolLevels(), panel );
+    QgsSymbolLevelsWidget *widget = new QgsSymbolLevelsWidget( r->legendSymbolItems(), r->usingSymbolLevels(), panel );
     widget->setPanelTitle( tr( "Symbol Levels" ) );
-    connect( widget, &QgsPanelWidget::widgetChanged, widget, &QgsSymbolLevelsWidget::apply );
-    connect( widget, &QgsPanelWidget::widgetChanged, this, [ = ]() { emit widgetChanged(); emit symbolLevelsChanged(); } );
+    connect( widget, &QgsPanelWidget::widgetChanged, this, [ = ]()
+    {
+      setSymbolLevels( widget->symbolLevels(), widget->usingLevels() );
+    } );
     panel->openPanel( widget );
-    return;
   }
-
-  QgsSymbolLevelsDialog dlg( r, r->usingSymbolLevels(), panel );
-  if ( dlg.exec() )
+  else
   {
-    emit widgetChanged();
-    emit symbolLevelsChanged();
+    QgsSymbolLevelsDialog dlg( r, r->usingSymbolLevels(), panel );
+    if ( dlg.exec() )
+    {
+      setSymbolLevels( dlg.symbolLevels(), dlg.usingLevels() );
+    }
   }
 }
 
@@ -378,6 +380,10 @@ QgsDataDefinedSizeLegendWidget *QgsRendererWidget::createDataDefinedSizeLegendWi
   return panel;
 }
 
+void QgsRendererWidget::setSymbolLevels( const QList< QgsLegendSymbolItem > &, bool )
+{
+
+}
 
 //
 // QgsDataDefinedValueDialog

diff --git a/src/gui/symbology/qgsrendererwidget.h b/src/gui/symbology/qgsrendererwidget.h
@@ -28,6 +28,7 @@ class QgsStyle;
 class QgsFeatureRenderer;
 class QgsMapCanvas;
 class QgsMarkerSymbol;
+class QgsLegendSymbolItem;
 
 /**
  * \ingroup gui
@@ -50,7 +51,9 @@ class GUI_EXPORT QgsRendererWidget : public QgsPanelWidget
     //! Returns pointer to the renderer (no transfer of ownership)
     virtual QgsFeatureRenderer *renderer() = 0;
 
-    //! show a dialog with renderer's symbol level settings
+    /**
+     * Show a dialog with renderer's symbol level settings.
+     */
     void showSymbolLevelsDialog( QgsFeatureRenderer *r );
 
     /**
@@ -92,8 +95,10 @@ class GUI_EXPORT QgsRendererWidget : public QgsPanelWidget
 
     /**
      * Emitted when the symbol levels settings have been changed.
+     *
+     * \deprecated since QGIS 3.20 -- no longer emitted.
      */
-    void symbolLevelsChanged();
+    Q_DECL_DEPRECATED void symbolLevelsChanged() SIP_DEPRECATED;
 
   protected:
     QgsVectorLayer *mLayer = nullptr;
@@ -131,6 +136,17 @@ class GUI_EXPORT QgsRendererWidget : public QgsPanelWidget
      */
     QgsDataDefinedSizeLegendWidget *createDataDefinedSizeLegendWidget( const QgsMarkerSymbol *symbol, const QgsDataDefinedSizeLegend *ddsLegend ) SIP_FACTORY;
 
+    /**
+     * Sets the symbol levels for the renderer defined in the widget.
+     *
+     * The \a levels argument defines the updated list of symbols with rendering passes set.
+     *
+     * The \a enabled arguments specifies if symbol levels should be enabled for the renderer.
+     *
+     * \since QGIS 3.20
+     */
+    virtual void setSymbolLevels( const QList< QgsLegendSymbolItem > &levels, bool enabled );
+
   protected slots:
     void  contextMenuViewCategories( QPoint p );
     //! Change color of selected symbols

diff --git a/src/gui/symbology/qgsrulebasedrendererwidget.cpp b/src/gui/symbology/qgsrulebasedrendererwidget.cpp
@@ -349,6 +349,23 @@ void QgsRuleBasedRendererWidget::refineRuleScalesGui( const QModelIndexList &ind
   mModel->finishedAddingRules();
 }
 
+void QgsRuleBasedRendererWidget::setSymbolLevels( const QList<QgsLegendSymbolItem> &levels, bool )
+{
+  if ( !mRenderer )
+    return;
+
+  for ( const QgsLegendSymbolItem &legendSymbol : std::as_const( levels ) )
+  {
+    QgsSymbol *sym = legendSymbol.symbol();
+    for ( int layer = 0; layer < sym->symbolLayerCount(); layer++ )
+    {
+      mRenderer->setLegendSymbolItem( legendSymbol.ruleKey(), sym->clone() );
+    }
+  }
+
+  emit widgetChanged();
+}
+
 QList<QgsSymbol *> QgsRuleBasedRendererWidget::selectedSymbols()
 {
   QList<QgsSymbol *> symbolList;
@@ -438,17 +455,20 @@ void QgsRuleBasedRendererWidget::setRenderingOrder()
     QgsSymbolLevelsWidget *widget = new QgsSymbolLevelsWidget( mRenderer, true, panel );
     widget->setForceOrderingEnabled( true );
     widget->setPanelTitle( tr( "Symbol Levels" ) );
-    connect( widget, &QgsPanelWidget::widgetChanged, widget, &QgsSymbolLevelsWidget::apply );
-    connect( widget, &QgsPanelWidget::widgetChanged, this, &QgsPanelWidget::widgetChanged );
+    connect( widget, &QgsPanelWidget::widgetChanged, this, [ = ]()
+    {
+      setSymbolLevels( widget->symbolLevels(), widget->usingLevels() );
+    } );
     panel->openPanel( widget );
-    return;
   }
-
-  QgsSymbolLevelsDialog dlg( mRenderer, true, panel );
-  dlg.setForceOrderingEnabled( true );
-  if ( dlg.exec() )
+  else
   {
-    emit widgetChanged();
+    QgsSymbolLevelsDialog dlg( mRenderer, true, panel );
+    dlg.setForceOrderingEnabled( true );
+    if ( dlg.exec() )
+    {
+      setSymbolLevels( dlg.symbolLevels(), dlg.usingLevels() );
+    }
   }
 }
 

diff --git a/src/gui/symbology/qgsrulebasedrendererwidget.h b/src/gui/symbology/qgsrulebasedrendererwidget.h
@@ -157,6 +157,8 @@ class GUI_EXPORT QgsRuleBasedRendererWidget : public QgsRendererWidget, private
     void refineRuleRangesGui();
     void refineRuleScalesGui( const QModelIndexList &index );
 
+    void setSymbolLevels( const QList< QgsLegendSymbolItem > &levels, bool enabled ) override;
+
     QgsRuleBasedRenderer::Rule *currentRule();
 
     QList<QgsSymbol *> selectedSymbols() override;

diff --git a/src/gui/symbology/qgssinglesymbolrendererwidget.cpp b/src/gui/symbology/qgssinglesymbolrendererwidget.cpp
@@ -60,12 +60,6 @@ QgsSingleSymbolRendererWidget::QgsSingleSymbolRendererWidget( QgsVectorLayer *la
   mSelector = new QgsSymbolSelectorWidget( mSingleSymbol, mStyle, mLayer, nullptr );
   connect( mSelector, &QgsSymbolSelectorWidget::symbolModified, this, &QgsSingleSymbolRendererWidget::changeSingleSymbol );
   connect( mSelector, &QgsPanelWidget::showPanel, this, &QgsPanelWidget::openPanel );
-  connect( this, &QgsRendererWidget::symbolLevelsChanged, [ = ]()
-  {
-    delete mSingleSymbol;
-    mSingleSymbol = mRenderer->symbol()->clone();
-    mSelector->loadSymbol( mSingleSymbol );
-  } );
 
   QVBoxLayout *layout = new QVBoxLayout( this );
   layout->setContentsMargins( 0, 0, 0, 0 );
@@ -113,6 +107,15 @@ void QgsSingleSymbolRendererWidget::setDockMode( bool dockMode )
     mSelector->setDockMode( dockMode );
 }
 
+void QgsSingleSymbolRendererWidget::setSymbolLevels( const QList<QgsLegendSymbolItem> &levels, bool enabled )
+{
+  mSingleSymbol.reset( levels.at( 0 ).symbol()->clone() );
+  mRenderer->setSymbol( mSingleSymbol->clone() );
+  mRenderer->setUsingSymbolLevels( enabled );
+  mSelector->loadSymbol( mSingleSymbol.get() );
+  emit widgetChanged();
+}
+
 void QgsSingleSymbolRendererWidget::changeSingleSymbol()
 {
   // update symbol from the GUI

diff --git a/src/gui/symbology/qgssinglesymbolrendererwidget.h b/src/gui/symbology/qgssinglesymbolrendererwidget.h
@@ -49,6 +49,9 @@ class GUI_EXPORT QgsSingleSymbolRendererWidget : public QgsRendererWidget
      */
     void setDockMode( bool dockMode ) override;
 
+  protected:
+    void setSymbolLevels( const QList< QgsLegendSymbolItem > &levels, bool enabled ) override;
+
   private slots:
     void changeSingleSymbol();