feat: fully reworked SCREEN module to provide better screen-content b…

…rightness adjustments. This is a conf-API break as SCREEN module configurations has changed. Refs #242 Signed-off-by: Federico Di Pierro <[email protected]>
FedeDP · Jan 30, 2022 · 405870e · 405870e
1 parent 409b255
commit 405870e
Show file tree

Hide file tree

Showing 17 changed files with 360 additions and 227 deletions.
diff --git a/Extra/clight.conf b/Extra/clight.conf
@@ -175,18 +175,15 @@ keyboard:
     ## in the corresponding AC state.
     # timeouts = [ 15, 5 ];
 
-    ## Curves used to match ambient brightness to keyboard backlight level for each AC state.
-    ## X axis: ambient brightness values (from 0 to 10)
-    ## Y axis: desired backlight level for corresponding ambient brightness.
+    ## Curves used to match screen backlight to keyboard backlight level for each AC state.
+    ## X axis: default screen backlight level (from 0 to 10)
+    ## Y axis: desired keyboard backlight level for corresponding screen backlight.
     ## Note: the array can be expanded up to 50 points for finer granularity.  
     ## Note also that most keyboard offers only 3 backlight levels (off, mid, high).
     ## Default curves are same as default backlight curves but upside down 
-    ## (ie: the lower the ambient brightness, the higer the keyboard backlight).
+    ## (ie: the lower the screen backlight, the higher the keyboard backlight).
     # ac_regression_points = [ 1.0, 0.97, 0.93, 0.88, 0.81, 0.74, 0.61, 0.45, 0.29, 0.15, 0.0 ];
     # batt_regression_points = [ 0.80, 0.78, 0.75, 0.71, 0.65, 0.59, 0.52, 0.36, 0.23, 0.15, 0.0 ];
-
-    ## Uncomment to switch off keyboard on dimming/dpms.
-    # dim = true;
 };
 
 ##############
@@ -222,9 +219,15 @@ gamma:
     ## no_smooth_gamma_transition, gamma_trans_step, gamma_trans_timeout
     # long_transition = true;
 
-    ## Let screen temperature match ambient brightness, like monitor backlight.
+    ## Let screen temperature match monitor backlight, using following algorithm:
+    ## ```pseudocode
+    ## diff = abs(temp[DAY] - temp[NIGHT])
+    ## min_temp = min(temp[NIGHT], temp[DAY]) 
+    ## new_temp = (diff * screen_bl) + min_temp;
+    ## ```
+    ## Ie: the higher the screen backlight, the colder the temp.
+    ## 
     ## When enabled, screen temperature won't be changed time-based.
-    ## Note that it uses same curve points as backlight.
     ## Note also that LOCATION is still needed to let BACKLIGHT module know current time of day.
     ## Finally, it requires BACKLIGHT module to be enabled, otherwise it gets disabled.
     # ambient_gamma = true;
