D5928: Introducing Night Color - KWin's native blue light filter at nighttime

[Roman Gilg]

subdiff added inline comments.

INLINE COMMENTS

> davidedmundson wrote in nightcolor.cpp:52
> rename the class or the file

Is there a general rule to it? Can I call the class NightColorManager instead and leave the file name untouched?

Is this about the generic class name or about the class name and the file name not being the same?

> davidedmundson wrote in nightcolor.cpp:111
> I don't understand.
> 
> we're coming from suspend if PreparingForSleep is true?

Yes: https://www.freedesktop.org/wiki/Software/systemd/logind/

It's true between signal `PrepareForSleep` is emitted with arg `true` and after sleep again with `false`.

> davidedmundson wrote in platform.h:382
> ok, if this is staying in KWin I still want it layered better. Otherwise you're going to find it hard to extend it.
> 
> Nothing in Platform and subclasses should mention nightmode. 
> They should all refer to supportsGammaCorrection or something.

Yes, you're probably right. I left the name because I was a little bit worried, that there are maybe platforms in the future, where the mechanism for changing gamma and changing color temperature is not working the same as in DRM. But probably when a platform can do one of the two things it can do the other one as well.

I'll wait until we settled on the right name of the whole (see Martin's comment about the older Color Correction / ICC / Kolor Manager code) to find a better method name for it. Otherwise I would have called it "supportsColorCorrection", in case there is a platform using a different mechanism than gamma ramp adjustment. Or should we ignore this possibility for now as well in your opinion?

REPOSITORY
  R108 KWin

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

To: subdiff, #kwin
Cc: cfeck, graesslin, davidedmundson, plasma-devel, kwin, ZrenBot, spstarr, progwolff, lesliezhai, ali-mohamed, hardening, jensreuterberg, abetts, eliasp, sebas, apol, mart, hein, lukas
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20170528/59080b80/attachment.html>