Ubuntu

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

Proposed by Philipp on 2012-04-19
Status: Work in progress
Proposed branch: lp:~sacridex/ubuntu/precise/unity-greeter/purple-background-on-startup-fix
Merge into: lp:ubuntu/precise/unity-greeter
Diff against target: 49 lines (+7/-3) 3 files modified
To merge this branch: bzr merge lp:~sacridex/ubuntu/precise/unity-greeter/purple-background-on-startup-fix
Reviewer Review Type Date Requested Status
Michael Terry 2012-04-24 Needs Fixing on 2012-05-29
Bryce Harrington patch Approve on 2012-04-24
Robert Ancell 2012-04-24 Pending
Ubuntu branches 2012-04-19 Pending
Review via email: mp+102679@code.launchpad.net

Description of the Change

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.

To post a comment you must log in.
Bryce Harrington (bryce) wrote :

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

review: Needs Fixing (code)
Philipp (sacridex) wrote :

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) wrote :

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
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) wrote :

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) wrote :

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) wrote :

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) wrote :

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) wrote :

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

Preview Diff

1=== modified file 'src/background.vala'
2--- src/background.vala 2012-04-18 17:21:34 +0000
3+++ src/background.vala 2012-04-24 17:39:18 +0000
4@@ -228,6 +228,7 @@
5 }
6
7 public string default_background {get; set; default = UGSettings.get_string (UGSettings.KEY_BACKGROUND_COLOR);}
8+ public string default_background_image {get; set; default = "";}
9 public string? current_background {get; set; default = null;}
10 public bool draw_grid {get; set; default = true;}
11 public double alpha {get; private set; default = 1.0;}
12
13=== modified file 'src/main-window.vala'
14--- src/main-window.vala 2012-04-18 17:21:34 +0000
15+++ src/main-window.vala 2012-04-24 17:39:18 +0000
16@@ -37,7 +37,9 @@
17 add_accel_group (accel_group);
18
19 var bg_color = Gdk.RGBA ();
20- bg_color.parse (UGSettings.get_string (UGSettings.KEY_BACKGROUND_COLOR));
21+ if (!bg_color.parse (UGSettings.get_string (UGSettings.KEY_BACKGROUND_COLOR)))
22+ /* backup color hardcoded for the worst case(UGSettings key is no parseable color) */
23+ bg_color.parse ("#2C001E");
24 override_background_color (Gtk.StateFlags.NORMAL, bg_color);
25 get_accessible ().set_name (_("Login Screen"));
26 has_resize_grip = false;
27@@ -46,7 +48,8 @@
28 realize ();
29 background = new Background (Gdk.cairo_create (get_window ()).get_target ());
30 background.draw_grid = UGSettings.get_boolean (UGSettings.KEY_DRAW_GRID);
31- background.default_background = UGSettings.get_string (UGSettings.KEY_BACKGROUND);
32+ /* Only default_background_image ist set here, not default_background(otherwise the default_background callback would trigger) */
33+ background.default_background_image = UGSettings.get_string (UGSettings.KEY_BACKGROUND);
34 background.set_logo (UGSettings.get_string (UGSettings.KEY_LOGO), UGSettings.get_string (UGSettings.KEY_BACKGROUND_LOGO));
35 background.show ();
36 add (background);
37
38=== modified file 'src/user-list.vala'
39--- src/user-list.vala 2012-04-18 17:21:34 +0000
40+++ src/user-list.vala 2012-04-24 17:39:18 +0000
41@@ -77,7 +77,7 @@
42 {
43 _offer_guest = value;
44 if (value)
45- add_entry ("*guest", _("Guest Session"));
46+ add_entry ("*guest", _("Guest Session"), background.default_background_image);
47 else
48 remove_entry ("*guest");
49 }

Subscribers

People subscribed via source and target branches

to all changes: