D5910: make shadows work on wayland

[David Edmundson]

davidedmundson added a comment.


  We're going to be leaking both Surface and Shadow objects in installWaylandFilter, no?

INLINE COMMENTS

> breezeshadowhelper.cpp:177
> +            QWidget* widget( static_cast<QWidget*>( object ) );
> +            if( event->type() == QEvent::Paint && ( !_widgets.contains(widget) || _widgets.value(widget) == 0 ) )
> +            {

we use Expose everywhere else, why not here? It gets called a lot less

> breezeshadowhelper.cpp:185
> +            {
> +                _widgets.insert( widget, 0 );
> +            }

why not just remove the widget from the list?

> breezeshadowhelper.cpp:464
>          using namespace KWayland::Client;
>          auto s = Surface::fromWindow( widget->windowHandle() );
>          if( !s ) return false;

this is currently leaking?

fromWindow constructs a new QObject

> breezeshadowhelper.cpp:467
>  
>          auto shadow = _shadowManager->createShadow( s, widget );
>          if( !shadow->isValid() ) return false;

this is going to be effectively leaking. on a show/hide/show we'll get now have two of these in memory, just one not attached to anything.

> breezeshadowhelper.cpp:482
>          shadow->commit();
>          s->commit( Surface::CommitFlag::None );
>  

off topic, but why do we have a commit on the surface? We haven't changed the surface, just attaced something to it.

REPOSITORY
  R31 Breeze

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

To: mart, #plasma, hpereiradacosta
Cc: davidedmundson, plasma-devel, ZrenBot, spstarr, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, lukas
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20170518/4d4a27a2/attachment.html>