Backlight tuning has not enough steps

Bug #1739805 reported by Jean Philippe Eimer
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Xfce4 Power Manager
Fix Released
Medium
xfce4-power-manager (Ubuntu)
New
Undecided
Unassigned
Focal
New
Undecided
Unassigned
Impish
Fix Released
Undecided
Unassigned

Bug Description

Backlight tuning with keyboard shortcuts has 10 steps.
On my system (DELL E5570), backlight at lowest value remains too high in dark ambiance.
The attached patch sets the step number at 30, instead of 10.

LUbuntu 17.10
xfce4-power-manager 1.4.4-4ubuntu2

Tags: patch
Revision history for this message
In , 2460acc (2460acc) wrote :

When using FN key and arrows there are only 10 increments in the screen brightness settings. The lowest one is a black screen and the next one is already mid-bright for me. So basically it is not possible to set a low screen brightness level.

Using xbacklight -dec 1 I have found there are 10 additional levels between the two xfce4-power-manager zero nd first level. Please can you let us access these.

Revision history for this message
In , 2460acc (2460acc) wrote :

The following lines in common/xfpm-brightness.c seem to be the cause of the course brightness steps.

http://git.xfce.org/xfce/xfce4-power-manager/tree/common/xfpm-brightness.c

214 brightness->priv->step = max <= 20 ? 1 : max / 10;
366 brightness->priv->step = ret <= 20 ? 1 : ret / 10;

I wonder if you could change these numbers to 20 or 30 to allow finer steps, e.g.

  brightness->priv->step = max <= 30 ? 1 : max / 30;

I'm going to try and recompile but it looks a bit complicated for me.

Revision history for this message
In , 2460acc (2460acc) wrote :

I just saw there is something called:

xfce4-settings-editor

Perhaps you could put the number of steps as a parameter in there so users can do whatever they want.

Revision history for this message
In , Simon Steinbeiß (ochosi) wrote :

*** Bug 12299 has been marked as a duplicate of this bug. ***

Revision history for this message
In , Timothy Lee (timothy-ty-lee) wrote :

I've made an attempt to add this feature. The code can be found on the brightness-step-count branch at: <email address hidden>:timothytylee/xfce4-power-manager.git

Please apply if the patch is acceptable.

Revision history for this message
In , Eric Koegel (eric-koegel) wrote :

Timothy, nice work. Now that you have a configuration option in the settings UI, you can react to that in the power manager plugin as well. If we do it in the backlight, you'll need to create a step_switch variable in the struct then, install it in the class_init
http://git.xfce.org/xfce/xfce4-power-manager/tree/src/xfpm-backlight.c#n313

Use xfconf_g_property_bind to be notified of changes.
http://git.xfce.org/xfce/xfce4-power-manager/tree/src/xfpm-backlight.c#n368

Then you can call xfpm_brightness_set_step_count in the set_property function
and return the step-size in the get function.
http://git.xfce.org/xfce/xfce4-power-manager/tree/src/xfpm-backlight.c#n444

Revision history for this message
In , Eric Koegel (eric-koegel) wrote :

Ugh, that's what I get for commenting before coffee. For the panel plugin you'll want to do something similar what the brightness_min_level does in the panel-plugins/power-manager-plugin/power-manager-button.c Hope I'm not too confusing.

Revision history for this message
In , Timothy Lee (timothy-ty-lee) wrote :

I believe the panel plugin doesn't need to be updated, since changing the number of keyboard controllable steps doesn't affect current brightness -- it only affects future brightness adjustment made by keyboard.

Revision history for this message
In , 2460acc (2460acc) wrote :

I wonder if you could make the increments logarithmic? I.e. 1, 2, 4, 6, 16. etc. On my screen, with linear steps the gaps are too large at the low levels (ie, the screen gets too bright too quickly) but then at the high levels there are too many steps needed to get to full brightness.

In terms of steps, something like 0, 1, 2, 3, 4, 5, 10, 20, 50, 75, 100 % would give fine control where you need it but you can also get to max brightness easily as well. Thanks for your help! Really appreciated.

Revision history for this message
In , Timothy Lee (timothy-ty-lee) wrote :

I've finished implementing the brightness step count functionality. In addition to the git repository, I'm attaching a patch.

Revision history for this message
In , Timothy Lee (timothy-ty-lee) wrote :

Created attachment 6754
Adjustable brightness steps

Revision history for this message
In , zetxx (opensuser) wrote :

