Added option to set transparency and blur behind menu frames such as right click context menu, toolbar menu, etc.
Transparency and blur is disabled by default.
Details
Diff Detail
- Repository
- R31 Breeze
- Branch
- master
- Lint
No Linters Available - Unit
No Unit Test Coverage
Looks great!
BUT, it would be nice to have a little bit bigger contrast. Could you please enable background contrast and upload a screenshot what it looks like?
kstyle/breeze.kcfg | ||
---|---|---|
188 | Redundant comment. |
I presume you mean the Background contrast effect. It was in the half-enabled state. I checked it to be fully enabled but I did not notice any difference.
However changing the transparency percent improves the contrast, since the default frame background color is (252, 252, 252).
Initially I uploaded 40% transparency to demonstrate the effect, anything lower than that might have been too subtle in a screenshot.
Anyways here are some transparency percent value screenshots with the background contrast effect enabled:
40%:
30%:
20%:
10%:
kstyle/breeze.kcfg | ||
---|---|---|
188 | What's redundant about it? |
Did you call KWindowEffects::enableBackgroundContrast?
kstyle/breeze.kcfg | ||
---|---|---|
188 | It's obvious that the following entry is about transparency. |
kstyle/breezestyle.cpp | ||
---|---|---|
3629 | The slider has 10 steps and goes from 0 to 10, that's why it's / 10.0 And we need to invert the value of it because the setAlphaF works inverted to our slider and it needs a value between 0.0 and 1.0 |
kstyle/breezestyle.cpp | ||
---|---|---|
3629 | IMO, transparency should be in range 0-100.
Why? |
Hello,
Hi,
thanks for the patch !
Few comments:
1/ the code does not compile under kde4 (cmake -DUSE_KDE4). You would need to use the proper #ifdef to prevent this
2/ I think the "blur" option should go. Blur should be controlled centrally by the desktop effect. In other words: BlurBehind should always be set to true, and then left to kwin to handle. Having an extra option here seems like micro-management. Why would you need blur behind plasma widgets (as handled by the desktop effect) and not behind menus ? (or vice versa). Also, right now, selecting "blur" in the style settings, but unselecting the desktop effect results in no blur behind menu, and hence inconsistency with what the style option says.
3/ the slider should follow the same convention and design as for the translucency effect: "transparent <----slider---> opaque". 0% should be transparent, 100% should be opaque.
Also, should have the same granularity, so ... percents. (not 10%)
4/ I would really like the VDG opinion on this. IMHO I am not sure if it fits the breeze "guidelines" or design-ideas, in the sense that I do not know why this is needed (what is the use case), aside from "this is pretty".
In other words: why would one need to be able to see what is behind the menu you are currently navigating ?
Navigating a menu is a very focused thing one wants to do: one is looking for a specific action to perform on the current content of the window. To me at least, seeing-through the menu is more a distraction to this task than anything else. (and adding blur to reduce this distraction would in fact go in the direction of not being translucent at all).
If the only reason is "this is pretty", then this has to be balanced with the extra option, code complexity, and configurations to be tested on the maintainer side, that this patch adds.
Dear Andy,
With all due respect, please stop "accepting" revisions. Revisions should be accepted by the code maintainer, who then endorse the responsibility that the code then remains in a workable state forever after. You are not the breeze maintainer.
In my understanding at least, accepting a revision allows the submitter to commit the code as is (as in "this revision is now accepted and ready to land").
In the current state, this revision is _not_ accepted and ready to land.
You can, of course, vote +1 or -1 to a change, and it is then up to the maintainer to accept/reject/request changes to the revision, taking into account (or not), your +1s or -1s.
Thanks in advance,
Hugo
Maybe I can add some extra reasons. I am not sure if having the extra menu to control Blur is "necessary." However, I see value in having it in our KCMs or as part of an effect. One thing that this patch accomplishes is to have a blurred effect consistent in more areas of the desktop. While now we are able to make windows transparent and blur them, an option to have menus follow that convention is not present. Imagine that maybe a window style or theme allows transparency but that the menus don't look well with the preset and you would like to change it independently, There is no option for that. My thought is that if is about theme/style consistency, this makes sense to have. I feel, however, that it fits better in a KCM or as part of the transparency effect options. Thoughts anyone else in the VDG? @colomar , @ngraham , @andreask
Also, please, take into account contrast issues. That's nice to have blur, but, please, use it wisely.
FWIW, I don't think I'd ever want this enabled by default, and I'm leery of adding the option to Breeze. I'm not sure how much it really fits the visual style. Only very very subtle blur transparency would ever work here without becoming garish and distracting; the screenshot of the feature looks terrible to me, I'm sorry to say. And using only a very a low level of blur transparency IMHO calls into question what the point even is.
For years, macOS experimented with making things transparent and blurry. People made fun of them. Then Microsoft did it, but even worse. People made fun of them, too. Since then, both of those companies have mostly moved away from blur transparency in their operating systems, or made it optional. I really don't see how letting people move closer towards their historical design mistakes helps us here.
Did they?
Microsoft is doing some work on Fluent design (https://fluent.microsoft.com/).
macOS is still using blur extensively.
Eh, maybe I'm wrong. Still... [shakes old man stick] I don't like it! These whippersnappers and their transparency! Back in my day things were opaque and we liked it that way!
I won't die on this hill though, since it's being proposed as an optional feature and not a new default.
@ngraham: blur is OK but it should be used wisely. Blurring the hell out from UI is not really cool.
IMO, changes like this, require some changes in the "design language".
PS. does Plasma have a design language? (like Fluent or Material design)
@hpereiradacosta
1/ Done. No more errors when compiling with -DUSE_KDE4. Blur is not enabled with KDE4 breeze.
2/ Done. Blur option gone. Now blurs by default.
3/ Done. Now looks and behaves like the translucency effect.
4/ This is pretty.
@zzag
I got enableBackgroundContrast to work. It is very very poorly documented.
I played around with the values but the endresult does not look better.
In my opinion it looks worse if I change the contrast or intensity.
@ngraham
Windows 10 moves closer and closer to make everything transparent and blurry.
Fluent designed UWP apps make like 30% of the window transparent/blurred.
Titlebars are getting blurred.
macOS blurs pretty much every menu too.
It's not a bad design choice, but maybe a little bit flawed implementation on my part.
The first screenshot I included is too transparent. I only set it so high so it is visible.
Normally you would use around 20% instead of 40%.
It would nice to have a fully-fledged design language which would specify grid stuff, animations, etc. Something like Fluent design(https://fluent.microsoft.com) or Material design(https://material.io/guidelines/#).
I think this should be a separate topic, so yeah...
Just to show you how it looks compared to something that is generally considered good design: Deepin
I choose the same wallpaper and menu locations.
I tested it with 30% and 40% transparency and 10/15 blur strength (that is likely going to be the new default).
KDE seems a little bit darker. That's because in Deepin the file manager background and the menu background is both white (255, 255, 255), while in KDE they are both gray (252, 252, 252).
I'm not sure a contrast trickery would improve it much.
+1 I wouldn't have this as a default for Breeze, but it looks really cool. I'm sure it'd be a popular option, specially among those trying to mimic macOS.
Hello,
thanks for taking care of 1, 2 and 3.
for 4 (the use case), I am adding @alake (one of the original designers of the breeze style) and @colomar as reviewer so that they have a chance to give their opinion too.
My personal (not so important) worry, is that we are adding yet another option to the style for something which is only eye candy (in my understanding it brings no improvement to usability, quite the contrary in fact) and the code complexity that goes with it. IMHO this change somewhat muddies Breeze's identity (flat and simple, not bloated) for no real gain. This said, I do appreciate your work.
Opinions welcome
Hugo
The exact opposite.
By default the Breeze style desktop theme is transparent and blurry. Kicker and Kickoff are transparent. The plasmoids on the panel are transparent.
This change can just make it more consistent.
I think the "blur" option should go. Blur should be controlled centrally by the desktop effect. In other words: BlurBehind should always be set to true, and then left to kwin to handle. Having an extra option here seems like micro-management. Why would you need blur behind plasma widgets (as handled by the desktop effect) and not behind menus ? (or vice versa). Also, right now, selecting "blur" in the style settings, but unselecting the desktop effect results in no blur behind menu, and hence inconsistency with what the style option says.
KWin has no way of knowing which pixels should be blurred behind a window. That's the reason the hint exists.
If you set the hint unconditionally, the effect will be applied unconditionally. Even when the menus are fully opaque.
Correct.
So the call to enableBlurBehind should be made only if "asAlphaChannel" _and_ opacity < 1
This still does not need an extra option.
Added comment in the code about this.
Thanks for the clarification.
kstyle/breezestyle.cpp | ||
---|---|---|
3629 | This should be called only if StyleConfigData::menuOpacity() is < 100 |
Now only enabled blur when the frame is transparent.
@hpereiradacosta
drawPanelMenuPrimitive only runs once per panel creation and not 60 times per second, right?
This is what enabled blur behind a window: https://github.com/KDE/kwindowsystem/blob/master/src/platforms/xcb/kwindoweffects.cpp#L245
I don't think it checks if it's already enabled.
But if it's called only once per panel then it's not an issue.
Well it is also probably called every time the selected (on mouse-over) item changes. So basically everytime you move the mouse in the list.
Now setting the property only once per menu (or rather, only at creation or when its sizes change), would require quite a bit more machinery. Probably something like a "breezeblurhelper" to which you would need to register the menu at creation (in ::polish) and then deal with resizeevent and configuration changes inside there.
Unless you feel like giving it a shot yourself, I could probably take care of that when this code is committed.
There is some code of this kind (more complex though), in the oxygen repository. It is called oxygenblurhelper.
This is what enabled blur behind a window: https://github.com/KDE/kwindowsystem/blob/master/src/platforms/xcb/kwindoweffects.cpp#L245
I don't think it checks if it's already enabled.
But if it's called only once per panel then it's not an issue.
Hello,
Thanks for the updated patch and for the work on BlurHelper.
Couple more comments below.
Hugo
kstyle/breezeblurhelper.cpp | ||
---|---|---|
144 ↗ | (On Diff #26407) | ok. So this is awkward. The whole code (the whole class in fact), does nothing for KDE4 (because of KWindowEffects not being available). For KF5, it is definitly better to use KWindowsEffect. so fine.
|
kstyle/breezestyle.cpp | ||
3632 | This should go to Breeze::Style::polish. |
Implemented the changes suggested by @hpereiradacosta
Only KDE5 Breeze has transparency and blur now
KDE4 Breeze has no trasnparency and blur anymore
@hpereiradacosta can you point me to some KDE4 apps that are still in use? I'm just curious what hasn't been ported yet.
last round of modifications: simplifications of the blurHelper class.
Also, for the config ui, maybe it makes more sense to call the "Transparency" tab, only "Menus" and change "Menu:" into "Transparency:"
Unless of course one wants to add transparency behind other widgets.
Then I think it is good to go.
kstyle/breezeblurhelper.h | ||
---|---|---|
138 ↗ | (On Diff #26814) | Because the use of this class is much simplified wrt the original one, it can be significantly simplified: _timer (and timerEvent) are unnecessary. |
Maybe in the future someone wants to add this transparency to (for example) dropdown menu, combobox, or other elements.
I say we should keep the naming vague.
Can you post an updated screenshot about the configuration option, and can we get VDG's opinion on it ?