Keyboard backlight handling improvements #404
Conversation
cwendling
force-pushed
the
kbd-backlight
branch
from
7c80bd9 to
606c145
Compare
|
You will want to rebase this against master to reflect that the last commit in this PR has been effectivelhy merged separately to fix the Travis problems |
There was a problem hiding this comment.
On my desktop with no backlit keyboard I've never seen any reference to a keyboard backlight control. Same on my laptops, none of which have ever had a backlit keyboard. Assuming this is shown in the power manager preferences only when the relevent hardware is detected.
That said, the build finished fine and nothing changed in a quick test running both mate-power-manager and mate-control-center with GDK_BACKEND=x11. This generates a ton of warnings in the terminal the power manager is running from but is enough to show the normal GUI in the preferences dialog.
The static value was reset on every call rather than only initially, resulting in failure to restore the initial value when toggling it back on.
Try and save the user-set brightness to be able to restore it on next restart. This is not perfect because we have complex interaction between settings which makes it not trivial, if at all possible, to save the value yet respect the dim settings. Here we try to save the value in a way that counteract battery dimming so that restoring the value yields expected results (e.g. a value saved on battery restores identical on battery again), but it cannot work when the value to save is larger than the dimmed maximum as at the moment the maximum value is 100%. It is however the best I could come up with working with the settings we currently have, and it's probably good enough at least as a starting point. It also switches from `master_percentage` to `brightness_percent` in the toggle code because the former is only initialized to the actual current brightness value once brightness changed at least once, while the former is properly initialized. It is otherwise quivalent for this feature, and less confusing as to when the value gets updated.
Now the on-AC value changes following user input, the internal `master_percentage` was just plain confusing and actually make things a bit weird at times because it doesn't follow dynamic user choices. So just get rid of it, replacing it with the on-AC value.
Handling of a disabled battery-reduce setting was too aggressive, which could reduce in not setting backlight value in unrelated cases, including initial startup and resume, as well as some cases of IDLE handling. Now the user choices are better followed, the whole special-casing could probably be removed as the internal state should always reflect the reality, but keep the specific check not to do anything on AC plug/unplug when this is disabled just in case, although the historical reasons why it was actually problematic should be gone now.
cwendling
force-pushed
the
kbd-backlight
branch
from
606c145 to
cf76826
Compare
|
I just rebased on top of fixed master. @lukefromdc thanks for the quick test. However, note that nothing I changed here is related to GUI preferences themselves, but solely to how hardware buttons react and how some settings combination interact. I'd be best to have others test this with the relevant hardware, but I also believe that it was broken badly enough (IMO) that even if there's some more fine-tuning to do to please everyone it's still way better than the state before. |
|
So presuming I should wait awhile to see if we can get another test, but if this just
sits with no further reviews merge it?
|
See commit log, but most important bits:
The rest is cleanups, and a small change to the handling of some settings combinations because they were just odd in some corner cases, and mostly irrelevant now the user changes are preserved.
I guess that not many people use keyboard backlight under MATE, because it seems really under-loved and especially the toggle bug seems both annoying and simple enough that it would have been spotted and fixed before. I myself just got my hands on a laptop with keyboard backlight with a proper Linux driver for controlling it requiring no hacks, and I couldn't help myself but to fix this 🙂
Hopefully I can still get some review on this, or just rely on street cred ✌️