----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125847/#review102537 -----------------------------------------------------------
it looks good to me, but I don't have a touch screen to test on unfortunately ui/presentationwidget.cpp (line 538) <https://git.reviewboard.kde.org/r/125847/#comment68336> always use {} - Martin Tobias Holmedahl Sandsmark On Aug. 13, 2016, 7:46 p.m., Oliver Sander wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/125847/ > ----------------------------------------------------------- > > (Updated Aug. 13, 2016, 7:46 p.m.) > > > Review request for Okular. > > > Bugs: 354012 > http://bugs.kde.org/show_bug.cgi?id=354012 > > > Repository: okular > > > Description > ------- > > This patch adds support for swipe gestures to the presentation mode of > okular. Swiping right-to-left goes to the previous page, swiping > left-to-right goes to the next page. > > Notes: > > 1) The swipe gesture used here is a three-finger swipe, which is what the Qt > QSwipeGesture class implements. I am not convinced that using three fingers > is optimal here, but it is the easiest to implement. Other swiping gestures > are possible, but you would need to implement a custom QGestureRecognizer. > That would make the patch larger and more error-prone. > > 2) Swiping from left to right(!) makes the presentation mode go to the > next(!) slide. This is the opposite direction compared to what you do when > you flip pages in a physical book, so it may feel strange. It is, however, > the direction used in the qt5 gesture example at > http://doc.qt.io/qt-5/gestures-overview.html Let me know if you want the > directions reversed. > > 3) By default, Qt5 synthesizes a mouse event for each unhandled touch event. > In particular, a left mouse click is synthesized for each finger tap to the > screen (which usually makes the presentation go to the next slide). This > mechanism gets in the way of gesture recognition, because the first finger > touch of your swipe gesture will already create a mouse-click and make your > presentation go to the next page, irrespective of the swipe direction. > > The patch solves this problem but switching off mouse event synthesis in > presentation mode. That's the line > > > QCoreApplication::setAttribute(Qt::AA_SynthesizeMouseForUnhandledTouchEvents,false); > > in the constructor of PresentationWidget. This has short-term side effects. > For example, with this, you cannot finger-touch on a movie to start it > anymore. The fix for this would be to implement proper handling of > QTouchEvents, which should not be difficult. > > An alternative would be to leave mouse event synthesis turned on, but > implement a dummy touch event handler. This will need only a few lines of > code, but it will not avoid the side effects mentioned above. > > 4) This patch applies to the 'frameworks' branch. I failed at backporting it > to 'master'. There, the event loop would never receive any touchscreen > events at all. This may be a Qt bug, but it may as well be my lack of > experience. > > > Diffs > ----- > > ui/presentationwidget.h 38bb679 > ui/presentationwidget.cpp 3680de1 > > Diff: https://git.reviewboard.kde.org/r/125847/diff/ > > > Testing > ------- > > Tested on a Thinkpad Yoga running Debian testing. > > > Thanks, > > Oliver Sander > >
