D10170: Added optional transparency/blur to menu frames

This revision was automatically updated to reflect the committed changes. Closed by commit R31:e5c0824adf8a: Added optional transparency/blur to menu frames (authored by anemeth, committed by zzag). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D10170?vs=27358=27368#toc REPOSITORY R31

D10170: Added optional transparency/blur to menu frames

zzag added a comment. I've reverted that commit. REPOSITORY R31 Breeze REVISION DETAIL https://phabricator.kde.org/D10170 To: anemeth, hpereiradacosta, #plasma, colomar, alake Cc: fredrik, alake, januz, abetts, colomar, andreask, zzag, ngraham, plasma-devel, ZrenBot, progwolff,

D10170: Added optional transparency/blur to menu frames

zzag reopened this revision. This revision is now accepted and ready to land. REPOSITORY R31 Breeze REVISION DETAIL https://phabricator.kde.org/D10170 To: anemeth, hpereiradacosta, #plasma, colomar, alake Cc: fredrik, alake, januz, abetts, colomar, andreask, zzag, ngraham, plasma-devel,

D10170: Added optional transparency/blur to menu frames

zzag added a comment. This breaks build. Missed breezeblurhelper.cpp and breezeblurhelper.h? REPOSITORY R31 Breeze REVISION DETAIL https://phabricator.kde.org/D10170 To: anemeth, hpereiradacosta, #plasma, colomar, alake Cc: fredrik, alake, januz, abetts, colomar, andreask, zzag,

D10170: Added optional transparency/blur to menu frames

This revision was automatically updated to reflect the committed changes. Closed by commit R31:336cf7b63a6a: Added option to set transparency and blur behind menu frames such as right… (authored by hpereiradacosta). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D10170?vs=27010=27358#toc

D10170: Added optional transparency/blur to menu frames

anemeth added a comment. @hpereiradacosta I don't have commit access. Could you please commit it for me? REPOSITORY R31 Breeze BRANCH master REVISION DETAIL https://phabricator.kde.org/D10170 To: anemeth, hpereiradacosta, #plasma, colomar, alake Cc: fredrik, alake, januz, abetts,

D10170: Added optional transparency/blur to menu frames

hpereiradacosta accepted this revision. hpereiradacosta added a comment. This revision is now accepted and ready to land. ok. No reaction on the UI. Ship it ! Thanks for the work and the patience. REPOSITORY R31 Breeze BRANCH master REVISION DETAIL

D10170: Added optional transparency/blur to menu frames

anemeth added a comment. In D10170#205000 , @hpereiradacosta wrote: > Can you post an updated screenshot about the configuration option, and can we get VDG's opinion on it ? This is how it looks now F5707716: cvb.PNG

D10170: Added optional transparency/blur to menu frames

anemeth marked 4 inline comments as done. REPOSITORY R31 Breeze REVISION DETAIL https://phabricator.kde.org/D10170 To: anemeth, hpereiradacosta, #plasma, colomar, alake Cc: fredrik, alake, januz, abetts, colomar, andreask, zzag, ngraham, plasma-devel, ZrenBot, progwolff, lesliezhai,

D10170: Added optional transparency/blur to menu frames

hpereiradacosta added a comment. In D10170#204976 , @anemeth wrote: > In D10170#204889 , @hpereiradacosta wrote: > > > Also, for the config ui, maybe it makes more sense to call the "Transparency"

D10170: Added optional transparency/blur to menu frames

anemeth updated this revision to Diff 27010. anemeth added a comment. Removed the _helper variable from the breezeblurhelper class REPOSITORY R31 Breeze CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10170?vs=27008=27010 BRANCH master REVISION DETAIL

D10170: Added optional transparency/blur to menu frames

