Mark <svg> elements (nsSVGOuterSVGFrame) as reflow roots
Categories
(Core :: Layout, defect, P3)
Tracking
()
People
(Reporter: dholbert, Assigned: dholbert)
References
Details
(Keywords: perf, perf:responsiveness)
Attachments
(1 file)
|
(deleted),
text/x-phabricator-request
|
Details |
In bug 1550511, there's an interactive SVG graph which changes its markup in response to cursor movements, in a way that requires a reflow -- and right now, it's triggering expensive "measuring" reflows in the rest of the page.
One way to ameliorate this would be to make the outer SVG frame into a dynamic reflow root. I'm pretty sure that outer-SVG-frames' sizing is not impacted by their descendants at all, so they should be safe to make into a dynamic reflow root (or perhaps even a "true" reflow root, as long as we're sure to mark them as needing to be reflowed by their parent whenever their width/height properties change).
I expect this will get us a pretty nice perf win on pages that have interactive SVG charts intermixed with other content.
|
Assignee | |
Updated•5 years ago
|
|
||
Comment 1•5 years ago
|
||
Yeah, this seems reasonable now that we can do overflow updates on reflow roots (bug 1159042 patch 2).
But why dynamic? Why shouldn't they just be regular reflow roots?
|
|
Assignee | |
Comment 2•5 years ago
|
||
Yeah, I think regular reflow roots would probably work (per comment 0 perhaps even a "true" reflow root).
--> removing "dynamic" from title.
|
|
Assignee | |
Comment 3•5 years ago
|
||
Let's see if this breaks any tests:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=6b743b2ba4a7bc4917a3a7c9418bb9f0b54a7514
|
|
Assignee | |
Comment 4•5 years ago
|
||
|
|
Assignee | |
Comment 5•5 years ago
|
||
The Try run looks good. Posted patch for review.
now that we can do overflow updates on reflow roots (bug 1159042 patch 2).
BTW, I tried to come up with an SVG-specific test for this, but I couldn't manage to create any overflowing content outside of an SVG element. Here's what I tested (with bbb/ccc attempting to be transformed to create overflow, but they just get clipped at the content-edge, i.e. at the black border):
<!DOCTYPE html>
<body>
<svg style="border: 1px solid black; width: 100px">
<foreignObject width="1000px" height="1000px">
<div xmnls="http://www.w3.org/1999/xhtml"
style="transform:translatex(50px)">aaa</div>
<div xmnls="http://www.w3.org/1999/xhtml"
style="transform:translatex(90px)">bbb</div>
<div xmnls="http://www.w3.org/1999/xhtml"
style="transform:translatex(-10px)">ccc</div>
</foreignObject>
</svg>
</body>
|
|
Assignee | |
Updated•5 years ago
|
|
|
Assignee | |
Comment 6•5 years ago
|
||
(In reply to Daniel Holbert [:dholbert] from comment #5)
BTW, I tried to come up with an SVG-specific test for this, but I couldn't manage to create any overflowing content outside of an SVG element.
Indeed, nsSVGOuterSVGFrame::Reflow just forces the overflow areas to match its border-rect:
// With regards to visual overflow, we always clip root-<svg> (see our
// BuildDisplayList method) regardless of the value of the 'overflow'
// property since that is per-spec, even for the initial 'visible' value. For
// that reason there's no point in adding descendant visual overflow to our
// own when this frame is for a root-<svg>. [...SNIP...]
[...]
aDesiredSize.SetOverflowAreasToDesiredBounds();
So even before we had the special reflow-root overflow-area-updating code, we would probably have been fine to mark outer SVG frames as reflow roots.
|
||
Updated•5 years ago
|
|
|
Assignee | |
Comment 7•5 years ago
|
||
I'll land this after the code freeze + merge have passed. (i.e. early next week)
Pushed by dholbert@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/09ef16b1c51f Mark outer svg frames as reflow roots. r=dbaron
|
|
Assignee | |
Updated•5 years ago
|
|
||
Comment 9•5 years ago
|
||
| bugherder | ||
|
||
Updated•5 years ago
|
|
||
Updated•2 years ago
|
Description
•