Bug 432331

Summary: Weird behavior with high-resolution scrolling
Product: [Plasma] plasmashell Reporter: Bernhard <micraft.b>
Component: Battery MonitorAssignee: Kai Uwe Broulik <kde>
Status: RESOLVED FIXED    
Severity: normal CC: katyaberezyaka, nate, plasma-bugs
Priority: NOR    
Version: master   
Target Milestone: 1.0   
Platform: Neon   
OS: Linux   
Latest Commit: Version Fixed In: 5.22

Description Bernhard 2021-01-31 04:27:32 UTC
Scrolling up on the panel icon to increase brightness works differently to scrolling down. When scrolling up slow scrolling is completely ignored. When slowly scrolling down it seems to enforce a 'minimum speed' where slow events automatically become faster.

To me it looks like some floating point rounding mistake where it always does a ceil() or something like that.

But the weirdness does not stop here: Brightness slider inside the widget works correctly (altough very slow) for scrolling up. When scrolling down however it _always_ changes at the same speed, no matter how fast or slow you go.

Operating System: KDE neon Unstable Edition
KDE Plasma Version: 5.21.80
KDE Frameworks Version: 5.79.0
Qt Version: 5.15.2
Kernel Version: 5.11.0-051100rc5-generic (64-bit)
Graphics Platform: Wayland
Processors: 8 × AMD Ryzen 7 4700U with Radeon Graphics
Memory: 15,1 GiB of RAM
Graphics Processor: AMD RENOIR
Comment 1 Bernhard 2021-01-31 04:38:42 UTC
I only mentionend in the title: This is specific to high-resolution scrolling.

Also:
For some reason horizontal scrolling is also accepted for changing brightness.
This has none of the problems of normal vertical scrolling while hovering over the panel.

Inside the applet horizontal scrolling is exactly as bugged as vertical scrolling.


After this is fixed I would also propose removing horizontal scrolling, as not scrolling perfectly straight down on a touchpad means you sometimes have unwanted 'side effects' when it is enabled.
Comment 2 Bernhard 2021-01-31 04:40:58 UTC
Sorry, misinformation. Horizontal scrolling is in every way the same as vertical when it comes to buggy behavior.
Comment 3 Nate Graham 2021-02-01 17:56:35 UTC
Works for me with my touchpad. What do you mean exactly by "high resolution scrolling?" What device are you using to scroll? And what drivers are you using? Libinput or Synaptics/Evdev?
Comment 4 Bernhard 2021-02-01 18:22:38 UTC
Considering this was a wayland session I would guess I used libinput - wouldn't know how to check tough.

I've got the exact same problem on this system running Arch with 5.20.5 on X11 though.

This is what xinput reports on Arch, I hope this helps:

