[GTK] BitmapImage::getGdkPixbuf does not handle alpha channels properly

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Martin Robinson]

Cairo surfaces store their information in CAIRO_FORMAT_ARGB32, while GdkPixbufs store their information in GDK_COLORSPACE_RGB. The current conversion process between the two types leads to transparency corruption on some images.

[Martin Robinson]

Created attachment 34910 [details]
Fix conversion

I've attached a patch fixing this issue.

[Martin Robinson]

Sorry to spam this bug, but I wanted to mention that this is reproducible by enabling images as cursors with the patch from #28346 and visiting: http://www.worldtimzone.com/mozilla/testcase/css3cursors.html and trying the red cursor at the bottom.

[Eric Seidel (no email)]

Comment on attachment 34910 [details]
Fix conversion

rowstride should probably be rowStride according to webkit style.

Are you sure there aren't gtk functions which abstract this?
+#if G_BYTE_ORDER == G_LITTLE_ENDIAN
+            guchar alpha = source[3];
+            dest[0] = alpha ? ((source[2] * 255) / alpha) : 0;
+            dest[1] = alpha ? ((source[1] * 255) / alpha) : 0;
+            dest[2] = alpha ? ((source[0] * 255) / alpha) : 0;
+            dest[3] = alpha;
+#else
Seems silly to make every program check those ifdefs.

[Martin Robinson]

Perhaps more recent versions of GTK+ or GDK have added functionality to accomplish this, but I could not find it. For reference here is an example where Abiword does something very similar: http://google.com/codesearch/p?hl=en&sa=N&cd=1&ct=rc#gQXpoyUxBIc/abiword-2.5.0/goffice-bits/goffice/utils/go-image.c&q=cairo_to_pixbuf&l=327

[Martin Robinson]

Created attachment 35016 [details]
Patch with variable names according to conventions

This patch aligns variable names according to WebKit conventions.

[Xan Lopez]

Comment on attachment 35016 [details]
Patch with variable names according to conventions

This looks good to me, and on top of fixing a bug it manages to do the data conversion on the client side. There's a NULL where we should use 0, but I'll fix that when landing.

[Xan Lopez]

Committed as r47689, closing.