Open Bug 537037 Opened 15 years ago Updated 2 years ago

slow painting after dynamic position changes involving a xul stack, display:block and onmousemove

Categories

(Core :: Layout, defect)

defect

Tracking

()

People

(Reporter: dao, Unassigned)

References

Details

(Keywords: perf, testcase)

Attachments

(2 files)

(deleted), application/vnd.mozilla.xul+xml
Details
(deleted), application/vnd.mozilla.xul+xml
Details
Attached file slow testcase (deleted) —
No description provided.
Attached file fast testcase w/o display:block (deleted) —
Blocks: 536945
Keywords: testcase
Dão, what do I need to do to reproduce? The two testcases act the same as far as I can tell over here (mac): I hover, stuff turns yellow under pointer, and I get a random X somewhere. Response time about the same on both testcases...
On my Linux mozilla-central debu build they're both pretty slow, but the slow one seems noticeably slower.
(And the slow one pegs the CPU, whereas the fast one doesn't peg the CPU.)
Aha. CPU pegging. OK, I can look into that.
I did a callgrind profile: 92% of the time is spent painting (i.e., inside expose_event_cb) 86% of the total is inside nsCSSRendering::PaintBorder 69.2% of the total is inside gfxContext::Fill 9.3% of the total is inside gfxContext::RoundedRectangle 5% of the time is cycle collection That leads me to guess that: * these styles lead to a very expensive painting situation * the block code is doing a bit more invalidation
(The trigger for the really really slow painting is having -moz-border-*-colors styles.)
The reason we invalidate a lot with blocks is that nsBlockFrame::ReflowLine invalidates the whole line when it reflows it (very end of the function). That said, it seems like we ought to be able to get away without reflowing the buttons at all, but we're not.
This really doesn't have anything to do with the :hover effect, though; it's the repaints resulting from the reflow from changing the position of the close button. It seems like the stack shouldn't need to reflow the children that didn't change.
Summary: slow :hover styling involving a xul stack, display:block and onmousemove → slow painting after dynamic position changes involving a xul stack, display:block and onmousemove
I think the stack is reflowing each child again to get its intrinsic size, even though only some are dirty. That's probably avoidable, although the code is pretty messy.
OK. So when I disable the :hover and just move over the buttons, we actually end up invalidating the whole viewport (at least according to quartz debug). The slow paint op is presumably the background fill inside the rounded border. If I give the stack a height, the invalidates are limited to what seems like a much more reasonable rectangle (generally one whose corners are the old and new position; since quartz debug only shows dirty region bounding rectangles, that's as good as I'll get out of it).
And yes, comment 10 seems to be right on, given comment 11 paragraph 3.
It looks like the stack gets reflowed at two different sizes: first its preferred width/height as reported by GetPrefSize (which is the height of _one_ line of the block, even with a width set, because nsFrame::GetPrefSize calls RefreshSizeCache, which computes the block pref width/height and then AddCSSPrefSize clobbers the pref width with the attr width) and then at the height actually needed if you reflow at that width. Since we have the size change, we end up reflowing all our kids in nsStackLayout::Layout. We could fix nsFrame::GetPrefSize, maybe, but then dropping the width attr would still cause issues, since the pref size would honestly be the block being all on one line...
The code that led to bug 536945 actually sets the width attribute. I removed it when building the testcase as it didn't make a difference there.
The testcase sets the width attribute too. The issue comes up whether the width attribute is set or not; it's just easier to fix if it's set. Setting the _height_ attribute, on the other hand, makes the problem go away, since it's the recomputation of height that screws things up.
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: