-
-
Notifications
You must be signed in to change notification settings - Fork 230
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Fix significant project load regressions when multiple layout legends…
… are present
- Loading branch information
Showing
2 changed files
with
91 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,90 @@ | ||
| diff --git a/src/core/layout/qgslayoutitemlegend.cpp b/src/core/layout/qgslayoutitemlegend.cpp | ||
| index b2223d74d5d..07d3c2ebd60 100644 | ||
| --- a/src/core/layout/qgslayoutitemlegend.cpp | ||
| +++ b/src/core/layout/qgslayoutitemlegend.cpp | ||
| @@ -75,12 +75,8 @@ QgsLayoutItemLegend::QgsLayoutItemLegend( QgsLayout *layout ) | ||
| { | ||
| // NOTE -- we do NOT connect to ::refresh here, as we don't want to trigger the call to onAtlasFeature() which sets mFilterAskedForUpdate to true, | ||
| // causing an endless loop. | ||
| - | ||
| - // TODO -- the call to QgsLayoutItem::refresh() is probably NOT required! | ||
| - QgsLayoutItem::refresh(); | ||
| - | ||
| - // (this one is definitely required) | ||
| - clearLegendCachedData(); | ||
| + invalidateCache(); | ||
| + update(); | ||
| } ); | ||
| } | ||
|
|
||
| @@ -1047,25 +1043,10 @@ void QgsLayoutItemLegend::mapThemeChanged( const QString &theme ) | ||
| mThemeName = theme; | ||
|
|
||
| // map's theme has been changed, so make sure to update the legend here | ||
| - if ( mLegendFilterByMap ) | ||
| - { | ||
| - // legend is being filtered by map, so we need to re run the hit test too | ||
| - // as the style overrides may also have affected the visible symbols | ||
| - updateFilterByMap( false ); | ||
| - } | ||
| - else | ||
| - { | ||
| - if ( mThemeName.isEmpty() ) | ||
| - { | ||
| - setModelStyleOverrides( QMap<QString, QString>() ); | ||
| - } | ||
| - else | ||
| - { | ||
| - // get style overrides for theme | ||
| - const QMap<QString, QString> overrides = mLayout->project()->mapThemeCollection()->mapThemeStyleOverrides( mThemeName ); | ||
| - setModelStyleOverrides( overrides ); | ||
| - } | ||
| - } | ||
| + | ||
| + // legend is being filtered by map, so we need to re run the hit test too | ||
| + // as the style overrides may also have affected the visible symbols | ||
| + updateFilterByMap( false ); | ||
|
|
||
| adjustBoxSize(); | ||
|
|
||
| @@ -1085,22 +1066,28 @@ void QgsLayoutItemLegend::updateFilterByMap( bool redraw ) | ||
|
|
||
| void QgsLayoutItemLegend::doUpdateFilterByMap() | ||
| { | ||
| - if ( mMap ) | ||
| + // There's an incompatibility here between legend handling of linked map themes and layer style overrides vs | ||
| + // how expression evaluation is made in legend content text. The logic below is hacked together to get | ||
| + // all the existing unit tests passing, but these two features are incompatible with each other and fixing | ||
| + // this is extremely non-trivial. Let's just hope no-one tries to use those features together! | ||
| + // Ideally, all the branches below would be consistently using either "setModelStyleOverrides" (which forces | ||
| + // a rebuild of each layer's legend, and breaks legend text expression evaluation) OR | ||
| + // "mLegendModel->setLayerStyleOverrides" which just handles the expression updates but which doesn't correctly | ||
| + // generate the legend content from the associated theme settings. | ||
| + if ( mMap && !mThemeName.isEmpty() ) | ||
| { | ||
| - if ( !mThemeName.isEmpty() ) | ||
| - { | ||
| - // get style overrides for theme | ||
| - const QMap<QString, QString> overrides = mLayout->project()->mapThemeCollection()->mapThemeStyleOverrides( mThemeName ); | ||
| - mLegendModel->setLayerStyleOverrides( overrides ); | ||
| - } | ||
| - else | ||
| - { | ||
| - mLegendModel->setLayerStyleOverrides( mMap->layerStyleOverrides() ); | ||
| - } | ||
| + // get style overrides for theme | ||
| + const QMap<QString, QString> overrides = mLayout->project()->mapThemeCollection()->mapThemeStyleOverrides( mThemeName ); | ||
| + setModelStyleOverrides( overrides ); | ||
| + } | ||
| + else if ( mMap ) | ||
| + { | ||
| + mLegendModel->setLayerStyleOverrides( mMap->layerStyleOverrides() ); | ||
| } | ||
| else | ||
| + { | ||
| mLegendModel->setLayerStyleOverrides( QMap<QString, QString>() ); | ||
| - | ||
| + } | ||
|
|
||
| const bool filterByExpression = QgsLayerTreeUtils::hasLegendFilterExpression( *( mCustomLayerTree ? mCustomLayerTree.get() : mLayout->project()->layerTreeRoot() ) ); | ||
|
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
808a70eThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎉 Ta-daaa, freshly created APKs are available: arm64-android
Other architectures