Closed Bug 745485 Opened 8 years ago Closed 6 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)
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.
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
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
Comment on attachment 810216 [details] [diff] [review] fix Not quite right yet.
Attachment #810216 - Attachment is obsolete: true
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: 6 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.
You need to log in before you can comment on or make changes to this bug.