Differential D5928

Introducing Night Color - KWin's native blue light filter at nighttime
ClosedPublic
Actions

Authored by romangg on May 20 2017, 9:09 PM.

Details

Reviewers
graesslin
Group Reviewers
KWin
Commits
R108:82d286046907: [colorcorrection] Night Color - blue light filter at nighttime
Summary

Introduction

With Wayland KWin needs to provide certain services, which were provided before that by the Xserver. One of these is gamma correction, which includes the - by many people beloved - functionality to reduce the blue light at nighttime. This patch provides the KWin part of that. It is self contained, but in the end will work in tandem with a lib in Plasma Workspace and a KCM in Plasma Desktop, which can be used to configure Night Color. Diffs for these parts will be posted this weekend.

Features:

  • Three modi:
    • Automatic: The location and sun timings are determined automatically (location data updates will be provided by the workspace)
    • Location: The sun timings are determined by fixed location data
    • Timings: The sun timings are set manually by the user
  • Color temperature value changes are smoothly applied:
    • Configuration changes, which lead to other current values are changed in a quick way over a few seconds
    • Changes on sunrise and sunset are applied slowly over the course of few minutes till several hours depending on the configuration
  • The current color value is set immediately at startup or after suspend phases and VT switches. There is no flickering.
  • All configuration is done via a DBus interface, changed values are tested on correctness and applied atomically
  • Self contained mechanism, speaks directly to the hardware by setting the gamma ramps on the CRTC
  • Currently working on DRM backend, extensible to other platform backends in the future
  • The code is written in a way to make the classes later easily extendable to also provide normal color correction, as it's currently done by KGamma on X

This patch solves the redshift part of T4465 and lays the foundations to solve the task in total in the near future.

Test Plan

Manually with the workspace parts and added integration tests in KWin using the virtual backend.

Diff Detail

Repository
R108 KWin
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
romangg created this revision.May 20 2017, 9:09 PM
Restricted Application added subscribers: kwin, plasma-devel. · View Herald TranscriptMay 20 2017, 9:09 PM
davidedmundson added a subscriber: davidedmundson.May 21 2017, 8:29 AM

In my opinion, and the impression you gave me when we talked about the design.

We shouldn't have Night mode stuff in kwin.
We should have generic colour correction in kwin (albeit with fading happening in kwin to reduce traffic)

I guess this changed mid way? I can see your class names start with "color correction" and then you have virtual "supportsNightMode" which is very specific.

romangg added a comment.EditedMay 21 2017, 11:04 AM
In D5928#110990, @davidedmundson wrote:

In my opinion, and the impression you gave me when we talked about the design.

We shouldn't have Night mode stuff in kwin.
We should have generic colour correction in kwin (albeit with fading happening in kwin to reduce traffic)

To make it more clear, I'll describe how I came in the end to the presented solution. In the beginning I wanted the workspace to only submit an updated color temperature value to KWin via DBus, KWin then calculates from this value the new gamma ramps. This has two drawbacks:

The first one is, that the fading phases generate much DBus traffic, as you said. To solve this we would need to do the fading in KWin. For that we need timers, at least one. But it would make sense to have two timers, one for quick adjustments and one for long fades. Because imagine the following:

While being in a fade to the neutral day temperature the user changes the night color temperature. KWin needs to quickly adjust the current color temperature to the new mix of night and day color temperature in the fade and at the same time proceed with the fade to the neutral day temperature.

If we allow only one timer with a target temperature workspace needs to calculate the current mix and then send this data to KWin with the information that it's supposed to be a quick change, after that Workspace again needs to send the neutral temperature and its target time to setup the timer. So we need a second timer in workspace to know when this can be done approximately or a callback from KWin when the quick change is over. In any case less complexity would be to let workspace send the informations {current (mixed) temperature, next target temperature, next target time}. In this case KWin would need two timers. We wouldn't need the slowUpdateStartTimer in KWin, but it would be in workspace and workspace would need to send the next target temperature/time combination at the beginning of every new long fade.

