Merge lp://staging/~kaihengfeng/unity-settings-daemon/add-brightness-limit-mechanism into lp://staging/unity-settings-daemon

Proposed by Kai-Heng Feng
Status: Rejected
Rejected by: Lars Karlitski
Proposed branch: lp://staging/~kaihengfeng/unity-settings-daemon/add-brightness-limit-mechanism
Merge into: lp://staging/unity-settings-daemon
Diff against target: 256 lines (+85/-13)
3 files modified
plugins/power/gpm-common.c (+32/-5)
plugins/power/gpm-common.h (+15/-4)
plugins/power/gsd-power-manager.c (+38/-4)
To merge this branch: bzr merge lp://staging/~kaihengfeng/unity-settings-daemon/add-brightness-limit-mechanism
Reviewer Review Type Date Requested Status
Lars Karlitski (community) Disapprove
Martin Pitt Disapprove
Ubuntu Desktop Pending
Review via email: mp+277416@code.staging.launchpad.net

Description of the change

Add a mechanism that can make user to choose the minimum/maximum adjustable brightness level.

To post a comment you must log in.
Revision history for this message
Martin Pitt (pitti) wrote :

TBH, I'm not a fan of introducing configurability for such things. It sounds too much like an "unbreak my desktop setting" and a hack for a bug, it introduces combinatorial explosion again which nobody will test properly, and it'll be hard to drop such settings again.

My opinion is not authoritative here of course, it's ultimately the desktop teams' decision.

review: Disapprove
Revision history for this message
Zygmunt Krynicki (zyga) wrote :

I'm tentatively +1 on this. I agree with Martin that this is an "unbreak my system" knob but ultimately the world where I have this knob is better than the one we are in now, where desktops _are_ broken.

I would love to see an user-visible checkbox that controls if backlight turns off at 0% and a system-vendor tunable that actually controls the 0-100% span.

Revision history for this message
Martin Pitt (pitti) wrote :

> I would love to see an user-visible checkbox that controls if backlight turns off at 0% and a system-vendor tunable that actually controls the 0-100% span.

Erk, please no. This is the absolutely worst UI.

Revision history for this message
Martin Pitt (pitti) wrote :

Also, this high-level user space is *not* the point to second-guess/workaround kernel drivers. Userspace should use the brightness range that the kernel offers, and if there's somehting wrong with that it needs to be fixed in that driver.

Revision history for this message
Lars Karlitski (larsu) wrote :

I strongly agree with Martin's remarks. This should be fixed in the respective drivers. Not only because that's the conceptually correct place, but also because otherwise we'll have to hack around this in every place that brightness is controlled in user space.

> I would love to see an user-visible checkbox that controls if backlight turns off at 0% and a system-vendor tunable that actually controls the 0-100% span.

Why would anyone ever want to turn off the backlight for LCD screens? Turning down the brightness is not the same as turning off the screen.

review: Disapprove
Revision history for this message
Martin Pitt (pitti) wrote :

FTR, https://code.launchpad.net/~fourdollars/unity-settings-daemon/fix-lowest-brightness/+merge/277326 addresses the same problem in an (IMHO) much better way, as it keeps the heuristics in the code (and thus changeable) without providing extra knobs.

Revision history for this message
Kai-Heng Feng (kaihengfeng) wrote :

Although I still prefer configuration for such an issue, I am happy to see this issue being addressed.

Revision history for this message
Mateo Salta (mateo-salta) wrote :

please no, I only see thing on windows devices to trick the battery life stats - setting it lower on battery vs plugged in is one thing(user can still increase to actual max), but forcing a limit that you can't turn up past unless you go into settings is a bad idea.

Unmerged revisions

4118. By Kai-Heng Feng

power: Add a mechanism that can limit min/max brightness percentage via gsettings.

In attempt to fix #1381625, I think it should be the user to decide the minimun
brightness policy to be "screen dim" or "screen off". This newly added mechanism
does not change the current behavior, user can decide it by changing
"min-brightness-percentage".

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches