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<QMenu*>( 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

Reply via email to