Closed Bug 745485 Opened 13 years ago Closed 11 years ago

Avoid reflows when a position change does not result in an actual change to the dimensions of a frame (top/left/right/bottom changes)

Categories

(Core :: Layout, defect, P3)

defect

Tracking

()

RESOLVED FIXED
mozilla28

People

(Reporter: ehsan.akhgari, Assigned: roc)

References

(Blocks 1 open bug)

Details

(Keywords: perf, Whiteboard: [qa-])

Attachments

(2 files, 1 obsolete file)

Bug 157681's optimization is a little bit more conservative than it could be for the abs-pos case. To avoid all unnecessary reflows, nsCSSFrameConstructor::RecomputePosition should be modified to compute the new dimensions where we suspect there to be a dimension change, and only fall back to reflow when the dimensions really change.
This is not a regression, it's an idea for further optimization. Definitely not needed to be tracked.
Keywords: regression
But bug 729597 is a regression, which has now regressed even further.
(In reply to Martijn Wargers [:mw22] (QA - IRC nick: mw22) from comment #3) > But bug 729597 is a regression, which has now regressed even further. It is not a regression, it's just a test case which used to do nothing because of bug 10209, and now does something, and the thing it does is expensive, and has always been so.
OS: Mac OS X → All
Priority: -- → P3
Hardware: x86 → All
Blocks: 876321
Blocks: 302909
Summary: Avoid reflows when a position change does not result in an actual change to the dimensions of a frame → Avoid reflows when a position change does not result in an actual change to the dimensions of a frame (top/left/right/bottom changes)
Assignee: nobody → roc
Attached patch fix (obsolete) (deleted) — Splinter Review
Comment on attachment 810216 [details] [diff] [review] fix Not quite right yet.
Attachment #810216 - Attachment is obsolete: true
Attached patch diff -w (deleted) — Splinter Review
This turned out to be pretty simple, I think!
Attachment #810249 - Flags: review?(dbaron)
Attachment #810249 - Flags: review?(dbaron) → review?(dholbert)
Comment on attachment 810249 [details] [diff] [review] diff -w Looks good! Just some comment-clarifying nits: >+++ b/layout/base/RestyleManager.cpp >@@ -379,27 +379,16 @@ RestyleManager::RecomputePosition(nsIFra > // For the absolute positioning case, set up a fake HTML reflow state for > // the frame, and then get the offsets from it. This contextual comment needs adjusting - s/offsets/offsets and size/ (due to the reflowState.ComputedWidth() & ComputedHeight() accesses added in this patch) BUT, while you're at it: it might be good to extend that comment slightly to explain the high-level idea of this chunk a little better now, along the lines of the comment higher up that says "For relative positioning, we can simply update the frame rect". Maybe something like: // For the absolute positioning case: if the frame's size hasn't changed, we // can simply update the frame rect (and otherwise, fall back to a reflow). // To determine whether the size has changed, and the frame's new position, // set up a fake reflow state and get the size/offsets from it. >+++ b/layout/style/nsStyleStruct.cpp >+ // Don't try to handle changes between "auto" and non-auto efficiently. Maybe add: ... "because such changes are likely to change the size and require reflow" (right?) r=me
Attachment #810249 - Flags: review?(dholbert) → review+
(er, s/and otherwise/or otherwise/, probably)
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
Does this mean that we do off-main-thread animations when top/left/bottom/right is animated? Except when reflow is actually needed of course.
(In reply to comment #13) > Does this mean that we do off-main-thread animations when top/left/bottom/right > is animated? Except when reflow is actually needed of course. No, we just avoid reflowing, and move the frame directly, but this all happens on the main thread.
Whiteboard: [qa-]
Depends on: 1014252
Depends on: 1200585
Depends on: 1485251
No longer depends on: 1485251
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: