Merge lp:~sacridex/ubuntu/precise/unity-greeter/purple-background-on-startup-fix into lp:ubuntu/precise/unity-greeter

[Philipp]

I think there is no synchronisation issue.
But there is too much stuff to be done between the initial background color and the actual background image is drawn.
There are 2 things that take longer than 0.2 seconds: launching Canberra and playing the startup sound takes about 0.26 seconds and LightDM.get_layout () (line 81 in menubar.vala) takes about 0.20 seconds.
So there was about a half second gap between those two backgrounds, unfotunately a ~0.1 second gap still remains.

The fact that the standard purple image is loaded at all, is a problem caused by appending the *other user to the user_list(which is the first user added, so the purple image is drawn) and then removing it later(which triggers a background redraw of the actual image).
But still, unity-greeter will fade from the initial background color to the actual background image, because the fade time is set and i am not able to disable it and reset it later. :(
But in my opinion that is no problem, the fade in actually looks nice.

I disabled canberra and loaded the LightDM standard layout earlier. Also i made the Guest background image relying on the Gsettings value of LightDM User.
Unfortunately there is no more background image for the *other user, otherwise this image would be drawn(as described above).

===============
Another UPDATE:
I reverted all the KeyFile parts.

[Bryce Harrington (bryce)]

The diff is showing some unfinished merge work in background.vala.

review: Needs Fixing (code)

[Philipp (sacridex)]

Dont be surprised by the Undecided/Invalid bug status it is linked to. Thats the wrong one actually. The bug is referred to unity-greeter not lightdm, thats why. If you click on the link to it, you will see.

[Michael Terry (mterry)]

We don't want to move back to keyfiles for the configuration. We used to have that, but switched to gsettings to make it easier for derivatives to change values. (i.e. we're optimizing ease-of-changes for derivatives, not the user)

review: Needs Fixing

[lp:~sacridex/ubuntu/precise/unity-greeter/purple-background-on-startup-fix]

35. By Philipp on 2012-04-24

* reverted all the KeyFile stuff
* the only change now is that the standard purple image is no longer shown before the user background image is shown

[Bryce Harrington (bryce)]

I don't know vala very well so can't comment on the quality of the code, but the patch looks clean now.

review: Approve (patch)

[Michael Terry (mterry)]

Just coming back to this. Why is default_background_image only used for the guest account? What about users that don't have a background set?

[Philipp (sacridex)]

Because the guest account has no image.
If a user has no background(which I think is impossible, but who knows), the default_background is used(background.vala:447). This is the backgroundcolor set for the lightdm user(background.vala:230).
So the guest account has a background image after all, and not just the background color.

But thinking about it now, it would be a better solution, not to put the "*other" user into the user_list at first(user-list.vala:257), but checking if there are any users in the user_list, after the real users have been added to the user_list(if not, then put "*other" to the user_list).

[Michael Terry (mterry)]

But it is possible and common for users not to have a set background (either they use a private image that the greeter doesn't have access to or encrypted home directories or the like).

So we need to do the same logic for them that we do for the *other user.

review: Needs Fixing

[Sebastien Bacher (seb128)]

ideally the merge request should be retargetted to an active serie as well, either precise-proposed or quantal

Unmerged revisions

35. By Philipp on 2012-04-24

* reverted all the KeyFile stuff
* the only change now is that the standard purple image is no longer shown before the user background image is shown

=== modified file 'src/background.vala'
--- src/background.vala	2012-04-18 17:21:34 +0000
+++ src/background.vala	2012-04-24 17:39:18 +0000
@@ -228,6 +228,7 @@
     }
 
     public string default_background {get; set; default = UGSettings.get_string (UGSettings.KEY_BACKGROUND_COLOR);}
+    public string default_background_image {get; set; default = "";}
     public string? current_background {get; set; default = null;}
     public bool draw_grid {get; set; default = true;}
     public double alpha {get; private set; default = 1.0;}

=== modified file 'src/main-window.vala'
--- src/main-window.vala	2012-04-18 17:21:34 +0000
+++ src/main-window.vala	2012-04-24 17:39:18 +0000
@@ -37,7 +37,9 @@
         add_accel_group (accel_group);
 
         var bg_color = Gdk.RGBA ();
-        bg_color.parse (UGSettings.get_string (UGSettings.KEY_BACKGROUND_COLOR));
+        if (!bg_color.parse (UGSettings.get_string (UGSettings.KEY_BACKGROUND_COLOR)))
+            /* backup color hardcoded for the worst case(UGSettings key is no parseable color) */
+            bg_color.parse ("#2C001E");
         override_background_color (Gtk.StateFlags.NORMAL, bg_color);
         get_accessible ().set_name (_("Login Screen"));
         has_resize_grip = false;
@@ -46,7 +48,8 @@
         realize ();
         background = new Background (Gdk.cairo_create (get_window ()).get_target ());
         background.draw_grid = UGSettings.get_boolean (UGSettings.KEY_DRAW_GRID);
-        background.default_background = UGSettings.get_string (UGSettings.KEY_BACKGROUND);
+        /* Only default_background_image ist set here, not default_background(otherwise the default_background callback would trigger) */
+        background.default_background_image = UGSettings.get_string (UGSettings.KEY_BACKGROUND);
         background.set_logo (UGSettings.get_string (UGSettings.KEY_LOGO), UGSettings.get_string (UGSettings.KEY_BACKGROUND_LOGO));
         background.show ();
         add (background);

=== modified file 'src/user-list.vala'
--- src/user-list.vala	2012-04-18 17:21:34 +0000
+++ src/user-list.vala	2012-04-24 17:39:18 +0000
@@ -77,7 +77,7 @@
         {
             _offer_guest = value;
             if (value)
-                add_entry ("*guest", _("Guest Session"));
+                add_entry ("*guest", _("Guest Session"), background.default_background_image);
             else
                 remove_entry ("*guest");
         }