D10170: Added optional transparency/blur to menu frames

[Vlad Zagorodniy]

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 Breeze

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D10170?vs=27358=27368

REVISION DETAIL
  https://phabricator.kde.org/D10170

AFFECTED FILES
  kstyle/CMakeLists.txt
  kstyle/breeze.kcfg
  kstyle/breezeblurhelper.cpp
  kstyle/breezeblurhelper.h
  kstyle/breezestyle.cpp
  kstyle/breezestyle.h
  kstyle/config/breezestyleconfig.cpp
  kstyle/config/ui/breezestyleconfig.ui

To: anemeth, hpereiradacosta, #plasma, colomar, alake
Cc: fredrik, alake, januz, abetts, colomar, andreask, zzag, ngraham, 
plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
sebas, apol, mart

D10170: Added optional transparency/blur to menu frames

[Vlad Zagorodniy]

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, lesliezhai, ali-mohamed, jensreuterberg, 
sebas, apol, mart

D10170: Added optional transparency/blur to menu frames

[Vlad Zagorodniy]

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, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
sebas, apol, mart

D10170: Added optional transparency/blur to menu frames

[Vlad Zagorodniy]

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, ngraham, 
plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
sebas, apol, mart

D10170: Added optional transparency/blur to menu frames

[Hugo Pereira Da Costa]

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

REPOSITORY
  R31 Breeze

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D10170?vs=27010=27358

REVISION DETAIL
  https://phabricator.kde.org/D10170

AFFECTED FILES
  kstyle/CMakeLists.txt
  kstyle/breeze.kcfg
  kstyle/breezestyle.cpp
  kstyle/breezestyle.h
  kstyle/config/breezestyleconfig.cpp
  kstyle/config/ui/breezestyleconfig.ui

To: anemeth, hpereiradacosta, #plasma, colomar, alake
Cc: fredrik, alake, januz, abetts, colomar, andreask, zzag, ngraham, 
plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
sebas, apol, mart

D10170: Added optional transparency/blur to menu frames

[Alex Nemeth]

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, colomar, andreask, zzag, ngraham, 
plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
sebas, apol, mart

D10170: Added optional transparency/blur to menu frames

[Hugo Pereira Da Costa]

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
  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, ali-mohamed, jensreuterberg, 
sebas, apol, mart

D10170: Added optional transparency/blur to menu frames

[Alex Nemeth]

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 
  I made it look like the Translucency effect config
  F5707719: bvc.PNG 

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, ali-mohamed, jensreuterberg, 
sebas, apol, mart

D10170: Added optional transparency/blur to menu frames

[Alex Nemeth]

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, ali-mohamed, jensreuterberg, 
sebas, apol, mart

D10170: Added optional transparency/blur to menu frames

[Hugo Pereira Da Costa]

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" tab, only "Menus" and change "Menu:" into "Transparency:"
  > >  Unless of course one wants to add transparency behind other widgets.
  >
  >
  > 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 ?

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, ali-mohamed, jensreuterberg, 
sebas, apol, mart

D10170: Added optional transparency/blur to menu frames

[Alex Nemeth]

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
  https://phabricator.kde.org/D10170

AFFECTED FILES
  kstyle/CMakeLists.txt
  kstyle/breeze.kcfg
  kstyle/breezeblurhelper.cpp
  kstyle/breezeblurhelper.h
  kstyle/breezestyle.cpp
  kstyle/breezestyle.h
  kstyle/config/breezestyleconfig.cpp
  kstyle/config/ui/breezestyleconfig.ui

To: anemeth, hpereiradacosta, #plasma, colomar, alake
Cc: fredrik, alake, januz, abetts, colomar, andreask, zzag, ngraham, 
plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
sebas, apol, mart

D10170: Added optional transparency/blur to menu frames

[Hugo Pereira Da Costa]

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 not needed

> breezeblurhelper.h:80
> +
> +protected Q_SLOTS:
> +

this can go.
No more slots :)

> breezeblurhelper.h:85
> +//* helper
> +Helper _helper;
> +

this can go

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, ali-mohamed, jensreuterberg, 
sebas, apol, mart

D10170: Added optional transparency/blur to menu frames

[Alex Nemeth]

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 transparency behind other widgets.
  
  
  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.

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, ali-mohamed, jensreuterberg, 
sebas, apol, mart

D10170: Added optional transparency/blur to menu frames

