subdiff added a comment.
In https://phabricator.kde.org/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 values 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 do the fading in KWin, for that we need timers, at least one. But it would make sense to have already 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 quick presentations 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 control 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. 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, then 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. REPOSITORY R108 KWin REVISION DETAIL https://phabricator.kde.org/D5928 To: subdiff, #kwin Cc: davidedmundson, plasma-devel, kwin, ZrenBot, spstarr, progwolff, lesliezhai, ali-mohamed, hardening, jensreuterberg, abetts, eliasp, sebas, apol, hein, lukas
