[sddm-theme/lock screen] Tighten clock and username shadows
ClosedPublic

Authored by rooty on Feb 26 2019, 5:30 AM.

Details

Summary

This patch tightens the shadows around the clock in the Breeze SDDM theme and lock screen so as to prevent the appearance of a diffuse dark glare that appears around the clock on top of very bright backgrounds.

In addition, it tightens the username/prompt shadows to match the shadows used in desktopcontainment.

Test Plan

Username shadows, before:


and after:

(the new shadows take after the ones in desktopcontainment)

Before (white background):


After (white background):

Before (Next):


After (Next):

Diff Detail

Repository
R120 Plasma Workspace
Branch
arcpatch-D19325_1
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 8891
Build 8909: arc lint + arc unit
rooty created this revision.Feb 26 2019, 5:30 AM
Restricted Application added a project: Plasma. · View Herald TranscriptFeb 26 2019, 5:30 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
rooty requested review of this revision.Feb 26 2019, 5:30 AM
rooty retitled this revision from [sddm-theme/lock screen] WIP, Tighten Clock shadows to [sddm-theme/lock screen] WIP, Tighten clock shadows.Feb 26 2019, 5:32 AM
rooty edited the test plan for this revision. (Show Details)
rooty added reviewers: VDG, ngraham.
rooty updated this revision to Diff 52578.Feb 26 2019, 5:33 AM

Remove unnecessary (pasted) lines of code

rooty edited the test plan for this revision. (Show Details)
rooty added a comment.Feb 26 2019, 6:02 AM

@ngraham there's actually another problem here, but one that's more readily seen in UserDelegate.qml

The shadow is rendered after the label - which results in the label being darker than it should be (the shadows should go under the label not on top). This is why usernames look weird in the current SDDM theme:

And with the shadows under the label (rendered after it):

There's not much difference when it comes to the clock though -
before (current master):


after (shadows rendered before label):

rooty updated this revision to Diff 52580.Feb 26 2019, 6:58 AM

Render shadows before labels

rooty added a comment.Feb 26 2019, 6:59 AM

Render shadows before labels

Is "visible:" enough here or do I need layer.enabled too?

mart added a subscriber: mart.Feb 26 2019, 10:24 AM

Render shadows before labels

Is "visible:" enough here or do I need layer.enabled too?

is ok

mart accepted this revision.Feb 26 2019, 10:24 AM
This revision is now accepted and ready to land.Feb 26 2019, 10:24 AM

I've been advocating this before (D16031#372706) so obviously I'm in favor of the change.

One thing that's been bothering me though is the blue offset that shows up to the left of the text:

It's not a big deal and actually helps out a bit with readability, but maybe we could tweak the settings a bit more.

ngraham requested changes to this revision.Feb 26 2019, 3:54 PM

I expected to hate this, but it turns out I don't. :) It looks legible enough with all the backgrounds I typically test against except for the most challenging ones of all for white text:

Other semi-challenging backgrounds work okay:

Also, +1 for making the color "black" again. In retrospect I don't think using a color from the theme makes sense. Shadows should always be black, not dark gray or any other color (which would make them into a glow, not a shadow).

However, the Request Changes is for two reasons:

  1. Shouldn't we do this for the shadows under the usernames too?
  2. I'm not sold on the visual appearance. No other shadow in KDE Software that I'm familiar with looks like this, with a low spread, a hard edge, and the light source shining from the top left. It's not that it looks bad, but it doesn't look like it really fits in with anything else. Thoughts?
This revision now requires changes to proceed.Feb 26 2019, 3:54 PM

However, the Request Changes is for two reasons:

  1. Shouldn't we do this for the shadows under the usernames too?
  2. I'm not sold on the visual appearance. No other shadow in KDE Software that I'm familiar with looks like this, with a low spread, a hard edge, and the light source shining from the top left. It's not that it looks bad, but it doesn't look like it really fits in with anything else. Thoughts?
  1. yes
  2. they're meant to follow the new style implemented here: https://phabricator.kde.org/D16968

Ah, I see now! I even implemented that, touché. :)

Also, why did you change the shadow from a layer effect to a separate item? If that's necessary, it would seem to be appropriate material for another patch since it's not a visual change (unless I'm not noticing it).

lookandfeel/contents/components/Clock.qml
44

If we're emulating the shadows in D16968, shouldn't this be 0.35?

rooty marked an inline comment as done.Feb 26 2019, 5:03 PM

Ah, I see now! I even implemented that, touché. :)

