speed up drawing of background colors with border radius

RESOLVED FIXED

Status

()

defect
RESOLVED FIXED
9 years ago
7 years ago

People

(Reporter: tnikkel, Assigned: tnikkel)

Tracking

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

Assignee

Description

9 years ago
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 :)
Assignee

Comment 2

9 years ago
This should be a 100% refactoring with no functional changes.
Assignee: nobody → tnikkel
Attachment #504580 - Flags: review?(roc)
Assignee

Comment 3

9 years ago
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+
Assignee

Comment 5

9 years ago
(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.
Assignee

Comment 6

9 years ago
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.
Assignee

Comment 8

9 years ago
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?
Assignee

Updated

9 years ago
Attachment #504580 - Flags: approval2.0?
Assignee

Updated

9 years ago
Attachment #504581 - Flags: approval2.0?
Assignee

Updated

9 years ago
No longer blocks: 623615
Assignee

Comment 10

9 years ago
http://hg.mozilla.org/mozilla-central/rev/481493fb0d84
http://hg.mozilla.org/mozilla-central/rev/44bcd49f1596
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.