Update breeze theme shadows
ClosedPublic

Authored by niccolove on Oct 28 2019, 8:35 PM.

Details

Summary

I have received negative feedback (and I agree with it) on the current state of breeze shadows: they are quite dark, narrow, and feel unnatural. I tried to adress that by making shadows more sparse and a bit lighter especially on the angles while trying to keep it distinguishable when on white background. I received some positive feedback on these shadows from T10891 and the VDG channel. More specifically, I changed dialogs/background.svg to have a) longer linear gradients and b) radial gradients instead of linear on the four sides to make the center darker than the angles.

Test Plan



Diff Detail

Repository
R242 Plasma Framework (Library)
Branch
breeze-shadows (branched from master)
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 18269
Build 18287: arc lint + arc unit
There are a very large number of changes, so older changes are hidden. Show Older Changes
Restricted Application added a project: Frameworks. · View Herald TranscriptOct 28 2019, 8:35 PM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
niccolove requested review of this revision.Oct 28 2019, 8:35 PM
niccolove retitled this revision from I have received negative feedback (and I agree with it) on the current state of breeze shadows: they are quite dark, narrow, and feel unnatural. I tried to adress that by making shadows more sparse and a bit lighter especially on the angles while... to Update breeze theme shadows.Oct 28 2019, 8:44 PM
niccolove edited the summary of this revision. (Show Details)
filipf added a subscriber: filipf.Oct 28 2019, 9:11 PM

Testing it. Usability wise the strength is perfectly fine; the shadows still do their job.

Visually it's a definitive improvement for me.

Only thing I'm not sure of is the weaker strength in the corners, but I'm going to keep testing to see if it's just a matter of getting used to it.

If anything I wonder if we should make them bigger too, to better match the default Breeze shadows.

If anything I wonder if we should make them bigger too, to better match the default Breeze shadows.

I'd prefer to avoid them making more sparse because a) they look worse to me when applied to small objects such as notifications (you have big shadows for a relatively small item, they don't give the impression to be casted by that item) b) lots and lots of techical problem when trying to change shadow size :-/

I hope this gets approved so T10470 doesn't look awkward with thin and dark shadows

Any update on this? I can also upload the shadow for the panel.

filipf added a comment.Nov 6 2019, 8:10 PM

I'd prefer the shadows to have equal strength all around, I couldn't get used to weaker shadows in the corners.

ndavis added a subscriber: ndavis.Nov 6 2019, 9:06 PM

I haven't said much about this idea because I just have a hard time seeing the difference from the screenshots. It seems like a good amount of effort went into this, and I know from experience that adjusting shadows in Inkscape is a major PITA, but I can't approve something just because of that. Maybe we're using the wrong approach for shadows in the first place?

I haven't said much about this idea because I just have a hard time seeing the difference from the screenshots.

I must admit having the same problem. :)

filipf added a comment.Nov 7 2019, 7:37 AM

There's definitely a difference, whereas the shadows are now rough and dark this is subtler.

I'd suggest testing this patch with a light color scheme and then switching to the unpatched theme plus testing against light backgrounds to notice the changes.

filipf added a comment.Nov 7 2019, 7:40 AM

Before:

After:

Ah, pixel-perfect before-and-after images make it much easier to see. I will reiterate my belief that the shadows now need to be bigger if we're going to make them lighter and weaker.

ndavis added a comment.Nov 7 2019, 9:11 PM

I see the difference now as well. @niccolove BTW, the top right corner is a bit messed up. It's rising above the rest of the shadow on the top edge.

Rather than painstakingly making shadows by hand in Inkscape, perhaps we should be using KWin for the shadows?

I'd prefer the shadows to have equal strength all around, I couldn't get used to weaker shadows in the corners.

Oh! That's a pity, I really liked them. I will update it as soon as I have time.

Ah, pixel-perfect before-and-after images make it much easier to see. I will reiterate my belief that the shadows now need to be bigger if we're going to make them lighter and weaker.

Uhm, I will try something and get back to you.

Rather than painstakingly making shadows by hand in Inkscape, perhaps we should be using KWin for the shadows?

Remember that there are third party desktop themes with SVG that we probably want to support. Wouldn't using KWin for shadows break those?

I'd prefer the shadows to have equal strength all around, I couldn't get used to weaker shadows in the corners.

Oh! That's a pity, I really liked them. I will update it as soon as I have time.

Ah, pixel-perfect before-and-after images make it much easier to see. I will reiterate my belief that the shadows now need to be bigger if we're going to make them lighter and weaker.

