[PATCH] [synaptic] swipe actions based on the tree-fingers touch

Fri Sep 5 09:51:19 PDT 2014

Hi Peter,
Hi Gabriele,

First of all thanks for reviewing the code and I'm glad, that you see
this patch as a valuable one. At the moment of writing it, I thought,
that it is a good approach to "hack" swipe functionality into the core
of the synaptic driver. However right now I'm not sure about that...

For a long time I was concerned, that such a approach might turn out to
be a dead end. At the moment on Mac one can define actions for gestures
with up to four fingers (and all of them are useful). There are also
possibilities to combine gesture with key modifiers. So I think, that
similar combinations should be possible in the X system. Furthermore,
one should be able to configure actions if the given gesture should
generate mouse click/release event or click->movie_pointer->release.
Such an event is useful for moving windows with touchpad gesture. And
the list of combinations goes on, including the xbindkeys mentioned by
Gabriele.

But long story short. I think, that gesture engine should be separated
from the driver itself. However, right now I'm not able to figure out
how to pass key press/release event into such a gesture engine without
an ugly hack with running daemon (like syndaemon). But maybe this idea
is completely wrong and I'm insane :)

So, as for now, I agree with you Peter, that only the right/left swipe
action is acceptable for strict incorporation into the synaptic driver.
If everyone agree with that, I can generate appropriate patch including
your corrections.

And lastly:

> > +    horizSwipeThreshold = width * 0.15;
> > +    vertSwipeThreshold = height * 0.25;  
> why two different values here?

The reason why I've used two different values, was based on the
ergonomic of hand movement. Moving fingers up and down is quit easy and
the generated delta y can be big, so setting threshold to a small value
might generate false positive triggers. On the other hand, so to speak,
moving hand horizontally is not very easy. So in order to trigger event
the threshold has to be sufficiently bigger. I've tested this
functionality on Samsung NP530 laptop which has rectangular touchpad.
Furthermore, this touchpad is not very big, so comfortable placing
three fingers on it and then moving hand horizontally generates small
delta x.

Best regards,
Arkadiusz

PS.
I have no subscription for xorg development list, so in general I'm able
to see posts in this thread only.

On Fri, 05 Sep 2014 13:35:08 +0200
Gabriele Mazzotta <gabriele.mzt at gmail.com> wrote:
> On Friday 05 September 2014 14:10:30 Peter Hutterer wrote:
> > Hi Arkadiusz,
> > 
> > sorry about the delay on the review here.
> > 
> > On Fri, Apr 25, 2014 at 07:31:44PM +0200, Arkadiusz Bokowy wrote:
> > > When touchpad can report more then 2 active fingers, we can generate action
> > > based on swipe gesture. It includes upward, downward, to the left and to the
> > > right swipes. To all of those gestures can be assigned one button event. By
> > > default (internal driver default) swipes are disabled (assigned button 0).
> > > The swipe length (hand displacement) required for triggering button event can
> > > be set independently for horizontal and vertical movement. There is also
> > > possibility for triggering repeatable events during swipe handling, however
> > > it is disabled by default.
> > > 
> > > The most preferable button assignment is button 8, 9, 10 and 11 for to the
> > > left, to the right, upward and downward swipes respectively. Such a
> > > configuration provides browser history navigation (e.g. Firefox) with
> > > horizontal swipes. Button 10 and 11 are not used, though.
> > > 
> > > Added Synaptic driver options:
> > > * SwipeUpButton, SwipeDownButton, SwipeLeftButton, SwipeRightButton
> > > * HorizSwipeThreshold, VertSwipeThreshold
> > > * SingleSwipe
> > > 
> > > This changes was developed on Samsung NP530U3C with the ETPS/2 Elantech
> > > Touchpad, which can report up to three-finger touch. Also tested on the
> > > SynPS/2 Synaptics TouchPad. Patch is generated for the driver version 1.7.4.
> > > 
> > > Signed-off-by: Arkadiusz Bokowy <arkadiusz.bokowy at gmail.com>
> > > ---
> > >  include/synaptics-properties.h | 10 +++++
> > >  man/synaptics.man              | 28 ++++++++++++
> > >  src/properties.c               | 44 +++++++++++++++++++
> > >  src/synaptics.c                | 99 +++++++++++++++++++++++++++++++++++++++++-
> > >  src/synapticsstr.h             | 21 +++++++++
> > >  tools/synclient.c              |  7 +++
> > >  6 files changed, 207 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/include/synaptics-properties.h b/include/synaptics-properties.h
> > > index 19bd2b2..e74ffd0 100644
> > > --- a/include/synaptics-properties.h
> > > +++ b/include/synaptics-properties.h
> > > @@ -104,6 +104,16 @@
> > >   * element. order: Finger 1, 2, 3 */
> > >  #define SYNAPTICS_PROP_CLICK_ACTION "Synaptics Click Action"
> > >  
> > > +/* 8 bit, up to MAX_SWIPE values (see synaptics.h), 0 disables an
> > > + * element. order: Up, Down, Left, Right */
> > > +#define SYNAPTICS_PROP_SWIPE_ACTION "Synaptics Swipe Action"
> > 
> > no references to header files please. Just say there are 4 values in it.
> > 
> > > +
> > > +/* 8 bit, 2 values, vertical, horizontal */
> > > +#define SYNAPTICS_PROP_SWIPE_THRESHOLD "Synaptics Swipe Threshold"
> > > +
> > > +/* 8 bit (BOOL) */
> > > +#define SYNAPTICS_PROP_SINGLE_SWIPE "Synaptics Single Swipe"
> > 
> > I don't think we need this one. I'm ok with adding support for three-finger
> > swipe but if we only map them to buttons anyway, a single button per gesture
> > is enough. The only time where multiple buttons are useful is for scroll
> > buttons and that's where we already have two-finger scrolling for.
> > 
> > that brings up the next question: should we even support vertical scroll. in
> > X, due to a number of limitations the only buttons that make sense to map to
> > are 8/9 for back/forward. Some specialised apps may be able to use 10/11
> > etc. but I'd rather have a small limited feature that has predictable
> > outcomes.
> > 
> > so I'm in favour of dropping the single swipe toggle, up/down and, quite
> > frankly, four finger swiping. Unless gabriele comes up with a really good
> > use-case, just exposing what the hw can do isn't enough tbh.
> 
> I'm using this patch and xbindkeys to perform specific actions. The reasons
> why I added four finger gestures were that first of all, the code was there
> already, I just did few changes, and second, I wanted to be able to perform
> more than two actions.
> For example, I wanted to be able to navigate in the browser history and at
> the same time control my music player running in background without
> conflicts.
> 
> Using the gestures to perform actions through programs like xbindkeys is 
> the only use-case in which having all these gestures could be useful.
> If you use the buttons sent for what they are, I agree with you, horizontal
> three-finger swipes should be enough.
> 
> PS: the patch I sent had a small mistake:
> 
> diff --git a/src/properties.c b/src/properties.c
> index ac2d8f0..dedb05d 100644
> --- a/src/properties.c
> +++ b/src/properties.c
> @@ -712,7 +712,7 @@ SetProperty(DeviceIntPtr dev, Atom property, XIPropertyValuePtr prop,
>          int i;
>          CARD8 *action;
>  
> -        if (prop->size > MAX_SWIPE || prop->format != 8 ||
> +        if (prop->size > 2 * MAX_SWIPE || prop->format != 8 ||
>              prop->type != XA_INTEGER)
>              return BadMatch;
>  
> 
> Regards,
> Gabriele