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.

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

Add 'legend_item_expression' variable to the Layout Legend expression scope #59276

Closed
wants to merge 4 commits into from

Conversation

roya0045
Copy link
Contributor

Description

This is an updated version of previous PR, not the function is directly within the scope creation and the expression can be fetched if the style of the layer is different between the canvas and the layout ( which was a limitation of the previous version).

This PR allows the user to access the expression representing a symbol rule in the Legend of a layout. Ultimately this allows the end user to perform calculation taking into acocunt the symbology in an aggregate operation.

Curerntly the only way to do such calculation is to have the label matching a single field(categorized) or single symbols, this does not work for complex symbology, which should not be the case anymore.

For instance the total area of each symbols can be displayed in the legend or any other operation with an aggregate expression,

PR aims to fix #21988 & fix #33312

Simpler version, handles style difference between layout and canvas.
@github-actions github-actions bot added this to the 3.42.0 milestone Oct 30, 2024
Copy link

github-actions bot commented Oct 30, 2024

🪟 Windows builds

Download Windows builds of this PR for testing.
Debug symbols for this build are available here.
(Built from commit 51108eb)

🪟 Windows Qt6 builds

Download Windows Qt6 builds of this PR for testing.
(Built from commit 51108eb)

Comment on lines 985 to 987
QgsMapLayerStyleOverride styleOverride( vl );
if ( modelstyles.contains( vl->id() ) )
styleOverride.setOverrideStyle( modelstyles.value( vl->id() ) );
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applying an override is a very slow operation -- is this actually required? Or just copied/pasted from elsewhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's required AFAIK, previously the implementation called the renderer directly, but if the style of a given layer in the canvas was different than in the layout ( cause by using a theme in the layout) then the expression could not be retreived by the renderer as legendKeyToExpression could not return the value.

If you think there is another way to retraive the expression from the renderer without using the override, this would be good. Though maybe some logic could be implemented to bypass the override and only perform the operation when required.

I'll let you make the call on what could be improved and I'll rework the PR as needed.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd have expected that this was already handled prior to calling any of this code -- otherwise even the simple case of a map with a different style having different color fills would be broken.

Can you try removing this code and retest?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As suggested, I removed the style override and both with local manual tests and in the test image, the lines are just NULL.

Copy link

Tests failed for Qt 6

One or more tests failed using the build from commit 51108eb

composer_legend_theme_expression_scope

composer_legend_theme_expression_scope

Test failed at testLegendExpressionWithStyles at tests/src/python/test_qgslayoutlegend.py:1131

Rendered image did not match tests/testdata/control_images/composer_legend/expected_composer_legend_theme_expression_scope/expected_composer_legend_theme_expression_scope.png (found 57430 pixels different)

The full test report (included comparison of rendered vs expected images) can be found here.

Further documentation on the QGIS test infrastructure can be found in the Developer's Guide.

Copy link

The QGIS project highly values your contribution and would love to see this work merged! Unfortunately this PR has not had any activity in the last 14 days and is being automatically marked as "stale". If you think this pull request should be merged, please check

  • that all unit tests are passing

  • that all comments by reviewers have been addressed

  • that there is enough information for reviewers, in particular

    • link to any issues which this pull request fixes

    • add a description of workflows which this pull request fixes

    • add screenshots if applicable

  • that you have written unit tests where possible
    In case you should have any uncertainty, please leave a comment and we will be happy to help you proceed with this pull request.
    If there is no further activity on this pull request, it will be closed in a week.

@github-actions github-actions bot added the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Nov 16, 2024
Copy link

While we hate to see this happen, this PR has been automatically closed because it has not had any activity in the last 21 days. If this pull request should be reconsidered, please follow the guidelines in the previous comment and reopen this pull request. Or, if you have any further questions, just ask! We love to help, and if there's anything the QGIS project can do to help push this PR forward please let us know how we can assist.

@github-actions github-actions bot closed this Nov 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale Uh oh! Seems this work is abandoned, and the PR is about to close.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unable to use aggregate fucntions in Text on Symbol expressions Expression for Map Composer Legend
2 participants