Bug 69866 - Chromium needs support for border radius clipping
Summary: Chromium needs support for border radius clipping
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Tom Hudson
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-10-11 13:48 PDT by Tom Hudson
Modified: 2012-10-04 21:29 PDT (History)
12 users (show)

See Also:


Attachments
Patch (797.20 KB, patch)
2012-10-02 18:25 PDT, dstockwell
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tom Hudson 2011-10-11 13:48:43 PDT
The speed of the current soft clipping implementation in Chromium is insufficient to support border radius clipping in websites that make heavy use of layers (https://bugs.webkit.org/show_bug.cgi?id=68733). Border radius clipping is turned off for chromium in https://bugs.webkit.org/show_bug.cgi?id=69844, but it needs to be reenabled - either when the fast path in #68733 is shown to work well enough in practice, or when the Windows/GDI/Skia issues blocking soft clipping for Chromium are resolved.
Comment 1 Tom Hudson 2011-11-11 05:20:54 PST
Status update: the first pass at anti-aliased clipping in Skia has landed and gets performance on targeted pages up to 10-15fps when we reenable border radius clipping; this is far better than it was when border radius clipping was disabled, but still a factor of two below our target. We're investigating.
Comment 2 Peter Beverloo 2012-02-23 04:11:38 PST
For reference: I just tried this with ToT Chromium and WebKit, unfortunately I'm still getting 80-100% CPU usage and only 15-20 fps on Twitter.
Comment 3 Tom Hudson 2012-02-23 13:47:26 PST
(In reply to comment #2)
> For reference: I just tried this with ToT Chromium and WebKit, unfortunately I'm still getting 80-100% CPU usage and only 15-20 fps on Twitter.

Actually, that's the pre-existing performance, and what we'll get after this bug is resolved.

The Skia optimizations required to support WebKit's border radius clipping in Chromium have landed, and enabling that feature produces no significant difference in the frame rate or profile, but we're rendering wrong for one of the four layout tests (fast/clip/overflow-border-radius-composited).

The Chromium CPU profile doesn't suggest any more obvious targets for microoptimzation in Skia; we're busy doing the direct work of clearing and blitting - further speedup for Twitter is likely going to take changing how much WebKit tries to draw.
Comment 4 dstockwell 2012-10-02 18:25:58 PDT
Created attachment 166792 [details]
Patch
Comment 5 dstockwell 2012-10-02 18:44:32 PDT
The test noted above (fast/clip/overflow-border-radius-composited) is still failing, but there is a separate issue for this: http://webkit.org/b/68196

A number of media control tests have subtle differences due to their uses of border-radius, I've included rebaselines for linux and will update expectations for the other platforms after landing.
Comment 6 noel gordon 2012-10-02 18:53:24 PDT
LGTM.

aisde: maybe file a bug about video-controls-rendering.html and it's use of scroll bars?
Comment 7 noel gordon 2012-10-02 19:03:01 PDT
Mr White: review please?
Comment 8 Stephen White 2012-10-03 07:25:26 PDT
This looks ok, but note that antialiased clipping is not (quite) implemented in the GPU path in Skia.  That's probably not an issue right now, but I'd like to get a thumbs-up from reed@ and robertphillips@ before we turn this on.
Comment 9 Mike Reed 2012-10-03 07:49:31 PDT
aaclipping is used for all other soft-clip requests in PlatformContext, so I see no reason to not use it here (from skia's perspective).
Comment 10 Stephen White 2012-10-03 07:50:41 PDT
Comment on attachment 166792 [details]
Patch

Thanks.  robertphillips@ gave me a thumbs-up offline, so r=me.
Comment 11 WebKit Review Bot 2012-10-03 19:17:39 PDT
Comment on attachment 166792 [details]
Patch

Clearing flags on attachment: 166792

Committed r130355: <http://trac.webkit.org/changeset/130355>
Comment 12 WebKit Review Bot 2012-10-03 19:17:43 PDT
All reviewed patches have been landed.  Closing bug.
Comment 13 Yury Semikhatsky 2012-10-04 00:17:22 PDT
The patch caused webkit_lint failure: http://build.chromium.org/p/chromium.webkit/builders/Webkit%20Win/builds/18128/steps/webkit_lint/logs/stdio
Comment 14 Yury Semikhatsky 2012-10-04 00:30:16 PDT
Updated expectations file in http://trac.webkit.org/changeset/130368.
Comment 15 dstockwell 2012-10-04 21:29:26 PDT
Landed mac/win rebaselines in http://trac.webkit.org/changeset/130447