D25015: Update breeze theme shadows

This revision was automatically updated to reflect the committed changes. Closed by commit R242:35351ba3f299: Update breeze theme shadows (authored by niccolove, committed by filipf). REPOSITORY R242 Plasma Framework (Library) CHANGES SINCE LAST UPDATE

D25015: Update breeze theme shadows

ndavis accepted this revision. REPOSITORY R242 Plasma Framework (Library) BRANCH bettershadows (branched from master) REVISION DETAIL https://phabricator.kde.org/D25015 To: niccolove, #vdg, ngraham, The-Feren-OS-Dev, ndavis Cc: The-Feren-OS-Dev, mart, broulik, cblack, ndavis, manueljlin,

D25015: Update breeze theme shadows

ngraham accepted this revision. ngraham added a comment. If @ndavis says it's okay, I say shipit! REPOSITORY R242 Plasma Framework (Library) BRANCH bettershadows (branched from master) REVISION DETAIL https://phabricator.kde.org/D25015 To: niccolove, #vdg, ngraham, The-Feren-OS-Dev

D25015: Update breeze theme shadows

The-Feren-OS-Dev accepted this revision. The-Feren-OS-Dev added a comment. 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.

D25015: Update breeze theme shadows

niccolove added a comment. 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

D25015: Update breeze theme shadows

