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

ViewBox: Only update auto-range before painting. #2947

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

agausmann
Copy link

Instead of recalculating autorange for each call to addItem, defer the calculation until the next paint event.

I was experiencing a lot of lag when creating >1000 curve items with 100 points each. Profiling showed that 70% of the runtime was spent in updateAutoRange. With this patch, updateAutoRange is less than 1% of the runtime.

Instead of recalculating autorange for each call to addItem, defer the calculation until the next paint event.

I was experiencing a lot of lag when creating >1000 curve items with 100 points each. Profiling showed that 70% of the runtime was spent in updateAutoRange.
With this patch, updateAutoRange is less than 1% of the runtime.
@j9ac9k
Copy link
Member

j9ac9k commented Mar 2, 2024

Here be dragons in this part of the code, would need to test on a number of platforms, and with log-mode to ensure plots are behaving as appropriate. I can't remember which PR touched on this last (would need to look at git blame) but we had some very undesirable consequences if memory serves...

@agausmann
Copy link
Author

I completely understand!

Give me some time and I can make an example script and open an issue. If this PR doesn't work out, we can still track the performance issue and discuss other solutions.

@j9ac9k
Copy link
Member

j9ac9k commented Mar 2, 2024

Not trying to suggest this PR is a nogo, just that it would need more scrutiny than one would think

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.

None yet

2 participants