[webkit-reviews] review denied: [Bug 53857] [Chromium] Layout Test canvas/philip/tests/2d.composite.globalAlpha.fill.html with --accelerated-2d-canvas. : [Attachment 81369] Patch

[bugzilla-daemon at webkit.org]

Kenneth Russell <kbr at google.com> has denied Naoki Takano
<takano.naoki at gmail.com>'s request for review:
Bug 53857: [Chromium] Layout Test
canvas/philip/tests/2d.composite.globalAlpha.fill.html with
--accelerated-2d-canvas.
https://bugs.webkit.org/show_bug.cgi?id=53857

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

------- Additional Comments from Kenneth Russell <kbr at google.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=81369&action=review

> Source/WebCore/platform/graphics/chromium/GLES2Canvas.cpp:103
> +#if PLATFORM(SKIA)
> +	   int a = SkAlphaMul(c.alpha(), s);
> +#else
> +	   int a = (c.alpha() * s) >> 8;
> +#endif

The Skia-specific code path is not really necessary here. I'd just remove it.

> Source/WebCore/platform/graphics/chromium/GLES2Canvas.cpp:104
> +	   return Color((c.rgb() & 0x00FFFFFF) | (a << 24));

I think you should avoid relying on Color's internal representation. You could
write this portably as

return Color(c.red(), c.green(), c.blue(), a);