niccolove updated this revision to Diff 74153. niccolove added a comment. Corrected masks to be more similiar to original shapes REPOSITORY R242 Plasma Framework (Library) CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D25015?vs=73844=74153 BRANCH bettershadows (branched from

D25015: Update breeze theme shadows

niccolove added a comment. 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 patch forward. REPOSITORY R242 Plasma Framework (Library) BRANCH

D25015: Update breeze theme shadows

niccolove added a comment. 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

D25015: Update breeze theme shadows

ndavis added a comment. 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

D25015: Update breeze theme shadows

ndavis added a comment. 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. REPOSITORY R242 Plasma Framework (Library) BRANCH bettershadows (branched from master) REVISION DETAIL

D25015: Update breeze theme shadows

niccolove added a comment. In D25015#596684 , @ndavis wrote: > The masks have a rather odd shape and it's not pixel aligned, which might mean something is wrong: > F7891523: Screenshot_20200118_225818.PNG

D25015: Update breeze theme shadows

ndavis added a comment. I noticed a few things. --- The masks have a rather odd shape and it's not pixel aligned, which might mean something is wrong: F7891523: Screenshot_20200118_225818.PNG It's different from master, which looks pretty

D25015: Update breeze theme shadows

ngraham accepted this revision. ngraham added a comment. 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? REPOSITORY R242

D25015: Update breeze theme shadows

niccolove updated this revision to Diff 73844. niccolove added a comment. Fix possible bug REPOSITORY R242 Plasma Framework (Library) CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D25015?vs=73843=73844 BRANCH bettershadows (branched from master) REVISION DETAIL

D25015: Update breeze theme shadows

niccolove updated this revision to Diff 73843. niccolove added a comment. Resized mask to better fit rounding shape REPOSITORY R242 Plasma Framework (Library) CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D25015?vs=73842=73843 BRANCH bettershadows (branched from master)

D25015: Update breeze theme shadows

niccolove updated this revision to Diff 73842. niccolove added a comment. Removed unrelated tabbar change REPOSITORY R242 Plasma Framework (Library) CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D25015?vs=73841=73842 BRANCH bettershadows (branched from master) REVISION DETAIL

D25015: Update breeze theme shadows

niccolove added a comment. Can you please doublecheck that you see everything correctly? The mask thing was weird, but I think I fixed it REPOSITORY R242 Plasma Framework (Library) BRANCH bettershadows (branched from master) REVISION DETAIL https://phabricator.kde.org/D25015 To:

D25015: Update breeze theme shadows

niccolove updated this revision to Diff 73841. niccolove added a comment. - Merge branch 'master' into bettershadows - Changed masks REPOSITORY R242 Plasma Framework (Library) CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D25015?vs=69886=73841 BRANCH bettershadows (branched

D25015: Update breeze theme shadows

niccolove added a comment. Got it. Blur is not rounded either to me. The shadow was hiding it. F7819795: Screenshot_20191215_170023.png In D25015#575297 , @mart wrote: > In D25015#568865

D25015: Update breeze theme shadows

niccolove added a comment. 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. REPOSITORY R242 Plasma Framework (Library) BRANCH bettershadows (branched from master) REVISION DETAIL

D25015: Update breeze theme shadows

mart added a comment. In D25015#568865 , @niccolove 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

D25015: Update breeze theme shadows

ngraham added subscribers: broulik, mart. ngraham added a comment. 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

D25015: Update breeze theme shadows

niccolove added a comment. 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: F7788618:

D25015: Update breeze theme shadows

filipf added a comment. 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. F7787609: image.png REPOSITORY R242 Plasma

D25015: Update breeze theme shadows

niccolove added a comment. In D25015#568331 , @ngraham wrote: > 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

D25015: Update breeze theme shadows

ngraham accepted this revision. ngraham added a comment. This revision is now accepted and ready to land. Actually it looks like that was due to some kind of caching issue. It's gone now. LGTM! Everyone else good with this? REPOSITORY R242 Plasma Framework (Library) BRANCH

D25015: Update breeze theme shadows

ngraham requested changes to this revision. ngraham added a comment. This revision now requires changes to proceed. This results in the corners of the System Tray popup no longer looking rounded. Current: F7785383: Rounded.png With patch:

D25015: Update breeze theme shadows

niccolove added a comment. F7780181: image.png I just can't get the padding right on angles when changing their size. What size should I change of the colored rectangles? REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL

D25015: Update breeze theme shadows

ngraham added a comment. 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. REPOSITORY

D25015: Update breeze theme shadows

filipf added a comment. +1 REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D25015 To: niccolove, #vdg Cc: cblack, ndavis, manueljlin, ngraham, filipf, kde-frameworks-devel, LeGast00n, GB_2, michaelh, bruns

D25015: Update breeze theme shadows

niccolove added a comment. Finally! I'm 100% this works now. In D25015#563300 , @niccolove wrote: > This is a comparison: F7766697: Screenshot_20191116_165659.png > The problem is not that you

D25015: Update breeze theme shadows

niccolove updated this revision to Diff 69886. niccolove added a comment. Updated breeze shadows REPOSITORY R242 Plasma Framework (Library) CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D25015?vs=69820=69886 BRANCH bettershadows (branched from master) REVISION DETAIL

D25015: Update breeze theme shadows

niccolove added a comment. This is a comparison: F7766697: Screenshot_20191116_165659.png 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. REPOSITORY R242 Plasma Framework

D25015: Update breeze theme shadows

ngraham added a comment. 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. :/ REPOSITORY R242 Plasma Framework

D25015: Update breeze theme shadows

niccolove added a comment. In D25015#563147 , @ngraham wrote: > 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

D25015: Update breeze theme shadows

ngraham added a comment. Do these shadows get cached or something? When I build the diff and restart plasmashell, the shadows I see are identical, pixel-for-pixel. REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D25015 To: niccolove, #vdg Cc:

D25015: Update breeze theme shadows

niccolove edited the test plan for this revision. REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D25015 To: niccolove, #vdg Cc: cblack, ndavis, manueljlin, ngraham, filipf, kde-frameworks-devel, LeGast00n, GB_2, michaelh, bruns

D25015: Update breeze theme shadows

niccolove updated this revision to Diff 69820. niccolove added a comment. Updated to new shadows REPOSITORY R242 Plasma Framework (Library) CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D25015?vs=68912=69820 BRANCH breeze-shadows (branched from master) REVISION DETAIL

D25015: Update breeze theme shadows

ngraham added a comment. In D25015#562992 , @niccolove wrote: > What do you think of: > F7765076: Screenshot_20191115_145942.png That feels smoother. REPOSITORY R242 Plasma Framework

D25015: Update breeze theme shadows

niccolove added a comment. This is what you think ends too early (for comparison): F7762929: Screenshot_20191114_100519.png What do you think of: F7765076: Screenshot_20191115_145942.png REPOSITORY R242

D25015: Update breeze theme shadows

ngraham added a comment. In D25015#562362 , @niccolove wrote: > 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? > > F7762929:

D25015: Update breeze theme shadows

niccolove added a comment. In D25015#560514 , @ndavis wrote: > > 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

D25015: Update breeze theme shadows

ndavis added a comment. In D25015#560516 , @cblack wrote: > In D25015#560515 , @ndavis wrote: > > > One thing we might need to do in order to finally stop changing the shadows is come up with a math

D25015: Update breeze theme shadows

cblack added a comment. In D25015#560515 , @ndavis wrote: > 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

D25015: Update breeze theme shadows

ndavis added a comment. One thing we might need to do in order to finally stop changing the shadows is come up with a 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.

D25015: Update breeze theme shadows

ndavis added a comment. In D25015#560498 , @niccolove wrote: > In D25015#559344 , @filipf wrote: > > > I'd prefer the shadows to have equal strength all around, I couldn't get used to weaker

D25015: Update breeze theme shadows

niccolove added a comment. In D25015#559344 , @filipf wrote: > 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

D25015: Update breeze theme shadows

ndavis added a comment. 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?

D25015: Update breeze theme shadows

ngraham added a comment. 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. REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL

D25015: Update breeze theme shadows

filipf added a comment. Before: F7746042: Screenshot_20191107_083508.png After: F7746044: Screenshot_20191107_083534.png REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL

D25015: Update breeze theme shadows

filipf added a comment. 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.

D25015: Update breeze theme shadows

ngraham added a comment. In D25015#559367 , @ndavis wrote: > 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. :) REPOSITORY R242

D25015: Update breeze theme shadows

ndavis added a comment. 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

D25015: Update breeze theme shadows

filipf added a comment. I'd prefer the shadows to have equal strength all around, I couldn't get used to weaker shadows in the corners. REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D25015 To: niccolove, #vdg Cc: manueljlin, ngraham, filipf,

D25015: Update breeze theme shadows

niccolove added a comment. Any update on this? I can also upload the shadow for the panel. REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D25015 To: niccolove, #vdg Cc: manueljlin, ngraham, filipf, kde-frameworks-devel, LeGast00n, GB_2, michaelh,

D25015: Update breeze theme shadows

manueljlin added a comment. I hope this gets approved so T10470 doesn't look awkward with thin and dark shadows REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D25015 To: niccolove, #vdg Cc: manueljlin,

D25015: Update breeze theme shadows

niccolove added a comment. In D25015#48 , @ngraham wrote: > 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

D25015: Update breeze theme shadows

ngraham added a comment. If anything I wonder if we should make them bigger too, to better match the default Breeze shadows. REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D25015 To: niccolove, #vdg Cc: ngraham, filipf, kde-frameworks-devel,

D25015: Update breeze theme shadows

filipf added a comment. 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

D25015: Update breeze theme shadows

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