Uhm, I will try something and get back to you.

Rather than painstakingly making shadows by hand in Inkscape, perhaps we should be using KWin for the shadows?

Remember that there are third party desktop themes with SVG that we probably want to support. Wouldn't using KWin for shadows break those?

If we remove the code for using those SVG shadows, it won't break them in the sense that they will stop working. It'll just mean those parts of the desktop themes won't be used. I suppose that could be seen as a form of visual breakage.

For complete theme sets, this shouldn't be such a problem since they also typically come with their own window decorations and recommend a specific widget style. For people who just want to get rid of the shadows we should probably provide an option somewhere to disable them rather than making them edit the plasma theme. I suppose we could wait until Plasma 6 to make that kind of change, but that's a ways away.

Regardless of the method we use to achieve it (including continuing to use SVGs), I think it might be best to make Plasmashell's shadows match the shadows cast by menus (such as context menus) in applications. These are defined in the Breeze widget style and have different sizes to match the different window shadow sizes.

ndavis added a comment.EditedNov 9 2019, 11:36 PM

One thing we might need to do in order to finally stop changing the shadows is come up with a math based system for deciding how shadows should look based on the elevation we want certain UI elements to have. We could copy Material Design shadows, but I don't think we should. MD's shadows get darker the larger they are, but that's not how real shadows work. Real shadows get darker the smaller they are because light bounces around.

cblack added a subscriber: cblack.Nov 9 2019, 11:39 PM

One thing we might need to do in order to finally stop changing the shadows is come up with a math based system for deciding how shadows should look based on the elevation we want certain UI elements to have. We could copy Material Design shadows, but I don't think we should. MD's shadows get darker the larger they are, but that's not how real shadows work. Real shadows get darker the smaller they are because light bounces around.

Material shadows get more voluminous, not darker.

One thing we might need to do in order to finally stop changing the shadows is come up with a math based system for deciding how shadows should look based on the elevation we want certain UI elements to have. We could copy Material Design shadows, but I don't think we should. MD's shadows get darker the larger they are, but that's not how real shadows work. Real shadows get darker the smaller they are because light bounces around.

Material shadows get more voluminous, not darker.

Well, they look darker, which still isn't realistic: https://material.io/design/environment/light-shadows.html#shadows

Remember that there are third party desktop themes with SVG that we probably want to support. Wouldn't using KWin for shadows break those?

If we remove the code for using those SVG shadows, it won't break them in the sense that they will stop working. It'll just mean those parts of the desktop themes won't be used. I suppose that could be seen as a form of visual breakage.

For complete theme sets, this shouldn't be such a problem since they also typically come with their own window decorations and recommend a specific widget style. For people who just want to get rid of the shadows we should probably provide an option somewhere to disable them rather than making them edit the plasma theme. I suppose we could wait until Plasma 6 to make that kind of change, but that's a ways away.

I'm against this; there are many desktop themes that tweak their shadows without being a theme set, and it would be an important regression to ignore them.

Regardless of the method we use to achieve it (including continuing to use SVGs), I think it might be best to make Plasmashell's shadows match the shadows cast by menus (such as context menus) in applications. These are defined in the Breeze widget style and have different sizes to match the different window shadow sizes.

This is interesting. I have to look more into it.

Meanwhile, I tried to do a more sparse and equal-all-around shadow (to address Nate+Filip), but I'm not 100% okay with it. Opinions?

Meanwhile, I tried to do a more sparse and equal-all-around shadow (to address Nate+Filip), but I'm not 100% okay with it. Opinions?

Hmm, that shadow feels too "hard," somehow. It feels like it ends rather abruptly rather than fading out smoothly.

This is what you think ends too early (for comparison):


What do you think of:

What do you think of:

That feels smoother.

niccolove updated this revision to Diff 69820.Nov 15 2019, 7:13 PM

Updated to new shadows

niccolove edited the test plan for this revision. (Show Details)Nov 15 2019, 7:15 PM

Do these shadows get cached or something? When I build the diff and restart plasmashell, the shadows I see are identical, pixel-for-pixel.

Do these shadows get cached or something? When I build the diff and restart plasmashell, the shadows I see are identical, pixel-for-pixel.

I don't know. But you can manually change them by replacing the .svgz in /usr/share/plasma/desktopthemes/default/ dialogs/background.svgz and widgets/panel-background.svgz with the new svg.

