I don't think there is an ideal solution for this problem, this one has the disadvantage that the code handling wheel scrolling is not related to the scroll bar any more. It still seems like the simplest one. What do you think?
Not yet thoroughly reviewed, but some thoughts:
- There is a bug report for this: https://bugs.kde.org/show_bug.cgi?id=256561 Could you verify that the issues in the bug report are addressed by this patch? See also the comments by @ngraham in this bug report.
- could you add: BUG: 256561
- Since the commit logs now always appear in the KF5 changelog, maybe the subject could read: Fix: Scroll view lines instead of not real lines for wheel and touchpad scrolling
I will modify the commit message as suggested.
I read through the bug report, and I think this patch does fix the main issue discussed there. The patch suggested by one user there is even quite similar (if a bit less complete) than this one. The original complaint (touchpad vs. mouse wheel) to me personally sounds a bit strange and I'm not sure if that is actually the point. I'm not even sure if that is what the original reporter was in fact annoyed by (in contrast to "what he wrote he was annoyed by").
Thanks for the patch, Sven!
There are two issues with scrolling:
- Multi-line strings are inappropriately treated as a single line for the purposes of scrolling
- Lack of per-pixel scrolling that touchpad users expect.
This patch fixes #1, but not #2. I've untangles the Bugzilla tickets such that https://bugs.kde.org/show_bug.cgi?id=256561 now only tracks #1, so I think you're free to add BUG: 256561 to this patch. We will track pixel-by-pixel scrolling separately with https://bugs.kde.org/show_bug.cgi?id=378275.
Hmm, I understood this differently. I thought this happens when you react to each input event sent by the kernel, regardless of its delta, like e.g. the old gwenview code did which you linked: it just zooms in once per event. Thus devices which generate more events with smaller deltas zoom in faster, which is not good. And this bug is definitely at the application level, not in libinput.
For me this works. It would be nice to get more feedback, but to be honest, I think we only get this feedback by integrating this patch and let the users test it. It's certainly not broken and fixes a usability issue we've had for a log time.
Please commit [and blog about it!].
The Gwenview issue you're talking about manifested in touchpad scrolling feeling too fast. IMHO, touchpad scrolling in KTextEditor views feels perfect right now, so we don't have that particular issue. However yes, we should probably be taking into account the delta. But maybe that's material for another patch?
I will indeed blog about this. If you'd like, I can request that people specifically test it out and offer feedback.
I'll test the patch ASAP, but I think there's a simple guideline to observe here (apologies if it's as much an open door as I think it is):
Does scrolling work as it should when dynamic wrapping is off, with and/or without this patch (and that would include the accidental wheel-zoom protection)?
The issue related to touchpad scrolling should indeed be tracked elsewhere because it's bound to have a platform-specific aspect (while this one here shouldn't).
IMHO, touchpad scrolling in KTextEditor views feels perfect right now
I have no issues with it either - never had on Mac (still the reference for anything touchpad related in my experience ... but for that you need to have experienced the Magic Trackpad :)). On Linux ... let's say it depends on how you well you manage to calibrate the thing with synclient.