battery: Improve the brightness responsiveness
ClosedPublic

Authored by apol on Dec 16 2019, 1:14 AM.

Details

Summary

Use QQC2.Slider, so that we have a moved signal. This way we can only
issue new brightnesses when the user actually interacts with it.
Don't adapt to the system brightness until we have finished interacting
with it.

Test Plan

Manual testing, flickering is very much reduced both when scrolling over the
compact plasmoid as well as the slider.

Diff Detail

Repository
R120 Plasma Workspace
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
apol created this revision.Dec 16 2019, 1:14 AM
Restricted Application added a project: Plasma. · View Herald TranscriptDec 16 2019, 1:14 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
apol requested review of this revision.Dec 16 2019, 1:14 AM

PlasmaComponents 3 Slider doesn't react to wheel events, which must be fixed before this can go in.

applets/batterymonitor/package/contents/ui/PopupDialog.qml
48

What about this?

applets/batterymonitor/package/contents/ui/batterymonitor.qml
109

Should we not forward the notion of "moved" here, too?

applets/batterymonitor/package/contents/ui/logic.js
53

Wouldn't that cause subsequent calls to fail if the previous hasn't finished yet? Wouldn't that be annoying if you drag the slider quickly?

apol updated this revision to Diff 71743.Dec 17 2019, 6:52 PM

Use a more meaningful step size for the screen. Having steps of 1 in a 1 to 7000 slider was awkward.

apol marked an inline comment as done.Dec 30 2019, 5:50 PM
apol added inline comments.
applets/batterymonitor/package/contents/ui/logic.js
53

No, it will be called eventually, which is all that matters.

broulik accepted this revision.Dec 30 2019, 5:55 PM

Seems to work well, thanks

This revision is now accepted and ready to land.Dec 30 2019, 5:55 PM
This revision was automatically updated to reflect the committed changes.
frenzie added inline comments.
applets/batterymonitor/package/contents/ui/PopupDialog.qml
73

Because it's artificially limited to 100 positions, this change makes it impossible to set a value by clicking on a position or by dragging the slider with the pointer. It snaps to one of the 100 positions and the precision we want just isn't there.

On my laptop the slider currently consists of ~582 pixels in width. (Just a quick screenshot selection-based estimate; the exact value is irrelevant other than to point out that it's very unlikely to be less than ~450 and extremely likely to be much more.) The organic limit is therefore 582 on my laptop. In the example given above, 7000/582 = 12.

I assume there has to be reason for this change, so an organic limit would mean something along the lines of:

stepSize: batterymonitor.maximumScreenBrightness / this.width

But intuitively it seems that a slider would have to do that all by itself no matter what, which just leaves me confused as to what the purpose behind this line is.

I hope it can simply be removed.