[webkit-reviews] review granted: [Bug 60750] Optimize outline rendering to avoid transparency layers : [Attachment 113618] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted David Barr <davidbarr at chromium.org>'s
request for review:
Bug 60750: Optimize outline rendering to avoid transparency layers
https://bugs.webkit.org/show_bug.cgi?id=60750

Attachment 113618: Patch
https://bugs.webkit.org/attachment.cgi?id=113618&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
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.