GNOME Bugzilla – Bug 411427
enhancements for face greeter
Last modified: 2010-06-04 20:09:34 UTC
For Fedora 7 we're planning to ship with a GDM theme that has uses the face greeter by default. While creating the theme I ran into a few issues - It's not possible to set the text color of the labels in the userlist - It looks weird how faces have different sizes So I wrote a patch to address this. It does the following 1. You can pass labelfgcolor to specify the text color in the user list 2. If a default face is found, we scale all other faces to be this size Maybe it's interesting to apply this patch upstream? Thanks for considering.
Created attachment 83211 [details] [review] proposed patch
Created attachment 83219 [details] [review] revised patch Also, it would be nice for gdm themes to ship a gtkrc file so here's a patch for that. Themes can now do <greeter gtkrc="MyThemeName.gtkrc"> to do this. Here's a screenshot showing this http://people.freedesktop.org/~david/gdm-with-gtkrc.png Thanks.
Mostly looks good. A few issues: 1) You include no updates to docs/C/gdm.xml. Could you include the new information about the theme changes there (e.g. new labelfgcolor element and mention that if a default face is found that all other faces are scaled) 2) I don't think that the <greeter gtkrc="foo.gtrkc"> syntax is required. I believe if you use the following that it works. <greeter gtk-theme="foo.gtkrc"> Can you verify? If this doesn't work, then I'm happy to add this feature as well (assuming you also add docs to docs/C/gdm.xml for it). If it does work, then please remove this part from the patch. If it does work, you might want to update the docs to be more clear that a gtkrc file can be used this way. 3) Would be nice to check if the default image and face image are the same scale and avoid calling gdk_pixbuf_scale_simple if not necessary. Calls to scale are slow so we want to avoid unnecessary scales, remember there could be hundreds of user images to scale. 4) Your code doesn't deal with the fact if the default image and the user image are different dimensions. So your scale would distort images unless they are the same dimensions. For example if the default image is 120x100 and the user image is 80x90 or 90x80. Might be better to scale the larger dimension of the user image to the default image dimension. I think it is better to not distort the images.
(In reply to comment #3) > Mostly looks good. A few issues: > > 1) You include no updates to docs/C/gdm.xml. Could you include the new > information about the theme changes there (e.g. new labelfgcolor element > and mention that if a default face is found that all other faces are scaled) Certainly. Wow, I didn't even know that file existed; would have been useful. Mea culpa I guess :-) > 2) I don't think that the <greeter gtkrc="foo.gtrkc"> syntax is required. I > believe if you use the following that it works. > > <greeter gtk-theme="foo.gtkrc"> > > Can you verify? If this doesn't work, then I'm happy to add this feature > as well (assuming you also add docs to docs/C/gdm.xml for it). If it does > work, then please remove this part from the patch. If it does work, you > might want to update the docs to be more clear that a gtkrc file can be > used this way. Just tested this - it doesn't work... gtk-theme doesn't pass the path to the theme directory. Plus, one would perhaps like to use both gtk-theme and gtkrc; e.g. a GDM theme could depend on a system-wide theme (say, Clearlooks) and also include a gtkrc to apply a style to a few particular widgets (say, the scrollbar). > 3) Would be nice to check if the default image and face image are the same > scale and avoid calling gdk_pixbuf_scale_simple if not necessary. Calls to > scale are slow so we want to avoid unnecessary scales, remember there could > be hundreds of user images to scale. Good point, I'll fix this. > 4) Your code doesn't deal with the fact if the default image and the user > image are different dimensions. So your scale would distort images unless > they are the same dimensions. > > For example if the default image is 120x100 and the user image is 80x90 or > 90x80. Might be better to scale the larger dimension of the user image to > the default image dimension. I think it is better to not distort the > images. That's a good point; I'll fix this too. I'll attach an updated patch sometime tomorrow. Thanks!
Where's the updated patch?
Is there an update, David?
Created attachment 87777 [details] [review] Look for gtk-2.0/gtkrc in the theme directory and use it Hi, I've found this patch in the Ubuntu gdm package, and it solves a part of this problem in reading gtk-2.0/gtkrc below the theme dir systematically; I think this is even easier to use than the original idea of setting gtkrc in the theme XML. Bye,
Loic: I'm a bit confused by this patch. Note GDM already provides a gui/GtkRC configuration key where you can specify the gtkrc file to use by default and this already works with both gdmgreeter and gdmlogin (whereas your patch only works for gdmgreeter). This patch would override any value in gui/GtkRC that is set, which seems broken. Perhaps a problem is that the GtkRC value as assigned by default in the GDM configuration isn't right for all systems. Perhaps it might be better for this key to support a value like "default" which would end up calling the sort of logic you have written? Still, it would be nice for gdmgreeter themes to be able to override the default GtkRC value if desired. So I still see value in allowing themes to override this value.
David, any update on your patch?
Thanks for taking the time to report this bug. However, you are using a version that is too old and not supported anymore. GNOME developers are no longer working on that version, so unfortunately there will not be any bug fixes for the version that you use. By upgrading to a newer version of GNOME you could receive bug fixes and new functionality. You may need to upgrade your Linux distribution to obtain a newer version of GNOME. Please feel free to reopen this bug if the problem still occurs with a newer version of GNOME.