(In reply to Timothy Lee from comment #10)
> Created attachment 6754 [details]
> Adjustable brightness steps

to which package should be applied this patch ? because i cannot see edited file in xfce4-power-manager

Revision history for this message
In , Timothy Lee (timothy-ty-lee) wrote :

I've added an option to toggle exponential step sizes.

Revision history for this message
In , Timothy Lee (timothy-ty-lee) wrote :

Created attachment 6757
Adjustable brightness steps with support for exponential step sizes

Revision history for this message
In , Timothy Lee (timothy-ty-lee) wrote :

This patch is applicable to the master branch of xfce4-power-manager at http://git.xfce.org/xfce/xfce4-power-manager/

Revision history for this message
In , zetxx (opensuser) wrote :

thanks

Revision history for this message
In , Timothy Lee (timothy-ty-lee) wrote :

Ping. Any comments on the patch?

Revision history for this message
In , 2460acc (2460acc) wrote :

The latest versions (up to 1.6) still have the original code with the brain-dead step increment.

https://github.com/xfce-mirror/xfce4-power-manager/releases

What exactly is the point of this bug tracker?

Logarithmic increments can be implemented with very little modification in common/xfpm-brightness.c. Look for "set_level =" and use the following for decrement and increment, respectively.

    set_level = MAX (hw_level/2, brightness->priv->min_level);

    set_level = MIN (hw_level*2 + (hw_level==0), brightness->priv->max_level );

I hope someone finds this useful... if anyone is even out there!

Revision history for this message
In , 2460acc (2460acc) wrote :

Works even better with a smaller decrement value, so you can access in-between values.

 set_level = MAX (hw_level/1.5, brightness->priv->min_level);

Revision history for this message
In , Marcel Partap (empee584) wrote :

Yes, please @maintainer integrate this into the next update : )

Revision history for this message
Jean Philippe Eimer (phileimer) wrote :
Revision history for this message
Ubuntu Foundations Team Bug Bot (crichton) wrote :

The attachment "Change brightness step number to 30 instead of 10" seems to be a patch. If it isn't, please remove the "patch" flag from the attachment, remove the "patch" tag, and if you are a member of the ~ubuntu-reviewers, unsubscribe the team.

[This is an automated message performed by a Launchpad user owned by ~brian-murray, for any issues please contact him.]

tags: added: patch
Revision history for this message
In , Timothy Lee (timothy-ty-lee) wrote :

Created attachment 7728
Adjustable brightness steps with support for exponential step sizes

Updated patch again for master branch of xfce4-power-manager at http://git.xfce.org/xfce/xfce4-power-manager/

Revision history for this message
Theo Linkspfeifer (lastonestanding) wrote :

Please forward this patch to the Xfce bug tracker. Thanks.

https://bugzilla.xfce.org/

Changed in xfce4-power-manager:
importance: Unknown → Medium
status: Unknown → Confirmed
Revision history for this message
In , 2460acc (2460acc) wrote :

Wow that is a godawful amount of patching for a simple 2-line fix. Are you sure there's not an NSA backdoor embedded in there somewhere? ;)

Revision history for this message
In , Hoy Tampoco (treschavales) wrote :

@majest I guess the bulk of the patch is the added config options in the GUI, please don't spread FUD (even ironic FUD).

I think this a very good feature and several users in the forum have expressed their desire for it and confirmed it works, e.g.: https://forum.xfce.org/viewtopic.php?id=13072

Thanks @Timothy Lee!

Revision history for this message
In , bjo (bjo81) wrote :

@Timothy Lee Could you provide a patch for the actual master branch?

Revision history for this message
In , Timothy Lee (timothy-ty-lee) wrote :

Created attachment 8652
Adjustable brightness steps with support for exponential step sizes

Rebased to latest master as of 2019-06-19.

Revision history for this message
In , Timothy Lee (timothy-ty-lee) wrote :

Created attachment 8653
Adjustable brightness steps with support for exponential step sizes

Fixed compilation problem.

Revision history for this message
In , Simon Steinbeiß (ochosi) wrote :

Sorry this got overlooked for a while...
We're not doing this on purpose, we're just not enough people and don't have enough free time to clean up the bugtracker. (FYI Eric, who previously reviewed and commented on the patch left the project so this is simply a leftover.)

I'll take a look so this can get included for 4.16.

Changed in xfce4-power-manager:
status: Confirmed → In Progress
Revision history for this message
In , Simon Steinbeiß (ochosi) wrote :

I started to look into this patch - could you rebase it one last time on top of master? (Please note that it mostly fails to apply because I cleaned up the code formatting in the sourcecode - no more tabs, 2 spaces instead.)

