Merge lp:~mzanetti/unity8/fix-lockscreen-wallpaper into lp:unity8

Proposed by Michael Zanetti
Status: Merged
Approved by: Michael Terry
Approved revision: 1428
Merged at revision: 1438
Proposed branch: lp:~mzanetti/unity8/fix-lockscreen-wallpaper
Merge into: lp:unity8
Diff against target: 177 lines (+47/-30)
5 files modified
qml/Components/Lockscreen.qml (+15/-0)
qml/Greeter/Greeter.qml (+1/-1)
qml/Greeter/GreeterContent.qml (+1/-18)
qml/Notifications/NotificationMenuItemFactory.qml (+1/-0)
qml/Shell.qml (+29/-11)
To merge this branch: bzr merge lp:~mzanetti/unity8/fix-lockscreen-wallpaper
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Needs Fixing
Michael Terry Approve
Olga Kemmet (community) design Approve
Review via email: mp+241067@code.launchpad.net

Commit message

unify Greeter and Lockscreen wallpaper again

This moves the actual reading of accountsservice into Shell.qml so we only have to read it once. Also this drops the GSettings for wallpaper as it hasn't been used any more (we can't change that in systemsettings any more anyways)

Description of the change

* Are there any related MPs required for this MP to build/function as expected? Please list.

no

 * Did you perform an exploratory manual test run of your code change and any related functionality?

yes

 * Did you make sure that your branch does not contain spurious tags?

yes

 * If you changed the packaging (debian), did you subscribe the ubuntu-unity team to this MP?

n/a

 * If you changed the UI, has there been a design review?

Yes. did a hangout with kemmko.

To post a comment you must log in.
Michael Terry (mterry) wrote :

- I might like a comment above the 0.4 alpha rectangle in Lockscreen to the effect of "This is only here to match the alpha of the main wallpaper for the greeter" -- makes a person wonder why its there otherwise

- We don't want to drop support for gsettings. That fallback is used so that image customizers can set a different default wallpaper. (They can easily change gsettings defaults, but not AS ones)

- I don't see design sign-off for this. I asked Olga about this in person in London and she preferred them to be separate backgrounds.

review: Needs Fixing
Michael Terry (mterry) wrote :

Adding nice fallback code for gsettings might be easier if we first merge the cleanup done in lp:~mterry/unity8/cache-greeter-bg

Olga Kemmet (olga-kemmet) wrote :

As seen and agreed, thanks.

review: Approve (design)
1423. By Michael Zanetti on 2014-11-11

fix review comments

Olga Kemmet (olga-kemmet) wrote :

Just as a side note: we should allow users to set at least the backgrounds for now. Further down the line, we should consider allowing to set separate backgrounds for lock and greeter screens.

Michael Zanetti (mzanetti) wrote :

> - I might like a comment above the 0.4 alpha rectangle in Lockscreen to the
> effect of "This is only here to match the alpha of the main wallpaper for the
> greeter" -- makes a person wonder why its there otherwise

Done

>
> - We don't want to drop support for gsettings. That fallback is used so that
> image customizers can set a different default wallpaper. (They can easily
> change gsettings defaults, but not AS ones)

Reverted back

> - I don't see design sign-off for this. I asked Olga about this in person in
> London and she preferred them to be separate backgrounds.

There has been, I asked Olga to leave a comment somewhere to make it more obvious

1424. By Michael Zanetti on 2014-11-11

make the black overlay cofigurable to not break the wizard

1425. By Michael Zanetti on 2014-11-11

merge trunk

1426. By Michael Zanetti on 2014-11-11

only set the gsettings image if there is no AS setting

1427. By Michael Zanetti on 2014-11-11

fix if

Michael Terry (mterry) wrote :

I tried deb from r1425 and copying over qml changes in 1426/1427. Still just shows me default bg. But reverting to vivid debs works.

review: Needs Fixing
1428. By Michael Zanetti on 2014-11-12

fix it for real

Michael Zanetti (mzanetti) wrote :

> I tried deb from r1425 and copying over qml changes in 1426/1427. Still just
> shows me default bg. But reverting to vivid debs works.

This was a bit racy. If GSettings loaded before AS, it broke the binding. I've removed imperative code now and keep it all to nice on-the-fly updating bindings. At the cost of adding a second dummy image though, but given sourceSize is 0x0 it shouldn't hurt us in terms of memory consumption.

Michael Terry (mterry) wrote :