@@ -326,32 +329,38 @@ dpms:
 screen:
 {
     ##############################################################################################################
-    ## Use this feature to provide screen-emitted brightness compensation to Clight.                             #
-    ## The idea here is:                                                                                         #
-    ##      dark ambient    -> higher monitor light contribution impact                                          #
-    ##      bright ambient  -> lower monitor light contribution impact                                           #
-    ## But, at the same time, obviously (as that is clight main feature):                                        #
-    ##      dark ambient    -> lower monitor backlight level    -> lower monitor light contribution impact       #
-    ##      high ambient    -> high monitor backlight level     -> higher monitor light contribution impact      #
-    ## We can assume these 2 contributions to zero each other, letting us just compute screen-emitted brightness #
-    ## and linearly multiplying it for our guessed ambient-brightness screen contribution value.                 #
+    ## Use this feature to enable screen-content based backlight adjustments to Clight.                          #
+    ## This means that monitord backlight won't be changed to match ambient brightness alone,                    #
+    ## but screen content brightness too, using following algorithm:                                             #
+    ## ```pseudocode                                                                                             #
+    ## window_min = ambient_br - contrib;                                                                        #
+    ## window_max = ambient_br + contrib;                                                                        #
+    ## bl_new = window_max - (0.4 * screen_br)                                                                   #
+    ## ```                                                                                                       #
+    ## Ie: ambient brightness is used as a sliding window 20% wide,                                              #
+    ## and screen content brightness is used to choose the correct point inside the window.                      #
+    ## Rule is: higher the screen content brightness, nearest to window max; and viceversa.                      #
+    ## Idea is fairly simple: given current ambient brightness, try to light up dark screen content, and dim     #
+    ## bright screen content.                                                                                    #
+    ##                                                                                                           #
+    ## This can impact battery life, that is the reason why SCREEN module is disabled on battery by default;     #
+    ## in this case, you might want to keep SCREEN module disabled and manually calibrating screen backlight     #
+    ## through Clight desktop file "Capture" quick action or its DBus command.                                   #
     ##############################################################################################################
 
     ## Uncomment to disable screen-emitted brightness compensation support
     # disabled = true;
 
+    ## How much does your screen brightness affect
+    ## ambient brightness, in percentage.
+    ## As above algorithm outlines, it is used to build the ambient brightness window.
+    ## Set it to 0.0 to disable screen-content based backlight compensation.
+    # contrib = 0.2;
+
     ## Screen timeouts on AC/on BATT.
-    ## Set any of these to <= 0 to disable screen-emitted brightness compensation
+    ## Set any of these to <= 0 to disable screen-content based backlight compensation
     ## in the corresponding AC state.
     ## Disabled by default on BATT because it is quite an heavy operation,
     ## as it has to take a snapshot of your X desktop and compute its brightness.
-    # timeouts = [ 30, -1 ];
-
-    ## How much does your screen-emitted brightness affect
-    ## ambient brightness, in your setup, in percentage.
-    # contrib = 0.1;
-
-    ## How many samples should be used to compute average 
-    ## screen-emitted brightness.
-    # num_samples = 10;
+    # timeouts = [ 5, -1 ];
 };
diff --git a/Extra/completions/_clight b/Extra/completions/_clight
@@ -20,7 +20,7 @@ arguments=(
   '--no-dimmer[Disable dimmer tool]'
   '--no-dpms[Disable dpms tool]'
   '--no-backlight[Disable backlight module]'
-  '--no-screen[Disable screen module]'
+  '--no-screen[Disable screen module (screen content based backlight adjustment)]'
   '--no-kbd[Disable keyboard backlight calibration]'
   '--dimmer-pct=[Backlight level used while screen is dimmed, in percentage]'
   '--verbose[Enable verbose mode]'

diff --git a/Extra/completions/clight.fish b/Extra/completions/clight.fish
@@ -18,7 +18,7 @@ complete -c clight -l no-gamma -f -d "Disable gamma correction tool"
 complete -c clight -l no-dimmer -f -d "Disable dimmer tool"
 complete -c clight -l no-dpms -f -d "Disable dpms tool"
 complete -c clight -l no-backlight -f -d "Disable backlight module"
-complete -c clight -l no-screen -f -d "Disable screen module"
+complete -c clight -l no-screen -f -d "Disable screen module (screen content based backlight adjustment)"
 complete -c clight -l no-kbd -f -d "Disable keyboard backlight calibration"
 complete -c clight -l dimmer-pct -x -d "Backlight level used while screen is dimmed, in percentage"
 complete -c clight -l verbose -f -d "Enable verbose mode"

diff --git a/Extra/man/clight.1 b/Extra/man/clight.1
@@ -14,7 +14,7 @@ Clight \- a user daemon utility that aims to fully manage your display.
 .br
 [\fB\fC\-\-no\-gamma\fR] [\fB\fC\-\-no\-dimmer\fR] [\fB\fC\-\-no\-dpms\fR] [\fB\fC\-\-no\-backlight\fR] [\fB\fC\-\-no\-screen\fR] [\fB\fC\-\-no\-kbd\fR]
 .br
-[\fB\fC\-\-dimmer\-pct\fR DOUBLE] [\fB\fC\-\-no\-auto\-calib\fR] [\fB\fC\-\-shutter\-thres\fR DOUBLE] [\fB\fC\-\-gamma\-long\-transition\fR] [\fB\fC\-\-ambient\-gamma\fR]
+[\fB\fC\-\-dimmer\-pct\fR DOUBLE] [\fB\fC\-\-no\-auto\-calib\fR] [\fB\fC\-\-shutter\-thres\fR DOUBLE] [\fB\fC\-\-gamma\-long\-transition\fR] [\fB\fC\-\-ambient\-gamma\fR] [\fB\fC\-\-content\-backlight\fR]
 .br
 [\fB\fC\-c, \-\-conf\-file\fR STRING] [\fB\fC\-w, \-\-wizard\fR] [\fB\fC\-\-verbose\fR] [\fB\fC\-v, \-\-version\fR] [\fB\fC\-?, \-\-help\fR] [\fB\fC\-\-usage\fR]
 
@@ -81,6 +81,10 @@ dim your screen after a timeout and manage screen DPMS.
 \fB\fC\-\-ambient\-gamma\fR
   Enable screen temperature matching ambient brightness instead of being time based.
 
+.PP
+\fB\fC\-\-content\-backlight\fR
+  Enable screen content-based backlight calibration.
+
 .PP
 \fB\fC\-\-lat\fR \fIdouble\fP
   Desired latitude.

diff --git a/TODO.md b/TODO.md
@@ -1,21 +1,47 @@
-## 4.9
+## 4.9 (5.0?)
 
 ### Generic
-- [ ] Port to libmodule 6.0.0 (?)
-- [ ] Add a Dump dbus method (and a DUMP_REQ request) to allow any module to dump their state (module_dump()) to a txt file
-
-### Pinephone
-- [ ] Finalize support
+- [ ] Fix MODULE_WITH_PAUSE modules: properly start in "paused" state if pause condition is met as soon as module starts
 
 ### Sensor
 - [ ] Allow multiple sensors to be specified in priority order; those sensors will be stored in a list and the first available will be used.
 - [ ] Changes: -> is_sensor_available() that will be called on each of the listed sensors; first avaiable will become highest_prio_available_dev_name
 - [ ] capture_frames_brightness -> will use highest_prio_available_dev_name
 
-## After Clightd API break:
+### Backlight
+- [x] Support content based backlight calibration
+- [x] Fix/improve set_new_backlight() from backlight and screen
+- [x] Fix: wait on state.screen_br update before starting, so that we can start with content-based if enabled
+
+### Screen
+- [x] start paused when starting with a timeout < 0
+- [x] Drop old "contrib" based SCREEN tool, and just keep content-backlight behavior?
+- [x] This would mean that if screen is not disabled, content-backlight is implicit
+- [x] Better manage clogged state
+- [x] New msg: SCREEN_UPD -> emit screen_br updates
+- [x] Expose state.screen_br in interface
+- [x] Drop state.content_based?
+
+### KbdBacklight
+- [x] Port to use BL_UPD instead of ambient_upd: this way it works fine while content_based backlight is enabled, and follows monitor specific_curves and ambient gamma rule (ie: use default backlight curve as target)
+- [x] Better still: it allows KEYBOARD to react to external backlight changes too
+- [ ] TEST -> bl_upd when smoothing (may be ok -> smoother transitions) or becoming dimmed (should be ok as there is already an option to pause kbd when dimmed)
+- [x] dim by default and drop config (why was it there in the first place?)
 
 ### Backlight
 - [ ] Drop is_smooth option (no need, just specify a step/wait > 0)
 
 ### Gamma
 - [ ] Drop is_smooth option (no need, just specify a step/wait > 0)
+
+### Dimmer
+- [ ] Drop is_smooth option (no need, just specify a step/wait > 0)
+
+## Future
+
+### Pinephone
+- [ ] Finalize support
+
+### Generic
+- [ ] Port to libmodule 6.0.0 (?)
+- [ ] Add a Dump dbus method (and a DUMP_REQ request) to allow any module to dump their state (module_dump()) to a txt file
diff --git a/src/commons.h b/src/commons.h
@@ -63,7 +63,6 @@ typedef struct {
 
 typedef struct {
     int disabled;                           // disable keyboard backlight automatic calibration (where supported)
-    int dim;                                // whether DPMS/Dimmer should switch keyboard off
     int timeout[SIZE_AC];                   // kbd dimming timeout
     curve_t curve[SIZE_AC];                 // curve used to match ambient brightness to certain keyboard backlight level
 } kbd_conf_t;
@@ -100,9 +99,8 @@ typedef struct {
 
 typedef struct {
     int disabled;
+    double contrib;
     int timeout[SIZE_AC];                   // screen timeouts
-    double contrib;                         // how much does screen-emitted brightness affect ambient brightness (eg 0.1)
-    int samples;                            // number of samples used to compute average screen-emitted brightness
 } screen_conf_t;
 
 typedef struct {
@@ -138,6 +136,8 @@ typedef struct {
  */
 typedef struct {
     bool looping;
+    bool content;                           // Whether content backlight is enabled (ie: SCREEN is running)
+    double screen_br;                       // screen content brightness
     int in_event;                           // Whether we are in a TIME event +- conf.event_duration
     int inhibited;                          // whether screensaver inhibition is enabled
     int pm_inhibited;                       // whether pm_inhibition is enabled
@@ -155,7 +155,6 @@ typedef struct {
     double current_bl_pct;                  // current backlight pct
     double current_kbd_pct;                 // current keyboard backlight pct
     double ambient_br;                      // last ambient brightness captured from CLIGHTD Sensor
-    double screen_comp;                     // current screen-emitted brightness compensation
     const char *clightd_version;            // Clightd found version
     const char *version;                    // Clight version
     jmp_buf quit_buf;                       // quit jump called by longjmp

diff --git a/src/conf/config.c b/src/conf/config.c
@@ -27,17 +27,16 @@ static void store_screen_settings(config_t *cfg, screen_conf_t *screen_conf);
 static void store_inh_settings(config_t *cfg, inh_conf_t *inh_conf);
 
 static void init_config_file(enum CONFIG file, char *filename) {
-    int len = 0;
     switch (file) {
         case LOCAL:
             if (getenv("XDG_CONFIG_HOME")) {
-                len = snprintf(filename, PATH_MAX, "%s/clight.conf", getenv("XDG_CONFIG_HOME"));
+                snprintf(filename, PATH_MAX, "%s/clight.conf", getenv("XDG_CONFIG_HOME"));
             } else {
-                len = snprintf(filename, PATH_MAX, "%s/.config/clight.conf", getpwuid(getuid())->pw_dir);
+                snprintf(filename, PATH_MAX, "%s/.config/clight.conf", getpwuid(getuid())->pw_dir);
             }
             break;
         case GLOBAL:
-            len = snprintf(filename, PATH_MAX, "%s/clight.conf", CONFDIR);
+            snprintf(filename, PATH_MAX, "%s/clight.conf", CONFDIR);
             break;
         default:
             return;
@@ -200,7 +199,6 @@ static void load_kbd_settings(config_t *cfg, kbd_conf_t *kbd_conf) {
     config_setting_t *kbd = config_lookup(cfg, "keyboard");
     if (kbd) {
         config_setting_lookup_bool(kbd, "disabled", &kbd_conf->disabled);
-        config_setting_lookup_bool(kbd, "dim", &kbd_conf->dim);
 
         /* Load timeouts */
         config_setting_t *timeouts, *points;
@@ -375,8 +373,6 @@ static void load_screen_settings(config_t *cfg, screen_conf_t *screen_conf) {
     if (screen) {
         config_setting_lookup_bool(screen, "disabled", &screen_conf->disabled);
         config_setting_lookup_float(screen, "contrib", &screen_conf->contrib);
-        config_setting_lookup_int(screen, "num_samples", &screen_conf->samples);
-
         config_setting_t *timeouts;
         if ((timeouts = config_setting_get_member(screen, "timeouts"))) {
             if (config_setting_length(timeouts) == SIZE_AC) {
@@ -540,9 +536,6 @@ static void store_kbd_settings(config_t *cfg, kbd_conf_t *kbd_conf) {
     config_setting_t *setting = config_setting_add(kbd, "disabled", CONFIG_TYPE_BOOL);
     config_setting_set_bool(setting, kbd_conf->disabled);
 
-    setting = config_setting_add(kbd, "dim", CONFIG_TYPE_BOOL);
-    config_setting_set_bool(setting, kbd_conf->dim);
-
     /* -1 here below means append to end of array */
     setting = config_setting_add(kbd, "ac_regression_points", CONFIG_TYPE_ARRAY);
     for (int i = 0; i < kbd_conf->curve[ON_AC].num_points; i++) {
@@ -670,10 +663,7 @@ static void store_screen_settings(config_t *cfg, screen_conf_t *screen_conf) {
     config_setting_t *setting = config_setting_add(screen, "disabled", CONFIG_TYPE_BOOL);
     config_setting_set_bool(setting, screen_conf->disabled);
 
-    setting = config_setting_add(screen, "num_samples", CONFIG_TYPE_INT);
-    config_setting_set_int(setting, screen_conf->samples);
-
-    setting = config_setting_add(screen, "contrib", CONFIG_TYPE_FLOAT);
+    setting = config_setting_add(screen, "content", CONFIG_TYPE_FLOAT);
     config_setting_set_float(setting, screen_conf->contrib);
 
     setting = config_setting_add(screen, "timeouts", CONFIG_TYPE_ARRAY);