[Alex Nemeth]

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 FILES
  kstyle/CMakeLists.txt
  kstyle/breeze.kcfg
  kstyle/breezeblurhelper.cpp
  kstyle/breezeblurhelper.h
  kstyle/breezestyle.cpp
  kstyle/breezestyle.h
  kstyle/config/breezestyleconfig.cpp
  kstyle/config/ui/breezestyleconfig.ui

To: anemeth, hpereiradacosta, #plasma, colomar, alake
Cc: fredrik, alake, januz, abetts, colomar, andreask, zzag, ngraham, 
plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
sebas, apol, mart

D10170: Added optional transparency/blur to menu frames

[Alex Nemeth]

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, ali-mohamed, jensreuterberg, 
sebas, apol, mart

D10170: Added optional transparency/blur to menu frames

[Hugo Pereira Da Costa]

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 behind other widgets.
  
  Then I think it is good to go.

INLINE COMMENTS

> breezeblurhelper.h:138
> +
> +#endif

Because the use of this class is much simplified wrt the original one, it can 
be significantly simplified:

_timer (and timerEvent) are unnecessary.
Then update() could be removed, as well as _widgets, as well as the 
widgetDestroyed callback.
Then update(widget) could be called directly in the event filter.

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, ali-mohamed, jensreuterberg, 
sebas, apol, mart

D10170: Added optional transparency/blur to menu frames

[Alex Nemeth]

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

AFFECTED FILES
  kstyle/CMakeLists.txt
  kstyle/breeze.kcfg
  kstyle/breezeblurhelper.cpp
  kstyle/breezeblurhelper.h
  kstyle/breezestyle.cpp
  kstyle/breezestyle.h
  kstyle/config/breezestyleconfig.cpp
  kstyle/config/ui/breezestyleconfig.ui

To: anemeth, hpereiradacosta, #plasma, colomar, alake
Cc: fredrik, alake, januz, abetts, colomar, andreask, zzag, ngraham, 
plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
sebas, apol, mart

D10170: Added optional transparency/blur to menu frames

[Hugo Pereira Da Costa]

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 at least, clementine (?), 
  vlc  (?)
  many qt4 based applications not ported to qt5, such as backintime
  
  Anyway, thanks for the updated patch !
  Lets wait for a couple more days for someone from VDG to show up (@colomar ? 
@alake ?), and if none shows up lets land it.

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, ali-mohamed, jensreuterberg, 
sebas, apol, mart

D10170: Added optional transparency/blur to menu frames

[Alex Nemeth]

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, andreask, zzag, ngraham, 
plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
sebas, apol, mart

D10170: Added optional transparency/blur to menu frames

[Alex Nemeth]

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

REVISION DETAIL
  https://phabricator.kde.org/D10170

AFFECTED FILES
  kstyle/CMakeLists.txt
  kstyle/breeze.kcfg
  kstyle/breezeblurhelper.cpp
  kstyle/breezeblurhelper.h
  kstyle/breezestyle.cpp
  kstyle/breezestyle.h
  kstyle/config/breezestyleconfig.cpp
  kstyle/config/ui/breezestyleconfig.ui

To: anemeth, hpereiradacosta, #plasma, colomar, alake
Cc: fredrik, alake, januz, abetts, colomar, andreask, zzag, ngraham, 
plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
sebas, apol, mart

D10170: Added optional transparency/blur to menu frames

[Hugo Pereira Da Costa]

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
> +KWindowEffects::enableBlurBehind(widget->winId(), true, 
> blurRegion);

ok. So this is awkward. The whole code (the whole class in fact), does nothing 
for KDE4 (because of KWindowEffects not being available).
We should then not have the event filters installed, nor even the blur helper 
created, etc.
Alternatively, one would need to duplicate the code from KWIndowEffect. as done 
in the original oxygen code.

For KF5, it is definitly better to use KWindowsEffect. so fine.
For kDE4 applications, not having the blur behind the translucent window is an 
issue (and there still are some kde4 applications around).
So there, one should either

- duplicate the code (for this #ifdef branch only
- disable the blur (as now), and possibly also the transparency (in order not 
to endup with transparent menus and no blur).

> breezestyle.cpp:3632
> +// force registration of widget
> +_blurHelper->registerWidget( widget->window() );
> +}

This should go to Breeze::Style::polish.
There is already a "if( qobject_cast( widget )" there. Just move this 
to the corresponding code block.

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, ali-mohamed, jensreuterberg, 
sebas, apol, mart

D10170: Added optional transparency/blur to menu frames

