D23011: Replace foreach loops and improve code style in composite

[Vlad Zagorodniy]

zzag added inline comments.

INLINE COMMENTS

> composite.cpp:272-273
>  
> -    kwinApp()->platform()->setSelectedCompositor(m_scene->compositingType() & OpenGLCompositing ? OpenGLCompositing : m_scene->compositingType());
> +    kwinApp()->platform()->setSelectedCompositor(m_scene->compositingType() & OpenGLCompositing ?
> +                                                    OpenGLCompositing : m_scene->compositingType());
>  

Personal opinion: I don't like that the ternary operator is minding its own business somewhere far right.

A better approach imho

  CompositingType compositingType = m_scene->compositingType();
  
  if (compositingType & OpenGLCompositing) {
      compositingType = OpenGLCompositing;
  }
  
  // ... one day
  // if (compositingType & VulkanCompositing) {
  //     compositingType = VulkanCompositing;
  // }
  
  kwinApp()->platform()->setSelectedCompositor(compositingType);

> composite.cpp:309
>  {
> -    auto c = kwinApp()->x11Connection();
> -    if (!c) {
> +    auto *con = kwinApp()->x11Connection();
> +    if (!con) {

No short names.

> composite.cpp:334
> -    if (auto w = waylandServer()) {
> -        const auto clients = w->clients();
> -        for (auto c : clients) {

Why did you remove auto?

> composite.cpp:350
> +
> +    for (Client *c : Workspace::self()->clientList()) {
>          c->setupCompositing();

If you want to follow the frameworks coding style 100%, then it should be ShellClient *client.

> composite.cpp:698-699
>          if (m_framesToTestForSafety == 0 && (m_scene->compositingType() & OpenGLCompositing)) {
> -            kwinApp()->platform()->createOpenGLSafePoint(Platform::OpenGLSafePoint::PostLastGuardedFrame);
> +            kwinApp()->platform()->createOpenGLSafePoint(Platform::
> +                                                         OpenGLSafePoint::PostLastGuardedFrame);
>          }

That's unusual line breaking.

A bit better imho

  kwinApp()->platform()->createOpenGLSafePoint(
      Platform::OpenGLSafePoint::PostLastGuardedFrame);

> composite.cpp:948
> +        const auto shortcuts = KGlobalAccel::self()->shortcut(
> +                    workspace()->findChild<QAction*>(QStringLiteral("Suspend Compositing")));
>          if (!shortcuts.isEmpty()) {

Indent only with 4 spaces.

REPOSITORY
  R108 KWin

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

To: romangg, #kwin
Cc: zzag, kwin, LeGast00n, sbergeron, jraleigh, fbampaloukas, GB_2, mkulinski, ragreen, jackyalcine, Pitel, iodelay, crozbo, bwowk, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, hardening, romangg, jensreuterberg, abetts, sebas, apol, mart
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kwin/attachments/20190808/943c688a/attachment-0001.html>