GNOME Bugzilla – Bug 355343
please add 'dropshadow' support for gdkpango renderer
Last modified: 2011-02-04 16:11:01 UTC
the new libgnome-applet stuff has a special 'gnomeappletlabel' that draws two labels on top of each other -- one black and one white. this is meant to allow labels on transparent panels with dark backgrounds to be more readable. the problem is, however, that if you take a wnck window list or something then it will use gtklabel -- not gnomeappletlabel. this means that complicated widgets that are put on the panel can't take advantage of the cool drop-shadowing effect. in order for this to work there would need to be an attribute added to gdkpango's renderer (similar to the current emboss attribute) that would produce the drop-shadow effect.
Created attachment 72748 [details] [review] a possible implementation this patch allows you to do the following: attr = gdk_pango_attr_dropshadow_new (TRUE); attrs = pango_attr_list_new (); pango_attr_list_insert (attrs, attr); widget = gtk_label_new ("This is a test label"); gtk_label_set_attributes (GTK_LABEL (widget), attrs); dropshadows are hardwired to be white on black (like nautilus)
oops. forgot to mention: the patch contains a small documentation fix for GdkPangoAttrEmbossed.
It seems to me that dropshadow_context_black() does exactly the same as emboss_context() except for the different color. In fact, the whole dropshadowing seems to be the same as embossing with black instead of white, and making sure the text color is white. If so, maybe we should instead generalize the emboss attribute to allow specifying the color used for the embossing ? That would also avoid the question what happens when you combine embossing and dropshadowing and text color.
I would have modified the embossed attribute instead of defining my own. I'd have changed the gboolean into a enum that allows normal/embossed/dropshadow. You can't really do this, though, since this is a public API. The structure is public too. That being said: gbooleans are ints and enums are (basically) ints too....
Created attachment 72750 [details] [review] new patch. Here's a new patch after discussion with mclasen on IRC that kills two birds with one stone. PangoColor is used instead of GdkColor since PangoColor is used already in the GdkPangoRendererPrivate structure and since a PangoColor is a GdkColor without the 'pixel' entry for colourmap allocation. (trivia: gdk_color_parse calls pango_color_parse!) Also, if the user is manipulating Pango attribute lists, they clearly know about pango and shouldn't mind using some of its other fine APIs :)
+ /* "#fff" is less expensive than "white" */ + pango_color_parse (&emboss_color, "#fff"); + even cheaper would be emboss_color.red = 65535; emboss_color.green = 65535; emboss_color.blue = 65535; the docs need a "Since: 2.12" And I hate to change my mind on that, but looking at the other attributes in gdkpango.h, I feel it would be more coherent to take a GdkColor in the constructor. Other than that, it looks great
Created attachment 72786 [details] [review] new, again documentation note added api switched back to GdkColor direct initialisation of emboss_color instead of calling pango_color_parse
I'd remove this comment /* less expensive than calling pango_color_parse */ After that, feel free to commit. I'd appreciate if you also add the new public api to gdk/gdk.symbols and docs/reference/gdk/gdk-sections.txt.
Thanks for the prompt review. Committed.
Apart from the changes to ChangeLog, the patch doesn't seem to be committed.
GNOME developers checklist: * basic CVS skills........ check! i have no idea what happened. the changed code was still sitting on my harddrive in the same directory that i'd checked in the changelog from. it's like the 'cvs commit' didn't recurse to gdk/ or something. fixed now -- thanks :)