Fix: Optimize Drawing.bBox calls to prevent unnecessary reflows in complex DOMs#7497
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.bBoxwhich 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 inDrawing.bBox`.Changes Introduced
Width/height optimization:
getBBox()when only size metrics are needed (avoiding full Drawing.bBox).Drawing.bBox logic
getBBoxfails to compute position fortextnode correctly. Raised here )Impact