[Alex Nemeth]

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
  kstyle/CMakeLists.txt
  kstyle/breeze.kcfg
  kstyle/breezeblurhelper.cpp
  kstyle/breezeblurhelper.h
  kstyle/breezestyle.cpp
  kstyle/breezestyle.h
  kstyle/config/breezestyleconfig.cpp
  kstyle/config/ui/breezestyleconfig.ui

To: anemeth, hpereiradacosta, #plasma, colomar, alake
Cc: fredrik, alake, januz, abetts, colomar, andreask, zzag, ngraham, 
plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
sebas, apol, mart

D10170: Added optional transparency/blur to menu frames

[Alex Nemeth]

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 FILES
  kstyle/CMakeLists.txt
  kstyle/breeze.kcfg
  kstyle/breezestyle.cpp
  kstyle/breezestyle.h
  kstyle/config/breezestyleconfig.cpp
  kstyle/config/ui/breezestyleconfig.ui

To: anemeth, hpereiradacosta, #plasma, colomar, alake
Cc: fredrik, alake, januz, abetts, colomar, andreask, zzag, ngraham, 
plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
sebas, apol, mart

D10170: Added optional transparency/blur to menu frames

[Hugo Pereira Da Costa]

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 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 was some code of this kind (more complex though), in the now 
decommissioned oxygen-transparent code. That was 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.

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, ali-mohamed, jensreuterberg, 
sebas, apol, mart

D10170: Added optional transparency/blur to menu frames

[Alex Nemeth]

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, ali-mohamed, jensreuterberg, 
sebas, apol, mart

D10170: Added optional transparency/blur to menu frames

[Alex Nemeth]

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: 
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.

REPOSITORY
  R31 Breeze

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D10170?vs=26278=26329

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D10170

AFFECTED FILES
  kstyle/breeze.kcfg
  kstyle/breezestyle.cpp
  kstyle/config/breezestyleconfig.cpp
  kstyle/config/ui/breezestyleconfig.ui

To: anemeth, hpereiradacosta, #plasma, colomar, alake
Cc: fredrik, alake, januz, abetts, colomar, andreask, zzag, ngraham, 
plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
sebas, apol, mart

D10170: Added optional transparency/blur to menu frames

[Hugo Pereira Da Costa]

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. 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.

INLINE COMMENTS

> breezestyle.cpp:3630
> +#if !BREEZE_USE_KDE4
> +KWindowEffects::enableBlurBehind(widget->winId(), true);
> +#endif

This should be called only if StyleConfigData::menuOpacity() is < 100
And in fact, I wonder if it is safe to make the call at every painting call. 
I guess it depends on how the underlying function is implemented (does it first 
check for the property ?)

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, ali-mohamed, jensreuterberg, 
sebas, apol, mart

D10170: Added optional transparency/blur to menu frames

[Fredrik Höglund]

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 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.

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, ali-mohamed, jensreuterberg, 
sebas, apol, mart

D10170: Added optional transparency/blur to menu frames

[Alex Nemeth]

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 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.

REPOSITORY
  R31 Breeze

REVISION DETAIL
  https://phabricator.kde.org/D10170

To: anemeth, hpereiradacosta, #plasma, colomar, alake
Cc: alake, januz, abetts, colomar, andreask, zzag, ngraham, plasma-devel, 
ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart

D10170: Added optional transparency/blur to menu frames

[Hugo Pereira Da Costa]

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 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

REPOSITORY
  R31 Breeze

REVISION DETAIL
  https://phabricator.kde.org/D10170

To: anemeth, hpereiradacosta, #plasma, colomar, alake
Cc: alake, januz, abetts, colomar, andreask, zzag, ngraham, plasma-devel, 
ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart

D10170: Added optional transparency/blur to menu frames

[Diego Gangl]

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, #plasma
Cc: januz, abetts, colomar, andreask, zzag, ngraham, plasma-devel, ZrenBot, 
progwolff, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart

D10170: Added optional transparency/blur to menu frames

[Nathaniel Graham]

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, ZrenBot, progwolff, 
lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart

D10170: Added optional transparency/blur to menu frames

[Alex Nemeth]

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).
  
  F5685908: qqq.PNG 
  
  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.

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

[Vlad Zagorodniy]

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 if I change the contrast or intensity.
  
  
  Could you please share screenshots?

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

