Bug 15654 - [GTK] GdkPixbuf support for ImageCairo
: [GTK] GdkPixbuf support for ImageCairo
Status: RESOLVED FIXED
Product: WebKit
Classification: Unclassified
Component: New Bugs
: 523.x (Safari 3)
: All All
: P2 Normal
Assigned To: Nobody
: Cairo, Gtk
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2007-10-23 19:28 PDT by Alp Toker
Modified: 2009-04-23 08:40 PDT (History)
4 users (show)

See Also:


Attachments
Converting Cairo's surfaces to GdkPixbuf (2.94 KB, patch)
2009-02-28 12:58 PST, Zan Dobersek
gns: review+
Details | Formatted Diff | Diff
Converting Cairo's surfaces to GdkPixbuf, take 2 (2.91 KB, patch)
2009-03-16 13:25 PDT, Zan Dobersek
gns: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alp Toker 2007-10-23 19:28:57 PDT
We need interoperability between GdkPixbuf and ImageCairo to implement certain features. It may even be the case that we want Image in the GTK+ port to be implemented using GdkPixbuf rather than a Cairo surface.
Comment 1 Dirk Schulze 2008-10-26 04:03:28 PDT
(In reply to comment #0)
> We need interoperability between GdkPixbuf and ImageCairo to implement certain
> features. It may even be the case that we want Image in the GTK+ port to be
> implemented using GdkPixbuf rather than a Cairo surface.

What is the benefit of GdkPixbuf? Why should we use GdkPixbuf instead of a cairo_surface? Expecially if you read http://live.gnome.org/GtkCairoIntegration. It is intented to replace GdkPixbuf in Gtk by cairo (at least in parts of it).
The biggest problem seems to be point 2 in the problems list.
We'll have to do something like in 'Workarounds & Solutions' on the same link.
Comment 2 Alp Toker 2008-10-28 11:37:28 PDT
(In reply to comment #1)
> (In reply to comment #0)
> > We need interoperability between GdkPixbuf and ImageCairo to implement certain
> > features. It may even be the case that we want Image in the GTK+ port to be
> > implemented using GdkPixbuf rather than a Cairo surface.
> 
> What is the benefit of GdkPixbuf? Why should we use GdkPixbuf instead of a
> cairo_surface? Expecially if you read
> http://live.gnome.org/GtkCairoIntegration. It is intented to replace GdkPixbuf
> in Gtk by cairo (at least in parts of it).
> The biggest problem seems to be point 2 in the problems list.
> We'll have to do something like in 'Workarounds & Solutions' on the same link.
> 

I don't think there's any need to switch to GdkPixbuf or stop using cairo_surface. This bug is just here to add some integration between the image classes in GTK+ and the WebCore Cairo backend so we can easily take GdkPixbufs handed to us by GTK+ and render them with Cairo, and vice-versa, generate GdkPixbufs based on Cairo surfaces if we need to expose them in the WebKit GTK+ API for example. This kind of integration could be helpful for Cursor.h and custom image cursors and I think the file chooser needed something like this too. Hope things are a little clearer now.
Comment 3 Zan Dobersek 2009-02-28 12:58:31 PST
Created attachment 28125 [details]
Converting Cairo's surfaces to GdkPixbuf

Implements a function for converting Cairo's surfaces to GdkPixbufs. There might be a more hackish way of doing this, but I vote this one for being the prettiest.
Comment 4 Xan Lopez 2009-03-07 13:12:54 PST
(In reply to comment #3)
> Created an attachment (id=28125) [review]
> Converting Cairo's surfaces to GdkPixbuf
> 
> Implements a function for converting Cairo's surfaces to GdkPixbufs. There
> might be a more hackish way of doing this, but I vote this one for being the
> prettiest.
> 

Looks reasonable to me.
Comment 5 Dirk Schulze 2009-03-14 12:10:15 PDT
(In reply to comment #3)
> Created an attachment (id=28125) [review]
> Converting Cairo's surfaces to GdkPixbuf
> 
> Implements a function for converting Cairo's surfaces to GdkPixbufs. There
> might be a more hackish way of doing this, but I vote this one for being the
> prettiest.
> 

Hi,

just a question about GdkPixmap* pixmap = gdk_pixmap_new(0, width, height, 24);
Won't we loose the alpha channel here? Isn't 32 a better choice here? (If 32 is possible).
We'll need the alpha channel for example on cursors with images.
Comment 6 Zan Dobersek 2009-03-16 13:25:08 PDT
Created attachment 28658 [details]
Converting Cairo's surfaces to GdkPixbuf, take 2

24 -> 32. Alpha channel should now be preserved through the conversion.
Comment 7 Gustavo Noronha (kov) 2009-04-20 19:14:56 PDT
(In reply to comment #6)
> Created an attachment (id=28658) [review]
> Converting Cairo's surfaces to GdkPixbuf, take 2
> 
> 24 -> 32. Alpha channel should now be preserved through the conversion.
> 

The patch looks good to me, too, but I still fail to think of a use-case. I think we should have at least one patch using this functionality so that it gets a bit of real-world testing before we take it. Otherwise this may end up being something we have but use for nothing, for a long time.
Comment 8 Dirk Schulze 2009-04-21 03:01:01 PDT
(In reply to comment #7)
> The patch looks good to me, too, but I still fail to think of a use-case. I
> think we should have at least one patch using this functionality so that it
> gets a bit of real-world testing before we take it. Otherwise this may end up
> being something we have but use for nothing, for a long time.

A possible use-case is a cursor icon: https://bugs.webkit.org/show_bug.cgi?id=24565
Comment 9 Gustavo Noronha (kov) 2009-04-21 15:42:26 PDT
Comment on attachment 28125 [details]
Converting Cairo's surfaces to GdkPixbuf

(In reply to comment #8)
> A possible use-case is a cursor icon:
> https://bugs.webkit.org/show_bug.cgi?id=24565

Right, thanks krit! This one is good to go then, IMO. What about you cook us a patch for those cursors now? =D
Comment 10 Gustavo Noronha (kov) 2009-04-22 20:22:31 PDT
Comment on attachment 28658 [details]
Converting Cairo's surfaces to GdkPixbuf, take 2

hah... of course I reviewed the wrong patch; I also found that there's a commented out function in Pasteboard::writeImage (at WebKit/WebCore/platform/gtk/) that is waiting on this.
Comment 11 Gustavo Noronha (kov) 2009-04-23 08:40:29 PDT
Landed as r42780. Thanks, Zan!