D22195: Split Compositor class in Wayland and X11 child classes

zzag added a comment.


  Will support properties stay in the Compositor class?

INLINE COMMENTS

> romangg wrote in composite.cpp:178
> Maybe. Is it a blocker or a necessity to this patch here? Otherwise let's tackle one idea after the other, shall we?

Well, some parts of KWin(as well Qt) follow this pattern, e.g. EffectsHandlerImpl, so it's a good idea to stick with this pattern in Compositor class. Given that the Compositor class still has X11 bits, I guess we can go with this approach for now.

> composite.h:49
>      ~Compositor() override;
> +    static Compositor* self();
>  

"For pointers or references, use a single space before '*' or '&', but not after"

(there are other places in this patch where whitespace for pointers needs to be fixed)

> composite.h:192
> +    static WaylandCompositor* create(QObject *parent = nullptr);
> +    ~WaylandCompositor() override = default;
> +

I guess we don't need it yet.

> dbusinterface.cpp:313
> +    if (kwinApp()->operationMode() == Application::OperationModeX11) {
> +        static_cast<X11Compositor*>(m_compositor)->resume(X11Compositor::ScriptSuspend);
> +    }

I know that I already mentioned the problem with whitespace before pointers, but this one may not be obvious.

"For pointers or references, use a single space before '*' or '&', but not after"

applies to templates as well, so the static_cast will look like

  static_cast<X11Compositor *>(m_compositor)->....

(I hope that one day we will be able to run clang-format over KWin's code bbase)

REPOSITORY
  R108 KWin

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

To: romangg, #kwin
Cc: anthonyfieroni, zzag, kwin, LeGast00n, sbergeron, jraleigh, fbampaloukas, GB_2, mkulinski, ragreen, jackyalcine, Pitel, iodelay, crozbo, bwowk, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, hardening, jensreuterberg, abetts, sebas, apol, mart