Use the default Plasma wallpaper on the lock screen
ClosedPublic

Authored by ngraham on Mar 14 2018, 4:27 AM.

Details

Summary

As discussed and agreed to by VDG (T7914), change the lock screen background from the blue color to the current Plasma wallpaper.

CCBUG: 381407

Closes T7914

Test Plan
  • rm ~/.config/kscreenlockerrc
  • Build and install kscreenlocker with this patch
  • Lock the screen

Diff Detail

Repository
R133 KScreenLocker
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
ngraham created this revision.Mar 14 2018, 4:27 AM
Restricted Application added a project: Plasma. · View Herald TranscriptMar 14 2018, 4:27 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
ngraham requested review of this revision.Mar 14 2018, 4:27 AM
ngraham edited the summary of this revision. (Show Details)Mar 14 2018, 4:30 AM
ngraham edited the test plan for this revision. (Show Details)
ngraham edited the summary of this revision. (Show Details)
graesslin requested changes to this revision.Mar 14 2018, 5:23 AM
graesslin added a subscriber: graesslin.

I think we first need to bring back some background for the UI elements. Otherwise it's too difficult to read.

Personally I'm very confused as it was the vdg's wish to have it blue. I hope our users don't take this negative. I would take it negative to see such a forward and backward.

This revision now requires changes to proceed.Mar 14 2018, 5:23 AM
mart added a subscriber: mart.Mar 14 2018, 8:31 AM

-1 from me too

mart added a comment.Mar 14 2018, 9:07 AM

