Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork1.9k
Fix: Optimize Drawing.bBox calls to prevent unnecessary reflows in complex DOMs#7497
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:master
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Thanks@imbansalaniket! There are two cases I'd be concerned about here, which are the original reasons we created
I suspect that when we originally wrote this Anyway if we can ensure these cases still work, and fix the various test failures (there are a bunch of image tests that fail, you can see them in the artifacts here:https://app.circleci.com/pipelines/github/plotly/plotly.js/12418/workflows/12a27ec0-ea1e-45f9-9d25-196d14e5e1b7/jobs/275741/artifacts - also looks like one jasmine test failed here:https://app.circleci.com/pipelines/github/plotly/plotly.js/12418/workflows/12a27ec0-ea1e-45f9-9d25-196d14e5e1b7/jobs/275729) then I'm all in favor of this change. I guess the other thing to say is we really shouldn't need to do this at all for non-text nodes. I know there are a few cases where we currently do call |
Hi@alexcjohnson, thanks for your feedback,
For the failing test and recommendation point Thanks |
Hi, any update on this PR? Thanks |
Description
This PR addresses a performance issue caused by repeated calls to
Drawing.bBox
which appends and remove test node to the tester element in the DOM, which were leading to unnecessary layout reflows—especially in scenarios involving complex DOM structures or a large number of CSS variables. These frequent reflows resulted in noticeable performance degradation during initial chart rendering and interactions like resizing, panning, zooming, or updating plots.Background
calls were forcing synchronous reflows in
Drawing.bBox`.Changes Introduced
Width/height optimization:
getBBox()
when only size metrics are needed (avoiding full Drawing.bBox).Drawing.bBox logic
getBBox
fails to compute position fortext
node correctly.Raised here )Impact