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
