|Summary:||Trackpad gestures and zooming behave very, very poorly on OSX|
|Component:||Data display||Assignee:||Nobody <nobody>|
|Severity:||major||CC:||ken.shirriff, martin-sigrokbugs, rgovostes, richardcoleman7, soeren, uwe, zfefm|
|Version:||unreleased development snapshot|
|OS:||Mac OS X|
Patch to disable anti-aliasing on high-resolution displays
Patch to use multiple drawLine() calls instead of drawPolyline()
Partial patch towards using QGestureEvents
Description gopiballava 2017-10-23 00:33:46 CEST
When running PulseView on a MacBook with the standard trackpad setup, scrolling results in slow and inconsistent behavior. I believe that PulseView is confused by the combination of horizontal and vertical scroll and zoom events that the trackpad generates. Disabling all of them using a combination of the Trackpad and Accessibility control panels is necessary for the app to not randomly slow down and freeze and slowly scroll and zoom for 20+ seconds after your last trackpad operation - the inertia that the system uses when generating events likely results in many small scroll events. With all of the scroll events disabled, the UI behaves reasonably - though it still seems to somehow be getting some scroll-like events in some situations.
Comment 1 gopiballava 2017-10-23 00:35:16 CEST
Oh - I'm running a nightly build, 0.5.0-get-83b9c07
Comment 2 Soeren Apel 2017-10-23 01:30:46 CEST
Probably a Qt bug, see https://bugreports.qt.io/browse/QTBUG-15869 https://bugreports.qt.io/browse/QTBUG-22269 https://bugreports.qt.io/browse/QTBUG-16684 I don't have OSX available for testing either way, so patches are welcome.
Comment 3 Ken Shirriff 2017-11-02 06:34:22 CET
I'm using PulseView 0.5.0-git-83b9c07 and I'm seeing similar behavior. If I do a trackpad pinch zoom, PulseView can spend 20 seconds slowly zooming in while I'm not touching anything.
Comment 4 Uwe Hermann 2017-11-02 12:44:47 CET
Since this seems to be an upstream Qt bug (of the Qt Mac OS X port) there's not all that much we can do about it, I guess. A potential workaround could be something like this (mentioned in one of the Qt bug reports) if anyone wants to provide (and test) a patch: https://bugreports.qt.io/browse/QTBUG-22269?focusedCommentId=374072&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-374072
Comment 5 Soeren Apel 2018-09-19 18:52:42 CEST
*** Bug 1282 has been marked as a duplicate of this bug. ***
Comment 6 Martin Ling 2018-09-19 19:06:22 CEST
There is a recent comment on https://bugreports.qt.io/browse/QTBUG-22269 questioning whether this is still a current issue, so might be worth responding to that (I don't have an account on that tracker atm and may not be the best point of contact). It seems like the issue is that OSX generates a large number of trackpad events in a short time, which end up in a queue that PV then processes sequentially.
Comment 7 Soeren Apel 2018-09-19 19:21:24 CEST
The thing about https://bugreports.qt.io/browse/QTBUG-22269 is that the author was quite specific: "QWebView and QGraphicsWebView are exhibiting very slow scrolling", leaving the impression that this is not a generic issue with Qt. QWebKit is deprecated as of Qt 5.5 and will eventually be removed. Hence the question whether it's still relevant. I found https://bugreports.qt.io/browse/QTBUG-44964 and while looking promising, it makes no mention of wheel events - so rather unlikely to be the solution here. The workaround mentioned in https://bugreports.qt.io/browse/QTBUG-22269 is not yet possible since the required change (https://bugreports.qt.io/browse/QTBUG-63681) has been implemented in the unreleased 5.12 branch. I'm rather stuck at this point.
Comment 8 Martin Ling 2018-09-21 18:14:57 CEST
Could we handle this on the PV side by: - In PV's handler for scroll events, add the events to a queue rather than acting on them immediately. - In an idle handler, loop through the queued scroll events and merge them, or select the most recent one, then act on that.
Comment 9 Ryan Govostes 2019-03-20 02:00:51 CET
I was attempting to change the gesture recognizer code in the Viewport class and discovered something: if I completely remove the touch_event() implementation, zooming and panning the trace becomes (a) smooth and (b) more intuitive, by which I mean, two-finger panning left and right Just Works(tm), and when I pan up and down, it zooms. My guess is that the Qt/macOS are already generating QWheelEvents for the two-finger scroll gestures, and this is interfering with Viewport's own gesture recognition. Changes with lesser effects: - I disabled anti-aliasing because on Retina displays it is not really necessary, and - I switched from using drawPolyline() in analogsignal.cpp to drawing lines with drawLine(); this creates a little bit of a Moiré effect at low zoom levels I suggest refactoring Viewport to use QGestureEvents to support pinch-to-zoom and other gestures as needed.
Comment 10 Ryan Govostes 2019-03-20 05:23:11 CET
Created attachment 518 [details] Patch to disable anti-aliasing on high-resolution displays
Comment 11 Ryan Govostes 2019-03-20 05:31:40 CET
Created attachment 519 [details] Patch to use multiple drawLine() calls instead of drawPolyline() Using drawLine() seems to be hundreds to thousands of times faster than drawPolyline().
Comment 12 Ryan Govostes 2019-03-20 05:39:46 CET
Created attachment 520 [details] Partial patch towards using QGestureEvents I'm attaching an incomplete patch that switches from manually interpreting touch events to just using QGestureEvents. This basically makes the Mac app usable again, but it isn't ready to merge. - Two-finger scrolling and zooming work fine (already handled as QWheelEvents) - Pinch-to-zoom works OK, but it doesn't center the zoom on the cursor, so it's not natural. - macOS doesn't seem to generate the QPanGesture event so I could not implement the handler for that. I don't have more to add to this patch, so if someone wants to complete it, please do.
Comment 13 Uwe Hermann 2019-03-28 19:53:38 CET
The anti-aliasing and hiDPI patches are merged, thanks! Not technically related to the trackpad issues per se I guess, hence the bug number is not mentioned in the commit messages.