Hi
thanks for the patch !

... see inline comment.
Also note that to more or less match the different scales shown in your original screenshot, you would need a 0.25% scale (or even 0.20%) rather than 0.5%
But I am also fine with the current choice.

Thanks for the advice, Hugo. Menu shadows now match the screenshot in the prior patch that people liked and that VDG approved.

Thanks Nate, your blog readership seems to like it too (congratulations on the blog, BTW ;) What about the inevitable "But can I change it back?". You claim we allow this, but if you actually try it, it does not work, i.e. the menu shadow is now almost completely gone for the former default shadow size of 16px. This is also the case for any user upgrading to 5.12 with a non-default size, e.g. 17px, where the shadow going away looks like an upgrade bug.

I won't bother you further with this, but I'm a bit concerned and I think you should at least comment on the problem. Ideas for consideration:

• Two comboboxes.
• Lower bound or different scaling function (but difficult to get right).
• Set of fixed values.

IMO new defaults are fine, but removing the possibility to go back to (or at least somewhat replicate) the old (default) values is the worst (also from a PR standpoint).

One more thing: Is the 100px max working for you? Mine is still capped at 64px (the defaults button has the new value, weirdly), but perhaps I'm just installing things wrongly and there are some config caching quirks. Could you check again?

(Also, the title of the Diff sounds off.)

Those are valid concerns, and I will think on them! I always admire your thoroughness, @rkflx.

That said, for upgrades, users who did not manually change the shadow size should get the new setting since they will not have a config file written out to override the defaults. You're right that users who did change the size manually to something similar to the old default will subsequently have almost no menu shadows. Since small 17px window shadows are a valid case, maybe I should put in a floor value for menu shadow size.

I will check the combobox size issue and if necessary, address that with another patch.

In D9627#185615, @ngraham wrote:

Those are valid concerns, and I will think on them! I always admire your thoroughness, @rkflx.

That said, for upgrades, users who did not manually change the shadow size should get the new setting since they will not have a config file written out to override the defaults. You're right that users who did change the size manually to something similar to the old default will not have almost no menu shadows. Since small window shadows are a valid case, maybe I should put in a floor value for menu shadow size.

... but then you also need the same floor value for the window shadow size. You do not want menu shadows to be bigger than window shadows.
In the end I think a finite set of hard coded shadow sizes (menu and window), controlled by a unique combobox, is what makes more sense. It hides unnecessary micromanagement to the user (honestly there is no need to allow user to change a shadow size from 31 to 32 pixels: the difference is not even visible to the eye); and gives the maximum flexibility to devs and designers to choose (and evolve) the "best" set of sizes that should accommodate most tastes.

@hpereiradacosta +1

Essentially this: D9549#184975 (but a "No" option could be added).

Excellent ideas, everyone. I'll see if I can work up something with a combobox. Also with that approach, I can fine-tune the menu shadows for each option to make sure they look good.

I'm working on the combobox patch, but in my KDE Neon machine, I'm having trouble getting my changes applied when I open the Breeze settings in the Window Decoration KCM. I'm building to /usr and can verify that /usr/lib/x86_64-linux-gnu/qt5/plugins/styles/breeze.so, /usr/lib/x86_64-linux-gnu/qt5/plugins/kstyle_breeze_config.so, and /usr/lib/x86_64-linux-gnu/qt5/plugins/org.kde.kdecoration2/breezedecoration.so have gotten updated. I still don't see my changes when I open the Breeze options in the Window Decoration KCM. Any idea what I'm doing wrong?

In D9627#185783, @ngraham wrote:

I'm working on the combobox patch, but in my KDE Neon machine, I'm having trouble getting my changes applied when I open the Breeze settings in the Window Decoration KCM. I'm building to /usr and can verify that /usr/lib/x86_64-linux-gnu/qt5/plugins/styles/breeze.so, /usr/lib/x86_64-linux-gnu/qt5/plugins/kstyle_breeze_config.so, and /usr/lib/x86_64-linux-gnu/qt5/plugins/org.kde.kdecoration2/breezedecoration.so have gotten updated. I still don't see my changes when I open the Breeze options in the Window Decoration KCM. Any idea what I'm doing wrong?

