graesslin added inline comments.

INLINE COMMENTS

> davidedmundson wrote in gestures.cpp:95
> I think you need to handle delta being zero in both directions and return 
> without cancelling

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.

> davidedmundson wrote in gestures.cpp:136
> Hypothetical question: Is it possible to have a start and end without an 
> update?

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.

> davidedmundson wrote in gestures.h:126
> I don't get what this is for.
> 
> You append to the list and you clear it, but it's used for any branch 
> decisions.
> 
> Unless it's a future thing for more advanced swipe analysis later?

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.

> davidedmundson wrote in globalshortcuts.cpp:36
> what's this for?
> 
> You don't make a QHash of SwipeDirections anywhere, and even if you did, it 
> should be implicity done for enums.

I do: QHash<Qt::KeyboardModifiers, QHash<SwipeDirection, GlobalShortcut*> > 
m_swipeShortcuts;

and no, it's not implicit. I only added because I got a compile error.

BRANCH
  gesture-support

REVISION DETAIL
  https://phabricator.kde.org/D5097

To: graesslin, #kwin, #plasma_on_wayland, davidedmundson
Cc: davidedmundson, plasma-devel, progwolff, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, eliasp, sebas, apol

Reply via email to