Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Backport release-3_0] Fix significant project load regressions when multiple layout legends are present #4713

Merged
merged 1 commit into from
Nov 1, 2023
Merged

[Backport release-3_0] Fix significant project load regressions when multiple layout legends are present #4713

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 90 additions & 0 deletions vcpkg/overlay/qgis-qt6/layout_fix.patch
View file
Open in desktop
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() ) );

1 change: 1 addition & 0 deletions vcpkg/overlay/qgis-qt6/portfile.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ vcpkg_from_github(
exiv2-0.28.patch # Remove when updating to QGIS 3.34
profiler.patch # Remove when updating to QGIS 3.34
exif_orientation_fix.patch # Remove when updating to QGIS 3.34.1
layout_fix.patch # Remove when updating to QGIS 3.34
)

file(REMOVE ${SOURCE_PATH}/cmake/FindQtKeychain.cmake)
Expand Down
Loading