Bug 60750

Summary: Optimize outline rendering to avoid transparency layers
Product: WebKit Reporter: Ben Wells <benwells>
Component: Layout and RenderingAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: benwells, davidbarr, dglazkov, mikelawther, noel.gordon, simon.fraser, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on: 60749    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Description Ben Wells 2011-05-12 20:39:03 PDT
Simple cases of borders and outlines, such as where all sides are the same color, are SOLID style, and don't have rounded corners, could easily be optimized to avoid using expensive transparency layers.
Comment 1 David Barr 2011-11-03 20:38:48 PDT
Created attachment 113617 [details]
Patch
Comment 2 David Barr 2011-11-03 20:50:28 PDT
Created attachment 113618 [details]
Patch
Comment 3 WebKit Review Bot 2011-11-03 21:26:09 PDT
Comment on attachment 113618 [details]
Patch

Attachment 113618 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/10314046

New failing tests:
fast/box-shadow/box-shadow-radius.html
fast/layers/self-painting-outline.html
Comment 4 David Barr 2011-11-03 21:54:09 PDT
> New failing tests:
> fast/box-shadow/box-shadow-radius.html
> fast/layers/self-painting-outline.html

These will likely need to be rebaselined due to Skia rounding differently for layered vs direct alpha paints.
Comment 5 Darin Adler 2011-11-04 10:13:38 PDT
Comment on attachment 113618 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=113618&action=review

> Source/WebCore/rendering/RenderObject.cpp:1145
> +    if (outlineStyle == SOLID && outlineColor.hasAlpha()) {

I think this code needs a why comment.

If you look at just this code, it's surprising that we have a special case just for the case where we have alpha. A good way to make that clear is to put this optimization inside the "use transparency layer" if statement. There it is easy to see that we are avoiding an expensive transparency layer. Otherwise, it is unclear why there is no fast case for the case where we don’t have alpha. The reason is that the code is already fast, and this alternate implemenation would probably be slower, in cases where we are not making a transparency layer.

So you could just add a comment, or you could even put this entire fast case inside the useTransparencyLayer if statement, or you could at least use the useTransparencyLayer boolean to trigger this fast case.

I think the key is that this is an optimization because it avoids the transparency layer and the code should make this crystal clear.
Comment 6 David Barr 2011-11-05 05:33:45 PDT
Created attachment 113752 [details]
Patch
Comment 7 David Barr 2011-11-05 05:37:32 PDT
Comment on attachment 113752 [details]
Patch

Moved the short path inside the useTransparencyLayer test for clarity.
Updated pixel tests for chromium-linux due to subtle change in alpha rounding with direct paint vs transparency layers.
Comment 8 WebKit Review Bot 2011-11-07 12:19:07 PST
Comment on attachment 113752 [details]
Patch

Clearing flags on attachment: 113752

Committed r99454: <http://trac.webkit.org/changeset/99454>
Comment 9 WebKit Review Bot 2011-11-07 12:19:12 PST
All reviewed patches have been landed.  Closing bug.
Comment 10 noel gordon 2011-11-08 13:04:45 PST
Rebaselines added in http://trac.webkit.org/changeset/99529