[webkit-reviews] review granted: [Bug 107560] [Qt] RGB -> BGR is wrong on big endian : [Attachment 186386] Patch

[bugzilla-daemon at webkit.org]

Jocelyn Turcotte <jocelyn.turcotte at digia.com> has granted Allan Sandfeld Jensen
<allan.jensen at digia.com>'s request for review:
Bug 107560: [Qt] RGB -> BGR is wrong on big endian
https://bugs.webkit.org/show_bug.cgi?id=107560

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

------- Additional Comments from Jocelyn Turcotte <jocelyn.turcotte at digia.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=186386&action=review


LGTM beside 2 nitpicks.

> Source/WebCore/platform/graphics/qt/ImageBufferQt.cpp:201
> +    to[0] = from.red();
> +    to[1] = from.green();
> +    to[2] = from.blue();
> +    to[3] = from.alpha();

Somebody in the future might come here and rewrite this using bit shifting if
he feels like it.
A comment would be great to explain why this is the only right way.

Maybe quote the spec at
http://www.w3.org/TR/2011/WD-2dcontext-20110525/#pixel-manipulation ?

> Source/WebCore/platform/graphics/qt/ImageBufferQt.cpp:253
> +	   const unsigned* srcRow = reinterpret_cast<const
unsigned*>(image.constScanLine(originy + y)) + originx;

This could use RGBA32 to keep the type abstraction of
colorFromPremultipliedARGB and some Color methods.

> Source/WebCore/platform/graphics/qt/ImageBufferQt.cpp:316
> +	   unsigned* destRow = destData + y * numColumns;

ditto