Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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

Open
imbansalaniket wants to merge4 commits intoplotly:master
base:master
Choose a base branch
Loading
fromimbansalaniket:fix/performance-issue-unnecessary-reflows

Conversation

imbansalaniket
Copy link

Description

This PR addresses a performance issue caused by repeated calls toDrawing.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

  • Profiling revealed that frequent ``getBoundingClientRectcalls were forcing synchronous reflows inDrawing.bBox`.
  • While this was negligible in simpler DOM trees, in real-world usage with thousands of CSS variables and deep nesting, these layout recalculations became expensive.
  • This resulted in degraded runtime performance, particularly visible during initial rendering and chart interaction which require layout updates.
image

Changes Introduced

  1. Width/height optimization:

    • Directly usegetBBox() when only size metrics are needed (avoiding full Drawing.bBox).
  2. Drawing.bBox logic

    • For non-text nodes: switched from getBoundingClientRect to getBBox.
    • For text nodes: retained getBoundingClientRect to ensure correct positioning. ( asgetBBox fails to compute position fortext node correctly.Raised here )

Impact

  • Significant reduction in forced reflows during plot rendering/updates.
  • Maintains accurate text positioning while optimizing other node calculations.
  • No changes to public API or rendering output.
image

@alexcjohnson
Copy link
Collaborator

Thanks@imbansalaniket! There are two cases I'd be concerned about here, which are the original reasons we createdDrawing.bBox in the first place:

  1. the<div> we're plotting into isn't part of the DOM yet.
  2. the<div> or its parents have CSS transforms applied.

I suspect that when we originally wrote thisgetBBox didn't have sufficient support or gave browser-dependent output, but it looks like at least the support has been good for years now. And perhaps its distinct characteristics vsgetBoundingClientRect mean we don't need to worry about CSS transforms anymore?

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 callDrawing.bBox on non-text nodes but in principle we can directly calculate what we need for these cases, which will always be better than asking the browser.

@gvwilsongvwilson requested a review fromemilyklAugust 1, 2025 15:31
@gvwilsongvwilson added communitycommunity contribution P1needed for current cycle fixfixes something broken labelsAug 1, 2025
@imbansalaniket
Copy link
Author

Hi@alexcjohnson, thanks for your feedback,

1. the <div> we're plotting into isn't part of the DOM yet. - yes this make sense asgetBBox return empty object for the node which are not in the DOM, for such node we still need to rely ongetBoundingClientRect.
2. the <div> or its parents have CSS transforms applied. - as you mentioned, yes, getBBox is independent from any transform applied on the node.

For the failing test and recommendation point1. the <div> we're plotting into isn't part of the DOM yet., I will add a fix in upcoming commit.

Thanks

@imbansalaniket
Copy link
Author

Hi,

any update on this PR?

Thanks

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@emilyklemilyklAwaiting requested review from emilykl

At least 1 approving review is required to merge this pull request.

Assignees

@emilyklemilykl

Labels
communitycommunity contributionfixfixes something brokenP1needed for current cycle
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

4 participants
@imbansalaniket@alexcjohnson@gvwilson@emilykl

[8]ページ先頭

©2009-2025 Movatter.jp