Skip to content
Permalink
Browse files
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

(cherry picked from commit 0ba03bd)
  • Loading branch information
nyalldawson committed May 31, 2021
1 parent bb19487 commit f19e8910b4eaae6feb852867e63ebf2af9bedf37
@@ -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();
@@ -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();
@@ -104,6 +104,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:
@@ -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();
@@ -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 );


};


@@ -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
@@ -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

};


@@ -1095,6 +1095,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() ) );
@@ -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;
@@ -910,6 +910,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 );
@@ -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 );
@@ -318,19 +318,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() );
}
}
}

@@ -376,6 +378,10 @@ QgsDataDefinedSizeLegendWidget *QgsRendererWidget::createDataDefinedSizeLegendWi
return panel;
}

void QgsRendererWidget::setSymbolLevels( const QList< QgsLegendSymbolItem > &, bool )
{

}

//
// QgsDataDefinedValueDialog
@@ -29,6 +29,7 @@ class QgsVectorLayer;
class QgsStyle;
class QgsFeatureRenderer;
class QgsMapCanvas;
class QgsLegendSymbolItem;

/**
* \ingroup gui
@@ -92,8 +93,10 @@ class GUI_EXPORT QgsRendererWidget : public QgsPanelWidget

/**
* Emitted when the symbol levels settings have been changed.
*
* \deprecated since QGIS 3.16.9 -- no longer emitted.
*/
void symbolLevelsChanged();
Q_DECL_DEPRECATED void symbolLevelsChanged() SIP_DEPRECATED;

protected:
QgsVectorLayer *mLayer = nullptr;
@@ -131,6 +134,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.16.9
*/
virtual void setSymbolLevels( const QList< QgsLegendSymbolItem > &levels, bool enabled );

protected slots:
void contextMenuViewCategories( QPoint p );
//! Change color of selected symbols
@@ -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() );
}
}
}

@@ -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;
@@ -59,12 +59,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 );
@@ -112,6 +106,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
@@ -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();

0 comments on commit f19e891

Please sign in to comment.