PowerDevil merge requestshttps://invent.kde.org/plasma/powerdevil/-/merge_requests2024-03-19T16:20:26Zhttps://invent.kde.org/plasma/powerdevil/-/merge_requests/342daemon: Limit KAuth backlighthelper calls to only one at a time2024-03-19T16:20:26ZJakob Petsovitsdaemon: Limit KAuth backlighthelper calls to only one at a timeTurns out #27 or https://invent.kde.org/plasma/plasma-workspace/-/merge_requests/3178 were not necessary to make the brightness slider laggy again. CC @nicolasfella, @nclarius, cheers! :beers:
---
KAuth invocations are slower than use...Turns out #27 or https://invent.kde.org/plasma/plasma-workspace/-/merge_requests/3178 were not necessary to make the brightness slider laggy again. CC @nicolasfella, @nclarius, cheers! :beers:
---
KAuth invocations are slower than users can move a slider.
So we get many brightness change requests, and KAuth can't keep up.
As a result, high-frequency brightness change requests for
`BacklightBrightness` (i.e. laptop displays) would feel very laggy
and get worse over time if not given a break to catch up.
This commit fixes the lagginess. Conceptually it's easy:
Don't start a new KAuth helper job until the current one has
reported a result. At that point, check if a different brightness
level was requested in the meantime, and start another job only then.
`BacklightHelper` can deal fine with interrupting a running animation
and starting a new one from the current brightness level. Hence,
there is no need to wait until the end of the brightness animation
to start a new KAuth job - this would only increase latency.
Starting the new job right after receiving a result is perfectly fine.
In the same go, we make two related improvements.
Firstly, the udev-powered brightness observer will not only ignore
events when the animation timer is running, but also in between
`setBrightness()` and the KAuth result handler where the timer
is started.
Secondly, the condition involving `m_brightnessAnimationThreshold`
is changed to something sensible. It makes no sense to simply
check the current brightness against a constant number (100) to
determine whether the brightness change should be animated.
What I think this meant to do is to ensure that there are enough
brightness steps available to animate. So following this commit,
we will now animate when the difference between the current and
the requested brightness is 100 or more integer steps.
Taken together, laptop brightness slider UX now seems decent.
BUG: 481003
---
Notes:
* I have not tested BUG: 470106 which is probably also fixed with this, but would have to spend some time reproducing the original "going out of sync" issue.
* Seems feasible enough to backport to [`PowerDevilUPowerBackend` in Plasma/6.0](https://invent.kde.org/plasma/powerdevil/-/blob/6c7b9727e72ecf0fb6f15c1183ba1576cae431d7/daemon/backends/upower/powerdevilupowerbackend.cpp), but damn has this code come a long way since then already.6https://invent.kde.org/plasma/powerdevil/-/merge_requests/133Fix minimal brightness on OLED screens2023-10-09T00:02:33ZBartosz TaudulFix minimal brightness on OLED screensWhen OLED screen brightness is set to zero, the display becomes
completely black. Unlike LCD screens, OLED pixels are self-lit, so if
the brightness becomes zero, there is no light emitted at all.
This can be fixed by clamping the minim...When OLED screen brightness is set to zero, the display becomes
completely black. Unlike LCD screens, OLED pixels are self-lit, so if
the brightness becomes zero, there is no light emitted at all.
This can be fixed by clamping the minimum brightness value to 1. OLED
screens will retain contents, even if barely visible. LCD screens may be
slightly brighter on the lowest setting.
Gnome uses similar logic:
https://gitlab.gnome.org/GNOME/gnome-settings-daemon/-/blob/e30c9cdea5dbc0bf5202142491a13213f3145c92/plugins/power/gsd-backlight.c#L233
The best place I found for this is BrightnessLogic::stepToValue(), the
implementation of which had to be split to only perform clamping for
screen brightness, but not for keyboard backlight brightness.
stepToValue() is only called from increased() and decreased(). The only
call path to both these functions is action() <-
BackendInterface::calculateNextStep() <-
PowerDevilUPowerBackend::brightnessKeyPressed(). This last function is
only called from BrightnessControl's increaseBrightness() and
decreaseBrightness() and KeyboardBrightnessControl's
increaseKeyboardBrightness(), decreaseKeyboardBrightness() and
toggleKeyboardBacklight(). All these functions are only called in
response to keyboard shortcut presses issued by the user.
Limiting clamping to user key interaction should minimize possible
impact on other functionality that may actually want to set zero
brightness, for example to disable display when the laptop lid is
closed. A comment in DimDisplay::onIdleTimeout() suggests that such
cases could be possible.
The OSD display still displays the minimum brightness as "0%", even
though the actual set value is 1. I have not checked why it works this
way.
---
See https://cdn.discordapp.com/attachments/745210855725203516/1071241573842567209/oled.mp4 for comparison of the new behavior (first part) and the old behavior (second part).5.27