Re: gtk_window_set_icons

Havoc Pennington <hp redhat com> writes:

> Hi,
> I attached a patch to
> The API docs and prototype are appended. I decided to allow passing in
> three standard sizes, rather than just one icon, in case people want
> to hand-draw the small one.
> It should perhaps be called gtk_window_set_icon() even though it takes
> three args, since it's one icon, just in three sizes.
> I don't have a getter because it would get some generated/scaled
> icons, it would be kind of strange.
> I don't have a property because I'm not sure how it would work. (A
> separate property for each size?)

getters and setters and properties all need to give back what was set.
If you go with a list (see below), then the behavior is obvious,
though limitations of our current set of types probably make the
property uninteresting.
> I just went ahead and required three stock sizes rather than allowing
> an arbitrary list to be passed in, because I think this is simpler to
> use and will result in more consistency. I know KDE sets 16x16 and
> 32x32 for _NET_WM_ICON, and 48x48 is the size of most GNOME icons and
> should be large enough for windowmaker/afterstep. So we provide those
> three in _NET_WM_ICON.

> void
> gtk_window_set_icons (GtkWindow  *window,
>                       GdkPixbuf  *icon_16_by_16,
>                       GdkPixbuf  *icon_32_by_32,
>                       GdkPixbuf  *icon_48_by_48)

Ugh. What an awful interface. Look how much text it took you took explain
it in the docs!

"Pick 1,2, or 3 icons, of sizes close to the sizes of the parameter names,
then match them up with the parameters" is just too painful.

I think a list + a recommendation in the docs: "a good choice for the sizes
to include is 16x16 and 48x48" would be much nicer. 

Also, I think GTK+ should give the window manager the icons in the
sizes that the app provides and let the window manager / user worry
about scaling. Remember, scaling twice provides considerably worse
results than scaling once, and scaling even by a small amount reduces

> I wasn't sure what size to use for the WM_NORMAL_HINTS pixmap though.
> Also, this patch changes gdk_window_set_icon_list() to unconditionally
> set the icons, rationale from ChangeLog:
>         * gdk/x11/gdkwindow-x11.c (gdk_window_set_icon_list): don't
>           check whether the hint is supported, just always set the
>           icon. A task list might want to use it even if the WM
>           doesn't, and the WM may change over time. Also,
>           XDeleteProperty() if list == NULL.

This is fine.

one obvious problem in your patch is:

  gdk_pixbuf_render_pixmap_and_mask (info->icon_32,

While violatation of the ICCCM for this is common, you _at least_ need
to use the system default colormap/visual for this, not the GdkRGB


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]