D5097: Add support for global touchpad swipe gestures

[David Edmundson]

davidedmundson accepted this revision.
davidedmundson added a comment.
This revision is now accepted and ready to land.


  All good I think, a few questions.

INLINE COMMENTS

> gestures.cpp:95
> +void GestureRecognizer::updateSwipeGesture(const QSizeF &delta)
> +{
> +    m_swipeUpdates << delta;

I think you need to handle delta being zero in both directions and return without cancelling

> gestures.cpp:136
> +
> +void GestureRecognizer::endSwipeGesture()
> +{

Hypothetical question: Is it possible to have a start and end without an update?

> gestures.h:126
> +    QMap<Gesture*, QMetaObject::Connection> m_destroyConnections;
> +    QVector<QSizeF> m_swipeUpdates;
> +};

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?

> globalshortcuts.cpp:36
>  
> +uint qHash(SwipeDirection direction)
> +{

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.

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20170327/b8c93128/attachment-0001.html>