Merge lp:~l-admin-3/gala/workspace-switcher-background-setting into lp:gala

[Marcus Wichelmann]

systembackground: add setting to change the background image of the system background

These changes are making it possible to change the background-image of the workspace-switcher (the default is this gray background) in org.pantheon.desktop.gala.appearance.

To test it, please download this branch, compile it and install it. Then it should be possible to change the setting (org.pantheon.desktop.gala.appearance => workspace-switcher-background) to an own image. After logging in and out the image should be used in the workspace-overview.

I think, this setting will be very helful if you want to customize the background without replacing the texture.png after every update.

[Robert Roth (evfool)]

Code looks fine, and a setting is indeed better than hardcoding the file to use. Maybe checking if the file exists and falling back to the default if not would be useful, but not absolutely required.

review: Approve (code)

[Robert Roth (evfool)]

By the way, please set a short commit message ("Set commit message" link above "Description of change")

[Rico Tzschichholz (ricotz)]

Yes an existence check to fallback to the internally shipped texture should be added.

Also do not touch the file endings. Please revert those.

You realize you are hardcoding the default texture location, despite having it adjusted to the install location. Please use an autoconf preprocessor variable (AC_SUBST) to adjust it accordingly in the schema.

review: Needs Fixing

[Marcus Wichelmann (l-admin-3)]

I have added a fallback now. But I didn't know how to use theese autoconf-variables.

I'm using scratch-text-editor and the last line will be modified automatically without touching anything. Tips for preventing that would be very helpful.

[Rico Tzschichholz (ricotz)]

I have pushed r421. There you can see how it works while looking at @GETTEXT_PACKAGE@

As the fallback check I would prefer a simple file-existence check (and *maybe* combined with a mime-type check to make sure it is an image-file)

Use any other text-editor to avoid stripping the file-ending ;)

Please rebase your work on latest trunk (squashed into on commit)

review: Needs Fixing

[Marcus Wichelmann (l-admin-3)]

Ok, I think I have understand what you mean, thanks!

I have choosen this fallback-check, because that checks if the file is readable, too. A filename check would only check if the file exists and not if the read permissions are set. Also I have used the same check like that to load the wallpaper, so the code would be more consistent.

What?! An elementary developer says I shouldn't use elementary software? ;) Isn't there any workaround for Scratch?
What is the correct format for the file endings? One empty line after the last braceright?

Do you mean I should merge trunk into my branch?

Sorry, I'm new to elementary-, Vala- and Linux-development...

[Marcus Wichelmann (l-admin-3)]

Oups, I saw yet, that a not readable texture.png would cause an infinite loop... Maybe I should restrict the count of tries. ;)

[Rico Tzschichholz (ricotz)]

Avoid this recursion completely and make sufficient checks before async-call.

[Rico Tzschichholz (ricotz)]

Completely rebase the branch, so don't merge trunk.

[Marcus Wichelmann (l-admin-3)]

Ok, I will create a new branch later.

But I don't understand what the advantage of making some checks before trying to load the background should be.
Theese checks would be executed on every start up. My solution would only be executed if the choosen wallpaper isn't existent/readable/broken. So the startup would be fast like ever and would only be a small bit slower if the wallpaper isn't useable.

[Marcus Wichelmann (l-admin-3)]

Please see: https://code.launchpad.net/~l-admin-3/gala/workspace-switcher-background-setting-2/+merge/245436

[Marcus Wichelmann (l-admin-3)]

Like suggested by ricotz I've now overwritten this branch and removed the second one.

review: Needs Resubmitting

[Rico Tzschichholz (ricotz)]

review: Abstain

[lp:~l-admin-3/gala/workspace-switcher-background-setting]

429. By Marcus Wichelmann on 2015-01-04

Code moved to InternalUtils.

[Tom Beckmann (tombeckmann)]

Looks great now

(in case anyone is wondering, the function was moved to utils, so that it can be used more easily from the 314 branch as well later)

review: Approve

=== modified file 'data/org.pantheon.desktop.gala.gschema.xml.in.in'
--- data/org.pantheon.desktop.gala.gschema.xml.in.in	2014-11-15 23:59:26 +0000
+++ data/org.pantheon.desktop.gala.gschema.xml.in.in	2015-01-04 12:17:10 +0000
@@ -153,6 +153,10 @@
 			<default>0.0</default>
 			<_summary>The opacity of the windows located in the background in the alt-tab-switcher.</_summary>
 		</key>
+		<key type="s" name="workspace-switcher-background">
+			<default>''</default>
+			<_summary>Background-image used in the workspace-switcher</_summary>
+		</key>
 	</schema>
 	
 	<schema path="/org/pantheon/desktop/gala/animations/" id="org.pantheon.desktop.gala.animations" gettext-domain="@GETTEXT_PACKAGE@">

=== modified file 'src/Background/SystemBackground.vala'
--- src/Background/SystemBackground.vala	2014-04-08 12:51:06 +0000
+++ src/Background/SystemBackground.vala	2015-01-04 12:17:10 +0000
@@ -27,7 +27,7 @@
 		construct
 		{
 			var cache = BackgroundCache.get_default ();
-			cache.load_image.begin (Config.PKGDATADIR + "/texture.png", 0,
+			cache.load_image.begin (InternalUtils.get_system_background_path (), 0,
 				GDesktop.BackgroundStyle.WALLPAPER, (obj, res) => {
 				content = cache.load_image.end (res);
 			});

=== modified file 'src/InternalUtils.vala'
--- src/InternalUtils.vala	2014-07-18 22:42:06 +0000
+++ src/InternalUtils.vala	2015-01-04 12:17:10 +0000
@@ -171,6 +171,21 @@
 			Util.set_stage_input_region (screen, xregion);
 		}
 
+		public static string get_system_background_path ()
+		{
+			var filename = AppearanceSettings.get_default ().workspace_switcher_background;
+			var default_file = Config.PKGDATADIR + "/texture.png";
+
+			if (filename == "") {
+				filename = default_file;
+			} else if (!FileUtils.test (filename, FileTest.IS_REGULAR)) {
+				warning ("Failed to load %s", filename);
+				filename = default_file;
+			}
+
+			return filename;
+		}
+
 		/**
 		 * Inserts a workspace at the given index. To ensure the workspace is not immediately
 		 * removed again when in dynamic workspaces, the window is first placed on it.

=== modified file 'src/Settings.vala'
--- src/Settings.vala	2014-04-19 12:57:09 +0000
+++ src/Settings.vala	2015-01-04 12:17:10 +0000
@@ -74,6 +74,7 @@
 		public bool attach_modal_dialogs { get; set; }
 		public bool dim_parents { get; set; }
 		public double alt_tab_window_opacity { get; set; }
+		public string workspace_switcher_background { get; set; }
 
 		static AppearanceSettings? instance = null;