[PATCH synaptics] Reset hw->x/y to INT_MIN and skip HandleState until we have x/y events

Chase Douglas chase.douglas at canonical.com
Thu May 17 07:10:48 PDT 2012


On 05/16/2012 06:38 PM, Peter Hutterer wrote:
> The driver assumes x/y is always valid but after coming from a resume we may
> get a few events with either ABS_X or ABS_Y (not both). Thus we process with
> hw->x == 0 and hw->y == somevalue, causing cursor jumps when calculating
> deltas whenver the real hw->x comes in.
> 
> Fix this by resetting hw->x/y to INT_MIN and skip state processing until
> both axes are available.
> 
> For clickpads, this means handling of data will be delayed until we get
> at least one motion on each axis. Button presses won't be recognised either
> until that happens. It requires some skill to not trigger motion on both
> axes, even more to press a button without doing so.
> 
> For non-clickpads, handling of motion events will be delayed likewise. If a
> physical button is pressed immediately after resume we have to assume deltas
> of x/y.
> - If the next event is a new touch, it will have ABS_X/ABS_Y set anyway
> - If the finger was already down, a button event is generated, and the
>   finger has generated ABS_X or ABS_Y only before the event, the next event
>   containing the missing data will cause a jump. The fix for this is more
>   invasive and this is quite a corner-case.
> 
> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> ---
>  src/synaptics.c |   13 +++++++++++++
>  src/synproto.c  |    4 ++--
>  2 files changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/src/synaptics.c b/src/synaptics.c
> index d912ee5..146afd8 100644
> --- a/src/synaptics.c
> +++ b/src/synaptics.c
> @@ -2767,6 +2767,19 @@ HandleState(InputInfoPtr pInfo, struct SynapticsHwState *hw, CARD32 now,
>          return delay;
>      }
>  
> +    /* 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
> +     * that case. */
> +    if (hw->x == INT_MIN || hw->y == INT_MAX) {
> +        if (para->clickpad)
> +            return delay;
> +        else if (hw->left || hw->right || hw->middle) {
> +            hw->x = (hw->x == INT_MIN) ? 0 : hw->x;
> +            hw->y = (hw->y == INT_MIN) ? 0 : hw->y;
> +        }
> +    }
> +
>      /* If a physical button is pressed on a clickpad, use cumulative relative
>       * touch movements for motion */
>      if (para->clickpad && (hw->left || hw->right || hw->middle)) {
> diff --git a/src/synproto.c b/src/synproto.c
> index d3f05ca..91e20e6 100644
> --- a/src/synproto.c
> +++ b/src/synproto.c
> @@ -123,8 +123,8 @@ void
>  SynapticsResetHwState(struct SynapticsHwState *hw)
>  {
>      hw->millis = 0;
> -    hw->x = 0;
> -    hw->y = 0;
> +    hw->x = INT_MIN;
> +    hw->y = INT_MIN;
>      hw->z = 0;
>      hw->cumulative_dx = 0;
>      hw->cumulative_dy = 0;

It's kind of hackish, but I trust your judgement that this is less
painful than actually fixing the driver for partial axis data.

Reviewed-by: Chase Douglas <chase.douglas at canonical.com>


More information about the xorg-devel mailing list