D23011: Replace foreach loops and improve code style in composite

Vlad Zagorodniy <[email protected]>
Newsgroups gmane.comp.kde.devel.kwin
Message-ID <[email protected]>
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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.