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

Reply via email to