Sadly, the current design doesn't really allow for arbitrary wallpapers (and since is possible already to put any image, that's quite a problem)
if a wallpaper should become the default again, the text elements should either have a visible background for contrast (so would mean a pretty heavy redesign) or, showing only the wallpaper by default and only after mouse move/any interaction the actual controls appear and the wallpaper gets either completely covered or with an heavy blur+contrast (blur alone isn't enough to ensure readability of text, the shader should modify the colors as well, like the plasma panel does)

there are several options for that, but all will mean some rewrite, in order for it to look like a godd quality product. So now the challenge is i guess arriving firt with a good design which is not too big of a departure, doesn't look too busy, and works well with any wallpaper.. sounds like a fun challenge :)

btw, for reference, the current is https://phabricator.kde.org/T2062

hein added a subscriber: hein.Mar 14 2018, 10:52 AM

FWIW: AIUI this patch isn't about making the lock screen use the current user wallpaper dynamically, which means the vendor gets control over making sure the default wallpaper they pick works with the UI elements at least.

ngraham added a comment.EditedMar 14 2018, 1:24 PM

I think we first need to bring back some background for the UI elements. Otherwise it's too difficult to read.

In D11308#225401, @mart wrote:

if a wallpaper should become the default again, the text elements should either have a visible background for contrast (so would mean a pretty heavy redesign) or, showing only the wallpaper by default and only after mouse move/any interaction the actual controls appear and the wallpaper gets either completely covered or with an heavy blur+contrast (blur alone isn't enough to ensure readability of text, the shader should modify the colors as well, like the plasma panel does)

I agree with you guys and would like to see those changes too, as they would resolve some ussbility issues that are tracked by https://bugs.kde.org/show_bug.cgi?id=388622 and https://bugs.kde.org/show_bug.cgi?id=369676

However, this patch doesn't make the image dynamic: it's just the standard default Plasma wallpaper. So the vendor (i.e. us, distros) can make sure that the image works against white text. The current one does, I think. So we don't need to tackle the readability of white text on arbitrary backgrounds here (though I agree with you that we need to tackle it elsewhere).

Personally I'm very confused as it was the vdg's wish to have it blue. I hope our users don't take this negative. I would take it negative to see such a forward and backward.

We have received a lot of negative feedback over the past year and a half regarding the blue color and have decided that the initiative was a failed experiment. On the contrary, I expect our users to appreciate the change because the current bright blur color is not popular. Visual papercuts and listening to user feedback are important. The blue background was not particularly well-received.

hein added a comment.Mar 14 2018, 4:15 PM

The blue background was not particularly well-received.

Are you sure you are not reacting to a loud minority? The bright background has some practical benefits, e.g. it allows people to type in their passwords in dark rooms without a backlit keyboard. There may be a silent majority who is served rather well by it. I'm concerned with just implementing the opinion of whoever screams the loudest.

progwolff added a subscriber: progwolff.EditedMar 14 2018, 4:30 PM

@hein might be right here.
If it's the single-click/double-click thing or this issue here: People who are happy with the status quo won't reach out to us.
Maybe we need to discuss at a different place if and how we can learn about the opinion of the majority of KDE/Plasma users, before we change things that are possibly only broken for few.

Pitel added a subscriber: Pitel.Mar 14 2018, 4:38 PM

+1 for picture instead of blue. With the current UI I would really appreciate option to choose text color independently on plasma color scheme (and such option would solve any static picture). Other possibility I was thinking about was 50 % opaque (default black) rectangle (with rounded corners) around control elements (i.e. like logout screen but only around controls not whole screen) but I have not found the way to position it where I want it to be...

ngraham added a comment.EditedMar 14 2018, 4:38 PM
In D11308#225711, @hein wrote:

The blue background was not particularly well-received.

Are you sure you are not reacting to a loud minority? The bright background has some practical benefits, e.g. it allows people to type in their passwords in dark rooms without a backlit keyboard.

Using the Plasma wallpaper as a background serves that purpose just fine, even on my low-contrast laptop keyboard:

It's hard to express something like this in a photo due to the extreme contrast involved, but I can confirm that the keyboard is perfectly readable in an otherwise totally dark room.

In D11308#225711, @hein wrote:

There may be a silent majority who is served rather well by it. I'm concerned with just implementing the opinion of whoever screams the loudest.

I'm happy to have the discussion again here if you'd like, but in a nutshell, entirely aside from public opinion, here are the substantive objections to the bright blue background:

  • Emotional: reminds former Windows users of the blue screen of death
  • Reflects a design error: an intentionally bright highlight color is used as a background color and becomes overwhelming and blinding
  • Doesn't reflect the depth of VDG's visual design skills; makes us look amateurish
  • Doesn't make use of any of our branding
  • Provides no continuity with the default desktop experience; nothing else uses the blue highlight color so starkly and totally

Honestly I think I would even be fine with a nice neutral gray color. Solid colors can work when used with subtlety (even if they don't show off our design skills). But IMHO the eyeball-searing blue has got to go.

zzag added a subscriber: zzag.EditedMar 14 2018, 4:50 PM
In D11308#225401, @mart wrote:

Sadly, the current design doesn't really allow for arbitrary wallpapers (and since is possible already to put any image, that's quite a problem)
if a wallpaper should become the default again, the text elements should either have a visible background for contrast (so would mean a pretty heavy redesign) or, showing only the wallpaper by default and only after mouse move/any interaction the actual controls appear and the wallpaper gets either completely covered or with an heavy blur+contrast (blur alone isn't enough to ensure readability of text, the shader should modify the colors as well, like the plasma panel does)

I agree with you, raw images will make everything worse so background should be somehow filtered. IMO, blur+contrast is a good idea.

Do we really need to use shaders? QML some useful effects like FastBlur and BrightnessContrast. Or I missed something?

zzag added a comment.Mar 14 2018, 4:54 PM
In D11308#225711, @hein wrote:

The blue background was not particularly well-received.

Are you sure you are not reacting to a loud minority? The bright background has some practical benefits, e.g. it allows people to type in their passwords in dark rooms without a backlit keyboard. There may be a silent majority who is served rather well by it. I'm concerned with just implementing the opinion of whoever screams the loudest.

The loud minority is still part of the community and KDE is all about community, isn't it? ;-)

This comment was removed by richardbowen.
hein added a comment.Mar 14 2018, 5:39 PM

The argument that it's a semantic abuse of the highlight color is quite convincing to me.

abetts accepted this revision.Mar 14 2018, 9:22 PM
mart added a comment.Mar 15 2018, 7:27 PM

That's a way i would like it:
https://www.youtube.com/watch?v=BOsclMNpK3M
(quick and dirty so the interaction and effects are very buggy)
have the lockscreen by default to be a slideshow of the default installed wallpapers and show only the clock (in this mode the text has a shadow as it might be invisible)
then at the minimum interaction (mouse, keyboard, anything) the rest of the ui appears and it becomes functional (after a timeout would go back to slideshow mode)
when in ui mode, either fades to a plain color (to look like the current blue default which i like a lot) or some effect like heavy blur (but still with coloring, again for readability reasons)

@mart You're essentially describing transforming the lock screen into a screensaver, with the default effect being an animated slideshow. I would favor this. Users like screensavers simply for the visual flair, and I've heard a lot of user feedback that they'd like something approximating a screensaver. However, that's a substantially different patch from this one, and we might run into memory pressure issues if we're not able to resolve https://bugs.kde.org/show_bug.cgi?id=368838 beforehand.

My worry is that we let the perfect be the enemy of the good and don't get something in time for 5.13, and ship yet another Plasma version with the eyeball-searing blue background. I support your initiative, but could we land this or a tweaked variant before then, so at we at least have a better fallback in case we aren't able to ship your idea in time?

@mart You're essentially describing transforming the lock screen into a screensaver, with the default effect being an animated slideshow. I would favor this. Users like screensavers simply for the visual flair, and I've heard a lot of user feedback that they'd like something approximating a screensaver. However, that's a substantially different patch from this one, and we might run into memory pressure issues if we're not able to resolve https://bugs.kde.org/show_bug.cgi?id=368838 beforehand.

ok for pushing this, but with the idea to change to a slideshow in the future (to me with the default wallpaper looks less quality refined than now, but is just a personal opinion)

(btw that leak i think should be solved, @davidedmundson also has a rewrite of the wallpaper code which should make it take less memory, as right now the first few times it changes memory gooes up not for an actual leak but more for the management not being correct)

Thanks @mart. I think using an animated slideshow by default will be really popular, especially if we do some work to add more nice images by default and polish up the slideshow plugin's UI a bit. I'm glad to hear that the memory issue is or should be resolved soon.

@graesslin, have we satisfactorily resolved your objections yet? It not, what more needs to be done here?

Thanks @mart. I think using an animated slideshow by default will be really popular, especially if we do some work to add more nice images by default and polish up the slideshow plugin's UI a bit. I'm glad to hear that the memory issue is or should be resolved soon.

@graesslin, have we satisfactorily resolved your objections yet? It not, what more needs to be done here?

Please be aware of two things:

  • Most distros only install one wallpaper by default. If we go this route we must reach out to distros before and check whether they are able to install more default wallpapers
  • If you talk about this please never, never,ever call it a screen saver. We don't have to save screens, we don't have crts anymore. We removed the screen saver support because of that and because of the energy saving. Call it animations or transitions, but not screen saver.

Personally I dislike animations on the lock screen by default due to the environment impact. We burn CPU and GPU cycles to show animations nobody will see (if the screen is locked the reason is mostly that the user went away). In times of global warming we should consider the impact of enabling animations for maybe millions of systems.

My suggestion is to:

  • Decrease the default power saving time
  • Ensure those animations stop when screen is power saved
  • Increase the time between wallpaper transitions in e.g. Fibonacci time.

@graesslin, those are good comments for @mart's ideas for the other approach. Do you have any remaining concerns with this one that I can work on?

@graesslin, those are good comments for @mart's ideas for the other approach. Do you have any remaining concerns with this one that I can work on?

As long as the lnf theme is not changed to provide sensible background contrast I'm against this change. We don't have control over the default background, this is done by distros. We cannot change in ways that it would break for them.

We can put the black bars back.
Problem is because the UI is now stretched out a lot taller in order to cover the clock and switch user button I'd have to take up the entire screen.

I strongly agree that this UI needs more background contrast, and I volunteer to spearhead the initiative to improve that (https://bugs.kde.org/show_bug.cgi?id=369676)

However, this patch as is doesn't require that. We as the upstream vendor have control over the default--and the default looks good and has sufficient contrast:

Screenshot:

Real-world, on my wife's laptop:

As downstream vendors, distros will also have a responsibility to ensure that the contrast is sufficient until https://bugs.kde.org/show_bug.cgi?id=369676 is resolved (which again I will commit to getting done). Since this change would go into 5.13, for quite some time the only distros that could possibly be affected will be rolling release distros. @graesslin, if you can find it in your heart to approve this, I will commit to reaching out to them and helping them make any necessary adjustments.

We can put the black bars back. Problem is because the UI is now stretched out a lot taller in order to cover the clock and switch user button I'd have to take up the entire screen.

We can make them vertical!

if you can find it in your heart to approve this

Lets keep review comments purely technical please.

Sorry, I was going for whimsical, but I guess it didn't come out right. Will stick to technical.

mart added a comment.EditedMar 16 2018, 3:23 PM

We can put the black bars back. Problem is because the UI is now stretched out a lot taller in order to cover the clock and switch user button I'd have to take up the entire screen.

We can make them vertical!

there was indeed on the vdg channel at some point a mockup with such appearance indeed :)
(which i even implemented, then.. lost it :p)

@ngraham Let's try to not work against our distros. Yes we could change that because we are upstream, we set the default. The result would be distros swapping the wallpaper and we have unreadable text. This is something which will happen, I have been too long in this business to not expect otherwise. We want to provide a good experience for everyone. And that means this change just cannot go in, sorry about that.

I understand that you want to improve the situation, but we need to think in the big picture. And that is all distros change wallpapers. Debian does, openSUSE does, Fedora does. The only distro I know which does not exchange the default wallpaper is Kubuntu. Sorry, we just cannot do a change which renders the text possibly unreadable on most systems.

@ngraham Let's try to not work against our distros. Yes we could change that because we are upstream, we set the default. The result would be distros swapping the wallpaper and we have unreadable text. This is something which will happen, I have been too long in this business to not expect otherwise. We want to provide a good experience for everyone. And that means this change just cannot go in, sorry about that.

Sorry, we just cannot do a change which renders the text possibly unreadable on most systems.

Do we know that the distros you mentioned currently do use wallpapers that will make white text unreadable, or is this a worry about the future?

And would you accept this chance if we fixed the login screen to eliminate the issue of bad contrast on light backgrounds? (https://bugs.kde.org/show_bug.cgi?id=369676)

Also, Martin, this isn't about re-using the desktop wallpaper on the lock screen. It's about using the current Plasma wallpaper on the lock screen. Distros can and do override the desktop wallpaper but to my knowledge most don't currently override the lockscreen background, right?

@ngraham Let's try to not work against our distros. Yes we could change that because we are upstream, we set the default. The result would be distros swapping the wallpaper and we have unreadable text. This is something which will happen, I have been too long in this business to not expect otherwise. We want to provide a good experience for everyone. And that means this change just cannot go in, sorry about that.

I understand that you want to improve the situation, but we need to think in the big picture. And that is all distros change wallpapers. Debian does, openSUSE does, Fedora does. The only distro I know which does not exchange the default wallpaper is Kubuntu. Sorry, we just cannot do a change which renders the text possibly unreadable on most systems.

When I worked for OpenSUSE and we used different wallpapers, we always went with darker ones to accommodate for the wallpaper. It wasn't always something we "had" to do, but it was a common request to use a wallpaper all of our own. However, we were careful to use something that wouldn't bury the content from on the screen. Although distributions use their own wallpapers probably doesn't make much of a difference for us whether we have a wallpaper there or a color instead. Therefore, it can be done. I see a lot of good branding coming from accepting this change.

Fuchs added a subscriber: Fuchs.Mar 16 2018, 4:02 PM

Personal opinions, based on discussions here and on TG

  • fixed, (default) wallpaper has the advantage of not risking privacy, not munching power and being usable in, at least from login to desktop to lock screen, have a consistent feel for new users. Downside is that it needs to be ensured, on every single new wallpaper release, that it looks good and is perfectly readable
  • slideshow: looks nice, but eats battery and is rarely ever seen, since the default use case of a lock screen is "I am away from my device". Also in this case it has to look good and be readable on every potential wallpaper that might show up.
  • fixed colour: consistent from boot to desktop, simple, efficient. But apparently some people think it's not sexy.

My personal preference is thus, in that order, default wallpaper, fixed colour, slide show. Assuming we test and ensure that the default wallpaper looks good and all text is easy to read.

Also, Martin, this isn't about re-using the desktop wallpaper on the lock screen. It's about using the current Plasma wallpaper on the lock screen. Distros can and do override the desktop wallpaper but to my knowledge most don't currently override the lockscreen background, right?

At least on Debian our default wallpaper is not installed and replaced by Debian's default wallpaper. This change is so deep that it even overrides my local plasma installation.

Also, Martin, this isn't about re-using the desktop wallpaper on the lock screen. It's about using the current Plasma wallpaper on the lock screen. Distros can and do override the desktop wallpaper but to my knowledge most don't currently override the lockscreen background, right?

At least on Debian our default wallpaper is not installed and replaced by Debian's default wallpaper. This change is so deep that it even overrides my local plasma installation.

  1. That seems like a downstream bug; they should be adding wallpapers and overriding defaults using config files, not replacing our defaults (with potentially invasive code changes?). Can you engage with the Debian folks to get that fixed?
  2. Does the wallpaper they chose make the white lock screen text unreadable?

@ngraham Let's try to not work against our distros. Yes we could change that because we are upstream, we set the default. The result would be distros swapping the wallpaper and we have unreadable text. This is something which will happen, I have been too long in this business to not expect otherwise. We want to provide a good experience for everyone. And that means this change just cannot go in, sorry about that.

Sorry, we just cannot do a change which renders the text possibly unreadable on most systems.

Do we know that the distros you mentioned currently do use wallpapers that will make white text unreadable, or is this a worry about the future?

I do not know how the situation is as I don't have tested all distros. But I also think that this is irrelevant. We have a technical problem here and that needs to be fixed.

And would you accept this chance if we fixed the login screen to eliminate the issue of bad contrast on light backgrounds? (https://bugs.kde.org/show_bug.cgi?id=369676)

I asked for addressing this issue, if it's addressed then it's fine for me. Personally I would recommend you against the change due to the history, but design decisions are up to the vdg and won't be blocked by me.

@ngraham Let's try to not work against our distros. Yes we could change that because we are upstream, we set the default. The result would be distros swapping the wallpaper and we have unreadable text. This is something which will happen, I have been too long in this business to not expect otherwise. We want to provide a good experience for everyone. And that means this change just cannot go in, sorry about that.

Sorry, we just cannot do a change which renders the text possibly unreadable on most systems.

Do we know that the distros you mentioned currently do use wallpapers that will make white text unreadable, or is this a worry about the future?

I do not know how the situation is as I don't have tested all distros. But I also think that this is irrelevant. We have a technical problem here and that needs to be fixed.

I very much agree that there is a technical problem in need of fixing. As you can see in https://bugs.kde.org/show_bug.cgi?id=369676, we're starting to scope out that work. The question is, does that work need to be done before this change can land? We can ensure that our upstream image looks good, and we can alert distros to the change and help them ensure that theirs do too. Plasma 5.13 isn't being released for several more months, which gives us time to work with rolling release distros to ensure that they're not shipping a lock screen background that's unreadable. We have even more time for the fixed release distros like Debian, Ubuntu, Fedora, and openSUSE Leap. In particular it will be years before Debian stable gets Plasma 5.13.

I volunteer to lead the distro outreach effort to ensure that distros don't ship with lockscreen backgrounds that make the text unreadable.

And medium-term, we will fix the lock screen's UI in https://bugs.kde.org/show_bug.cgi?id=369676 to eliminate the issue entirely.

Does this sound like a sensible path forward, Martin?

Also, Martin, this isn't about re-using the desktop wallpaper on the lock screen. It's about using the current Plasma wallpaper on the lock screen. Distros can and do override the desktop wallpaper but to my knowledge most don't currently override the lockscreen background, right?

At least on Debian our default wallpaper is not installed and replaced by Debian's default wallpaper. This change is so deep that it even overrides my local plasma installation.

  1. That seems like a downstream bug; they should be adding wallpapers and overriding defaults using config files, not replacing our defaults (with potentially invasive code changes?). Can you engage with the Debian folks to get that fixed?

I'm certainly not going to play don Quixote against Debian packaging policies. I currently don't even have time to review KWin changes...

  1. Does the wallpaper they chose make the white lock screen text unreadable?

Debian tends to change the wallpaper late in the cycle. I cannot tell you how the wallpaper will look like together with this change. I can tell you that Debian will ship it broken and their policies will make it impossible to change once the freeze is in place. And I can tell you that Debian will certainly not adjust the lnf package. Nobody looks at the big picture - there's a team doing keep packaging and a team doing wallpapers. Expecting them to work together seems unreasonable. Apparently the ice team does not even have influence on what's getting installed in a default ice install, because that's another team.

Welcome to the reality of distros. And in many other distros it doesn't look different. Most distros are understaffed and only do packaging. Nobody will notice if this breaks till it's too late. And then we will get the blame for doing such a change. And we cannot say we didn't know.

@ngraham Let's try to not work against our distros. Yes we could change that because we are upstream, we set the default. The result would be distros swapping the wallpaper and we have unreadable text. This is something which will happen, I have been too long in this business to not expect otherwise. We want to provide a good experience for everyone. And that means this change just cannot go in, sorry about that.

Sorry, we just cannot do a change which renders the text possibly unreadable on most systems.

Do we know that the distros you mentioned currently do use wallpapers that will make white text unreadable, or is this a worry about the future?

I do not know how the situation is as I don't have tested all distros. But I also think that this is irrelevant. We have a technical problem here and that needs to be fixed.

I very much agree that there is a technical problem in need of fixing. As you can see in https://bugs.kde.org/show_bug.cgi?id=369676, we're starting to scope out that work. The question is, does that work need to be done before this change can land? We can ensure that our upstream image looks good, and we can alert distros to the change and help them ensure that theirs do too. Plasma 5.13 isn't being released for several more months, which gives us time to work with rolling release distros to ensure that they're not shipping a lock screen background that's unreadable. We have even more time for the fixed release distros like Debian, Ubuntu, Fedora, and openSUSE Leap. In particular it will be years before Debian stable gets Plasma 5.13.

I volunteer to lead the distro outreach effort to ensure that distros don't ship with lockscreen backgrounds that make the text unreadable.

And medium-term, we will fix the lock screen's UI in https://bugs.kde.org/show_bug.cgi?id=369676 to eliminate the issue entirely.

Does this sound like a sensible path forward, Martin?

Sorry but no. I want to see it addressed before flipping it. We have shipped half finished things too often - especially on the look screen. Sorry but sometimes I wonder what you want to achieve: improve the situation for all of change for the sake of change. If you ignore that most users won't get the wallpaper you think of then it's unfortunately the latter.

ngraham added a comment.EditedMar 16 2018, 5:31 PM

Sorry but no. I want to see it addressed before flipping it. We have shipped half finished things too often - especially on the look screen. Sorry but sometimes I wonder what you want to achieve: improve the situation for all of change for the sake of change. If you ignore that most users won't get the wallpaper you think of then it's unfortunately the latter.

I think the people who have worked with me a lot will tell you that I am very much against "change for the sake of change". I outlined my reasons in the top, and again in T7914 and https://bugs.kde.org/show_bug.cgi?id=381407

I understand your position and I very much agree on fixing the usability problem. If that unlocks our path forward, then I will focus on that. Let's continue in https://bugs.kde.org/show_bug.cgi?id=369676

(btw that leak i think should be solved, @davidedmundson also has a rewrite of the wallpaper code which should make it take less memory, as right now the first few times it changes memory gooes up not for an actual leak but more for the management not being correct)

At the risk of getting offtopic. The leak is not in Plasma code, no matter how amazingly I rewrote the wallpaper it won't make a difference there. Sorry :/

fvogt added a subscriber: fvogt.Mar 21 2018, 2:00 PM

I'll write my viewpoint as downstream maintainer here: We removed the blue background as default as soon as it landed as it is was complained about in various places.
Also, our sddm theme also uses the wallpaper as background so it looks similiar now.

(if a user installed the -branding-upstream package they'd get the blue background again, but AFAICT not many did that)

My personal opinion is that the blue background is incredibly hard to look at, too bright and not enough contrast.

+1 from me.

davidedmundson accepted this revision.Apr 22 2018, 9:26 AM
mart accepted this revision.Apr 22 2018, 2:24 PM
This revision was not accepted when it landed; it landed in state Needs Revision.Apr 22 2018, 2:28 PM
This revision was automatically updated to reflect the committed changes.

Landing per Marco's instructions, since the whole lockscreen thingy is going in.