Regarding the second drawback: On bootup and after suspend phases KWin needs to know immediately, i.e. before the first frame is painted, the current color temperature mix, something which can be any value between night and day temperature depending on the time of day. My assumption was, that it would be way slow if we wait on workspace to give KWin this information, to not lead to some frames being painted with wrong color temperature values.

In any case we would have lots of traffic going on between workspace and KWin and an higher complexity for little gain in my opinion. Yes, KWin wouldn't have to calculate the sun timings itself, which is somewhat a stretch, but I wrote it as a single function with little to no footprint. On the other side, we have a single DBus interface for configuration, which is only firing on manual configuration changes by the user, and additionally one DBus function to update the automatic location data (but KWin doesn't need that to work in Automatic mode). Other than that KWin can work on its own. All configuration is saved and provided by KWin, which is imo the nicer solution to having it in workspace or worst case having it split up on multiple applet config files, if people decide to write their own applets for controlling the color temperature.

I can see your class names start with "color correction" and then you have virtual "supportsNightMode" which is very specific.

Certain names start with "ColorCorrect" and I created a new directory "colorcorrection", because the idea is to later extend the classes in this directory with functionality to control the gamma of the connected screens in general and what's currently done by KGamma on X. Basically what most people would understand under color correction. But since setting a color temperature value for night time is working in the same way (by adjusting the gamma ramps), it has to be done in tandem, when we have the general gamma control later (it's basically another factor multiplied to the general gamma ramp adjustments). I just started with Night Color, because it was more straight forward without the need to remember different values for different screens. And in my opinion it's more needed. At least I need it to use Wayland at night.

graesslin added a subscriber: graesslin.May 23 2017, 9:54 AM

I wouldn't call it color correction as we used to have a mechanism called color correction in the past which did something very different. Personal suggestion: gamma correction or color temperature correction.

Sorry cannot do a full review for the next few weeks.

Personally I do not mind whether the functionality is in KWin or extern. The argument of first frane after suspend is a good one. I didn't check the code in detail so ignote if it doesn't matter: in long term I want to forbid KWin to have Internet access. So the geolocation neefs to happen outside of KWin.

And congrats for top item on r/linux :-P