hpereiradacosta added inline comments. INLINE COMMENTS > breezeblurhelper.cpp:45 > +QObject(parent), > +_helper(helper) > +{ helper argument is not needed > breezeblurhelper.h:53 > +//! constructor > +BlurHelper( QObject*, Helper& ); > + helper argument is

D10170: Added optional transparency/blur to menu frames

anemeth added a comment. In D10170#204889 , @hpereiradacosta wrote: > 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

D10170: Added optional transparency/blur to menu frames

anemeth updated this revision to Diff 27008. anemeth added a comment. Simplified the breezeblurhelper class REPOSITORY R31 Breeze CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10170?vs=26814=27008 BRANCH master REVISION DETAIL https://phabricator.kde.org/D10170 AFFECTED

D10170: Added optional transparency/blur to menu frames

anemeth marked an inline comment as done. REPOSITORY R31 Breeze REVISION DETAIL https://phabricator.kde.org/D10170 To: anemeth, hpereiradacosta, #plasma, colomar, alake Cc: fredrik, alake, januz, abetts, colomar, andreask, zzag, ngraham, plasma-devel, ZrenBot, progwolff, lesliezhai,

D10170: Added optional transparency/blur to menu frames

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

D10170: Added optional transparency/blur to menu frames

anemeth updated this revision to Diff 26814. anemeth added a comment. Fixed a visual glitch when using a scaled display REPOSITORY R31 Breeze CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10170?vs=26658=26814 BRANCH master REVISION DETAIL https://phabricator.kde.org/D10170

D10170: Added optional transparency/blur to menu frames

hpereiradacosta added a comment. In https://phabricator.kde.org/D10170#202023, @anemeth wrote: > 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. Amarok

D10170: Added optional transparency/blur to menu frames

anemeth marked 2 inline comments as done. anemeth added a comment. KDE4 Breeze has no trasnparency and blur anymore REPOSITORY R31 Breeze REVISION DETAIL https://phabricator.kde.org/D10170 To: anemeth, hpereiradacosta, #plasma, colomar, alake Cc: fredrik, alake, januz, abetts, colomar,

D10170: Added optional transparency/blur to menu frames

anemeth updated this revision to Diff 26658. anemeth added a comment. Implemented the changes suggested by @hpereiradacosta Only KDE5 Breeze has transparency and blur now REPOSITORY R31 Breeze CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10170?vs=26407=26658 BRANCH master

D10170: Added optional transparency/blur to menu frames

hpereiradacosta added a comment. Hello, Thanks for the updated patch and for the work on BlurHelper. Couple more comments below. Hugo INLINE COMMENTS > breezeblurhelper.cpp:144 > +if (!blurRegion.isEmpty()) { > +#if !BREEZE_USE_KDE4 > +

D10170: Added optional transparency/blur to menu frames

anemeth updated this revision to Diff 26407. anemeth added a comment. Added breezeblurhelper files REPOSITORY R31 Breeze CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10170?vs=26406=26407 BRANCH master REVISION DETAIL https://phabricator.kde.org/D10170 AFFECTED FILES

D10170: Added optional transparency/blur to menu frames

anemeth updated this revision to Diff 26406. anemeth added a comment. Moved blur handling to breezeblurhelper REPOSITORY R31 Breeze CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10170?vs=26329=26406 BRANCH master REVISION DETAIL https://phabricator.kde.org/D10170 AFFECTED

D10170: Added optional transparency/blur to menu frames

hpereiradacosta added a comment. In https://phabricator.kde.org/D10170#199153, @anemeth wrote: > Now only enabled blur when the frame is transparent. > > @hpereiradacosta > drawPanelMenuPrimitive only runs once per panel creation and not 60 times per second, right? Well it

D10170: Added optional transparency/blur to menu frames

anemeth marked 4 inline comments as done. REPOSITORY R31 Breeze REVISION DETAIL https://phabricator.kde.org/D10170 To: anemeth, hpereiradacosta, #plasma, colomar, alake Cc: fredrik, alake, januz, abetts, colomar, andreask, zzag, ngraham, plasma-devel, ZrenBot, progwolff, lesliezhai,

D10170: Added optional transparency/blur to menu frames

anemeth updated this revision to Diff 26329. anemeth added a comment. 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:

D10170: Added optional transparency/blur to menu frames

hpereiradacosta added a comment. In https://phabricator.kde.org/D10170#199003, @fredrik wrote: > > 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.

D10170: Added optional transparency/blur to menu frames

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

D10170: Added optional transparency/blur to menu frames

anemeth added a comment. In https://phabricator.kde.org/D10170#198972, @hpereiradacosta wrote: > IMHO this change somewhat muddies Breeze's identity (flat and simple, not bloated) for no real gain. This said, I do appreciate your work. The exact opposite. By default the Breeze

D10170: Added optional transparency/blur to menu frames

hpereiradacosta added reviewers: colomar, alake. hpereiradacosta added a subscriber: alake. hpereiradacosta added a comment. 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

D10170: Added optional transparency/blur to menu frames

januz added a comment. +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. REPOSITORY R31 Breeze REVISION DETAIL https://phabricator.kde.org/D10170 To: anemeth, hpereiradacosta,

D10170: Added optional transparency/blur to menu frames

ngraham added a comment. OK, I have to admit that those examples look really, really good. 30% more than 40%. REPOSITORY R31 Breeze REVISION DETAIL https://phabricator.kde.org/D10170 To: anemeth, hpereiradacosta, #plasma Cc: abetts, colomar, andreask, zzag, ngraham, plasma-devel,

D10170: Added optional transparency/blur to menu frames

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

D10170: Added optional transparency/blur to menu frames

zzag added a comment. In https://phabricator.kde.org/D10170#198659, @anemeth wrote: > 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

D10170: Added optional transparency/blur to menu frames

zzag added a comment. In https://phabricator.kde.org/D10170#198656, @ngraham wrote: > Sort of/mostly: https://community.kde.org/KDE_Visual_Design_Group/HIG It would nice to have a fully-fledged design language which would specify grid stuff, animations, etc. Something like Fluent

D10170: Added optional transparency/blur to menu frames

anemeth updated this revision to Diff 26278. anemeth added a comment. . REPOSITORY R31 Breeze CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10170?vs=26160=26278 BRANCH master REVISION DETAIL https://phabricator.kde.org/D10170 AFFECTED FILES kstyle/breeze.kcfg

D10170: Added optional transparency/blur to menu frames

ngraham added a comment. In https://phabricator.kde.org/D10170#198655, @zzag wrote: > @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

D10170: Added optional transparency/blur to menu frames

zzag added a comment. @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) REPOSITORY R31

D10170: Added optional transparency/blur to menu frames

abetts added a comment. In https://phabricator.kde.org/D10170#198634, @ngraham wrote: > 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

D10170: Added optional transparency/blur to menu frames

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

D10170: Added optional transparency/blur to menu frames

zzag added a comment. In https://phabricator.kde.org/D10170#198591, @ngraham wrote: > Since then, both of those companies have mostly moved away from blur transparency in their operating systems, or made it optional. Did they? Microsoft is doing some work on Fluent design

D10170: Added optional transparency/blur to menu frames

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

D10170: Added optional transparency/blur to menu frames

zzag added a comment. Also, please, take into account contrast issues. That's nice to have blur, but, please, use it wisely. REPOSITORY R31 Breeze REVISION DETAIL https://phabricator.kde.org/D10170 To: anemeth, hpereiradacosta, #plasma Cc: abetts, colomar, andreask, zzag, ngraham,

D10170: Added optional transparency/blur to menu frames

abetts removed a reviewer: VDG. REPOSITORY R31 Breeze REVISION DETAIL https://phabricator.kde.org/D10170 To: anemeth, hpereiradacosta, #plasma Cc: abetts, colomar, andreask, zzag, ngraham, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart

D10170: Added optional transparency/blur to menu frames

abetts added subscribers: andreask, colomar, abetts. abetts added a comment. In https://phabricator.kde.org/D10170#198471, @hpereiradacosta wrote: > Hello, > Hi, > thanks for the patch ! > > Few comments: > 1/ the code does not compile under kde4 (cmake -DUSE_KDE4). You

D10170: Added optional transparency/blur to menu frames

abetts removed a reviewer: abetts. This revision now requires review to proceed. REPOSITORY R31 Breeze REVISION DETAIL https://phabricator.kde.org/D10170 To: anemeth, hpereiradacosta, #plasma, #vdg Cc: zzag, ngraham, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed,

D10170: Added optional transparency/blur to menu frames

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

D10170: Added optional transparency/blur to menu frames

abetts accepted this revision. This revision is now accepted and ready to land. REPOSITORY R31 Breeze BRANCH master REVISION DETAIL https://phabricator.kde.org/D10170 To: anemeth, hpereiradacosta, #plasma, #vdg, abetts Cc: zzag, ngraham, plasma-devel, ZrenBot, progwolff, lesliezhai,

D10170: Added optional transparency/blur to menu frames

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

D10170: Added optional transparency/blur to menu frames

zzag added a comment. In https://phabricator.kde.org/D10170#197851, @anemeth wrote: > Just tried that. > Nothing changed. Try to change `contrast`, `intensity`, and `saturation` args. REPOSITORY R31 Breeze REVISION DETAIL https://phabricator.kde.org/D10170 To: anemeth,

D10170: Added optional transparency/blur to menu frames

zzag added inline comments. INLINE COMMENTS > anemeth wrote in breezestyle.cpp:3630 > The slider has 10 steps and goes from 0 to 10, that's why it's `/ 10.0` > 100 steps would be too much in my opinion. > > And we need to invert the value of it because the setAlphaF works inverted to > our

D10170: Added optional transparency/blur to menu frames

anemeth added inline comments. INLINE COMMENTS > zzag wrote in breezestyle.cpp:3630 > buggy.. > > `background.setAlphaF(StyleConfigData::menuTransparency() / 100.0f);` The slider has 10 steps and goes from 0 to 10, that's why it's `/ 10.0` 100 steps would be too much in my opinion. And we

D10170: Added optional transparency/blur to menu frames

anemeth added a comment. In https://phabricator.kde.org/D10170#197849, @zzag wrote: > In https://phabricator.kde.org/D10170#197845, @anemeth wrote: > > > 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

D10170: Added optional transparency/blur to menu frames

zzag added a comment. In https://phabricator.kde.org/D10170#197845, @anemeth wrote: > 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. Did you call

D10170: Added optional transparency/blur to menu frames

anemeth added inline comments. INLINE COMMENTS > zzag wrote in breeze.kcfg:188 > Redundant comment. What's redundant about it? REPOSITORY R31 Breeze REVISION DETAIL https://phabricator.kde.org/D10170 To: anemeth, hpereiradacosta, #plasma, #vdg Cc: zzag, ngraham, plasma-devel, ZrenBot,

D10170: Added optional transparency/blur to menu frames

anemeth added a comment. In https://phabricator.kde.org/D10170#197812, @zzag wrote: > 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? I presume you mean the

D10170: Added optional transparency/blur to menu frames

zzag added inline comments. INLINE COMMENTS > breeze.kcfg:189 > + > + > +0 0 100 maybe something like this? > breezestyle.cpp:3630 > +if (hasAlpha) { > +background.setAlphaF(1.0 - > StyleConfigData::menuTransparencyPercent() / 10.0); > +

D10170: Added optional transparency/blur to menu frames

zzag added inline comments. INLINE COMMENTS > breeze.kcfg:188 > > + > + Redundant comment. REPOSITORY R31 Breeze REVISION DETAIL https://phabricator.kde.org/D10170 To: anemeth, hpereiradacosta, #plasma, #vdg Cc: zzag, ngraham, plasma-devel, ZrenBot, progwolff, lesliezhai,

D10170: Added optional transparency/blur to menu frames

zzag added a comment. Looks great! BUT, it would be nice to have a little bit bigger contrast. Could you please enable background contrast and upload what it looks like? REPOSITORY R31 Breeze REVISION DETAIL https://phabricator.kde.org/D10170 To: anemeth, hpereiradacosta, #plasma,

D10170: Added optional transparency/blur to menu frames

anemeth added a project: Breeze. REPOSITORY R31 Breeze REVISION DETAIL https://phabricator.kde.org/D10170 To: anemeth, hpereiradacosta, #plasma, #vdg Cc: ngraham, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D10170: Added optional transparency/blur to menu frames

anemeth retitled this revision from "Added transparency/blur to menu frames" to "Added optional transparency/blur to menu frames". REPOSITORY R31 Breeze REVISION DETAIL https://phabricator.kde.org/D10170 To: anemeth, hpereiradacosta, #plasma, #vdg Cc: plasma-devel, ZrenBot, progwolff,