Bug 28345

Summary: [GTK] BitmapImage::getGdkPixbuf does not handle alpha channels properly
Product: WebKit Reporter: Martin Robinson <mrobinson>
Component: WebKitGTKAssignee: Nobody <webkit-unassigned>
Severity: Normal CC: krit, xan.lopez
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Linux   
Bug Depends on:    
Bug Blocks: 28346    
Description Flags
Fix conversion
eric: review-
Patch with variable names according to conventions xan.lopez: review+

Description Martin Robinson 2009-08-15 19:08:21 PDT
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.
Comment 1 Martin Robinson 2009-08-15 19:17:56 PDT
Created attachment 34910 [details]
Fix conversion

I've attached a patch fixing this issue.
Comment 2 Martin Robinson 2009-08-15 19:33:33 PDT
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.
Comment 3 Eric Seidel (no email) 2009-08-17 17:17:04 PDT
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?
+            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;
Seems silly to make every program check those ifdefs.
Comment 4 Martin Robinson 2009-08-17 17:29:20 PDT
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
Comment 5 Martin Robinson 2009-08-17 23:02:23 PDT
Created attachment 35016 [details]
Patch with variable names according to conventions

This patch aligns variable names according to WebKit conventions.
Comment 6 Xan Lopez 2009-08-23 00:11:44 PDT
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.
Comment 7 Xan Lopez 2009-08-23 00:18:54 PDT
Committed as r47689, closing.