davidedmundson added inline comments.May 26 2017, 5:18 PM
colorcorrection/nightcolor.cpp
52 ↗(On Diff #14725)

rename the class or the file

111 ↗(On Diff #14725)

I don't understand.

we're coming from suspend if PreparingForSleep is true?

colorcorrection/nightcolor.h
109 ↗(On Diff #14725)

use kwinApp()->platform() when you need it and remove it from the ctor

org.kde.kwin.ColorCorrect.xml
18

don't include signal in a signal name.

I assume you're doing it to avoid the conflict with the method?
(fun fact, that's valid DBus, just invalid QtDBus)

typical convention is to have the set method called "setNightModeConfig"

platform.h
473

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.

plugins/platforms/drm/drm_object_crtc.cpp
119

so we have 3 arrays of size m_gammaSize

we can pass the GammaRamp object and do:

gammaRamp.red,
gammaRamp.green,
gammaRamp.blue,

also it'll be safer to use the gammaSize from the gammaramp. otherwise if it changes, you're going to crash.

if you are trying for separation pass 3 uint*s instead of doing pointer maths.

davidedmundson added a comment.May 26 2017, 5:45 PM

As for your comments:

The first one is, that the fading phases generate much DBus traffic, as you said. To solve this we would need to do the fading in KWin. For that we need timers, at least one. But it would make sense to have two timers, one for quick adjustments and one for long fades. >Because imagine the following:

Not really. With any gradient or animation, you specify: startpoint, startvalue, endpoint, endvalue

With that your "imagine the following" is easily solved, if you get a new sunset time or filter value you can immediately deduce the correct "current" value.

(talking of which, look at QVariantAnimation instead of your quicksteps timer, it basically does the same thing, but wrapped away. Set the duration to the entire fade time, then use setCurrentTime on suspend)
Also you shouldn't fear DBus traffic so much. Even if it was updating every second it wouldn't to be a problem.

The suspend argument makes sense...though I'm not sure you can really use that argument whilst you currently have a call to logind in the code.

romangg added a comment.May 28 2017, 1:53 PM
In D5928#111282, @graesslin wrote:

I wouldn't call it color correction as we used to have a mechanism called color correction in the past which did something very different. Personal suggestion: gamma correction or color temperature correction.

I have looked it up now and it's something about ICC profiles for screens. Is this stuff functionally right now in KWin? Don't forget that Night Color is only the first part of the planned work here. In the end it is also about giving the user control over gamma / color values of the attached screens in general, which sounds similar to the ICC profiles stuff and maybe should be integrated anyway. Which means then it would make sense again to use the proposed name. But if not, gamma correction would be the way to go, because as I said in the end not only color temperature

Sorry cannot do a full review for the next few weeks.

No problem, we have lots of time till next release. :)

Personally I do not mind whether the functionality is in KWin or extern. The argument of first frane after suspend is a good one. I didn't check the code in detail so ignote if it doesn't matter: in long term I want to forbid KWin to have Internet access. So the geolocation neefs to happen outside of KWin.

Geolocation is outside (in Plasma Workspace). No internet access needed from within KWin.

And congrats for top item on r/linux :-P

At least we now can be sure, that there is general interest for such a feature.

cfeck added a subscriber: cfeck.EditedMay 28 2017, 2:17 PM
In D5928#112334, @subdiff wrote:
In D5928#111282, @graesslin wrote:

I wouldn't call it color correction as we used to have a mechanism called color correction in the past which did something very different. Personal suggestion: gamma correction or color temperature correction.

I have looked it up now and it's something about ICC profiles for screens. Is this stuff functionally right now in KWin? Don't forget that Night Color is only the first part of the planned work here. In the end it is also about giving the user control over gamma / color values of the attached screens in general, which sounds similar to the ICC profiles stuff and maybe should be integrated anyway. Which means then it would make sense again to use the proposed name. But if not, gamma correction would be the way to go, because as I said in the end not only color temperature

The difference between gamma correction and color correction is that the former only uses three 1D LUTs, while the latter can use three 3D LUTs. In other words, gamma correction cannot tint your blue more green, while color correction can.

EDIT: We had code to do color correction, but that was deleted. For details, see the commit message.

https://cgit.kde.org/kwin.git/commit/?id=17e0bad922c7b9b17164987d2a4d64bff936edae

cfeck added a comment.EditedMay 28 2017, 2:58 PM

I agree that this code should not talk about night mode at all. Indeed we only need code in KWin to control the gamma LUTs per screen, and maybe even compute the LUTs from a number of gamma/contrast/brightness control points so that we do not have to transfer a lot of data via inter-process communication. If new values are submitted from the night mode daemon, they can be marked as "apply LUTs immediately", or marked as "fade to new LUTs smoothly within a given timeframe", e.g. a minute.

Likewise for color correction, we would not have a full ICC specification in KWin, only code that manages the 3D LUTs and applies them per window or per screen.

That reminds me about another important difference between gamma correction and color correction. The former is a hardware feature, and cannot be applied per window, maybe only per full-screen windows.

romangg updated this revision to Diff 14916.EditedMay 28 2017, 4:30 PM
romangg marked 2 inline comments as done.
  • No m_platform in ColorCorrect::Manager
  • Dbus iface methods renamed
  • Give GammaRamp object to DrmCrtc (and use its ramp size)
  • Separate GammaRamp struct for better includes
  • Merge current master
romangg added inline comments.May 28 2017, 4:32 PM
colorcorrection/nightcolor.cpp
52 ↗(On Diff #14725)

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?

111 ↗(On Diff #14725)

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.

platform.h
473

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?

davidedmundson added inline comments.May 28 2017, 4:58 PM
colorcorrection/nightcolor.cpp
81 ↗(On Diff #14916)

This won't work on BSD, TimeEngine::init() has a relevant ifdef and a fallback.

111 ↗(On Diff #14725)

How confusing. In any case we have a race condition.

We're triggering this on timeChangedFd so we have behaviour that basically changes depending on whether logind has processed resuming and sent a signal before or after kwin processes this.

Why do we have this check anyway? We need to resetAllTimers if the system time changes due to a timezone change too.

romangg added a comment.May 28 2017, 5:28 PM
In D5928#111959, @davidedmundson wrote:

As for your comments:

The first one is, that the fading phases generate much DBus traffic, as you said. To solve this we would need to do the fading in KWin. For that we need timers, at least one. But it would make sense to have two timers, one for quick adjustments and one for long fades. >Because imagine the following:

Not really. With any gradient or animation, you specify: startpoint, startvalue, endpoint, endvalue

You forgot that we have a step size bigger than 1 on shifting the integer value of the color temperature (right now 50). QVariantAnimation also doesn't seem to support it directly.

With that your "imagine the following" is easily solved, if you get a new sunset time or filter value you can immediately deduce the correct "current" value.

It's not about deducing the current value (we already do that in currentTargetTemp() - without use of the timers). It's about a two step operation: Changing to the currentTargetTemp and then (re)starting the slow fade process. The change to currentTargetTemp is suppposed to be faded as well (that's done by m_quickAdjustTimer) and afterwards in step two we go through the slow fade (m_slowUpdateTimer) or create a timer for the next slow fade to start (m_slowUpdateStartTimer).

romangg added a comment.EditedMay 28 2017, 6:24 PM
In D5928#112339, @cfeck wrote:

The difference between gamma correction and color correction is that the former only uses three 1D LUTs, while the latter can use three 3D LUTs. In other words, gamma correction cannot tint your blue more green, while color correction can.

Can you give a source for that definition of the two notions? When I look it up Wikipedia explains Color Correction as a way to change color values in a generic fashion. It's relatively broad. I haven't found a very specific definition like you. In contrast gamma correction is explained as the adjustment of the gamma of the light in general - i.e. all colors at once. You can influence the color balance though by varying the gamma of the separate gamma color channels, which is then again part of color correction. There is also the notion of color management, which is somewhat all of that as well in our context.

EDIT: We had code to do color correction, but that was deleted. For details, see the commit message.

https://cgit.kde.org/kwin.git/commit/?id=17e0bad922c7b9b17164987d2a4d64bff936edae

Thanks for the link! Seems to be not relevant anymore, although the goal was somewhat the same as for my later planned color correction (using 1D LUTs), which is simpler but still better than nothing (also the prime objective is in this case to offer the functionality of KGamma on Wayland, which uses 1D LUTs as well).

romangg added inline comments.May 28 2017, 6:46 PM
colorcorrection/nightcolor.cpp
111 ↗(On Diff #14725)

Well, in my tests logind always changed the value of this property way later then the notifier was triggered (like 1 second), so I assumed that this is always the case. But you're right, that I don't know for sure.

Why do we have this check anyway?

It's only a small difference in look and it's working anyway, that's why I probably found the possible race condition not that important. Take a look at line 117. If it's true, we change the color temperature value without fade, otherwise we do. The else case is for example triggered on timezone change while running, so it fades nicely in. But this is a corner case, and we might just ignore it.

cfeck added a comment.May 28 2017, 7:02 PM
In D5928#112357, @subdiff wrote:
In D5928#112339, @cfeck wrote:

The difference between gamma correction and color correction is that the former only uses three 1D LUTs, while the latter can use three 3D LUTs. In other words, gamma correction cannot tint your blue more green, while color correction can.

Can you give a source for that definition of the two notions? When I look it up Wikipedia explains Color Correction as a way to change color values in a generic fashion. It's relatively broad. I haven't found a very specific definition like you. In contrast gamma correction is explained as the adjustment of the gamma of the light in general - i.e. all colors at once. You can influence the color balance though by varying the gamma of the separate gamma color channels, which is then again part of color correction. There is also the notion of color management, which is somewhat all of that as well in our context.

http://www.color.org/faqs.xalter

romangg added a comment.May 28 2017, 7:25 PM
In D5928#112359, @cfeck wrote:

http://www.color.org/faqs.xalter

Sorry, but I can't find the relevant part here. Can you quote it?

cfeck added a comment.May 28 2017, 8:13 PM
In D5928#112364, @subdiff wrote:
In D5928#112359, @cfeck wrote:

http://www.color.org/faqs.xalter

Sorry, but I can't find the relevant part here. Can you quote it?

I wanted you to read it all ;)

First, the difference between "color management" and "color correction" is that color correction is only one step in the color management workflow. KWin would need (again) be able to apply color correction to take part in the color management workflow. As such, the terms color correction and color management can be used interchangeably in the context of KWin discussion.

Gamma correction (as implemented by the hardware LUTs) cannot be used for color correction, because it lacks the cross-component modifications computed by the 3x3 matrix or the 3D LUT.

The term "gamma correction" has been used since decades in the context of video cards, even if they are not only used to apply gamma correction, but also brigthness/contrast adjustments, color inversion effects, and temperature adjustments.

Anyway, http://www.color.org/faqs.xalter#p1 says:

Q. What is the structure of a display profile?

A. Display profiles are commonly of the Matrix/TRC type, in which case they contain (in addition to the tags which all profiles are required to have, such as white point and copyright information):

  • a 3x3 matrix of the colorant primaries tristimulus values
  • a one-dimensional tone curve for each colorant

They can also be of the multi-dimensional look-up table (or LUT) type, in which case they have:

  • a 3x3 matrix
  • a one-dimensional tone curve for each colour channel
  • a three-dimensional look-up table
  • a second 1D tone curve for each channel
romangg added a comment.May 29 2017, 1:37 AM
In D5928#112368, @cfeck wrote:

First, the difference between "color management" and "color correction" is that color correction is only one step in the color management workflow. KWin would need (again) be able to apply color correction to take part in the color management workflow. As such, the terms color correction and color management can be used interchangeably in the context of KWin discussion.

Gamma correction (as implemented by the hardware LUTs) cannot be used for color correction, because it lacks the cross-component modifications computed by the 3x3 matrix or the 3D LUT.

The term "gamma correction" has been used since decades in the context of video cards, even if they are not only used to apply gamma correction, but also brigthness/contrast adjustments, color inversion effects, and temperature adjustments.

Thanks for the explanation in your own words, but there has to be some misunderstanding. I wanted to see a quote from a second source besides you where it says:

"We define Color Correction as..." or "Color Correction is..."
and
"We define Gamma Correction as..." or "Gamma Correction is..."

Because currently to me it seems Wikipedia and you have different notions of what color correction is supposed to be (also other sources disagree: https://creativepro.com/color-management-not-color-correction/). But I'll leave this naming discussion for now aside, because it's not really important. I would still say color correction is the right term to call it, since in the end the perceived result is a change of color temperature or color balance and not one of the overall gamma in particular. Since the old stuff, which got called color correction back then, has been removed and and when I look at the code likely won't come back, I also don't see a problem with naming my stuff that way. The old stuff should've been called differently anyway according to the above link. But I'll leave the naming decision to Martin, if this is ok with him.

cfeck added a comment.May 29 2017, 7:20 AM

Yes, let's not argue about the names. I just wanted to make sure you understand the difference between 1D and 3D LUTs, and why color management (should it ever come back to KWin), needs 3D LUTs.

behrmann added a subscriber: behrmann.May 29 2017, 8:09 AM
In D5928#112398, @subdiff wrote:
In D5928#112368, @cfeck wrote:

First, the difference between "color management" and "color correction" is that color correction is only one step in the color management workflow. KWin would need (again) be able to apply color correction to take part in the color management workflow. As such, the terms color correction and color management can be used interchangeably in the context of KWin discussion.

Gamma correction (as implemented by the hardware LUTs) cannot be used for color correction, because it lacks the cross-component modifications computed by the 3x3 matrix or the 3D LUT.

The term "gamma correction" has been used since decades in the context of video cards, even if they are not only used to apply gamma correction, but also brigthness/contrast adjustments, color inversion effects, and temperature adjustments.

Thanks for the explanation in your own words, but there has to be some misunderstanding. I wanted to see a quote from a second source besides you ...

The 1D gamma business is for proper white point and gray behavior. Most people name the 1D LUT stuff "Monitor Calibration". It would be much more clear to stick to the "Gamma" or "Calibration" terms here. DRM names it as well "drmModeCrtcSetGamma()".

Btw. in terms of ICC the 1D LUT manipulation is called "Calibration" too.

leezu added a subscriber: leezu.Oct 11 2017, 2:59 PM
ngraham added a subscriber: ngraham.Oct 11 2017, 5:05 PM

What's the status of this patch?

romangg added a comment.Oct 12 2017, 9:30 AM
In D5928#154362, @ngraham wrote:

What's the status of this patch?

There is some minor problem @davidedmundson pointed to. When I've finished the thesis I currently need to work on, I'll look into that, rebase on current master and also try to reignite the review for this patch in general. I need some review of the overall structure from @graesslin. D5931 needs to be approved as well.

davidedmundson added a comment.Oct 12 2017, 11:23 AM

If you're busy, tell me and I'll make those minor changes I want and follow this up.

romangg added a comment.Oct 13 2017, 5:31 AM
In D5928#154584, @davidedmundson wrote:

If you're busy, tell me and I'll make those minor changes I want and follow this up.

Thanks for the offer, but I should be able to do it myself early enough before next release. Would be nice to get a review from you on D5931 though.

romangg updated this revision to Diff 20971.EditedOct 19 2017, 11:13 AM

Merge current master.

romangg updated this revision to Diff 20972.Oct 19 2017, 11:29 AM

Ifdef Linux specific time change detection code.

romangg marked an inline comment as done.Oct 19 2017, 11:31 AM
romangg marked an inline comment as done.Oct 19 2017, 11:34 AM
romangg marked 4 inline comments as done.Oct 19 2017, 6:18 PM
romangg added inline comments.
colorcorrection/nightcolor.cpp
111 ↗(On Diff #14725)

@davidedmundson and I talked about it and he said he is content with it.

romangg marked 2 inline comments as done.Oct 19 2017, 6:18 PM
graesslin requested changes to this revision.Oct 21 2017, 6:40 PM
graesslin added a subscriber: • jriddell.

The general concept looks good to me.

colorcorrection/constants.h
35–40

What's the license for this? Looking at the linked readme I do not see any license. If the Redshift license applies it would be GPLv3+ which is something I have not allowed so far in KWin/Core as it would remove the GPLv2 option. But to me this looks like a database so other licensing might apply.

summoning @jriddell for support.

colorcorrection/logging.cpp
20–21 ↗(On Diff #20972)

same for this file: use the ecm command.

colorcorrection/logging.h
20–26 ↗(On Diff #20972)

please use the ecm command to generate the logging.h file instead of adding a new one.

colorcorrection/nightcolor.cpp
152 ↗(On Diff #20972)

You could also consider to use a kcfgc here. See https://techbase.kde.org/Development/Tutorials/Using_KConfig_XT

We use it in e.g. all effects and it reduces quite a lot the boiler plate code. Especially for things like mapping to an enum.

561–588 ↗(On Diff #20972)

Please use initializer list:

return QHash<QString, QVariant>{
    { QStringLiteral{"Avaliable"}, kwinApp()->platform()->supportsNightColor()},
    {QStringLiterall{"ActiveEnabled"}, true},
   /* and so on */
};
52 ↗(On Diff #14725)

I cannot answer for David, but I would say that class name and file name should be the same. I would have called it manager.h and manager.cpp. Though that's a little bit generic - also the class name Manager is rather generic. Granted it's in a namespace, but still.

colorcorrection/nightcolor.h
40–41 ↗(On Diff #20972)

I wouldn't use all uppercase. Just use normal CamelCase: DateTimes, Times. All uppercase is normally reserved for defines or constants.

platform.h
473

I agree with David here: just make it gamma. If in the future we have a difference we can still introduce more variants.

This revision now requires changes to proceed.Oct 21 2017, 6:40 PM
evpokp added a subscriber: evpokp.Nov 20 2017, 6:27 PM
romangg updated this revision to Diff 23698.Dec 9 2017, 9:19 PM
romangg marked 10 inline comments as done.
  • Cover comments,
  • simplify sun timing calculation,
  • merge current master
romangg added inline comments.Dec 9 2017, 9:21 PM
colorcorrection/constants.h
35–40

I have rewritten the lines, such that it can not count any longer as a copy. I also contacted Ingo Thies, and he gave me permission in an email to use the data in KWin (although raw data, as he also mentioned in the mail, is probably not copyrightable anyway).

Hence I think this issue has been resolved and I mark the comment as done.

romangg marked an inline comment as done.Dec 9 2017, 9:21 PM
graesslin accepted this revision.Dec 10 2017, 1:33 PM
This revision is now accepted and ready to land.Dec 10 2017, 1:33 PM
maspons added a subscriber: maspons.Dec 10 2017, 3:54 PM
maspons added inline comments.
colorcorrection/constants.h
82

5000K ?

romangg marked an inline comment as done.Dec 11 2017, 9:43 AM
Closed by commit R108:82d286046907: [colorcorrection] Night Color - blue light filter at nighttime (authored by romangg). · Explain WhyDec 11 2017, 10:22 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 23749

View Options

CMakeLists.txt

Loading...
View Options

autotests/integration/CMakeLists.txt

Loading...
View Options

autotests/integration/colorcorrect_nightcolor_test.cpp

Loading...
View Options

colorcorrection/colorcorrect_settings.kcfg

Loading...
View Options

colorcorrection/colorcorrect_settings.kcfgc

Loading...
View Options

colorcorrection/colorcorrectdbusinterface.h

Loading...
View Options

colorcorrection/colorcorrectdbusinterface.cpp

Loading...
View Options

colorcorrection/constants.h

Loading...
View Options

colorcorrection/gammaramp.h

Loading...
View Options

colorcorrection/manager.h

Loading...
View Options

colorcorrection/manager.cpp

Loading...
View Options

colorcorrection/suncalc.h

Loading...
View Options

colorcorrection/suncalc.cpp

Loading...
View Options

org.kde.kwin.ColorCorrect.xml

Loading...
View Options

platform.h

Loading...
View Options

platform.cpp

Loading...
View Options

plugins/platforms/drm/drm_backend.h

Loading...
View Options

plugins/platforms/drm/drm_backend.cpp

Loading...
View Options

plugins/platforms/drm/drm_object_crtc.h

Loading...
View Options

plugins/platforms/drm/drm_object_crtc.cpp

Loading...
View Options

plugins/platforms/virtual/virtual_backend.h

Loading...
View Options

plugins/platforms/virtual/virtual_backend.cpp

Loading...