Hi Nathan,
Are there other libraries in the path you mention (e.g. Aurorae and oxygen in /usr/lib/x86_64-linux-gnu/qt5/plugins/org.kde.kdecoration2/ ?
Also, you could try "breeze-settings5" which just loads the two breeze-related kcm into its own window.
Other than that, no clue.

In D9627#185604, @rkflx wrote:

One more thing: Is the 100px max working for you? Mine is still capped at 64px

Figured that one out: The problem is clearly with D9549, because it only changed breezesettingsdata.kcfg, i.e. the values which would be considered valid in the config file. As most users use the GUI to edit, this would've needed changes to breezeconfigurationui.ui so the spinbox would've allowed bigger values.

Going for the combobox, this is now moot. However, I want to beg everyone (again!) for the sake of quality: Please, if you do changes or "accept" a Diff, try them in the actual application. Try different user-set values, too. Only focussing on the code in an editor leads to bugs, which are much more expensive to fix after the fact.

In D9627#185783, @ngraham wrote:

I still don't see my changes when I open the Breeze options in the Window Decoration KCM. Any idea what I'm doing wrong?

First, determine whether you are actually trying to load those files, e.g. by removing them (now it should not work anymore). Next, check whether you aren't accidentally doing code edits in the build dir (ui_*.h) instead of the .ui files (been there, done that ;).

For me, installing locally as a user works fine, the trick is then to set QT_PLUGIN_PATH so the new files take precedence over the system installation.

Would you consider changing the word "Normal" for something more related to measurements? For example, regular, default, current. Normal works better in other languages like Spanish where it is related to measurements. In English that link is not as strong.

How about "Medium"?

In D9627#186582, @abetts wrote:

Would you consider changing the word "Normal" for something more related to measurements? For example, regular, default, current. Normal works better in other languages like Spanish where it is related to measurements. In English that link is not as strong.

For the record, normal is already used at many places: decoration border sizes, decoration button sizes to cite a few.
Back in the days I was explicitly asked to change Default to Normal.

In D9627#186583, @ngraham wrote:

How about "Medium"?

It's not really of size medium, isn't it? However, compared to before 3 different sizes are not that many anyway, so we could insert an extra / actual "Medium", rename the new default to "Large", and finally rename the last one to "Huge" or "Very Large".

In D9627#186584, @hpereiradacosta wrote:

In D9627#186582, @abetts wrote:

Would you consider changing the word "Normal" for something more related to measurements? For example, regular, default, current. Normal works better in other languages like Spanish where it is related to measurements. In English that link is not as strong.

For the record, normal is already used at many places: decoration border sizes, decoration button sizes to cite a few.
Back in the days I was explicitly asked to change Default to Normal.

That is why I was thinking that maybe using words to describe it is harder than numbers. Could we consider using numbers instead of words? @hpereiradacosta Normal is not a bad words but doesn't work well in English when explaining measurements. Hence my suggestion.

FWIW, I rather prefer abstract words instead of numbers, since then we have more freedom to tweak the numbers under the hood in the future. The actual numbers aren't important enough to show to the user IMHO. And if we do expose the number, people will eventually ask for a spinbox to precisely input arbitrary values, and we'll be back to where we started from. :-)

In D9627#186592, @ngraham wrote:

FWIW, I rather prefer abstract words instead of numbers, since then we have more freedom to tweak the numbers under the hood in the future. The actual numbers aren't important enough to show to the user IMHO. And if we do expose the number, people will eventually ask for a spinbox to precisely input arbitrary values, and we'll be back to where we started from. :-)

I agree. IMO numbers in themselves are technical implementations irrelevant for the users, especially when you actually paint a gaussian gradient (for which the actual size is not quite well defined).

I am ok with changing normal by medium, adding more sizes, etc.

