[Differential] [Changed Subscribers] D2330: [xrandr backend] fall back to preferred mode, fix crash

[graesslin (Martin Gräßlin)]

graesslin added inline comments.

INLINE COMMENTS

> xrandrconfig.cpp:517-520
> +    int modeId = kscreenOutput->currentModeId().toInt();
> +    if (!kscreenOutput->currentMode()) {
> +        modeId = kscreenOutput->preferredModeId().toInt();
> +    }

it looks strange to me that you first access the currentModeId and afterwards check whether there is a currentMode.

What about a one liner?

  const int modeId = kscreenOutput->currentMode() ? kscreenOutput->currentModeId().toInt() : kscreenOutput->preferredModeId().toInt();

If you don't like the one-liner (which I can understand) I would inverse the logic and init the modeId with preferredModeId and only set to currentModeId if there is a currentMode.

> xrandrconfig.cpp:559-562
> +    int modeId = kscreenOutput->currentModeId().toInt();
> +    if (!kscreenOutput->currentMode()) {
> +        modeId = kscreenOutput->preferredModeId().toInt();
> +    }

As you have twice the same code:

  static inline int getBestModeId(auto *kscreenOutput) {
      return ....
  }

That could also address my other comment nicely as then you can do nice if-else-return switches.

REPOSITORY
  rLIBKSCREEN KScreen Library

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

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: sebas, dvratil
Cc: graesslin, plasma-devel, #plasma, ali-mohamed, jensreuterberg, abetts, sebas
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20160802/1037deb3/attachment-0001.html>