604557 - SVG Animations in content with display animated to "none" still triggers repaints

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Daniel Holbert [:dholbert]]

See attached testcase, which has the mouse from bug 562815's testcase being rotated back and forth.  It has two buttons that apply display:none to the mouse -- one via animated style, and one via script.

CPU USAGE OF TESTCASE:
On my dual-core laptop, using a Linux trunk build...
 (A) After initially loading testcase:   CPU usage goes to ~99%
 (B) After animating "display:none":     CPU reduces to 80%
 (C) After setting  "display:none" w JS: CPU reduces to 10%

INTERPRETATION:
 - (A) tells us how much time we spend on SMIL sampling & fully repainting.
 - (C) tells us how much CPU we spend just on our SMIL sampling.

I'd expect (B) to match (C), but it's actually way closer to (A).  That indicates that we're doing unnecessary repaints in that case.  And indeed, after profiling (B) with sysprof, I found that we're basically spending all our time there in nsLayoutUtils::PaintFrame.

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: testcase 1

Comment 2

[Daniel Holbert [:dholbert]]

To be clear: in case (B), nothing is visibly painted -- I think we're doing unnecessary invalidates on regions that we think might need to be repainted, and then we repaint them as blank.

Comment 3

[Brian Birtles (:birtles)]

It sounds like you're already looking into this Daniel but if you find you don't have time, feel free to assign to me. I think if we don't fix bug 562815 then we should at least fix this. But ideally we should get to bug 562815 too.

Comment 4

[Stebs]

Maybe this bug causes the high CPU usage on http://codedread.com/ ?
Also, move over the "Home", "Blog" etc. images which start svg animations.

Comment 5

[Brian Birtles (:birtles)]

I can't seem to reproduce this on Windows. Perhaps it was fixed by bug 603917 since we now no longer unset and reset the display property on each sample--perhaps that was triggering the unnecessary repaints?

Daniel, can you confirm if this is still a problem in Linux?

I currently see something like this:
(A) ~50%   (transform animation running)
(B) 1~10%  (<set> display:none)
(C) 1~5%   (JS: display:none)

Comment 6

[Daniel Holbert [:dholbert]]

Awesome -- my numbers match yours, Brian!  Seems likely to have been fixed by bug 603917, for the reason you described.  Feel free to resolve as WORKSFORME, or as FIXED with "depends-on: bug 603917" if you're pretty sure that that's the patch that did it.

@Stebs: I still get ~20% CPU usage at http://codedread.com/ so I think Comment 4 is still an issue.  Bug 562815 likely covers that.

Comment 7

[Brian Birtles (:birtles)]

(In reply to comment #6)
> Awesome -- my numbers match yours, Brian!  Seems likely to have been fixed by
> bug 603917, for the reason you described.  Feel free to resolve as WORKSFORME,
> or as FIXED with "depends-on: bug 603917" if you're pretty sure that that's the
> patch that did it.

Cheers Daniel, thanks for verifying. I'm pretty sure bug 603917 is what fixed this.