Summary: | Optimize outline rendering to avoid transparency layers | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Ben Wells <benwells> | ||||||||
Component: | Layout and Rendering | Assignee: | 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
Ben Wells
2011-05-12 20:39:03 PDT
Created attachment 113617 [details]
Patch
Created attachment 113618 [details]
Patch
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 > 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 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. Created attachment 113752 [details]
Patch
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 on attachment 113752 [details] Patch Clearing flags on attachment: 113752 Committed r99454: <http://trac.webkit.org/changeset/99454> All reviewed patches have been landed. Closing bug. Rebaselines added in http://trac.webkit.org/changeset/99529 |