Okay, after you taught me how to delete the cache files rm ~/.cache/plasma*, I can see other changes to the plasma theme SVG files but I must admit I still don't see a difference between the old and new shadows in this patch. :/

This is a comparison:


The problem is not that you don't see, it's that it doesn't show. I'll try to understand what's going on and get back to you.

niccolove updated this revision to Diff 69886.Nov 17 2019, 5:44 PM

Updated breeze shadows

niccolove added a comment.EditedNov 17 2019, 5:45 PM

Finally! I'm 100% this works now.

This is a comparison:


The problem is not that you don't see, it's that it doesn't show. I'll try to understand what's going on and get back to you.

Please refer to the comparison to see the difference now that this patch works, although I think it's quite noticeable :-)

Yep, can confirm that it works now. It is indeed a more subtle and pleasing effect. I still think the distance away from the window would be lengthened to make it even nicer-looking. :) But if nobody else agrees, I'm okay with this in its current form.


I just can't get the padding right on angles when changing their size. What size should I change of the colored rectangles?

ngraham requested changes to this revision.Nov 26 2019, 5:05 PM

This results in the corners of the System Tray popup no longer looking rounded.

Current:

With patch:

This revision now requires changes to proceed.Nov 26 2019, 5:05 PM
ngraham accepted this revision.Nov 27 2019, 2:41 PM

Actually it looks like that was due to some kind of caching issue. It's gone now. LGTM!

Everyone else good with this?

This revision is now accepted and ready to land.Nov 27 2019, 2:41 PM

Actually it looks like that was due to some kind of caching issue. It's gone now. LGTM!

Everyone else good with this?

Actually, it seems to me that borders are not correctly rounded except when on bright background, and sometimes. I'm not sure why would that be. Do you also see this, or is it just me?


My impression is that the rounding is currently just faked by the shadow. So when the shadow gets lighter, it becomes more obvious that the object itself isn't really rounded.

niccolove added a comment.EditedNov 28 2019, 2:38 PM

Got it. The shape is actually rounded BUT the desktop contract effect is applied in the whole rectangle underneath, and it makes everything look like a rectangle. Here's the widget with fully transparent background, but with the contract effect on:


Of course, I can't just turn off the contrast desktop effect. I'm unsure how to move forward with this.
Here's Plasma without desktop effect:

As you can see, shadows are working!

So I guess we can either fix the Background Contrast effect, or stop using it and do the opacity entirely within the Plasma Breeze theme (e.g. by bumping it up from 0.6 to 0.9 or 0.95 or so).

Probably needs Plasma input, especially from @broulik and @mart who I believe are familiar with the history here.

mart added a comment.EditedDec 11 2019, 10:35 AM

Got it. The shape is actually rounded BUT the desktop contract effect is applied in the whole rectangle underneath, and it makes everything look like a rectangle. Here's the widget with fully transparent background, but with the contract effect on:


Of course, I can't just turn off the contrast desktop effect. I'm unsure how to move forward with this.
Here's Plasma without desktop effect:

As you can see, shadows are working!

Not sure why it looks rectangular, on current theme the mask- prefix is correctly used and the blur is correctly shaped with rounded borders

I just compared the blur and contrast effects and the mask is used in the exact same way :-/ I have no idea what could be wrong there.

Got it. Blur is not rounded either to me. The shadow was hiding it.

In D25015#575297, @mart wrote:

Got it. The shape is actually rounded BUT the desktop contract effect is applied in the whole rectangle underneath, and it makes everything look like a rectangle. Here's the widget with fully transparent background, but with the contract effect on:


Of course, I can't just turn off the contrast desktop effect. I'm unsure how to move forward with this.
Here's Plasma without desktop effect:

As you can see, shadows are working!

Not sure why it looks rectangular, on current theme the mask- prefix is correctly used and the blur is correctly shaped with rounded borders

If you remove the shadows, desktop effect and background, do you still see rounded blur? If so, it's my machine.

niccolove updated this revision to Diff 73841.Sat, Jan 18, 7:54 PM
  • Merge branch 'master' into bettershadows
  • Changed masks
niccolove added a comment.EditedSat, Jan 18, 7:58 PM

Can you please doublecheck that you see everything correctly? The mask thing was weird, but I think I fixed it

(Accidentally put in a file from a different branch, sorry, just a sec)

niccolove updated this revision to Diff 73842.Sat, Jan 18, 8:02 PM

Removed unrelated tabbar change

niccolove updated this revision to Diff 73843.Sat, Jan 18, 8:14 PM

Resized mask to better fit rounding shape

