626536 - speed up drawing of background colors with border radius

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Timothy Nikkel (:tnikkel)]

We can get significant speed ups when drawing the background color of elements that have border radius by filling a rounded rect and clipping to a rect instead of clipping to a rounded rect and filling a rect. I.e.

ctx->RoundedRect(backgroundRect, corners)
ctx->Clip()
ctx->Rect(dirty)
ctx->Fill()

is a lot slower than

ctx->Rect(dirty);
ctx->Clip()
ctx->RoundedRect(backgroundRect, corners)
ctx->Fill()

Jeff, Bas, if the above is not a good idea for any of our cairo backends please speak up.

Drawing a page with only a single rounded rect elements gets just under twice as fast.

cairo 1.10 must include optimizations to do something like this, because the last time I tried this idea together with cairo 1.10 I got no further speed up.

Comment 1

[Bas Schouten (:bas.schouten)]

The above is an excellent idea I've suggested exactly this before :)

Comment 2

[Timothy Nikkel (:tnikkel)]

Attachment: Part 1. Refactor the background clip code.

This should be a 100% refactoring with no functional changes.

Comment 3

[Timothy Nikkel (:tnikkel)]

Attachment: Part 2. Skip the rounded rect clip.

This actually implements the optimization in comment 0.

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 504580 [details] [diff] [review]
Part 1. Refactor the background clip code.

+  // Whether we are being asked to draw with a caller provided background
+  // clipping area. If this is true we also disable rounded corners.
+  PRBool mCustomClip;
+
+  gfxCornerSizes mClippedRadii;
+  PRBool mRadiiAreOuter;

PRPackedBool

Why not just return BackgroundClipState as the result of GetBackgroundClip?

Otherwise OK

Comment 5

[Timothy Nikkel (:tnikkel)]

(In reply to comment #4)
> PRPackedBool

Done.
 
> Why not just return BackgroundClipState as the result of GetBackgroundClip?

I thought it was kind of big to copy around, and I don't think the return value optimization can be applied here.

Comment 6

[Timothy Nikkel (:tnikkel)]

I replied to your comment in comment 5.

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Why can't it be applied here? I'm sure it can. Just declare a "BackgroundClipState result;" and always return that from GetBackgroundClip.

Comment 8

[Timothy Nikkel (:tnikkel)]

Isn't there always going to be at least one copy happening, because we make multiple GetBackgroundClip calls and will only have one clip state in the caller for the results?

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

OK, fine.

Comment 10

[Timothy Nikkel (:tnikkel)]

http://hg.mozilla.org/mozilla-central/rev/481493fb0d84
http://hg.mozilla.org/mozilla-central/rev/44bcd49f1596