$ xinput list-props 'SYNA32A1:00 06CB:CE17 Touchpad'
Device 'SYNA32A1:00 06CB:CE17 Touchpad':
        Device Enabled (158):   1
        Coordinate Transformation Matrix (160): 0.000000, 0.000000, 0.000000, 0.000000, 0.000000, 0.000000, 0.000000, 0.000000, 0.000000
        Device Accel Profile (292):     1
        Device Accel Constant Deceleration (293):       2.500000
        Device Accel Adaptive Deceleration (294):       1.000000
        Device Accel Velocity Scaling (295):    12.500000
        Synaptics Edges (319):  53, 1279, 44, 784
        Synaptics Finger (320): 25, 30, 0
        Synaptics Tap Time (321):       180
        Synaptics Tap Move (322):       58
        Synaptics Tap Durations (323):  180, 180, 100
        Synaptics ClickPad (324):       1
        Synaptics Middle Button Timeout (325):  0
        Synaptics Two-Finger Pressure (326):    282
        Synaptics Two-Finger Width (327):       7
        Synaptics Scrolling Distance (328):     25, 25
        Synaptics Edge Scrolling (329): 0, 0, 1
        Synaptics Two-Finger Scrolling (330):   1, 1
        Synaptics Move Speed (331):     1.977633, 255.000000, 0.062808, 0.000000
        Synaptics Off (332):    2
        Synaptics Locked Drags (333):   0
        Synaptics Locked Drags Timeout (334):   1000
        Synaptics Tap Action (335):     0, 3, 0, 0, 1, 3, 2
        Synaptics Click Action (336):   1, 3, 2
        Synaptics Circular Scrolling (337):     0
        Synaptics Circular Scrolling Distance (338):    0.104720
        Synaptics Circular Scrolling Trigger (339):     0
        Synaptics Circular Pad (340):   0
        Synaptics Palm Detection (341): 0
        Synaptics Palm Dimensions (342):        10, 200
        Synaptics Coasting Speed (343): 20.000000, 50.000000
        Synaptics Pressure Motion (344):        30, 160
        Synaptics Pressure Motion Factor (345): 1.000000, 1.000000
        Synaptics Grab Event Device (346):      0
        Synaptics Gestures (347):       1
        Synaptics Capabilities (348):   1, 0, 0, 1, 1, 0, 0
        Synaptics Pad Resolution (349): 12, 12
        Synaptics Area (350):   0, 0, 0, 0
        Synaptics Soft Button Areas (351):      666, 0, 678, 0, 0, 0, 0, 0
        Synaptics Noise Cancellation (352):     7, 7
        Device Product ID (285):        1739, 52759
        Device Node (284):      "/dev/input/event6"


If you want to know anything more specific you'd have to provide me with a command for that.
Comment 5 Nate Graham 2021-02-01 19:24:38 UTC
Thanks. I wonder if it's touchpad-specific, or a quirk of Libinput's scroll hysteresis.

