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 <[email protected]> > --- > 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. > + > /* 8 bit (BOOL) */ > #define SYNAPTICS_PROP_CIRCULAR_SCROLLING "Synaptics Circular Scrolling" > > diff --git a/man/synaptics.man b/man/synaptics.man > index 7da7527..d3e4fb3 100644 > --- a/man/synaptics.man > +++ b/man/synaptics.man > @@ -358,6 +358,34 @@ Left/Right/Top/BottomEdge parameters. > . > For circular touchpads. Property: "Synaptics Circular Pad" > .TP > +.BI "Option \*qSwipeUpButton\*q \*q" integer \*q > +Which mouse button is reported on a upward tree-finger swipe. Property: > +"Synaptics Swipe Action" see Gabriele's patch, this should have Three in the name to be less ambiuous. > +.TP > +.BI "Option \*qSwipeDownButton\*q \*q" integer \*q > +Which mouse button is reported on a downward tree-finger swipe. Property: > +"Synaptics Swipe Action" > +.TP > +.BI "Option \*qSwipeLeftButton\*q \*q" integer \*q > +Which mouse button is reported on a tree-finger swipe to the left. Property: > +"Synaptics Swipe Action" > +.TP > +.BI "Option \*qSwipeRightButton\*q \*q" integer \*q > +Which mouse button is reported on a tree-finger swipe to the right. Property: > +"Synaptics Swipe Action" > +.TP > +.BI "Option \*qHorizSwipeThreshold\*q \*q" integer \*q > +Horizontal threshold for triggering swipe event. Property: "Synaptics Swipe > +Threshold" > +.TP > +.BI "Option \*qVertSwipeThreshold\*q \*q" integer \*q > +Vertical threshold for triggering swipe event. Property: "Synaptics Swipe > +Threshold" both thresholds need a unit specification. The rest looks good, but as I said above I think we should limit this patch to do a lot less. Cheers, Peter > +.TP > +.BI "Option \*qSingleSwipe\*q \*q" boolean \*q > +Trigger only one button click event per swipe. Property: "Synaptics Single > +Swipe" > +.TP > .BI "Option \*qPalmDetect\*q \*q" boolean \*q > If palm detection should be enabled. > . > diff --git a/src/properties.c b/src/properties.c > index 886d8c2..140cf6e 100644 > --- a/src/properties.c > +++ b/src/properties.c > @@ -80,6 +80,9 @@ Atom prop_circscroll = 0; > Atom prop_circscroll_dist = 0; > Atom prop_circscroll_trigger = 0; > Atom prop_circpad = 0; > +Atom prop_swipeaction = 0; > +Atom prop_swipethreshold = 0; > +Atom prop_singleswipe = 0; > Atom prop_palm = 0; > Atom prop_palm_dim = 0; > Atom prop_coastspeed = 0; > @@ -298,6 +301,18 @@ InitDeviceProperties(InputInfoPtr pInfo) > prop_circpad = > InitAtom(pInfo->dev, SYNAPTICS_PROP_CIRCULAR_PAD, 8, 1, > ¶->circular_pad); > + > + memcpy(values, para->swipe_action, MAX_SWIPE * sizeof(int)); > + prop_swipeaction = > + InitAtom(pInfo->dev, SYNAPTICS_PROP_SWIPE_ACTION, 8, MAX_SWIPE, > values); > + values[0] = para->swipe_threshold_x; > + values[1] = para->swipe_threshold_y; > + prop_swipethreshold = > + InitAtom(pInfo->dev, SYNAPTICS_PROP_SWIPE_THRESHOLD, 32, 2, values); > + prop_singleswipe = > + InitAtom(pInfo->dev, SYNAPTICS_PROP_SINGLE_SWIPE, 8, 1, > + ¶->single_swipe); > + > prop_palm = > InitAtom(pInfo->dev, SYNAPTICS_PROP_PALM_DETECT, 8, 1, > ¶->palm_detect); > @@ -675,6 +690,35 @@ SetProperty(DeviceIntPtr dev, Atom property, > XIPropertyValuePtr prop, > > para->circular_pad = *(BOOL *) prop->data; > } > + else if (property == prop_swipeaction) { > + int i; > + CARD8 *action; > + > + if (prop->size > MAX_SWIPE || prop->format != 8 || > + prop->type != XA_INTEGER) > + return BadMatch; > + > + action = (CARD8 *) prop->data; > + > + for (i = 0; i < MAX_SWIPE; i++) > + para->swipe_action[i] = action[i]; > + } > + else if (property == prop_swipethreshold) { > + INT32 *swipe_thresholds; > + > + if (prop->size != 2 || prop->format != 32 || prop->type != > XA_INTEGER) > + return BadMatch; > + > + swipe_thresholds = (INT32 *) prop->data; > + para->swipe_threshold_x = swipe_thresholds[0]; > + para->swipe_threshold_y = swipe_thresholds[1]; > + } > + else if (property == prop_singleswipe) { > + if (prop->size != 1 || prop->format != 8 || prop->type != XA_INTEGER) > + return BadMatch; > + > + para->single_swipe = *(BOOL *) prop->data; > + } > else if (property == prop_palm) { > if (prop->size != 1 || prop->format != 8 || prop->type != XA_INTEGER) > return BadMatch; > diff --git a/src/synaptics.c b/src/synaptics.c > index 0986e20..ea6c7e3 100644 > --- a/src/synaptics.c > +++ b/src/synaptics.c > @@ -548,6 +548,7 @@ set_default_parameters(InputInfoPtr pInfo) > Bool vertTwoFingerScroll, horizTwoFingerScroll; > int horizResolution = 1; > int vertResolution = 1; > + int horizSwipeThreshold, vertSwipeThreshold; > int width, height, diag, range; > int horizHyst, vertHyst; > int middle_button_timeout; > @@ -614,6 +615,12 @@ set_default_parameters(InputInfoPtr pInfo) > vertTwoFingerScroll = priv->has_double ? TRUE : FALSE; > horizTwoFingerScroll = FALSE; > > + /* Calculate the minimal required swipe distance for horizontal and > + * vertical events. Default is 15 and 25 per cent of the width and > + * the height respectively. */ > + horizSwipeThreshold = width * 0.15; > + vertSwipeThreshold = height * 0.25; why two different values here? > + > /* Use resolution reported by hardware if available */ > if ((priv->resx > 0) && (priv->resy > 0)) { > horizResolution = priv->resx; > @@ -702,6 +709,19 @@ set_default_parameters(InputInfoPtr pInfo) > xf86SetBoolOption(opts, "CircularScrolling", FALSE); > pars->circular_trigger = xf86SetIntOption(opts, "CircScrollTrigger", 0); > pars->circular_pad = xf86SetBoolOption(opts, "CircularPad", FALSE); > + pars->swipe_action[UP_SWIPE] = > + xf86SetIntOption(opts, "SwipeUpButton", 0); > + pars->swipe_action[DOWN_SWIPE] = > + xf86SetIntOption(opts, "SwipeDownButton", 0); > + pars->swipe_action[LEFT_SWIPE] = > + xf86SetIntOption(opts, "SwipeLeftButton", 0); > + pars->swipe_action[RIGHT_SWIPE] = > + xf86SetIntOption(opts, "SwipeRightButton", 0); > + pars->swipe_threshold_x = > + xf86SetIntOption(opts, "HorizSwipeThreshold", horizSwipeThreshold); > + pars->swipe_threshold_y = > + xf86SetIntOption(opts, "VertSwipeThreshold", vertSwipeThreshold); > + pars->single_swipe = xf86SetBoolOption(opts, "SingleSwipe", TRUE); > pars->palm_detect = xf86SetBoolOption(opts, "PalmDetect", FALSE); > pars->palm_min_width = xf86SetIntOption(opts, "PalmMinWidth", > palmMinWidth); > pars->palm_min_z = xf86SetIntOption(opts, "PalmMinZ", palmMinZ); > @@ -1862,10 +1882,13 @@ HandleTapProcessing(SynapticsPrivate * priv, struct > SynapticsHwState *hw, > > touch = finger >= FS_TOUCHED && priv->finger_state == FS_UNTOUCHED; > release = finger == FS_UNTOUCHED && priv->finger_state >= FS_TOUCHED; > + /* TODO: better way to determine how many fingers was actually used > + * for gesture - there could be up to 5 fingers (or 6 ?) */ > move = (finger >= FS_TOUCHED && > (priv->tap_max_fingers <= > - ((priv->horiz_scroll_twofinger_on || > - priv->vert_scroll_twofinger_on) ? 2 : 1)) && > + (priv->swipe.threefinger_on ? 3 : > + ((priv->horiz_scroll_twofinger_on || > + priv->vert_scroll_twofinger_on) ? 2 : 1))) && > ((abs(hw->x - priv->touch_on.x) >= para->tap_move) || > (abs(hw->y - priv->touch_on.y) >= para->tap_move))); > press = (hw->left || hw->right || hw->middle); > @@ -2528,6 +2551,35 @@ HandleScrolling(SynapticsPrivate * priv, struct > SynapticsHwState *hw, > return delay; > } > > +static void > +HandleSwipe(SynapticsPrivate *priv, struct SynapticsHwState *hw, Bool finger) > +{ > + SynapticsParameters *para = &priv->synpara; > + > + /* not enough fingers touched, clear swipe tracking */ > + if (!finger || hw->numFingers < 3) { > + priv->swipe.threefinger_on = FALSE; > + priv->swipe.fourfinger_on = FALSE; > + priv->swipe.posted = FALSE; > + return; > + } > + > + if (priv->swipe.threefinger_on || priv->swipe.fourfinger_on) { > + /* swipe was activated, accumulate delta */ > + priv->swipe.delta_x += hw->x - priv->swipe.last_x; > + priv->swipe.delta_y += hw->y - priv->swipe.last_y; > + } > + else { > + priv->swipe.threefinger_on = hw->numFingers == 3; > + priv->swipe.fourfinger_on = hw->numFingers == 4; > + priv->swipe.delta_x = 0; > + priv->swipe.delta_y = 0; > + } > + > + priv->swipe.last_x = hw->x; > + priv->swipe.last_y = hw->y; > +} > + > /** > * Check if any 2+ fingers are close enough together to assume this is a > * ClickFinger action. > @@ -2808,6 +2860,43 @@ repeat_scrollbuttons(const InputInfoPtr pInfo, > return delay; > } > > +static void > +post_swipe_events(const InputInfoPtr pInfo) { > + SynapticsPrivate *priv = (SynapticsPrivate *) (pInfo->private); > + SynapticsParameters *para = &priv->synpara; > + > + /* there is no need to go any further if those conditions are not met */ > + if (!(priv->swipe.threefinger_on || priv->swipe.fourfinger_on) || > + (priv->swipe.posted && para->single_swipe)) > + return; > + > + /* swipes are intrinsically related - one big if stack */ > + if (para->swipe_action[UP_SWIPE] && > + priv->swipe.delta_y < -para->swipe_threshold_y) { > + post_button_click(pInfo, para->swipe_action[UP_SWIPE]); > + priv->swipe.posted = TRUE; > + priv->swipe.delta_y = 0; > + } > + else if (para->swipe_action[DOWN_SWIPE] && > + priv->swipe.delta_y > para->swipe_threshold_y) { > + post_button_click(pInfo, para->swipe_action[DOWN_SWIPE]); > + priv->swipe.posted = TRUE; > + priv->swipe.delta_y = 0; > + } > + else if (para->swipe_action[LEFT_SWIPE] && > + priv->swipe.delta_x < -para->swipe_threshold_x) { > + post_button_click(pInfo, para->swipe_action[LEFT_SWIPE]); > + priv->swipe.posted = TRUE; > + priv->swipe.delta_x = 0; > + } > + else if (para->swipe_action[RIGHT_SWIPE] && > + priv->swipe.delta_x > para->swipe_threshold_x) { > + post_button_click(pInfo, para->swipe_action[RIGHT_SWIPE]); > + priv->swipe.posted = TRUE; > + priv->swipe.delta_x = 0; > + } > +} > + > /* Update the open slots and number of active touches */ > static void > UpdateTouchState(InputInfoPtr pInfo, struct SynapticsHwState *hw) > @@ -3039,6 +3128,8 @@ HandleState(InputInfoPtr pInfo, struct SynapticsHwState > *hw, CARD32 now, > if (timeleft > 0) > delay = MIN(delay, timeleft); > > + HandleSwipe(priv, hw, (finger >= FS_TOUCHED)); > + > /* > * Compensate for unequal x/y resolution. This needs to be done after > * calculations that require unadjusted coordinates, for example edge > @@ -3109,6 +3200,10 @@ HandleState(InputInfoPtr pInfo, struct > SynapticsHwState *hw, CARD32 now, > priv->scroll.last_millis = hw->millis; > } > > + /* Process swipe events only in the active area */ > + if (inside_active_area) > + post_swipe_events(pInfo); > + > if (double_click) { > post_button_click(pInfo, 1); > post_button_click(pInfo, 1); > diff --git a/src/synapticsstr.h b/src/synapticsstr.h > index 54bc154..8135fb6 100644 > --- a/src/synapticsstr.h > +++ b/src/synapticsstr.h > @@ -83,6 +83,14 @@ enum ClickFingerEvent { > MAX_CLICK > }; > > +enum SwipeEvent { > + UP_SWIPE = 0, /* Swipe upward, three fingers */ > + DOWN_SWIPE, /* Swipe downward, three fingers */ > + LEFT_SWIPE, /* Swipe to the left, three fingers */ > + RIGHT_SWIPE, /* Swipe to the right, three fingers */ > + MAX_SWIPE > +}; > + > > typedef struct _SynapticsMoveHist { > int x, y; > @@ -187,6 +195,10 @@ typedef struct _SynapticsParameters { > int locked_drag_time; /* timeout for locked drags */ > int tap_action[MAX_TAP]; /* Button to report on tap events */ > int click_action[MAX_CLICK]; /* Button to report on click with > fingers */ > + int swipe_action[MAX_SWIPE]; /* Button to report on swipe action > */ > + int swipe_threshold_x; /* Threshold for horizontal swipe event */ > + int swipe_threshold_y; /* Threshold for vertical swipe event */ > + Bool single_swipe; /* Allow only one action per swipe */ > Bool circular_scrolling; /* Enable circular scrolling */ > double scroll_dist_circ; /* Scrolling angle radians */ > int circular_trigger; /* Trigger area for circular scrolling */ > @@ -243,6 +255,15 @@ struct _SynapticsPrivateRec { > double coast_delta_y; /* Accumulated vertical coast delta */ > int packets_this_scroll; /* Events received for this scroll */ > } scroll; > + struct { > + int last_x; /* last x-swipe position */ > + int last_y; /* last y-swipe position */ > + double delta_x; /* accumulated horiz swipe delta */ > + double delta_y; /* accumulated vert swipe delta */ > + Bool posted; /* indicate that event was posted */ > + Bool threefinger_on; /* swipe event with three fingers */ > + Bool fourfinger_on; /* XXX: ?? swipe event with four fingers */ > + } swipe; > int count_packet_finger; /* packet counter with finger on the > touchpad */ > int button_delay_millis; /* button delay for 3rd button emulation */ > Bool prev_up; /* Previous up button value, for double > click emulation */ > diff --git a/tools/synclient.c b/tools/synclient.c > index ac31a66..94dad62 100644 > --- a/tools/synclient.c > +++ b/tools/synclient.c > @@ -122,6 +122,13 @@ static struct Parameter params[] = { > {"CircScrollDelta", PT_DOUBLE, .01, 3, > SYNAPTICS_PROP_CIRCULAR_SCROLLING_DIST, 0 /* float */, 0}, > {"CircScrollTrigger", PT_INT, 0, 8, > SYNAPTICS_PROP_CIRCULAR_SCROLLING_TRIGGER, 8, 0}, > {"CircularPad", PT_BOOL, 0, 1, > SYNAPTICS_PROP_CIRCULAR_PAD, 8, 0}, > + {"SwipeUpButton", PT_INT, 0, SYN_MAX_BUTTONS, > SYNAPTICS_PROP_SWIPE_ACTION, 8, 0}, > + {"SwipeDownButton", PT_INT, 0, SYN_MAX_BUTTONS, > SYNAPTICS_PROP_SWIPE_ACTION, 8, 1}, > + {"SwipeLeftButton", PT_INT, 0, SYN_MAX_BUTTONS, > SYNAPTICS_PROP_SWIPE_ACTION, 8, 2}, > + {"SwipeRightButton", PT_INT, 0, SYN_MAX_BUTTONS, > SYNAPTICS_PROP_SWIPE_ACTION, 8, 3}, > + {"HorizSwipeThreshold", PT_INT, 0, 10000, > SYNAPTICS_PROP_SWIPE_THRESHOLD, 32, 0}, > + {"VertSwipeThreshold", PT_INT, 0, 10000, > SYNAPTICS_PROP_SWIPE_THRESHOLD, 32, 1}, > + {"SingleSwipe", PT_BOOL, 0, 1, > SYNAPTICS_PROP_SINGLE_SWIPE, 8, 0}, > {"PalmDetect", PT_BOOL, 0, 1, > SYNAPTICS_PROP_PALM_DETECT, 8, 0}, > {"PalmMinWidth", PT_INT, 0, 15, > SYNAPTICS_PROP_PALM_DIMENSIONS, 32, 0}, > {"PalmMinZ", PT_INT, 0, 255, > SYNAPTICS_PROP_PALM_DIMENSIONS, 32, 1}, > -- > 1.8.3.2 > _______________________________________________ > [email protected]: X.Org development > Archives: http://lists.x.org/archives/xorg-devel > Info: http://lists.x.org/mailman/listinfo/xorg-devel > _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
