Closed Bug 626536 Opened 11 years ago Closed 11 years ago

speed up drawing of background colors with border radius

Categories

(Core :: Layout, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: tnikkel, Assigned: tnikkel)

References

Details

Attachments

(2 files)

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.
The above is an excellent idea I've suggested exactly this before :)
This should be a 100% refactoring with no functional changes.
Assignee: nobody → tnikkel
Attachment #504580 - Flags: review?(roc)
This actually implements the optimization in comment 0.
Attachment #504581 - Flags: review?(roc)
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
Attachment #504580 - Flags: review?(roc) → review+
(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.
I replied to your comment in comment 5.
Why can't it be applied here? I'm sure it can. Just declare a "BackgroundClipState result;" and always return that from GetBackgroundClip.
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?
Attachment #504580 - Flags: approval2.0?
Attachment #504581 - Flags: approval2.0?
Blocks: 623615
No longer blocks: 623615
http://hg.mozilla.org/mozilla-central/rev/481493fb0d84
http://hg.mozilla.org/mozilla-central/rev/44bcd49f1596
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.