Does this behavior manifest in any other scrollviews? Or only when scrolling over the brightness applet? And what about when scrolling over the volume applet?
Comment 6 Bernhard 2021-02-01 19:45:45 UTC
I have not noticed similar behavior in any other application or applet. The volume applet works perfectly and at a reasonable speed (for me)
Comment 7 Nate Graham 2021-02-01 19:50:23 UTC
All right, in that case there must be something odd with the scroll handling implementation specific to the battery monitor. Thanks for your patience.
Comment 8 Bernhard 2021-02-01 19:51:41 UTC
(In reply to Bernhard from comment #6)
> I have not noticed similar behavior in any other application or applet. The
> volume applet works perfectly and at a reasonable speed (for me)

Okay, my answer was perhaps a bit hasty. 'Perfectly' is not exactly true, if I scroll to fast the volume applet seems to miss almost all scroll events. At any decently slow speed it works fine though, without weird direction dependent behavior.
Comment 11 Bernhard 2021-02-01 20:19:19 UTC
> This might have been fixed by
> https://invent.kde.org/plasma/plasma-workspace/-/merge_requests/590 Any
> chance you can test that?

I would I if I knew how to. Is there a 'simple' way to compile it to a .deb file for neon? The build instructions (https://community.kde.org/Guidelines_and_HOWTOs/Build_from_source/Plasma_5_on_Ubuntu_14.04_LTS) seem pretty old and complicated.
Comment 12 Bernhard 2021-02-03 00:39:13 UTC
It goes away when not using smooth scrolling, but I have to say that is quit unpleasant because you lose the line you're reading.

Smooth scrolling otoh is something I can only describe as extremely unresponsive. It does not seem to accelerate or slow down to with the mouse wheel but on it's own time. If I want to scroll one page exactly it will slowly begin scrolling (no matter how fast the wheel goes) and then decelerate (while the scroll wheel is already stopped) going pages further than it should.
Comment 13 Bernhard 2021-02-03 00:40:40 UTC
(In reply to Bernhard from comment #12)
> It goes away when not using smooth scrolling, but I have to say that is quit
> unpleasant because you lose the line you're reading.
> 
> Smooth scrolling otoh is something I can only describe as extremely
> unresponsive. It does not seem to accelerate or slow down to with the mouse
> wheel but on it's own time. If I want to scroll one page exactly it will
> slowly begin scrolling (no matter how fast the wheel goes) and then
> decelerate (while the scroll wheel is already stopped) going pages further
> than it should.

Sorry that comment was meant for another bug report. Please ignore it (or delete, if possible)
Comment 14 Nate Graham 2021-02-03 01:32:39 UTC
In this case the simplest thing would probably be to directly edit the file on disk--after making a backup of course!

The file in question is /usr/share/plasma/plasmoids/org.kde.plasma.battery/contents/ui/batterymonitor.qml. Open it up and edit it to mimic the changes in the diff at https://invent.kde.org/plasma/plasma-workspace/-/merge_requests/590/diffs. Lines in green are added, and lines in red are removed.

After you do this, restart plasmashell with `plasmashell --replace` and see if the changes helped.
Comment 15 Nate Graham 2021-02-03 01:32:58 UTC
(and if you're complaining about smooth scrolling in Okular, you can turn it off in the config window)
Comment 16 Bernhard 2021-02-04 17:12:14 UTC
(In reply to Nate Graham from comment #14)
> In this case the simplest thing would probably be to directly edit the file
> on disk--after making a backup of course!
> 
> The file in question is
> /usr/share/plasma/plasmoids/org.kde.plasma.battery/contents/ui/
> batterymonitor.qml. Open it up and edit it to mimic the changes in the diff
> at
> https://invent.kde.org/plasma/plasma-workspace/-/merge_requests/590/diffs.
> Lines in green are added, and lines in red are removed.
> 
> After you do this, restart plasmashell with `plasmashell --replace` and see
> if the changes helped.


Okay I tested this and it works, kind of. It does however remove fine-grained control completely and also has rounding errors where (depending on current brightness) it changes 6% instead of the otherwise 5% increments.
Comment 17 Bernhard 2021-02-04 18:12:05 UTC
Okay I fixed it.
Basically the only thing that's missing is explicit (and actually correct) rounding in the last line of the function:

Instead of this
batterymonitor.screenBrightness = Math.max(minimumBrightness, Math.min(maximumBrightness, batterymonitor.screenBrightness + deltaSteps * steps));

I should be this
batterymonitor.screenBrightness = Math.max(minimumBrightness, Math.min(maximumBrightness, batterymonitor.screenBrightness + Math.round(deltaSteps * steps)));

What the patch is trying to do (storing the rounding error for future events) would be the correct thing to do, but the way that it's done makes it worse than just this simple fix.
Comment 18 Nate Graham 2021-02-04 20:02:50 UTC
Thanks for testing. Could you please post that as a comment on that merge request? I know it's closed, but perhaps it can be re-opened. Also reference this bug report by its URL.
Comment 19 Bernhard 2021-02-04 20:32:54 UTC
(In reply to Nate Graham from comment #18)
> Thanks for testing. Could you please post that as a comment on that merge
> request? I know it's closed, but perhaps it can be re-opened. Also reference
> this bug report by its URL.

I have tried to really fix this properly and have started a merge request for that here:
https://invent.kde.org/plasma/plasma-workspace/-/merge_requests/632

Please let me know it there is anything wrong with it as I am not too experienced
Comment 20 Nate Graham 2021-04-30 16:14:54 UTC
Git commit ff4fa345c5119000809c8bbb1357285cfd168a1e by Nate Graham, on behalf of Bernhard Sulzer.
Committed on 30/04/2021 at 16:14.
Pushed by ngraham into branch 'master'.

Fix battery applet high-res scrolling

Rounding errors made scrolling behave weirdly for devices supplying very
low scroll delta values. This is fixed now. For non-high resolution devices,
scrolling is now also aligned to 5%, just like the keyboard shortcuts.
FIXED-IN: 5.22

M  +1    -1    applets/batterymonitor/package/contents/ui/BrightnessItem.qml
M  +17   -7    applets/batterymonitor/package/contents/ui/CompactRepresentation.qml

https://invent.kde.org/plasma/plasma-workspace/commit/ff4fa345c5119000809c8bbb1357285cfd168a1e