synaptics - double taps are ignored in some applications

[Peter Hutterer]

On Mon, Jan 12, 2015 at 09:32:13PM +0100, Gabriele Mazzotta wrote:
> Hi,
> 
> I have some problems with double taps in some applications.
> These applications, e.g. Firefox, often (if not always) ignores double
> taps. This never happens with double clicks and it happens only with
> the same applications.
> 
> After some tests I found that the problem is that there's no delay
> between the second ButtonPress and the second ButtonRelease.
> 
> The patch here below seems to solve the problem (the delay should be
> handled better), but I don't know if it can be considered a good
> solution. Before I try to improve it, does anyone know a better way
> to address this problem?
> 
> I tried to fix the problem from HandleTapProcessing(), but I could only
> have reliable double taps breaking tap and drag gestures.

docs/tapndrag.dia has the state diagram, though I'm not 100% it's still
accurate. Based on that I think there may be a solution: when we transition
from 3 to 2B, we could send up + down. From 2B we have two transitions, back
to start (Up) or back to 3 (no event needed).
Since 2B->start is based on a timeout, that adds the required delay.

Does that sound sensible? 
(of course, this could all be bogus if the diagram doesn't reflect the code
anymore :)
 
> diff --git a/src/synaptics.c b/src/synaptics.c
> index 3b11fda..8a3e5c5 100644
> --- a/src/synaptics.c
> +++ b/src/synaptics.c
> @@ -1109,6 +1109,7 @@ SynapticsReset(SynapticsPrivate * priv)
>      priv->mid_emu_state = MBE_OFF;
>      priv->nextRepeat = 0;
>      priv->lastButtons = 0;
> +    priv->mustRelease = -1;

fwiw, I'd expect something named "mustRelease" to be a boolean yes/no, not a
button number. something like pressedButton or activeButton or so would be
more obvious. also, X doesn't have a button 0, so that'd be fine to use as
default.

>      priv->prev_z = 0;
>      priv->prevFingers = 0;
>      priv->num_active_touches = 0;
> @@ -3245,6 +3246,12 @@ HandleState(InputInfoPtr pInfo, struct SynapticsHwState *hw, CARD32 now,
>      Bool using_cumulative_coords = FALSE;
>      Bool ignore_motion;
>  
> +    if (priv->mustRelease != -1) {
> +        xf86PostButtonEvent(pInfo->dev, FALSE, priv->mustRelease, FALSE, 0, 0);
> +        priv->mustRelease = -1;
> +        return 0;
> +    }
> +
>      /* We need both and x/y, the driver can't handle just one of the two
>       * yet. But since it's possible to hit a phys button on non-clickpads
>       * without ever getting motion data first, we must continue with 0/0 for
> @@ -3360,6 +3367,7 @@ HandleState(InputInfoPtr pInfo, struct SynapticsHwState *hw, CARD32 now,
>  
>          if (priv->tap_button_state == TBS_BUTTON_DOWN_UP) {
>              if (tap_mask != (priv->lastButtons & tap_mask)) {
> +                priv->mustRelease = priv->tap_button;
>                  xf86PostButtonEvent(pInfo->dev, FALSE, priv->tap_button, TRUE,
>                                      0, 0);
>                  priv->lastButtons |= tap_mask;
> @@ -3387,8 +3395,9 @@ HandleState(InputInfoPtr pInfo, struct SynapticsHwState *hw, CARD32 now,
>      while (change) {
>          id = ffs(change);       /* number of first set bit 1..32 is returned */
>          change &= ~(1 << (id - 1));
> -        xf86PostButtonEvent(pInfo->dev, FALSE, id, (buttons & (1 << (id - 1))),
> -                            0, 0);
> +        if (priv->mustRelease != id)
> +            xf86PostButtonEvent(pInfo->dev, FALSE, id,
> +                                (buttons & (1 << (id - 1))), 0, 0);
>      }
>  
>      if (priv->has_scrollbuttons)
> @@ -3422,6 +3431,9 @@ HandleState(InputInfoPtr pInfo, struct SynapticsHwState *hw, CARD32 now,
>      if (inside_active_area)
>          store_history(priv, hw->x, hw->y, hw->millis);
>  
> +    if (priv->mustRelease)
> +        delay = 50;

I think delay of 50 is too long, that may be noticeable (gut feeling, no
hard data). I'd probably for for 10 or 20 at max. Whatever is the minimum to
make the applications work.

Ack in general to the patch if we can't work it into the state machine.

Cheers,
   Peter

> +
>      return delay;
>  }
>  
> diff --git a/src/synapticsstr.h b/src/synapticsstr.h
> index 9a5542f..2fd2b5a 100644
> --- a/src/synapticsstr.h
> +++ b/src/synapticsstr.h
> @@ -318,6 +318,7 @@ struct _SynapticsPrivateRec {
>      double horiz_coeff;         /* normalization factor for x coordintes */
>      double vert_coeff;          /* normalization factor for y coordintes */
>  #endif
> +    int mustRelease;
>  
>      int minx, maxx, miny, maxy; /* min/max dimensions as detected */
>      int minp, maxp, minw, maxw; /* min/max pressure and finger width as detected */