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

Recalculate layout when legend item size changes #38882

Merged
merged 2 commits into from
Sep 20, 2020

Conversation

m-kuhn
Copy link
Member

@m-kuhn m-kuhn commented Sep 19, 2020

This triggers an update() of the legend viewport when the size of the legend item changes.

The solution is only partially satisfying. This should be done automatically by the model / view themselves. And it seems to work correctly for WMS. I couldn't find out why.

This patch works without any side effects. Happy to dump it in favor of an alternative approach.

Fixes #38881

@nyalldawson
Copy link
Collaborator

Could you test with a project involving 1000s of layers and confirm there's no regressions in project handling speed here? I've previously seen that legend update code like this can cause temporary qgis hangs on large projects (that's an ongoing issue, but I don't want it to get any worse then it currently is!)

@github-actions github-actions bot added this to the 3.16.0 milestone Sep 19, 2020
@m-kuhn
Copy link
Member Author

m-kuhn commented Sep 19, 2020

What is the requirement exactly? Can I duplicate a vector layer 1000 times and add an AMS layer and test with that?

@nyalldawson
Copy link
Collaborator

That'd be a good test. Also throw a vector layer with rule based renderer or categorised renderer and 10000 categories in there.

@m-kuhn
Copy link
Member Author

m-kuhn commented Sep 19, 2020

I'm confident this doesn't introduce any noticeable performance impact. It triggers a (deferred) repaint like any user interface interaction does (clicking somewhere, hitting a random key on the keyboard, scrolling, ...) and only does this under a specific condition.

I'll try to create meaningful results. It fear it won't be easy.

Talking about... Debugging the legend code I've seen code paths triggered repeatedly without any good reason, so if you say you've observed bad performance ... I'm not surprised.

@m-kuhn
Copy link
Member Author

m-kuhn commented Sep 20, 2020

Confirmed with 10000 categories legend, 34 seconds to add the layer with and without the patch.

@m-kuhn
Copy link
Member Author

m-kuhn commented Sep 20, 2020

Followup issue #38890

Copy link
Collaborator

@nyalldawson nyalldawson left a comment

Choose a reason for hiding this comment

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

Thanks, proceed at will!

@nyalldawson nyalldawson merged commit 8a53235 into qgis:master Sep 20, 2020
@m-kuhn m-kuhn deleted the legendResize branch September 21, 2020 05:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AMS legend overlapping existing legend entries
2 participants