niccolove updated this revision to Diff 73844.Sat, Jan 18, 8:17 PM

Fix possible bug

ngraham accepted this revision.Sat, Jan 18, 8:23 PM

Looks like you fixed the rounded corner issues, nice. This does look much better IMO. +1 from me.

Are other VDG folks good with this? @ndavis do the SVG changes look appropriate from a technical point of view?

ndavis added a comment.EditedSun, Jan 19, 4:22 AM

I noticed a few things.


The masks have a rather odd shape and it's not pixel aligned, which might mean something is wrong:


It's different from master, which looks pretty normal:


In the panel-background and tooltip SVGs, the shadow was changed, but the tooltip/panel background was not.


I get the impression that these were changed with the node tool rather than creating the corners from rounded rectangles. I guess that works as long as all the svg corners are consistent with each other, but it's inconsistent with the QStyle and could still lead to visible pixel misalignment when the UI is scaled up.


Why is widgets/background.svg different from the other ones? Maybe there's a good reason, since it's also different in the master branch, but its very odd.

The masks have a rather odd shape and it's not pixel aligned, which might mean something is wrong:


It's different from master, which looks pretty normal:

I know this was going to seem weird, but: the only way to get pixel-perfect mask is to add nodes to the shape, but if I do that, the mask is no longer correctly detected. I tried to group different shapes to be pixel perfect, but that does not work as well. They only way seem to be a single curve, that can only approximate the actual shape. Currently, Breeze Masks are *not* working, although they sure look better than mine. No idea why. Do you have any tip here?

the panel-background and tooltip SVGs, the shadow was changed, but the tooltip/panel background was not.

Mhh, weird. I'll fix that.

I get the impression that these were changed with the node tool rather than creating the corners from rounded rectangles. I guess that works as long as all the svg corners are consistent with each other, but it's inconsistent with the QStyle and could still lead to visible pixel misalignment when the UI is scaled up.

This is already what is done in master though, right? I'm not sure rounded rectangles corner can be used, and all the pieces should be different elements

Why is widgets/background.svg different from the other ones? Maybe there's a good reason, since it's also different in the master branch, but its very odd.

No idea. I did not change that file.

I'll trust your judgement on the masks and corners. If there's a problem, it can be patched. I know this stuff can be super janky at times.

Just in case you misunderstood me, when I said create the corners from rounded rectangles, I meant create the rounded rectangle and cut off the parts you don't need so that you're left with corners that you can place however you need. If you're not aware of it, check out Inkscape's Path menu. It has a ton of useful stuff for sculpting shapes.

Regarding the mask: I'd keep it as is right now and look more into it in the (very near) future. Currently the difference is not noticeable even on different scalings and I cannot find another working solution, so I think it should not be a blocking issue. But yes, I will surely look more into it. But right now I have little time and I spent the last four days without studying and working on KDE and I know I can't keep up that, so I'm just a bit afraid that this patch would end up blocked for weeks again.
Regarding the corners, I will also look into it but not in this patch, as that would be an unrelated change.

niccolove added a comment.EditedSun, Jan 19, 10:39 PM

I posted the previous comment in a eccessive state of tiredness and nervousness :-) I will look into doing a better mask *before* asking to land the patch, so I can be sure it's the right path forward.

niccolove updated this revision to Diff 74153.Wed, Jan 22, 5:56 PM

Corrected masks to be more similiar to original shapes

I found a way to make masks that follow the current shape by copying it and making them slightly bigger than original ones.
Regarding panels and tooltips: using different roundings (1px vs 3px) seemed inconsistent to me, so I tried to ask in the VDG chat if they though that moving panels and tooltips to 3px was a good idea, which I also think (e.g.: panel endings should be rounded rather than sharp). I thus moved them to 3px rounding as well. I think it's barely noticeable but it looks better than before. I put it in this patch as it was closely related and on the same files, but I could create a different patch if you think it's more appropriate.

The-Feren-OS-Dev accepted this revision.Wed, Jan 22, 6:05 PM
The-Feren-OS-Dev added a subscriber: The-Feren-OS-Dev.

I feel like the shadows might be slightly overcompensating in shadow strength for the small shadow span (it looks kinda weird on first glance), but apart from that minor nitpick, I'd say this is good to go.

ngraham accepted this revision.Wed, Jan 22, 10:58 PM

If @ndavis says it's okay, I say shipit!

ndavis accepted this revision.Thu, Jan 23, 12:50 AM
This revision was automatically updated to reflect the committed changes.