Bug 15654 – [GTK] GdkPixbuf support for ImageCairo

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Alp Toker]

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.

[Dirk Schulze]

(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.

[Alp Toker]

(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.

[Zan Dobersek]

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.

[Xan Lopez]

(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.

[Dirk Schulze]

(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.

[Zan Dobersek]

Created attachment 28658 [details]
Converting Cairo's surfaces to GdkPixbuf, take 2

24 -> 32. Alpha channel should now be preserved through the conversion.

[Gustavo Noronha (kov)]

(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.

[Dirk Schulze]

(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

[Gustavo Noronha (kov)]

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

[Gustavo Noronha (kov)]

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.

[Gustavo Noronha (kov)]

Landed as r42780. Thanks, Zan!