[Vlad Zagorodniy]

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 
design(https://fluent.microsoft.com) or Material 
design(https://material.io/guidelines/#).
  
  I think this should be a separate topic, so yeah...

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

[Alex Nemeth]

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
  kstyle/breezestyle.cpp
  kstyle/config/breezestyleconfig.cpp
  kstyle/config/ui/breezestyleconfig.ui

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

[Nathaniel Graham]

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 have a design language? (like Fluent or Material design)
  
  
  Sort of/mostly: https://community.kde.org/KDE_Visual_Design_Group/HIG

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

[Vlad Zagorodniy]

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 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

[Andres Betts]

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 this hill though, since it's being proposed as an optional 
feature and not a new default.
  
  
  I think that's a good way to look at it. I would be OK to make it an option.

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

[Nathaniel Graham]

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 not a new default.

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

[Vlad Zagorodniy]

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 (https://fluent.microsoft.com/).
  macOS is still using blur extensively.

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

[Nathaniel Graham]

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 such a low level 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.

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

[Vlad Zagorodniy]

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, plasma-devel, ZrenBot, progwolff, 
lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart

D10170: Added optional transparency/blur to menu frames

[Andres Betts]

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

[Andres Betts]

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 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  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.
  
  
  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

REPOSITORY
  R31 Breeze

REVISION DETAIL
  https://phabricator.kde.org/D10170

To: anemeth, hpereiradacosta, #plasma, #vdg
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

[Andres Betts]

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, 
jensreuterberg, abetts, sebas, apol, mart

D10170: Added optional transparency/blur to menu frames

[Hugo Pereira Da Costa]

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. 
  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

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, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D10170: Added optional transparency/blur to menu frames

[Andres Betts]

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, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D10170: Added optional transparency/blur to menu frames

[Hugo Pereira Da Costa]

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 
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  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.

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, 
jensreuterberg, abetts, sebas, apol, mart

D10170: Added optional transparency/blur to menu frames

[Vlad Zagorodniy]

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, hpereiradacosta, #plasma, #vdg
Cc: zzag, ngraham, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D10170: Added optional transparency/blur to menu frames

[Vlad Zagorodniy]

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 slider and it needs a value between 0.0 and 1.0
> That's why we need the `1.0 - `
> It would be great if we could set the slider to go from 10 to 0 instead, but 
> I don't see how.

IMO, transparency should be in range 0-100.

> It would be great if we could set the slider to go from 10 to 0 instead, but 
> I don't see how.

Why?

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, 
jensreuterberg, abetts, sebas, apol, mart

D10170: Added optional transparency/blur to menu frames

[Alex Nemeth]

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 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
That's why we need the `1.0 - `
It would be great if we could set the slider to go from 10 to 0 instead, but I 
don't see how.

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, 
jensreuterberg, abetts, sebas, apol, mart

D10170: Added optional transparency/blur to menu frames

[Alex Nemeth]

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 not notice any 
difference.
  >
  >
  > Did you call `KWindowEffects::enableBackgroundContrast`?
  
  
  Just tried that.
  Nothing changed.

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, 
jensreuterberg, abetts, sebas, apol, mart

D10170: Added optional transparency/blur to menu frames

[Vlad Zagorodniy]

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 `KWindowEffects::enableBackgroundContrast`?

INLINE COMMENTS

> anemeth wrote in breeze.kcfg:188
> What's redundant about it?

It's obvious that the following entry is about transparency.

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, 
jensreuterberg, abetts, sebas, apol, mart

D10170: Added optional transparency/blur to menu frames

[Alex Nemeth]

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, progwolff, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D10170: Added optional transparency/blur to menu frames

[Alex Nemeth]

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 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%:
  
  F5681942: 40percent.PNG 
  
  30%:
  
  F5681944: 30percent.PNG 
  
  20%:
  
  F5681946: 20percent.PNG 
  
  10%:
  
  F5681948: 10percent.PNG 

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, 
jensreuterberg, abetts, sebas, apol, mart

D10170: Added optional transparency/blur to menu frames

[Vlad Zagorodniy]

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);
> +}

buggy..

`background.setAlphaF(StyleConfigData::menuTransparency() / 100.0f);`

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, 
jensreuterberg, abetts, sebas, apol, mart

D10170: Added optional transparency/blur to menu frames

[Vlad Zagorodniy]

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, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D10170: Added optional transparency/blur to menu frames

[Vlad Zagorodniy]

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, #vdg
Cc: zzag, ngraham, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D10170: Added optional transparency/blur to menu frames

[Alex Nemeth]

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

[Alex Nemeth]

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, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart