Merge lp:~fourdollars/unity-settings-daemon/trunk into lp:unity-settings-daemon

[Shih-Yuan Lee]

[Sebastien Bacher (seb128)]

Thank you for your work, could you give some details/explanation about the change though? what values do you have for now/step/min in your case? How can max(something, value) be lower than max(something, 0)?

review: Needs Information

[Shih-Yuan Lee (fourdollars)]

The maximum brightness is 7500 in this case.
When I keep stepping down the screen brightness, it will be down to 375 by 375 for each step.
When it stays in 375, I can not make it becomes 0 or 1.
The lowest brightness should be 1 in this case because the maximum brightness is greater than 99.
0 is not reachable so I used max(something, min) here. min is 1 in this case.

[Sebastien Bacher (seb128)]

The issue then is that usd goes by step and there is not a full step left before reaching 0, so you want to go down to 1 rather by doing a smaller step in this case?

The reason gsd/use refuse to go to 0 is that it would make it look like the screen is off/not working anymore which can be confusing to users, is that a risk of that being true on some configs with a level set at "1"?

Do you know if current GNOME still has the issue? In which case we should probably report an upstream bug as well, could you maybe do that?

[Shih-Yuan Lee (fourdollars)]

By the original design, the step down of 375 should be 0, but u-s-d refused to accept 0 so it makes the UI looks bad with the trailing tail.
I was trying to proposal a solution to https://bugzilla.gnome.org/show_bug.cgi?id=734267, but the upstream has changed the codebase a lot that makes the issue have been solved in upstream but still exist in unity-settings-daemon.
u-s-d still used some kind of old codebase from gnome-settings-daemon that makes the problem still exist.

[Sebastien Bacher (seb128)]

Good to know that the issue has been fixed upstream, do you know what change of them addressed it? Or is that been solved in some refactoring which doesn't make it possible to pinpoint a specific changeset?

You didn't reply to part of my comment though, what you are doing is making the step less than a full one to reach 1? are we sure than 1 isn't creating issues on some config? (what value is GNOME getting down to in their fixed version)? Oh also is the actual screen value the issue or that the osd bar is not reaching 0 (it is for me but I guess it is config dependant)

[Shih-Yuan Lee (fourdollars)]

Sorry. I don't have time to answer your questions.
Let me reply you in next week.

[Shih-Yuan Lee (fourdollars)]

Since the following commit, g-s-d doesn't use xrandr to control the backlight.

commit dbd3b138df814e370952149a5097470af44a8e86
Refs: GNOME_SETTINGS_DAEMON_3_15_4-15-gdbd3b13
Author: Rui Matos <email address hidden>
AuthorDate: Tue Feb 10 16:24:06 2015 +0100
Commit: Rui Matos <email address hidden>
CommitDate: Mon Feb 16 18:08:18 2015 +0100

    power: Use the backlight helper on Linux unconditionally

    This makes us use the same code paths on both X11 and Wayland
    sessions.

    https://bugzilla.gnome.org/show_bug.cgi?id=744277

The following commit clamps the minimum backlight.

commit bb5c1aec2b7e0684c55ee8b1d7025ff0c1acba8e
Refs: GNOME_SETTINGS_DAEMON_3_15_4-18-gbb5c1ae
Author: Rui Matos <email address hidden>
AuthorDate: Tue Feb 10 16:56:48 2015 +0100
Commit: Rui Matos <email address hidden>
CommitDate: Mon Feb 16 18:08:18 2015 +0100

    power: On raw backlight types, clamp the value to a minumum

    On raw backlight types, it's likely that setting it to zero will
    completely turn the backlight off which isn't very useful. Instead,
    let's always clamp the value to a minimum in that case.

    For now we just clamp if the backlight has at least 100 levels but we
    might tweak it as needed.

    https://bugzilla.gnome.org/show_bug.cgi?id=744278

Ubuntu 16.04 still uses xrandr to control the backlight so this problem still exists, but not in the upstream.

[Sebastien Bacher (seb128)]

somehow that got forgotten, would you still like to see that fix landing or should we close it now?

Unmerged revisions

4163. By Shih-Yuan Lee on 2017-04-21

Make brightness step down able to reach the minimum backlight. (LP: #1685062)

=== modified file 'gnome-settings-daemon/gsd-rr.c'
--- gnome-settings-daemon/gsd-rr.c	2016-03-14 15:24:41 +0000
+++ gnome-settings-daemon/gsd-rr.c	2017-04-21 04:48:57 +0000
@@ -1938,6 +1938,10 @@
 
     g_return_val_if_fail (output != NULL, FALSE);
 
+    /* Make brightness step down able to reach the minimum backlight */
+    if (value == 0 && output->backlight_min == 1)
+	value = 1;
+
     /* check this is sane */
     if (value < output->backlight_min ||
         value > output->backlight_max)

=== modified file 'plugins/power/gpm-common.c'
--- plugins/power/gpm-common.c	2015-11-16 04:51:45 +0000
+++ plugins/power/gpm-common.c	2017-04-21 04:48:57 +0000
@@ -1615,7 +1615,7 @@
                 if (now < 0)
                        return percentage_value;
                 step = BRIGHTNESS_STEP_AMOUNT (max - min + 1);
-                discrete = MAX (now - step, 0);
+                discrete = MAX (now - step, min);
                 ret = gsd_rr_output_set_backlight (output,
                                                      discrete,
                                                      error);
@@ -1632,7 +1632,7 @@
         if (max < 0)
                 return percentage_value;
         step = BRIGHTNESS_STEP_AMOUNT (max - min + 1);
-        discrete = MAX (now - step, 0);
+        discrete = MAX (now - step, min);
         ret = backlight_helper_set_value ("set-brightness",
                                           discrete,
                                           error);