Skip to content

daemon: Limit KAuth backlighthelper calls to only one at a time

Jakob Petsovits requested to merge work/jpetso/backlight-snappy-slider into master

Turns out #27 or plasma-workspace!3178 (closed) were not necessary to make the brightness slider laggy again. CC @nicolasfella, @nclarius, cheers! 🍻

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, but damn has this code come a long way since then already.

Merge request reports