Created attachment 173868 [details] [review]
gdk-pixbuf: implement the GIcon interface

What about serialisation: shouldn't GIcon::to_tokens and GIcon::from_tokens also be implemented?

Implementation of to_tokens and from_tokens is not actually required (the docs say "Don't implement if the GIcon can't be serialized".
I think you could actually implement those by means of gdk_pixdata_from_pixbuf() and gdk_pixbuf_from_pixdata() (and then use gdk_pixdata_serialize/deserialize), but I wonder if it's actually useful.

Well, since GdkPixbuf *can* be serialised, that comment from the docs doesn't apply :)
And IMHO it is useful to be able to serialise/deserialise any GIcon that can support it.

I'm not sure using gdk_pixdata_serialize directly is suitable, since the GIcon:to_tokens docs say that "The tokens must not contain any whitespace.". Possibly need to base64 encode afterwards, or something.

I don't know if serialization is going to be hugely useful here, but we can always add it later.

Also, this is not going to help too much, because of the basic problem of GIcon: each user has to have a bit switch checking for all known types implementing GIcon...

Attachment 173868 [details] pushed as 23e39b9 - gdk-pixbuf: implement the GIcon interface

(In reply to comment #3)
> Implementation of to_tokens and from_tokens is not actually required (the docs
> say "Don't implement if the GIcon can't be serialized".
> I think you could actually implement those by means of
> gdk_pixdata_from_pixbuf() and gdk_pixbuf_from_pixdata() (and then use
> gdk_pixdata_serialize/deserialize), but I wonder if it's actually useful.

Trying to use GtkNumerableIcon in GNOME Shell, this actually turned out to be a problem. The problem is that the shell is using g_icon_to_string() to get a caching key, see

 http://git.gnome.org/browse/gnome-shell/tree/src/st/st-texture-cache.c?id=3.0.1#n1182

It could very well be that the shell could instead use g_icon_hash() instead, I don't know. Owen?

Either way, the lack of to_tokens/from_tokens means that a GtkNumerableIcon cannot be sent across from one process to another. This is a problem in GVfs where the system is implemented as many processes.

My gut feeling is that GtkNumerableIcon shouldn't be using GdkPixbuf at all ... instead it should use predefined emblems for low numbers (say, 1 through 10) and one emblem that reads "10+" or just "MANY". Of course we can't do that now since there is already gtk_numerable_icon_set_label() ... which is of questionable value given the space constraints (as its docs properly mention). Maybe we can add a GNumerableIcon icon in GLib that works this way and then deprecate GtkNumerableIcon. Mmm. I don't know. Thoughts?