<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/125847/">https://git.reviewboard.kde.org/r/125847/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On February 17th, 2017, 7:43 p.m. UTC, <b>Martin Tobias Holmedahl Sandsmark</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">it looks good to me, but I don't have a touch screen to test on unfortunately</p></pre>
</blockquote>
<p>On February 19th, 2017, 11:45 a.m. UTC, <b>Albert Astals Cid</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I recently got a laptop with a touchscreen, it's not my main devel machine but i'll try to find time to setup stuff there and give it a try.</p></pre>
</blockquote>
<p>On February 19th, 2017, 11:10 p.m. UTC, <b>Albert Astals Cid</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">tested, don't think it's a good idea to add a feature and remove others, for example without your patch i can use the drawing stuff to draw on screen, with it i can not.</p></pre>
</blockquote>
<p>On February 27th, 2017, 9:05 p.m. UTC, <b>Oliver Sander</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I am aware of this problem, but I don't know what the solution is. Here are a few thoughts:</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The current behavior has to change at least a little bit if you want gesture support. For example, currently, for each single-finger touch, Qt synthesizes a MouseEvent, which makes the presentation proceed to the next page. This will always interfere with swipe gestures, because when you put three fingers onto the screen for a swipe, usually one finger touches the screen before the others, and you are off to the next page before Qt could notice that you are planning to do a three finger swipe. Therefore this single-finger-synthesizes-a-mouse-click has to be switched off.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Same with the drawing stuff you mention. I haven't tried it, but I am pretty sure that if you implement single-finger drawing together with page-changing-by-swiping, then you will always get a small line drawing every time you try to swipe. For the same reason: it is very difficult to put three fingers onto the screen at the same time.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I don't really know how this would be solved ideally. Disabling swipe gestures when drawing mode is disabled? Doesn't sound good. I am afraid there is not one single behavior that can make everybody happy. For you, drawing with the finger is the natural thing to do (and I agree!). However, my machine comes with a stylus, which creates TabletEvents. I would never want to draw with my finger anyways, because I have that stylus. Therefore, the lost ability to draw with my fingers hasn't bothered me. Do we need to add configuration options for the touch/gesture behavior? What would those be?</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I'd be happy to change my patch, but I don't know what direction to take. Suggestions would be helpful.</p></pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">As far as I can see, the easiest solution is move the slotNextPage/slotPrevPage from mousePressEvent to mouseReleaseEvent. This shouldn't be a problem for the typical "click to go to next page", but allows you to put the three fingers on the screen and to a swipe before the next page code is triggered.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Do I make sense?</p></pre>
<br />
<p>- Albert</p>
<br />
<p>On February 17th, 2017, 8:06 p.m. UTC, Oliver Sander wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
<div>Review request for Okular.</div>
<div>By Oliver Sander.</div>
<p style="color: grey;"><i>Updated Feb. 17, 2017, 8:06 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="http://bugs.kde.org/show_bug.cgi?id=354012">354012</a>
</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
okular
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Notes:</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The patch solves this problem but switching off mouse event synthesis in presentation mode. That's the line</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;"><div class="codehilite" style="background: #f8f8f8"><pre style="line-height: 125%"><span></span><span style="color: #008000; font-weight: bold">QCoreApplication</span><span style="color: #666666">:</span><span style="color: #AA22FF">:setAttribute</span><span style="color: #666666">(</span><span style="color: #008000; font-weight: bold">Qt</span><span style="color: #666666">:</span><span style="color: #AA22FF">:AA_SynthesizeMouseForUnhandledTouchEvents</span><span style="color: #666666">,</span><span style="color: #008000; font-weight: bold">false</span><span style="color: #666666">);</span>
</pre></div>
</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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.</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Tested on a Thinkpad Yoga running Debian testing.</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>ui/presentationwidget.h <span style="color: grey">(69574d22)</span></li>
<li>ui/presentationwidget.cpp <span style="color: grey">(526c235e)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/125847/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>