Thanks in advance!

Revision history for this message
In , Simon Steinbeiß (ochosi) wrote :

Nvm, I rebased it myself.

So yes, it works, but I'm not sure I agree with not updating the panel plugin with the brightness steps. I understand you bound it visually to the brightness *buttons*, and that there is a separate GtkScale widget in the panel plugin's menu.

What feels a little strange to have two brightness steps, one on the brightness keys and the other for the mousewheel (which you can use on hovering the plugin to change the brightness).

Revision history for this message
In , Timothy Lee (timothy-ty-lee) wrote :

Sorry, I completely missed the slider on the pop-up menu.

Revision history for this message
In , Timothy Lee (timothy-ty-lee) wrote :

Created attachment 9307
Adjustable brightness steps with support for exponential step sizes

I've updated the patch against latest master, with whitespace fixes.

The slider on the pop-up menu now also obeys uses the same steps as the brightness buttons (allowing for rounding errors).

Revision history for this message
In , Dawid M. (dawid.mlyn) wrote :

The lowest one is a black screen for me too and the next one is already mid-bright on my Dell XPS 13. While I don't mind having the possibility to turn the screen brightness off completely, for many years I was also adding "xbacklight -dec 1" to the keyboard shortcuts, just to be able to dim my display more, when needed.

Thanks guys for fixing it. Can't wait to have it available in XFCE 4.16, when it comes out.

Revision history for this message
In , Gitbot (gitbot) wrote :

Timothy Lee referenced this bugreport in commit c866cf9faec6333e60f6b55a56b92ec9494747f3

Make brightness steps configurable (Bug #12062)

https://git.xfce.org/xfce/xfce4-power-manager/commit?id=c866cf9faec6333e60f6b55a56b92ec9494747f3

Revision history for this message
In , Simon Steinbeiß (ochosi) wrote :

Thanks for updating the patch and sticking to the coding style.
I only had very few places with odd spaces to fix (but I acknowledge that the source code is still quite messy and inconsistent in this respect, so it's hard to stick to a "correct style").

Changed in xfce4-power-manager:
status: In Progress → Fix Released
Revision history for this message
In , Main-haarp (main-haarp) wrote :

Thank you, Timothy Lee. This was long needed. I've applied attachment 8653 from comment 25 on top of xfce4-power-manager-1.6.5. I couldn't apply the newer patches due to the aforementioned code style changes.

Where are the settings to be found? I can't find anything new in the Power Manager's setting screen. I ended up manually adding them with the settings editor.

The exponential feature is really comfortable to use. However there's one caveat. It starts too slow, so that the first 3 of 10 steps have pretty much no effect at all. Is there a way to weigh it a little more towards the later steps?

Cheers!

Revision history for this message
In , Timothy Lee (timothy-ty-lee) wrote :

(In reply to haarp from comment #34)
> Where are the settings to be found? I can't find anything new in the Power
> Manager's setting screen. I ended up manually adding them with the settings
> editor.

With the patch applied, on the General tab of Xfce Power Manager, there should be a "Brightness step count" field and an "Exponential" checkbox next to it.

> The exponential feature is really comfortable to use. However there's one
> caveat. It starts too slow, so that the first 3 of 10 steps have pretty much
> no effect at all. Is there a way to weigh it a little more towards the later
> steps?

I believe the effect of exponential steps is very much dependent on the characteristics of the LCD backlight. For my last two laptops, there are very visible differences between every step on the scale. Perhaps you can try reducing the number of steps to 6?

Enlarging the step sizes towards the lower end complicates the maths quite a bit, and may not suit everyone.

Revision history for this message
In , Main-haarp (main-haarp) wrote :

> With the patch applied, on the General tab of Xfce Power Manager, there should be a "Brightness step count" field and an "Exponential" checkbox next to it.

Weird, I'm not seeing those. But don't let that worry you, I'm sure the next proper release will have those setting show up.

> I believe the effect of exponential steps is very much dependent on the characteristics of the LCD backlight. For my last two laptops, there are very visible differences between every step on the scale. Perhaps you can try reducing the number of steps to 6?

Tried that. Funnily enough, it did nothing to change the lower end. Now I have 3 steps that do nothing and 3 steps that quickly increase brightness :)

Seems like a large case of YMMV.

Revision history for this message
Sean Davis (bluesabre) wrote :

Resolved in version 1.7.0.

Changed in xfce4-power-manager (Ubuntu Impish):
status: New → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.