Like it! Yay for declarative backgrounds! Works for me now too.

 * Did you perform an exploratory manual test run of the code change and any related functionality?
 Yes

 * Did CI run pass? If not, please explain why.
 Yes it did! omg

 * Did you make sure that the branch does not contain spurious tags?
 Yes (can we stop doing this now? Seems like we got rid of all of them)

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'qml/Components/Lockscreen.qml'
--- qml/Components/Lockscreen.qml 2014-10-30 21:42:44 +0000
+++ qml/Components/Lockscreen.qml 2014-11-12 11:52:42 +0000
@@ -56,6 +56,9 @@
56 property int maxPinLength: -156 property int maxPinLength: -1
5757
58 property url background: ""58 property url background: ""
59 // Use this to put a black overlay above the background
60 // 0: normal background, 1: black background
61 property real darkenBackground: 0
5962
60 readonly property string passphrase: (pinPadLoader.item && pinPadLoader.item.passphrase) ? pinPadLoader.item.passphrase : ""63 readonly property string passphrase: (pinPadLoader.item && pinPadLoader.item.passphrase) ? pinPadLoader.item.passphrase : ""
6164
@@ -100,10 +103,22 @@
100 anchors {103 anchors {
101 fill: parent104 fill: parent
102 }105 }
106 // Limit how much memory we'll reserve for this image
107 sourceSize.height: height
108 sourceSize.width: width
103 source: root.required ? root.background : ""109 source: root.required ? root.background : ""
104 fillMode: Image.PreserveAspectCrop110 fillMode: Image.PreserveAspectCrop
105 }111 }
106112
113 // This is to
114 // a) align it with the greeter and
115 // b) keep the white fonts readable on bright backgrounds
116 Rectangle {
117 anchors.fill: parent
118 color: "black"
119 opacity: root.darkenBackground
120 }
121
107 MouseArea {122 MouseArea {
108 anchors.fill: root123 anchors.fill: root
109 onClicked: {124 onClicked: {
110125
=== modified file 'qml/Greeter/Greeter.qml'
--- qml/Greeter/Greeter.qml 2014-11-04 11:19:52 +0000
+++ qml/Greeter/Greeter.qml 2014-11-12 11:52:42 +0000
@@ -25,7 +25,7 @@
25 enabled: shown25 enabled: shown
26 created: greeterContentLoader.status == Loader.Ready && greeterContentLoader.item.ready26 created: greeterContentLoader.status == Loader.Ready && greeterContentLoader.item.ready
2727
28 property url defaultBackground28 property url background
29 property bool loadContent: required29 property bool loadContent: required
3030
31 // 1 when fully shown and 0 when fully hidden31 // 1 when fully shown and 0 when fully hidden
3232
=== modified file 'qml/Greeter/GreeterContent.qml'
--- qml/Greeter/GreeterContent.qml 2014-10-01 13:22:00 +0000
+++ qml/Greeter/GreeterContent.qml 2014-11-12 11:52:42 +0000
@@ -48,9 +48,6 @@
48 color: "black"48 color: "black"
49 }49 }
5050
51 property url backgroundValue: AccountsService.backgroundFile != undefined && AccountsService.backgroundFile.length > 0 ? AccountsService.backgroundFile : greeter.defaultBackground
52 onBackgroundValueChanged: background.source = backgroundValue
53
54 CrossFadeImage {51 CrossFadeImage {
55 id: background52 id: background
56 objectName: "greeterBackground"53 objectName: "greeterBackground"
@@ -62,21 +59,7 @@
62 // Limit how much memory we'll reserve for this image59 // Limit how much memory we'll reserve for this image
63 sourceSize.height: height60 sourceSize.height: height
64 sourceSize.width: width61 sourceSize.width: width
65 }62 source: greeter.background
66
67 // See Shell.qml's backgroundSettings treatment for why we need a separate
68 // Image, but it boils down to avoiding binding loop detection.
69 Image {
70 source: background.source
71 height: 0
72 width: 0
73 sourceSize.height: 0
74 sourceSize.width: 0
75 onStatusChanged: {
76 if (status == Image.Error && source != greeter.defaultBackground) {
77 background.source = greeter.defaultBackground
78 }
79 }
80 }63 }
8164
82 Rectangle {65 Rectangle {
8366
=== modified file 'qml/Notifications/NotificationMenuItemFactory.qml'
--- qml/Notifications/NotificationMenuItemFactory.qml 2014-11-05 15:23:43 +0000
+++ qml/Notifications/NotificationMenuItemFactory.qml 2014-11-12 11:52:42 +0000
@@ -145,6 +145,7 @@
145 errorText: errorAction.valid ? errorAction.state : ""145 errorText: errorAction.valid ? errorAction.state : ""
146 retryText: notification.body146 retryText: notification.body
147 background: shell.background147 background: shell.background
148 darkenBackground: 0.4
148149
149 onEntered: {150 onEntered: {
150 menuModel.changeState(menuIndex, passphrase);151 menuModel.changeState(menuIndex, passphrase);
151152
=== modified file 'qml/Shell.qml'
--- qml/Shell.qml 2014-11-07 14:30:42 +0000
+++ qml/Shell.qml 2014-11-12 11:52:42 +0000
@@ -51,7 +51,8 @@
5151
52 property real edgeSize: units.gu(2)52 property real edgeSize: units.gu(2)
53 property url defaultBackground: Qt.resolvedUrl(shell.width >= units.gu(60) ? "graphics/tablet_background.jpg" : "graphics/phone_background.jpg")53 property url defaultBackground: Qt.resolvedUrl(shell.width >= units.gu(60) ? "graphics/tablet_background.jpg" : "graphics/phone_background.jpg")
54 property url background54 property url background: asImageTester.status == Image.Ready ? asImageTester.source
55 : gsImageTester.status == Image.Ready ? gsImageTester.source : defaultBackground
55 readonly property real panelHeight: panel.panelHeight56 readonly property real panelHeight: panel.panelHeight
5657
57 readonly property bool locked: LightDM.Greeter.active && !LightDM.Greeter.authenticated && !forcedUnlock58 readonly property bool locked: LightDM.Greeter.active && !LightDM.Greeter.authenticated && !forcedUnlock
@@ -98,6 +99,31 @@
98 shell.activateApplication(app);99 shell.activateApplication(app);
99 }100 }
100101
102 // This is a dummy image to detect if the custom AS set wallpaper loads successfully.
103 Image {
104 id: asImageTester
105 source: AccountsService.backgroundFile != undefined && AccountsService.backgroundFile.length > 0 ? AccountsService.backgroundFile : ""
106 height: 0
107 width: 0
108 sourceSize.height: 0
109 sourceSize.width: 0
110 }
111
112 GSettings {
113 id: backgroundSettings
114 schema.id: "org.gnome.desktop.background"
115 }
116
117 // This is a dummy image to detect if the custom GSettings set wallpaper loads successfully.
118 Image {
119 id: gsImageTester
120 source: backgroundSettings.pictureUri != undefined && backgroundSettings.pictureUri.length > 0 ? backgroundSettings.pictureUri : ""
121 height: 0
122 width: 0
123 sourceSize.height: 0
124 sourceSize.width: 0
125 }
126
101 Binding {127 Binding {
102 target: LauncherModel128 target: LauncherModel
103 property: "applicationManager"129 property: "applicationManager"
@@ -114,15 +140,6 @@
114 }140 }
115 }141 }
116142
117 GSettings {
118 id: backgroundSettings
119 schema.id: "org.gnome.desktop.background"
120 }
121 property url gSettingsPicture: backgroundSettings.pictureUri != undefined && backgroundSettings.pictureUri.length > 0 ? backgroundSettings.pictureUri : shell.defaultBackground
122 onGSettingsPictureChanged: {
123 shell.background = gSettingsPicture
124 }
125
126 VolumeControl {143 VolumeControl {
127 id: volumeControl144 id: volumeControl
128 }145 }
@@ -328,6 +345,7 @@
328 width: parent.width345 width: parent.width
329 height: parent.height - panel.panelHeight346 height: parent.height - panel.panelHeight
330 background: shell.background347 background: shell.background
348 darkenBackground: 0.4
331 alphaNumeric: AccountsService.passwordDisplayHint === AccountsService.Keyboard349 alphaNumeric: AccountsService.passwordDisplayHint === AccountsService.Keyboard
332 minPinLength: 4350 minPinLength: 4
333 maxPinLength: 4351 maxPinLength: 4
@@ -523,7 +541,7 @@
523541
524 locked: shell.locked542 locked: shell.locked
525543
526 defaultBackground: shell.background544 background: shell.background
527545
528 width: parent.width546 width: parent.width
529 height: parent.height547 height: parent.height

Subscribers

People subscribed via source and target branches