GNOME Bugzilla – Bug 667979
Modifying background color ... when in fullscreen mode.
Last modified: 2019-02-22 03:16:44 UTC
- I ask for having the chance to modify the background color ... when in fullscreen mode. - There are some circumstances when it becomes very useful having the availability to modify the background color ... when in fullscreen mode. When eog works with images that are actually 'documents' (screenshots from text webpages for example) ... and eog is treated as a 'document' viewer instead of as an image viewer, the VERY HIGH contrast between the black (default) background and the usually white coloured document's paper make pretty unconfortable the read itself ... I have attached an example of this phenom. Being able to set the color as a convenience would be a step forward when it comes to ergonomics in these kind of cases. Cheers.
Created attachment 205322 [details] Example of the annoying very high contrast
Link to the discussion about this on eog-list: http://mail.gnome.org/archives/eog-list/2012-January/msg00000.html Alright, as I said/wrote in the given discussion this should work as a nice little plugin. That would allow to leave eog's own code untouched considering that this is a rather rare usecase. If anyone wants to beat me to doing it (should be a nice task for beginners -> gnome-love): The plugin would react to the window going fullscreen (there's a GtkWindow-signal) and would simply override the background color a second time (eog's first). That should be sufficient as eog should reset the background color automatically when fullscreen mode is left.
Created attachment 207078 [details] [review] Plugin solving issue I have two questions related to the code. Is it right that I use global setting or should I create my own settings dialog where user can select background for fullscreen? Plugin works for two modes: slideshow and fullscreen - I think it should work for both, but I would like to know your opinion.
Review of attachment 207078 [details] [review]: Hey Adrian, (In reply to comment #3) > I have two questions related to the code. Is it right that I use global setting > or should I create my own settings dialog where user can select background for > fullscreen? I'd say that's a matter of taste. The current way might be sufficient for most. But if you could add a dialog that allows switching (follow global setting <-> custom color), that would be really flexible. Feel free to use GSettings to save the settings of the dialog (you can use the path /org/gnome/eog/plugins/fullscreenbg for this). If this is too much work for now this can easily be postponed until later though. Generally your plugin does very well already there's just a few small nitpicks outlined below. I also noticed your plugin is missing the build system bits (Makefile.am, configure.ac). For example the fullscreenbg.plugin file is usually autogenerated to allow translation of the description strings. Can you add the necessary parts as well? I can help you with that if you need help. The slideshowshuffle plugin could serve as an example for this. ::: plugins/fullscreenbackground/fullscreenbg.plugin @@ +2,3 @@ +Loader=python +Module=fullscreenbg +IAge=3 Please use IAge=2 for now. We don't do any filtering based on this value yet, but one might never know. @@ +7,3 @@ +Authors=Adrian Zgorzałek <a.zgorzalek@gmail.com> +Copyright=Copyright © 2012 Adrian Zgorzałek <a.zgorzalek@gmail.com> +Website=http://www.gedit.org GEdit? If you don't have a website of your own, you can add eog's (slightly outdated) http://projects.gnome.org/eog ::: plugins/fullscreenbackground/fullscreenbg.py @@ +1,1 @@ +from gi.repository import GObject, Eog, Gdk Please add a license header above. You should be able to use the slideshowshuffle plugin as reference. @@ +20,3 @@ + color = scroll_view.get_property("background-color") + + if mode == Eog.WindowMode(2) or mode == Eog.WindowMode(3): Can't you use the literal terms here? I mean sth. like Eog.WindowMode(FULLSCREEN)? That would make it independent from the actual numeric value of the enum.
Hi Adrian, I don't have any problem if you want to implement the plug-in for both fullscreen and slideshow mode. Thank you for your work.
Created attachment 207126 [details] [review] Improved version of plugin I hope that I fixed all the problems which Felix pointed out. I also added this preference dialog for plugin and corresponding entries in GSettings. However, I've encountered few problems with Makefile. I have UI of preference dialog in XML file and GSettings layout in another file. As far as I remember they should go to /data directory in source(but for such small plugin I believe they can stay here) then during install XML file with GSettings layout should go to /usr/share/lib-2.0/schemas/ and the second... where? I just don't know how to express it in Makefile language. If you can point a nice source of information about writing Makefiles (make, autotools manuals are awful) with Gnome specific macros and philosophy of organising files I would be grateful.
(In reply to comment #6) > Created an attachment (id=207126) [details] [review] > Improved version of plugin > > I hope that I fixed all the problems which Felix pointed out. I also added this > preference dialog for plugin and corresponding entries in GSettings. Yes, I think the ones above are fixed. Will look at the new patch in detail after writing this. > However, I've encountered few problems with Makefile. > I have UI of preference dialog in XML file and GSettings layout in another > file. As far as I remember they should go to /data directory in source(but for > such small plugin I believe they can stay here) then during install XML file > with GSettings layout should go to /usr/share/lib-2.0/schemas/ and the > second... where? I just don't know how to express it in Makefile language. It's not that complicated actually. For the GSettings you just need to assign the XML filename to gsettings_SCHEMAS. Then you can simply add a line "@GSETTINGS_RULES@" below and everything will be taken care of (copying, compiling,???). Remember to append the XML file to EXTRA_DIST as well, so it is picked up during tarball creation. See also http://developer.gnome.org/gio/unstable/ch29s06.html (roughly in the middle) Installing the UI file is also just two additional lines. I'd try it the following way: ui_DATA = preferences_dialog.ui #(add the file to EXTRA_DIST as well) uidir = $(plugindir)/fullscreenbg This would copy the ui file to $(plugindir)/fullscreenbg. The installation of the fullscreenbg.plugin file works the same way. You can then use self.plugin_info.get_data_dir() in your code to get that folder again. Note that this is technically not the correct folder for the ui file (which would be $(EOG_PLUGINS_DATA_DIR)/fullscreenbg), but until I find out how to make libpeas pass the correct data_dir path we'll have to keep it that way for Python scripts. > If you can point a nice source of information about writing Makefiles (make, > autotools manuals are awful) with Gnome specific macros and philosophy of > organising files I would be grateful. http://www.wanderinghorse.net/computing/make/ served as a nice make reference. Leaves out automake/autoconf though. There might be some references for those, but it worked better for me to learn from other people's autotools code together with the stock manuals (of course this counts for Makefiles as well).
Review of attachment 207126 [details] [review]: Overall I think we are pretty much set. :) The configuration dialog needs some makeover though. Use a GtkDialog as base widget not just a GtkBox. This will automatically include space for a Close button and also give a proper window. The text of the checkbox should likely be something like "Use custom color:" and have the inverted meaning (if the box is checked use the color from the colorbutton) if you leave it next to the colorbutton. ::: plugins/fullscreenbackground/Makefile.am @@ +1,2 @@ +# Fullscreen Background plugin +plugindir = $(libdir)/eog/plugins You can use $(EOG_PLUGINS_LIBS_DIR) instead of $(libdir)/eog/plugins here. ::: plugins/fullscreenbackground/fullscreenbg.py @@ +44,3 @@ + self.settings.get_boolean('use-global')) + self.choose_color.set_color(\ + Gdk.Color.parse(self.settings.get_string('background-color'))[1]) Is there a reason for the [1] here? GdkColor should be able to parse the color together with the leading "#" char.
Created attachment 207166 [details] [review] Improved version of plugin (In reply to comment #8) > Review of attachment 207126 [details] [review]: > > Overall I think we are pretty much set. :) > > The configuration dialog needs some makeover though. > Use a GtkDialog as base widget not just a GtkBox. This will automatically > include space for a Close button and also give a proper window. I cannot use GtkDialog since I get assertion !gtk_widget_is_toplevel failed. I looks like libpeas just needs a widget which is then packed into dialog created by library. > ::: plugins/fullscreenbackground/fullscreenbg.py > @@ +44,3 @@ > + self.settings.get_boolean('use-global')) > + self.choose_color.set_color(\ > + Gdk.Color.parse(self.settings.get_string('background-color'))[1]) > > Is there a reason for the [1] here? > GdkColor should be able to parse the color together with the leading "#" char. This [1] refers to object produced by the Gdk.Color.parse which is a tuple (bool, Gdk.Color) so I extract the color. I know that that it looks strange. You mentioned that I can use self.plugin_info.get_data_dir() and it works fine with just one downside: I cannot use it in constructor. Thus I moved all the code related to instantiating preferences dialog into do_create_configure_widget with one additional boolean variable in order to init it just once. Makefile wasn't really that difficult as I expected. Thanks to your explanation I overcame my fear :) In this patch you will also find modifications in /configure.ac where I added my plugin to the list. Not all lists probably, because there are such lists like DEFAULT_PLUGINS, USEFUL_PLUGINS so I leave it to you. I am looking forward any hints you have for me.
Created attachment 207167 [details] [review] Whoops, previous patch was wrong file.
(In reply to comment #9) > Created an attachment (id=207166) [details] [review] > Improved version of plugin > > (In reply to comment #8) > > Review of attachment 207126 [details] [review] [details]: > > > > Overall I think we are pretty much set. :) > > > > The configuration dialog needs some makeover though. > > Use a GtkDialog as base widget not just a GtkBox. This will automatically > > include space for a Close button and also give a proper window. > I cannot use GtkDialog since I get assertion !gtk_widget_is_toplevel failed. I > looks like libpeas just needs a widget which is then packed into dialog created > by library. Sorry, my fault. I looked at the other plugins using GtkBuilder for this and while they use a GtkDialog as base widget in their UI file they only return the contained GtkBox to libpeas. > > ::: plugins/fullscreenbackground/fullscreenbg.py > > @@ +44,3 @@ > > + self.settings.get_boolean('use-global')) > > + self.choose_color.set_color(\ > > + Gdk.Color.parse(self.settings.get_string('background-color'))[1]) > > > > Is there a reason for the [1] here? > > GdkColor should be able to parse the color together with the leading "#" char. > This [1] refers to object produced by the Gdk.Color.parse which is a tuple > (bool, Gdk.Color) so I extract the color. I know that that it looks strange. I see. This is basically the way Python handles functions with multiple outputs. > You mentioned that I can use self.plugin_info.get_data_dir() and it works fine > with just one downside: I cannot use it in constructor. Thus I moved all the > code related to instantiating preferences dialog into > do_create_configure_widget with one additional boolean variable in order to > init it just once. I'm not sure if that's actually strictly necessary. The C plugins recreate the dialog everytime the configure button is clicked. But let's leave it that way for now. > Makefile wasn't really that difficult as I expected. Thanks to your explanation > I overcame my fear :) > In this patch you will also find modifications in /configure.ac where I added > my plugin to the list. Not all lists probably, because there are such lists > like DEFAULT_PLUGINS, USEFUL_PLUGINS so I leave it to you. > > I am looking forward any hints you have for me. Adding it to PYTHON_*_PLUGINS lists is absolutely sufficient. :) These lists will be appended to the other plugin lists and as such will be built automatically. Alright, I'm going to give it a try now. If nothing bad happens this one is going in. :)
And there we go: commit bdb7a3b75ff6d8dc8654ebff806ce2bd3855ab11 Author: Adrian Zgorzałek <> Date: Wed Feb 8 11:22:36 2012 +0100 New plugin fullscreenbg Plugin enables custom background in fullscreen and slideshow mode. It can use global settings or plugin's setttings for background of ScrollView. Fixes Bug#667979: https://bugzilla.gnome.org/show_bug.cgi?id=667979 Thanks Adrian. There wasn't much left besides hooking the plugin up with intltool which was a little hairy even for me. I also rolled the eog-plugins-3.3.5 tarball together with your plugin. ---- This problem has been fixed in the development version. The fix will be available in the next major software release. Thank you for your bug report.
Wow !! ... I am impressed, it was my first bug report ever... and I didn't expect such a quick resolution. I though that maybe ... if I were lucky I had to wait for some months ... if not ... maybe a couple of years. Well ... Adrian ... thanks a lot for your work ... and Felix ... thank you for your mentoring. I guess I will wait for Kubuntu 12.04 to enjoy the bugfix ... but ... who knows ... if I feel brave enough ... I may compile myself (if it's not very complex) and check it sooner. Thank you very, very much.