But please not changing to "default", since "default size" means essentially nothing (and I agree that "normal size" does not mean much either).

In D9627#186592, @ngraham wrote:

FWIW, I rather prefer abstract words instead of numbers, since then we have more freedom to tweak the numbers under the hood in the future. The actual numbers aren't important enough to show to the user IMHO. And if we do expose the number, people will eventually ask for a spinbox to precisely input arbitrary values, and we'll be back to where we started from. :-)

Exactly my thoughts.

As we now have three comboboxes (sizes of shadow, border, window buttons) all with slightly different options, we could think about unifying these, both in the number of options (not too many) and the wording (no "normal"/"default" etc.). For example Tiny → Small → Medium → Large → Huge plus special cases (e.g. No [Side] Borders). Different patch, of course. Thoughts?

Small is the old default value for people who preferred that.

Works for the menu, but the window shadow is not as before for me. Maybe insert Tiny with 16px + 16px?

As for the rest, it's working well for me (did not look at the code, though). Thank you Hugo + Nate!

@ngraham BTW, how did you solve the .so problem in the end?

In D9627#186631, @rkflx wrote:

As we now have three comboboxes (sizes of shadow, border, window buttons) all with slightly different options, we could think about unifying these, both in the number of options (not too many) and the wording (no "normal"/"default" etc.). For example Tiny → Small → Medium → Large → Huge plus special cases (e.g. No [Side] Borders). Different patch, of course. Thoughts?

Great idea. I even wonder if we actually need Tiny as an option. It seems to me that the kind of person who wants Tiny window borders generally prefers None at all (disclosure: I am one such person). Same for shadows, really. But yes, this is a discussion for another patch.

---

Small is the old default value for people who preferred that.

Works for the menu, but the window shadow is not as before for me. Maybe insert Tiny with 6px + 6px?

Are you sure? the old shadow default was 16px, which is replicated in the Small option. Of course it doesn't look exactly the same as before since the old shadow was right-biased, so it looked bigger than 16px on the right side, and almost non-existent on the left side. But technically, we do still have a 16px shadow option.

@ngraham BTW, how did you solve the .so problem in the end?

In the end the .sos weren't actually being compiled with my changes due to stale files in my checkout. After doing a clean, the libraries were generated correctly and then installing them using my prior procedure worked fine.

In D9627#186637, @ngraham wrote:

In D9627#186631, @rkflx wrote:

Works for the menu, but the window shadow is not as before for me. Maybe insert Tiny with 16px + 16px?

Are you sure? the old shadow default was 16px, which is replicated in the Small option. Of course it doesn't look exactly the same as before since the old shadow was right-biased, so it looked bigger than 16px on the right side, and almost non-existent on the left side. But technically, we do still have a 16px shadow option.

For me Small looks like this currently (note the bigger window shadow, while you claim it should be smaller with centering):

Before, it looked like this (note the size of the window shadow is the same small size as the menu shadow):

Essentially all I want is that we provide a fallback for those not liking the change (personally I like it), they have to accept the centering anyway. I guess that's what we would get with 12px menu shadow + 12 px window shadow? (Not 16px, because we have to account for the centering). As far as I understand the code, currently Small is 12px + 32px. We would not need to change that, just add another which gets us the old behaviour.

Edit: Scratch my centering remark, window size should be 16px to look as before (menu 12px). I just noticed that the menu shadow is a bit odd in the sense that for the same input size it creates a stronger shadow compared to the window shadow.

Thanks for the update. I won't need to test again, as the screenshots clearly show it working fine ;)

If Hugo is happy with the code, this could land.

In D9627#186696, @rkflx wrote:

I won't need to test again, as the screenshots clearly show it working fine ;)

Never trust a screenshot…

Other than that ... Ship it ! I'm fine with the change and happy to maintain it in the future.

Thanks for the review and your help ;)

I'm agnostic about the default style, but I do see the utility of putting it on top. I'll leave it that way unless anybody has strong objections.

Is this shippable now?