GdkPixbufFromSkBitmap needs to un-premultiply the alpha before

GdkPixbufFromSkBitmap needs to un-premultiply the alpha before
passing off the data to gdk_pixbuf.  This is required in the theme
code for tinted bitmaps.

[Dean McNamee, 2009-07-01 06:32:01]

How about using SkUnPreMultiply ?

[Nico, 2009-07-06 19:14:56]

Surprise review!

http://codereview.chromium.org/150151/diff/1/2
File base/gfx/gtk_util.cc (right):

http://codereview.chromium.org/150151/diff/1/2#newcode48
Line 48: divided[i + 2] = (SkColorGetB(pixel) << 8) / alpha;
This has the same bug as the code it was copied from:
http://codereview.chromium.org/149164 . I wonder if there might be a lesson
hidden in here :-)

[tony, 2009-07-07 17:49:17]

Switched to SkUnPreMultiply.  Can you take another look?

[tony, 2009-07-08 16:41:23]

ping, anyone?

[Dean McNamee, 2009-07-08 16:46:38]

ok

http://codereview.chromium.org/150151/diff/3001/2003
File base/gfx/gtk_util.cc (right):

http://codereview.chromium.org/150151/diff/3001/2003#newcode37
Line 37: const int bbp = 4;
static const int kBytesPerPixel ?

http://codereview.chromium.org/150151/diff/3001/2003#newcode38
Line 38: guchar* divided = static_cast<guchar*>(malloc(height * stride));
should we use g_malloc or anything?

http://codereview.chromium.org/150151/diff/3001/2003#newcode41
Line 41: for (int y = 0; y < height; y++) {
int i = 0, y = 0

[brettw, 2009-07-08 16:47:01]

http://codereview.chromium.org/150151/diff/3001/2003
File base/gfx/gtk_util.cc (right):

http://codereview.chromium.org/150151/diff/3001/2003#newcode37
Line 37: const int bbp = 4;
This should be kBpp.

http://codereview.chromium.org/150151/diff/3001/2003#newcode38
Line 38: guchar* divided = static_cast<guchar*>(malloc(height * stride));
Seems weird to use GTK-specific types here. How about either uint8 or at least
unsigned char.

[tony, 2009-07-08 17:08:50]

http://codereview.chromium.org/150151/diff/3001/2003
File base/gfx/gtk_util.cc (right):

http://codereview.chromium.org/150151/diff/3001/2003#newcode37
Line 37: const int bbp = 4;
On 2009/07/08 16:46:38, Dean McNamee wrote:
> static const int kBytesPerPixel ?

Done.

http://codereview.chromium.org/150151/diff/3001/2003#newcode38
Line 38: guchar* divided = static_cast<guchar*>(malloc(height * stride));
On 2009/07/08 16:46:38, Dean McNamee wrote:
> should we use g_malloc or anything?

Went with brettw's suggestions of just using uint8, which I guess should just go
with malloc.

http://codereview.chromium.org/150151/diff/3001/2003#newcode41
Line 41: for (int y = 0; y < height; y++) {
On 2009/07/08 16:46:38, Dean McNamee wrote:
> int i = 0, y = 0

Done.

[brettw, 2009-07-08 17:25:05]

LGTM