Also, why did you change the shadow from a layer effect to a separate item? If that's necessary, it would seem to be appropriate material for another patch since it's not a visual change (unless I'm not noticing it).

Because it says "Cannot assign to non-existent property "layers""
I don't know what I'm doing wrong

However, the Request Changes is for two reasons:

  1. Shouldn't we do this for the shadows under the usernames too?
  2. I'm not sold on the visual appearance. No other shadow in KDE Software that I'm familiar with looks like this, with a low spread, a hard edge, and the light source shining from the top left. It's not that it looks bad, but it doesn't look like it really fits in with anything else. Thoughts?
  1. Separate patch (I've written it but I haven't committed it yet). The action buttons too.
  2. DesktopContainment does, that's where I got the idea (@flipwise's mockups too)
lookandfeel/contents/components/Clock.qml
44

Yes, but doesn't it look too jagged when it's 0.35?

0.35:


0.25:

I think we should also be a little big careful with trying to apply the same shadow settings everywhere. The DesktopContainment settings work great for 10pt and 11pt text (meaning they will be great for most of the labels in the sddm theme), but the clock is rather big.

Small text -> there is little meat for the drop shadow to use so the higher the radius, the weaker the shadow
Large point size -> means more meat for the drop shadow to use so you don't have to make it as hard or even tight as the ones used for smaller text

The point would be to implement the same offset logic, and to try not to deviate much with radius and spread settings - but some flexibility might produce a better result.

OK, if 0.35 spread is too much, how about increasing the radius a bit instead? The thing that I don't like so much about this new style is how hard-edged the shadows look in comparison to the folder view icons' text shadows

OK, if 0.35 spread is too much, how about increasing the radius a bit instead? The thing that I don't like so much about this new style is how hard-edged the shadows look in comparison to the folder view icons' text shadows

+1, here's what it would look like with:

horizontalOffset: 1
verticalOffset: 1
radius: 6
samples: 14
spread: 0.30
color: "black"

Looks like a shadow again IMO.

+1, here's what it would look like with:

horizontalOffset: 1
verticalOffset: 1
radius: 6
samples: 14
spread: 0.30
color: "black"

Looks like a shadow again IMO.

Lovely!!!!

rooty updated this revision to Diff 52650.Feb 26 2019, 5:48 PM
rooty marked an inline comment as done.

Use filipf's settings

rooty retitled this revision from [sddm-theme/lock screen] WIP, Tighten clock shadows to [sddm-theme/lock screen] Tighten clock shadows.Feb 26 2019, 6:19 PM
rooty updated this revision to Diff 52656.Feb 26 2019, 6:35 PM

Use layer syntax instead

rooty updated this revision to Diff 52662.Feb 26 2019, 7:25 PM

Use new master

rooty updated this revision to Diff 52663.Feb 26 2019, 7:28 PM

Use DropShadow

rooty updated this revision to Diff 52664.Feb 26 2019, 7:48 PM

Add UserDelegate shadows

rooty retitled this revision from [sddm-theme/lock screen] Tighten clock shadows to [sddm-theme/lock screen] Tighten clock and username shadows.Feb 26 2019, 7:49 PM
rooty updated this revision to Diff 52665.Feb 26 2019, 7:50 PM

Remove >>>HEAD

rooty updated this revision to Diff 52666.Feb 26 2019, 7:51 PM

Round clock shadow spread to a single decimal place

rooty edited the summary of this revision. (Show Details)Feb 26 2019, 7:57 PM
rooty edited the test plan for this revision. (Show Details)

+1 visually! Just one little thing...

lookandfeel/contents/components/Clock.qml
34–48

"New" isn't going to be accurate for very long. :)

We should also explain in the comment why we're hardcoding a color since this reflects a reversion of that change (continuity with Breeze shadow color, black looks better than gray, not even necessary to use a theme color since the text color is always white, etc)

lookandfeel/contents/components/UserDelegate.qml
160 ↗(On Diff #52666)

Let's put the same comment here, too.

rooty updated this revision to Diff 52674.Feb 26 2019, 8:21 PM

Better comments

rooty marked 3 inline comments as done.Feb 26 2019, 11:15 PM
filipf accepted this revision.Feb 26 2019, 11:22 PM
rooty edited the test plan for this revision. (Show Details)Feb 26 2019, 11:29 PM
ngraham accepted this revision.Feb 27 2019, 2:29 PM

Very classy. I think this is a nice improvement.

This revision is now accepted and ready to land.Feb 27 2019, 2:29 PM
This revision was automatically updated to reflect the committed changes.