[PATCH] ClickPad support v4

[Peter Hutterer]

On Thu, Dec 09, 2010 at 08:34:11AM -0600, Chris Bagwell wrote:
> On Wed, Dec 8, 2010 at 1:55 AM, Yan Li <yan.i.li at intel.com> wrote:
> > This patch adds the support for Synaptics Clickpad devices.
> > It requires the change in Linux kernel synaptics input driver, found in
> >    https://patchwork.kernel.org/patch/92435/
> > The kernel patch is already included in 2.6.34 and later releases.
> >
> > When the kernel driver sets only the left-button bit evbit and no
> > multi-finger is possible, Clickpad mode is activated.  In this mode,
> > the bottom touch area is used as button emulations.  Clicking at the
> > bottom-left, bottom-center and bottom-right zone corresponds to a left,
> > center and right click.
> >
> > v2->v3: Fix the mis-detection of Clickpad device with double-tap feature
> >        (e.g. MacBook)
> >        Fix one forgotten spacing issue Peter suggested
> >
> > v3->v4: Ported to HEAD by Yan Li for MeeGo, also added ClickPad
> >        description to man page.
> >
> > Signed-off-by: Takashi Iwai <tiwai at suse.de>
> > Signed-off-by: Yan Li <yan.i.li at intel.com>
> > ---
> >  man/synaptics.man  |    8 ++++++
> >  src/eventcomm.c    |    7 +++++
> >  src/synaptics.c    |   72 +++++++++++++++++++++++++++++++++++++++++++++++++++-
> >  src/synapticsstr.h |    2 +
> >  4 files changed, 88 insertions(+), 1 deletions(-)
> >
> > diff --git a/man/synaptics.man b/man/synaptics.man
> > index 3f1ca9d..25f1115 100644
> > --- a/man/synaptics.man
> > +++ b/man/synaptics.man
> > @@ -56,6 +56,14 @@ Pressure dependent motion speed.
> >  .IP \(bu 4
> >  Run-time configuration using shared memory. This means you can change
> >  parameter settings without restarting the X server.
> > +.IP \(bu 4
> > +Synaptics ClickPad support: ClickPad is a new kind of device from
> > +Synaptics that has no visible physical keys. Instead, the whole board
> > +is clickable and the device sends out BTN_MIDDLE only. It's the
> > +driver's duty to judge whether the click is a left or right one
> > +according to finger location. If the driver detects that the touchpad
> > +has only one button, the ClickPad mode will be activated and handles
> > +the action correctly.
> 
> I'm not sure a man page entery is needed.  There is nothing
> configurable to user.  The single button is more internal knowledge.
> Now, if we expose a Clickable property then thats a different story.

I'd say it is good to have one. The behaviour of the touchpad is
non-standard and leaves some room for interpretation (20%, lmr buttons being
a certain size). having a short blurb in the man page is the most visible
approach to signal to the user that the driver is doing magic here.

Cheers,
  Peter

> >  .LP
> >  Note that depending on the touchpad firmware, some of these features
> >  might be available even without using the synaptics driver. Note also
> > diff --git a/src/eventcomm.c b/src/eventcomm.c
> > index faa66ab..7da5a40 100644
> > --- a/src/eventcomm.c
> > +++ b/src/eventcomm.c
> > @@ -269,6 +269,13 @@ event_query_axis_ranges(LocalDevicePtr local)
> >        }
> >
> >        xf86Msg(X_PROBED, "%s: buttons:%s\n", local->name, buf);
> > +
> > +       /* clickpad device reports only the single left button mask */
> > +       if (priv->has_left && !priv->has_right && !priv->has_middle && !priv->has_double) {
> > +               priv->is_clickpad = TRUE;
> > +               xf86Msg(X_INFO, "%s: is Clickpad device\n", local->name);
> > +       }
> > +
> 
> The part about !priv->has_double should probably be removed.  For
> synaptics at least, there is a good chance these clickpads will start
> reporting has_double with some new kernel patches.
> 
> There is talk of ioctl() to query if a touchpad is a clickpad soon as
> well but I think single button detection is OK short term.
> 
> >     }
> >  }
> >
> > diff --git a/src/synaptics.c b/src/synaptics.c
> > index 53c3685..2e5f8ae 100644
> > --- a/src/synaptics.c
> > +++ b/src/synaptics.c
> > @@ -506,6 +506,18 @@ static void set_default_parameters(LocalDevicePtr local)
> >         vertResolution = priv->resy;
> >     }
> >
> > +    /* Clickpad mode -- bottom area is used as buttons */
> > +    if (priv->is_clickpad) {
> > +        int button_bottom;
> > +    /* Clickpad devices usually the button area at the bottom, and
> > +     * its size seems ca. 20% of the touchpad height no matter how
> > +     * large the pad is.
> > +     */
> > +    button_bottom = priv->maxy - (abs(priv->maxy - priv->miny) * 20) / 100;
> > +    if (button_bottom < b && button_bottom >= t)
> > +        b = button_bottom;
> > +    }
> > +
> >     /* set the parameters */
> >     pars->left_edge = xf86SetIntOption(opts, "LeftEdge", l);
> >     pars->right_edge = xf86SetIntOption(opts, "RightEdge", r);
> > @@ -2153,6 +2165,59 @@ handle_clickfinger(SynapticsParameters *para, struct SynapticsHwState *hw)
> >     }
> >  }
> >
> > +/* clickpad event handling */
> > +static void
> > +HandleClickpad(LocalDevicePtr local, struct SynapticsHwState *hw, edge_type edge)
> > +{
> > +    SynapticsPrivate *priv = (SynapticsPrivate *) (local->private);
> > +    SynapticsParameters *para = &priv->synpara;
> > +
> > +    if (edge & BOTTOM_EDGE) {
> > +   /* button area */
> > +   int width = priv->maxx - priv->minx;
> > +   int left_button_x, right_button_x;
> > +
> > +   /* left and right clickpad button ranges;
> > +    * the gap between them is interpreted as a middle-button click
> > +    */
> > +   left_button_x = width * 2 / 5 + priv->minx;
> > +   right_button_x = width * 3 / 5 + priv->minx;
> > +
> > +   /* clickpad reports only one button, and we need
> > +    * to fake left/right buttons depending on the touch position
> > +    */
> > +   if (hw->left) { /* clicked? */
> > +       hw->left = 0;
> > +       if (hw->x < left_button_x)
> > +       hw->left = 1;
> > +       else if (hw->x > right_button_x)
> > +       hw->right = 1;
> > +       else
> > +       hw->middle = 1;
> > +   }
> > +
> > +   /* Don't move pointer position in the button area during clicked,
> > +    * except for horiz/vert scrolling is enabled.
> 
> I don't understand the scrolling part.  Why do you want a mouse click
> and scroll events to be sent?
> 
> > +    *
> > +    * The synaptics driver tends to be pretty sensitive.  This hack
> > +    * is to avoid that the pointer moves slightly and misses the
> > +    * poistion you aimed to click.
> > +    *
> > +    * Also, when the pointer movement is reported, the dragging
> > +    * (with a sort of multi-touching) doesn't work well, too.
> > +    */
> > +   if (hw->left || !(para->scroll_edge_horiz ||
> > +             ((edge & RIGHT_EDGE) && para->scroll_edge_vert)))
> > +       hw->z = 0; /* don't move pointer */
> > +
> > +    } else if (hw->left) {
> > +   /* dragging */
> > +   hw->left = priv->prev_hw.left;
> > +   hw->right = priv->prev_hw.right;
> > +   hw->middle = priv->prev_hw.middle;
> > +    }
> > +    priv->prev_hw = *hw;
> > +}
> >
> >  /* Update the hardware state in shared memory. This is read-only these days,
> >  * nothing in the driver reads back from SHM. SHM configuration is a thing of the past.
> > @@ -2344,6 +2409,12 @@ HandleState(LocalDevicePtr local, struct SynapticsHwState *hw)
> >     if (para->touchpad_off == 1)
> >        return delay;
> >
> > +    edge = edge_detection(priv, hw->x, hw->y);
> 
> Should this be protected by if() that it was in below?
> 
> Chris
> 
> > +
> > +    /* Clickpad handling for button area */
> > +    if (priv->is_clickpad)
> > +       HandleClickpad(local, hw, edge);
> > +
> >     inside_active_area = is_inside_active_area(priv, hw->x, hw->y);
> >
> >     /* now we know that these _coordinates_ aren't in the area.
> > @@ -2373,7 +2444,6 @@ HandleState(LocalDevicePtr local, struct SynapticsHwState *hw)
> >
> >     /* no edge or finger detection outside of area */
> >     if (inside_active_area) {
> > -       edge = edge_detection(priv, hw->x, hw->y);
> >        finger = SynapticsDetectFinger(priv, hw);
> >     }
> >
> > diff --git a/src/synapticsstr.h b/src/synapticsstr.h
> > index 658721c..5e4090e 100644
> > --- a/src/synapticsstr.h
> > +++ b/src/synapticsstr.h
> > @@ -235,6 +235,8 @@ typedef struct _SynapticsPrivateRec
> >     Bool has_pressure;                 /* device reports pressure */
> >     Bool has_width;                    /* device reports finger width */
> >     Bool has_scrollbuttons;            /* device has physical scrollbuttons */
> > +    Bool is_clickpad;          /* is Clickpad device (one-button) */
> > +    struct SynapticsHwState prev_hw;   /* previous h/w state (for clickpad) */
> >
> >     enum TouchpadModel model;          /* The detected model */
> >  } SynapticsPrivate;
> > --
> > 1.7.2.3
> >
> >