D9487: [effects/slide] Handle moving clients

[Martin Flöser]

graesslin requested changes to this revision.
graesslin added inline comments.
This revision now requires changes to proceed.
Restricted Application edited projects, added KWin; removed Plasma.

INLINE COMMENTS

> slide.cpp:93-94
>              }
> +        } else if (w == m_movingWindow) {
> +            // A window which is being moved is always inside our viewport, do nothing.
>          } else if (w->isOnDesktop(painting_desktop)) {

I don't like empty if blocks. Please handle this e.g. with if (slide && w != m_movingWindow)

> slide.cpp:282
> +
> +    m_movingWindow = with;
> +

The window is nowhere reset, also it's not handling the windowClosed/windowRemoved case. This means the pointer could dangle.

> slide.h:67
>      QPoint slide_painting_diff;
> +    EffectWindow* m_movingWindow;
>  

You can simplify:

  EffectWindow *m_movingWindow = nullptr;

REPOSITORY
  R108 KWin

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

To: zzag, #kwin, #plasma, graesslin
Cc: graesslin, plasma-devel, kwin, bwowk, ZrenBot, progwolff, lesliezhai, ali-mohamed, hardening, jensreuterberg, abetts, sebas, apol, mart
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20171223/d5577d70/attachment.html>