After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 741224 - new functionality request: Screen shader (control color temperature and brightness)
new functionality request: Screen shader (control color temperature and brigh...
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: extensions
unspecified
Other Linux
: Normal enhancement
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
: 778445 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2014-12-07 20:19 UTC by Marius Andreiana
Modified: 2017-02-16 18:09 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Screenshot of display with Screen shader enabled (120.29 KB, image/png)
2014-12-07 20:25 UTC, Marius Andreiana
  Details
Patched Ubuntu 16.10 amd64 deb #1 (551.19 KB, application/vnd.debian.binary-package)
2016-11-23 01:41 UTC, jackieb
  Details
Patched Ubuntu 16.10 amd64 deb #2 (527.55 KB, application/vnd.debian.binary-package)
2016-11-23 01:42 UTC, jackieb
  Details
redshift Gnome Extension 3.14-3.20 (18.47 KB, application/zip)
2016-11-23 01:43 UTC, jackieb
  Details
status: Add naturalLight indicator (5.69 KB, patch)
2017-02-10 13:28 UTC, Florian Müllner
none Details | Review
status: Add naturalLight indicator (5.51 KB, patch)
2017-02-10 13:49 UTC, Florian Müllner
none Details | Review
status: Add nightLight indicator (5.53 KB, patch)
2017-02-10 16:30 UTC, Florian Müllner
committed Details | Review

Description Marius Andreiana 2014-12-07 20:19:58 UTC
Monitors nowadays are very bright, even with brightness set to minimum.
Spending time in front of a screen outside the regular daylight hours hurts eyes and sleep patterns.

Software techniques such as shading screen prove effective. For example:
https://chrome.google.com/webstore/detail/screen-shader/fmlboobidmkelggdainpknloccojpppi

Please provide this as a native functionality, which would apply to all display. With the above extension, switching to Nautilus or other white-background apps is a pain during evening.

Thanks!
Comment 1 Marius Andreiana 2014-12-07 20:22:24 UTC
Basically, this would the be equivalent of "Brightness: Auto" of phones, but relying on local time instead of a brightness sensor.
Comment 2 Marius Andreiana 2014-12-07 20:25:14 UTC
Created attachment 292266 [details]
Screenshot of display with Screen shader enabled
Comment 3 Bastien Nocera 2014-12-07 21:57:12 UTC
Somebody should implement this as an extension for gnome-shell before we review it for inclusion as a builtin feature.
Comment 4 Marius Andreiana 2015-03-24 22:04:46 UTC
For inspiration: https://justgetflux.com/faq.html
https://github.com/Kilian/f.lux-indicator-applet
Comment 5 Marius Andreiana 2015-03-24 22:07:19 UTC
And a newer, open source alternative: http://jonls.dk/redshift/
Comment 6 Jonathan 2015-11-20 19:37:01 UTC
Now that Gnome got automatic brightness adjustment, it would also be great to integrate functionality like redshift, or f.lux in the standard system. It's already in Windows and Apple. I think it really helps for late work sessions.

Redshift is great but is often behind on the updating to new gnome versions making it unusable. F.lux works really well, but you need to know about it to add the repo. I figure it would be great to include this in Gnome as standard.
Comment 7 Bastien Nocera 2015-11-20 20:23:40 UTC
*** Bug 742149 has been marked as a duplicate of this bug. ***
Comment 8 jackieb 2016-04-21 04:49:01 UTC
Here's a red hat feature request
A redshift GNOME extension exists, but has no effect for GNOME on Wayland
https://bugzilla.redhat.com/show_bug.cgi?id=1285590
Comment 9 Allan Day 2016-04-22 17:21:14 UTC
The display settings mockups contain an initial idea for how to present this feature:

https://raw.githubusercontent.com/gnome-design-team/gnome-mockups/master/system-settings/displays/displays-v3.png

To properly integrate this into the shell, it would be good to show an icon in the top bar and to have controls in the system status menu.
Comment 10 Ray Strode [halfline] 2016-04-22 20:47:34 UTC
So one thought is we have these weird settings:

Universal Access -> Zoom -> Color Effects

That mess around with color/contrast/brightness and have nothing to do with zoom, but do seem at least tangentially related to sleep assistance/shade mode.  

I don't have any good answers, but just wanted to point out that other piece to the puzzle.
Comment 11 Bastien Nocera 2016-04-22 23:03:40 UTC
(In reply to Ray Strode [halfline] from comment #10)
> So one thought is we have these weird settings:
> 
> Universal Access -> Zoom -> Color Effects
> 
> That mess around with color/contrast/brightness and have nothing to do with
> zoom, but do seem at least tangentially related to sleep assistance/shade
> mode.  
> 
> I don't have any good answers, but just wanted to point out that other piece
> to the puzzle.

The problem is that we need settings that would take into account the fact that you can change the "colour" of what's on the screen through:
- the Colour panel
- the Universal Access panel (as above) (for example, reducing colour to the minimum gets you a black & white mode that Lapo loves)
- however we end up implementing this mode

We'd also need to make sure that the different places that change colour management don't trample on each other. (Does colour management work in Wayland? Are all the types of colour correction above done from gnome-shell itself)
Comment 12 Marius Andreiana 2016-04-23 07:16:33 UTC
Brightness is important too. Even at 0%, most monitors today are too bright to be used at night.
Redshift controls both color temp and brightness.
Comment 13 Benjamin Berg 2016-04-23 11:23:40 UTC
My guess is that you want to modify the gamma curve of each monitor to implement this, but keep in mind that modifying the gamma curve is useful for different purposes:
 * full screen color calibration
 * screen brightness setting
 * redshift of screen

Also, the color applet needs to be able to force a linear gamma curve for the whole screen for calibration purposes.

In my opinion what we need is:
 * A wayland API to set a monitor specific gamma curve (as we have in X11)
 * code in the color applet (see #742149) to calculate and set the gamma curves
 * a shell extension to control parts of this (also a bit of code in #742149)
Comment 14 Allan Day 2016-04-25 16:34:47 UTC
(In reply to Bastien Nocera from comment #11)
... 
> The problem is that we need settings that would take into account the fact
> that you can change the "colour" of what's on the screen through:
> - the Colour panel
> - the Universal Access panel (as above) (for example, reducing colour to the
> minimum gets you a black & white mode that Lapo loves)
> - however we end up implementing this mode
...

We would handle the interaction between these functions, indeed. My initial thought is for each one to override the others, and to communicate this through the UI.

So, if you're using color calibration on your display, you see a message like: "Sleep assistance is incompatible with display calibration. Color calibration will be disabled if sleep assistance is turned on."

It's a bit sucky, but what can you do?
Comment 15 Bastien Nocera 2016-04-25 16:48:58 UTC
(In reply to Allan Day from comment #14)
> (In reply to Bastien Nocera from comment #11)
> ... 
> > The problem is that we need settings that would take into account the fact
> > that you can change the "colour" of what's on the screen through:
> > - the Colour panel
> > - the Universal Access panel (as above) (for example, reducing colour to the
> > minimum gets you a black & white mode that Lapo loves)
> > - however we end up implementing this mode
> ...
> 
> We would handle the interaction between these functions, indeed. My initial
> thought is for each one to override the others, and to communicate this
> through the UI.
> 
> So, if you're using color calibration on your display, you see a message
> like: "Sleep assistance is incompatible with display calibration. Color
> calibration will be disabled if sleep assistance is turned on."
> 
> It's a bit sucky, but what can you do?

As long as it's clear that it's one or the other, that sounds fine by me.
Comment 16 Ray Strode [halfline] 2016-04-28 19:36:39 UTC
So in the color panel, the monitor is listed and can have multiple color profiles associated with it, and the user can switch between profiles.

Maybe Sleep Assist should be an additional one there.
Comment 17 Benjamin Berg 2016-04-29 09:17:38 UTC
(In reply to Bastien Nocera from comment #15)
> (In reply to Allan Day from comment #14)
> > So, if you're using color calibration on your display, you see a message
> > like: "Sleep assistance is incompatible with display calibration. Color
> > calibration will be disabled if sleep assistance is turned on."
> > 
> > It's a bit sucky, but what can you do?
> 
> As long as it's clear that it's one or the other, that sounds fine by me.

Yeah, it is more complicated, but I would prefer to have full screen colour correction and redshift/brightness at the same time. I use the full screen colour correction so that
 * the thinkpad screen isn't too blue, and
 * the thinkpad/external screen look relatively similar.

I doubt I would use a redshift plugin if I had to disable the colour correction.

My guess is that this could be possible if gnome-shell has an API to register multiple gamma curves that are composited or if this is integrated into the colour management code (#742149).
Comment 18 Jean-François Fortin Tam 2016-05-05 02:14:35 UTC
> I doubt I would use a redshift plugin if I had to disable colour correction

From a logic standpoint, I'm not sure how it's possible to have a calibrated monitor at the same time as tinting à la redshift is happening. Doesn't it mostly defeat the point of color calibration and correction? What am I missing here?
Comment 19 Benjamin Berg 2016-05-05 12:21:02 UTC
Yes and no.

Yes, you are not getting the calibrated colours at 6500K.

And no, you are still calibrated, but you are also changing the colour temperature. This is basically simulating not using a proper 6500K lamp to look at your samples but using e.g. an incandescent light bulb at 4500K.

Nobody in their right mind would be using a 4500K lamp if they want the "proper" colour perception. But I still think it is totally fair to simulate e.g. a 4500K lighting environment in a calibrated fashion.
Comment 20 Allan Day 2016-05-06 17:11:16 UTC
(In reply to Ray Strode [halfline] from comment #16)
> So in the color panel, the monitor is listed and can have multiple color
> profiles associated with it, and the user can switch between profiles.
> 
> Maybe Sleep Assist should be an additional one there.

That's a lot of digging to find it...

I would honestly prefer it if the color panel didn't exist. It is really specialised and the vast majority of users are never going to touch it. It would be much better if it were devolved into each of the hardware panels, so displays and printers each have a color profile as a part of their settings.
Comment 22 Ray Strode [halfline] 2016-05-06 18:25:16 UTC
(In reply to Allan Day from comment #20)
> It
> would be much better if it were devolved into each of the hardware panels,
> so displays and printers each have a color profile as a part of their
> settings.

Makes sense to me.
Comment 23 stevengruspier 2016-08-13 00:04:38 UTC
Any news on this?
Comment 24 Jeremy Newton 2016-08-24 17:55:10 UTC
I would like to add that this isn't necessarily just about insomnia, as I find it helps with eye strain at night too.

I.e. I suggest changing the string to "this helps prevent insomnia and eye strain"

It seems there is a lot of studies suggesting this too, often calling it "computer vision syndrome". I can at least offer anecdotal evidence that it helps.
Comment 25 Leho Kraav (@lkraav :macmaN) 2016-08-24 18:23:29 UTC
I'm missing redshift every day of the current Wayland life. It really made things better.
Comment 26 stevengruspier 2016-08-25 00:01:11 UTC
Here is a request to implement it in Gnome:

https://bugzilla.gnome.org/show_bug.cgi?id=741224#c24
Comment 27 Patrick Griffis (tingping) 2016-11-14 07:11:14 UTC
I'm not too familiar with Mutter but I looked into this somewhat. The basic code Redshift uses for its DRM backend[1] should work if duct-taped into mutter and exposed to gnome-shell somehow. I tried to use libdrm in a gnome-shell extension (as I understand it only one process can control it?) but the basic drm calls I tried simply failed. Maybe somebody knows more about it.


[1] - https://github.com/jonls/redshift/blob/master/src/gamma-drm.c
Comment 28 Bastien Nocera 2016-11-16 12:24:44 UTC
(In reply to Patrick Griffis (tingping) from comment #27)
> I'm not too familiar with Mutter but I looked into this somewhat. The basic
> code Redshift uses for its DRM backend[1] should work if duct-taped into
> mutter and exposed to gnome-shell somehow. I tried to use libdrm in a
> gnome-shell extension (as I understand it only one process can control it?)
> but the basic drm calls I tried simply failed. Maybe somebody knows more
> about it.

There's probably no need to import code from anywhere, we already have code to handle colour effects for a11y[1]. Not sure where the colour calibration code lives, but merging all 3 codepaths could help reduce the interaction problems between the various reasons why we'd want colour filters in the desktop.

[1]: https://git.gnome.org//browse/gnome-shell/tree/js/ui/magnifier.js
Comment 29 Benjamin Berg 2016-11-18 00:40:16 UTC
I just tested this again and my code from #742149 actually still works entirely fine (one hunk needs manual merging) and it actually *does* work on wayland. The code works by adding the redshift feature into the colour calibration (see attachment #293518 [details]).

So if it is fine for this to live inside the color applet of gnome-settings-daemon, then it can be implemented there. And that code works on both X11 and wayland.
Comment 30 Benjamin Berg 2016-11-19 22:37:21 UTC
So, I brought my experimental extension into a better shape, it lives at:
  https://github.com/benzea/gnome-shell-extension-redshift

The extension requires a patched gnome-settings-daemon, and if installed will adjust the displays color temperature automatically based on the local sunset and sunrise times. It also allows changing it using a slider.

It is all more of a hack, but it seems stable and works fine for me.
Comment 31 jackieb 2016-11-23 01:41:30 UTC
Created attachment 340571 [details]
Patched Ubuntu 16.10 amd64 deb #1
Comment 32 jackieb 2016-11-23 01:42:19 UTC
Created attachment 340572 [details]
Patched Ubuntu 16.10 amd64 deb #2
Comment 33 jackieb 2016-11-23 01:43:08 UTC
Created attachment 340573 [details]
redshift Gnome Extension 3.14-3.20
Comment 34 jackieb 2016-11-23 01:48:20 UTC
The GNOME project does not have an alternative, so the patch in comment 29 should be applied

Here's how the debs were won:

mkdir redshift-patch
cd redshift-patch
sudo nano /etc/apt/sources.list
- uncomment all deb-src
sudo apt-get update
apt-get source gnome-settings-daemon
sudo apt-get build-dep gnome-settings-daemon
- package build dependencies
sudo apt-get install build-essential checkinstall devscripts
- patch
wget https://bug742149.bugzilla-attachments.gnome.org/attachment.cgi?id=293518
cd gnome-settings-daemon-3.22.1
patch --strip=1 <../attachment.cgi\?id\=293518
- fix problem with plugins/color/gsd-color-state.c
- build package:
dch --increment # add changelog comment
debuild -us -uc --build=binary # --unsigned-source --unsigned-changes won’t work

Here's how the extension was built:
git clone --depth 1 https://github.com/benzea/gnome-shell-extension-redshift
cd gnome-shell-extension-redshift
./make-zip.sh
Comment 35 jackieb 2016-11-23 01:55:34 UTC
Installing:

wget --output-document=gnome-settings-daemon_3.22.1-0ubuntu2_amd64.deb https://bugzilla.gnome.org/attachment.cgi?id=340571
sha256sum gnome-settings-daemon_3.22.1-0ubuntu2_amd64.deb
017cabe1af701fdb80cd87cca473ddbfd357efecf41eb94368484e6cf33e3220

wget --output-document=gnome-settings-daemon-schemas_3.22.1-0ubuntu2_all.deb https://bugzilla.gnome.org/attachment.cgi?id=340572
sha256sum gnome-settings-daemon-schemas_3.22.1-0ubuntu2_all.deb
017cabe1af701fdb80cd87cca473ddbfd357efecf41eb94368484e6cf33e3220

sudo dpkg --install gnome-settings-daemon_3.22.1-0ubuntu2_amd64.deb gnome-settings-daemon-schemas_3.22.1-0ubuntu2_all.deb

wget --output-document=redshift@benjamin.sipsolutions.net.zip https://bugzilla.gnome.org/attachment.cgi?id=340573
sha256sum redshift@benjamin.sipsolutions.net.zip
0190e6c7ed7e6c1fd14be83f9c79cdf2b75d97ce6ce536a45cfb96113c28c7d9

in GNOME launch Tweak Tool, go to Extensions tab, in lower right click the (None) button, navigate to the zip, click Open

Under your username in the systems menu upper right, select Log Out
- log back in

launch Tweak Tool, go to Extensions tab, toggle the switch to the left of Redshift gnome to ON

DONE!
Comment 36 jackieb 2016-11-23 01:56:36 UTC
sha256sum gnome-settings-daemon-schemas_3.22.1-0ubuntu2_all.deb
65d0c972d0aa8b07b452fe2ef6d8a8c6583d26715d6c5bd4b5dd38418bc68516
Comment 37 Bastien Nocera 2016-11-23 02:03:02 UTC
(In reply to jackieb from comment #34)
> The GNOME project does not have an alternative, so the patch in comment 29
> should be applied

Yes, we have plenty of alternatives. Don't treat this as a forum please and don't upload binary packages to the bug tracking tool.
Comment 38 jackieb 2016-11-23 05:56:48 UTC
It takes 20 minutes to apply that patch

Spending 20 minutes in 2014, when the patch was provided, could have provided GNOME users with two years of Redshift
Comment 39 Cole Mickens 2016-11-23 07:36:38 UTC
Bastien, can you point us at the alternatives? I haven't been able to find any with the search yet.

This post was shared on reddit and I was quite excited. This feature is the last remaining thing keeping me from being able to adopt the default Gnome on Wayland session.

Thanks in advance.
Comment 40 Benjamin Berg 2016-11-23 10:49:37 UTC
(In reply to Cole Mickens from comment #39)
> Bastien, can you point us at the alternatives? I haven't been able to find
> any with the search yet.

I linked this information here, so that people can find it more easily and to get feedback on the approach. Adding more information for end users on how to use it is out of scope.

You could for example make a pull request to add instructions into the README of the extension repository that I provided. Or add it to the wiki there.

PS: To jackieb, your "20 minute" estimation is not correct. This patch is not ready to be applied upstream! The purpose is to show that the general approach works fine and that implementing the feature by hooking into the color management code in gnome-settings-daemon is a viable solution.
Comment 41 Cole Mickens 2016-11-23 18:31:14 UTC
I wasn't asking for usage instructions at all; it was not hard to get running... I was asking Bastien where the alternative approaches are that they mentioned in #37.
Comment 42 Bastien Nocera 2016-11-24 16:39:47 UTC
(In reply to jackieb from comment #38)
> It takes 20 minutes to apply that patch
> 
> Spending 20 minutes in 2014, when the patch was provided, could have
> provided GNOME users with two years of Redshift

I'm sorry to say but you have no idea about software development if you think that this is 20 minutes worth of work.

(In reply to Cole Mickens from comment #39)
> Bastien, can you point us at the alternatives?

I'm talking about alternative approaches. We have color calibration support code, we have accessibility features with can change colours as well (Universal Access -> Zoom -> set a 100% zoom and go to the Colour Effects tab). This piggybacks on top of the colour calibration, but doesn't integrate with it, so there's no saying what the results of using those together would be.

First this would need end-user interface design, then moving all those colour correction algorithms in one place, and integrating it all into gnome-shell and gnome-control-center.
Comment 43 Florian Müllner 2017-02-10 13:27:58 UTC
*** Bug 778445 has been marked as a duplicate of this bug. ***
Comment 44 Florian Müllner 2017-02-10 13:28:24 UTC
Created attachment 345455 [details] [review]
status: Add naturalLight indicator

The display configuration now exposes a setting to automatically
shift the display color at nighttime. As there are cases where
disabling the filtering temporarily is useful, it makes sense to
expose the feature in the system menu for quick access.
Comment 45 Florian Müllner 2017-02-10 13:49:51 UTC
Created attachment 345459 [details] [review]
status: Add naturalLight indicator

Updated to use the newly added NaturalLightActive property.
Comment 46 Richard Hughes 2017-02-10 14:15:16 UTC
Patch works for me, thanks!
Comment 47 Carlos Soriano 2017-02-10 15:27:10 UTC
Review of attachment 345459 [details] [review]:

Few open questions, most probably because I'm not aware of the details, so as always feel free to ignore :)

::: js/ui/status/naturalLight.js
@@ +31,3 @@
+                                         if (error) {
+                                             log(error.message);
+                                             return;

if there is an error, we still continue to create the items and do calls to the proxy, which will be eventually a critical and have some UI reprensentation that doesn't work. Although I can see other submenu items like Bluetooth etc do the same.

@@ +38,3 @@
+                                     });
+
+        this._item = new PopupMenu.PopupSubMenuMenuItem("", true);

maybe we should have some fallback text in here while in case the dbus connection is taking long?

@@ +41,3 @@
+        this._item.icon.icon_name = 'natural-light-filter-symbolic';
+        this._disableItem = this._item.menu.addAction('', () => {
+            this._proxy.DisabledUntilTomorrow = !this._proxy.DisabledUntilTomorrow;

If "new ColorProxy" creates an async connection to dbus but was not ready yet, accesing this._proxy here will cause missbehaviours/errors no?

@@ +43,3 @@
+            this._proxy.DisabledUntilTomorrow = !this._proxy.DisabledUntilTomorrow;
+        });
+        this._item.menu.addAction(_("Turn Off"), () => {

This should follow also the Enable/Disable On/Off discussion later on.

@@ +45,3 @@
+        this._item.menu.addAction(_("Turn Off"), () => {
+            let settings = new Gio.Settings({ schema_id: 'org.gnome.settings-daemon.plugins.color' });
+            settings.set_boolean('natural-light-enabled', false);

Turn off here means completely off, as in the item will disappear under user's pointer, but I see we have "disable" and "enable" as well. Does the design specifies this?
Also, I see the dbus property is read only, so we modify the settings directly, is it not more clear if the dbus property was readwrite and let settingsDaemon set the setting when this is changed? If we do it for the DisableUntilTomorrow already...

@@ +64,3 @@
+
+        this._item.label.text = disabled ? _("Natural Light Filter Disabled")
+                                         : _("Natural Light Filter On");

This is kinda mixing two kind of words, I would say either use Disabled/Enabled or On/Off.

@@ +65,3 @@
+        this._item.label.text = disabled ? _("Natural Light Filter Disabled")
+                                         : _("Natural Light Filter On");
+        this._disableItem.label.text = disabled ? _("Restart Filter")

restart means here "resume" as in start it again?
Comment 48 Florian Müllner 2017-02-10 16:06:34 UTC
(In reply to Carlos Soriano from comment #47)
> if there is an error, we still continue to create the items and do calls to
> the proxy, which will be eventually a critical and have some UI
> reprensentation that doesn't work. Although I can see other submenu items
> like Bluetooth etc do the same.

Well, the proxy object does exist and has properties according to the XML description - before the proxy is initialized or in case of an error, those properties remain unset (i.e. "null"), but that should turn out OK in this case: NaturalLightActive of null is "false enough" to hide all elements in that case.


> @@ +38,3 @@
> +                                     });
> +
> +        this._item = new PopupMenu.PopupSubMenuMenuItem("", true);
> 
> maybe we should have some fallback text in here while in case the dbus
> connection is taking long?

We shouldn't show anything until we have a connection, so I'll add a call to _sync() at the end of init to ensure that.


> @@ +41,3 @@
> +        this._item.icon.icon_name = 'natural-light-filter-symbolic';
> +        this._disableItem = this._item.menu.addAction('', () => {
> +            this._proxy.DisabledUntilTomorrow =
> !this._proxy.DisabledUntilTomorrow;
> 
> If "new ColorProxy" creates an async connection to dbus but was not ready
> yet, accesing this._proxy here will cause missbehaviours/errors no?

Mmh, not sure to be honest - *reading* should be fine, but I don't know what'll happen if we attempt to write it. It's a theoretical question anyway, as the item isn't shown unless the proxy is ready.



> @@ +45,3 @@
> +        this._item.menu.addAction(_("Turn Off"), () => {
> +            let settings = new Gio.Settings({ schema_id:
> 'org.gnome.settings-daemon.plugins.color' });
> +            settings.set_boolean('natural-light-enabled', false);
> 
> Turn off here means completely off, as in the item will disappear under
> user's pointer, but I see we have "disable" and "enable" as well. Does the
> design specifies this?

Yes, I'm following https://raw.githubusercontent.com/gnome-design-team/gnome-mockups/master/system-settings/displays/displays-v4.png. Basically "disable" is for turning the feature of temporarily (let's say for watching a movie with proper colors), while "turn off" actually turns off the feature entirely.


> Also, I see the dbus property is read only, so we modify the settings
> directly, is it not more clear if the dbus property was readwrite and let
> settingsDaemon set the setting when this is changed? If we do it for the
> DisableUntilTomorrow already...

DBus property and GSetting are not the same. The setting enables the feature, while the property indicates whether any filtering is active (that is, the feature is enabled and we're in a time range where filtering should apply according to the settings).


> @@ +64,3 @@
> +
> +        this._item.label.text = disabled ? _("Natural Light Filter
> Disabled")
> +                                         : _("Natural Light Filter On");
> 
> This is kinda mixing two kind of words, I would say either use
> Disabled/Enabled or On/Off.

I don't think "Enabled" or "Off" are right, maybe "Active"/"Disabled"? Anyway, I'll leave the wording to Allan ...


> +        this._disableItem.label.text = disabled ? _("Restart Filter")
> 
> restart means here "resume" as in start it again?

Yes.
Comment 49 Florian Müllner 2017-02-10 16:30:16 UTC
Created attachment 345467 [details] [review]
status: Add nightLight indicator

Added additional _sync() call as mentioned, and went on a rename spree after changing the feature to "Night Light".
Comment 50 Juraj Fiala 2017-02-10 18:32:40 UTC
I liked Natural Light better, it describes it exactly--make the screen red when it’s red outside, white when it’s white outside. Night Light doesn’t really say anything about the feature, when I hear ‘Night Light’ I picture a small bedside lamp.
Comment 51 Jeremy Bicha 2017-02-10 18:40:36 UTC
Natural is a very ambiguous term.

It's only red outside during the transition between day and night, not during the night itself.

"Night Light" is the term used by Android 7 and by Windows 10 Creators Update. Although the word has a double meaning, it is a fairly accurate description of the feature: it changes the light emitted by your device screen at night.
Comment 52 Amit Shah 2017-02-10 18:50:27 UTC
Some laptops have ambient light sensors, so even if it's day time, the 'night light' mode can be activated based on current environment readings.  So if this feature is extended to make use of such sensors, it makes sense to rename it to something general.

However, thanks a lot for working on this!
Comment 53 Bastien Nocera 2017-02-10 18:53:08 UTC
(In reply to Amit Shah from comment #52)
> Some laptops have ambient light sensors, so even if it's day time, the
> 'night light' mode can be activated based on current environment readings. 
> So if this feature is extended to make use of such sensors, it makes sense
> to rename it to something general.

Using an ambient light sensor would have the wrong effect. This is about diminishing the amount of blue light from the display, not about the brightness of the screen per se. Both features can and will get combined though.
Comment 54 Amit Shah 2017-02-10 19:02:53 UTC
(In reply to Bastien Nocera from comment #53)
> Using an ambient light sensor would have the wrong effect. This is about
> diminishing the amount of blue light from the display, not about the
> brightness of the screen per se. Both features can and will get combined
> though.

I meant it in the "it's too dark in the room, might as well be night" sort of a way.  I'd want to the blue light to be reduced in such a case as well.
Comment 55 Bastien Nocera 2017-02-10 19:35:18 UTC
(In reply to Amit Shah from comment #54)
> (In reply to Bastien Nocera from comment #53)
> > Using an ambient light sensor would have the wrong effect. This is about
> > diminishing the amount of blue light from the display, not about the
> > brightness of the screen per se. Both features can and will get combined
> > though.
> 
> I meant it in the "it's too dark in the room, might as well be night" sort
> of a way.  I'd want to the blue light to be reduced in such a case as well.

Light sensing is relative, and frankly not that precise. Passing through a corridor, closing the curtains, or having the sun hide behind clouds would make it think that it's night, and conversely, turning on a houselight, or streetlights coming on would make it think it's day. It's just not workable, and we already have a way to detect night and day anyway.
Comment 56 Florian Müllner 2017-02-16 00:42:15 UTC
Attachment 345467 [details] pushed as 785c813 - status: Add nightLight indicator
Comment 57 Joel Schaerer 2017-02-16 17:42:48 UTC
Any idea in which release this will be included?
Comment 58 Florian Müllner 2017-02-16 18:09:48 UTC
In 3.23.90, a.k.a. 3.24 beta.