<table><tr><td style="">graesslin added inline comments.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D5097" rel="noreferrer">View Revision</a></tr></table><br /><div><strong>INLINE COMMENTS</strong><div><div style="margin: 6px 0 12px 0;"><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D5097#inline-21343" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">davidedmundson</span> wrote in <span style="color: #4b4d51; font-weight: bold;">gestures.cpp:95</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">I think you need to handle delta being zero in both directions and return without cancelling</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">I think a delta of 0/0 just cannot happen. libinput should not send us an update in such a case. We only get updates if the fingers moved. Thus 0/0 is just impossible.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D5097#inline-21345" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">davidedmundson</span> wrote in <span style="color: #4b4d51; font-weight: bold;">gestures.cpp:136</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">Hypothetical question: Is it possible to have a start and end without an update?</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Theoretically yes, practically I doubt that any user would be able to rest four fingers on the touchpad, trigger the swipe gesture (which already needs movements) and then raise without further movement. Similar for touch screen events: it would mean the user clicks a point without moving the finger even a little bit.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D5097#inline-21344" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">davidedmundson</span> wrote in <span style="color: #4b4d51; font-weight: bold;">gestures.h:126</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">I don't get what this is for.</p>

<p style="padding: 0; margin: 8px;">You append to the list and you clear it, but it's used for any branch decisions.</p>

<p style="padding: 0; margin: 8px;">Unless it's a future thing for more advanced swipe analysis later?</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">It's a future thing and used in the follow up patch for touch screen swipes where we need to get the minimum swipe distance.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D5097#inline-21375" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">davidedmundson</span> wrote in <span style="color: #4b4d51; font-weight: bold;">globalshortcuts.cpp:36</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">what's this for?</p>

<p style="padding: 0; margin: 8px;">You don't make a QHash of SwipeDirections anywhere, and even if you did, it should be implicity done for enums.</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">I do: QHash<Qt::KeyboardModifiers, QHash<SwipeDirection, GlobalShortcut*> > m_swipeShortcuts;</p>

<p style="padding: 0; margin: 8px;">and no, it's not implicit. I only added because I got a compile error.</p></div></div></div></div></div><br /><div><strong>BRANCH</strong><div><div>gesture-support</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D5097" rel="noreferrer">https://phabricator.kde.org/D5097</a></div></div><br /><div><strong>To: </strong>graesslin, KWin, Plasma on Wayland, davidedmundson<br /><strong>Cc: </strong>davidedmundson, plasma-devel, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, eliasp, sebas, apol<br /></div>