[PATCH 1/2] Add Clickpad support
Peter Hutterer
peter.hutterer at who-t.net
Tue Apr 20 21:16:41 PDT 2010
Thank you for the patch. Just as a heads-up, I'd like to wait with this
until there's at least one kernel released with this support.
My comments to the patch are inline.
On Thu, Apr 15, 2010 at 06:12:10PM +0200, Takashi Iwai 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/
>
> When the kernel driver sets only the left-button bit evbit, 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.
>
> There are two new parameters added. One is a parameter to specify
> the size of the bottom button-area, and another is to toggle the touch-
> sensing in the button area.
>
> Signed-off-by: Takashi Iwai <tiwai at suse.de>
> ---
> include/synaptics-properties.h | 6 ++++
> man/synaptics.man | 21 +++++++++++++++
> src/eventcomm.c | 15 ++++++++++
> src/properties.c | 19 +++++++++++++
> src/synaptics.c | 56 ++++++++++++++++++++++++++++++++++++++++
> src/synapticsstr.h | 4 +++
> tools/synclient.c | 2 +
> 7 files changed, 123 insertions(+), 0 deletions(-)
>
> diff --git a/include/synaptics-properties.h b/include/synaptics-properties.h
> index cf330d8..c77afd3 100644
> --- a/include/synaptics-properties.h
> +++ b/include/synaptics-properties.h
> @@ -155,4 +155,10 @@
> /* 32 bit, 4 values, left, right, top, bottom */
> #define SYNAPTICS_PROP_AREA "Synaptics Area"
>
> +/* 8 bit (0-100) */
> +#define SYNAPTICS_PROP_TOUCH_BUTTON_AREA "Synaptics Touch Button Area"
> +
> +/* 8 bit (BOOL) */
> +#define SYNAPTICS_PROP_TOUCH_BUTTON_SENSE "Synaptics Touch Button Sense"
> +
> #endif /* _SYNAPTICS_PROPERTIES_H_ */
> diff --git a/man/synaptics.man b/man/synaptics.man
> index 59fbaac..f92ea0c 100644
> --- a/man/synaptics.man
> +++ b/man/synaptics.man
> @@ -509,6 +509,27 @@ Ignore movements, scrolling and tapping which take place below this edge.
> The option is disabled by default and can be enabled by setting the
> AreaBottomEdge option to any integer value other than zero. Property: "Synaptics Area"
> .
> +.TP
> +.BI "Option \*qTouchButtonArea\*q \*q" integer \*q
> +The percentage of touch-button area of Clickpad device.
> +.
> +The synaptics driver supports "ClickZone" mode, which emulates the buttons
> +to click at the bottom of touchpad area. This option specifies how large
> +is the area to be used as the button area. Passing zero to this disables
> +the Clickpad behavior. The default value is 20.
where does the 0-100 come from? what unit is this? it's answered in the
header file but this should be in the man page as well.
also, I'd rather have this stored in device units and leave things like
"this is 20% of the area" to higher-level tools.
> +Property: "Synaptics Touch Button Area"
please make this so that the Synaptics Area is re-used instead of
introducing another area property. This should remove the need for this
property altogether and just need the enable/disable value in the other one.
how do you get the default of 20? isn't this something that should be
auto-scaled from the device in case future devices have a different
resolution?
> +.TP
> +.BI "Option \*qTouchButtonSense\*q \*q" integer \*q
> +Disable the touch-sensing in the Clickpad button area.
> +.
> +Since the touchpad is often quite sensitive, the pointer would move
> +slightly when you click via Clickpad, and user may miss the aimed
> +position. When this option is set, the cursor movement in the
> +Clickpad button area is suppressed, thus the point won't move while
> +clicking the button. On the other hand, this reduces the available
> +space on the touchpad. The default is on.
> +Property: "Synaptics Touch Button Sense"
> +.
> .LP
> A tap event happens when the finger is touched and released in a time
> interval shorter than MaxTapTime, and the touch and release
> diff --git a/src/eventcomm.c b/src/eventcomm.c
> index d00d810..ca99f3a 100644
> --- a/src/eventcomm.c
> +++ b/src/eventcomm.c
> @@ -166,6 +166,19 @@ event_query_info(LocalDevicePtr local)
> }
> }
>
> +static void event_query_clickpad(LocalDevicePtr local)
> +{
> + SynapticsPrivate *priv = (SynapticsPrivate *)local->private;
> +
> + priv->is_clickpad = FALSE;
> +
> + /* clickpad device reports only the single left button mask */
> + if (priv->has_left && !priv->has_right && !priv->has_middle) {
> + priv->is_clickpad = TRUE;
> + xf86Msg(X_INFO, "%s: is Clickpad device\n", local->name);
please move this check into the part that checks for LMR buttons.
> + }
> +}
> +
> /* Query device for axis ranges */
> static void
> event_query_axis_ranges(LocalDevicePtr local)
> @@ -253,6 +266,8 @@ event_query_axis_ranges(LocalDevicePtr local)
> strcat(buf, " triple");
> xf86Msg(X_INFO, "%s: buttons:%s\n", local->name, buf);
> }
> +
> + event_query_clickpad(local);
> }
>
> static Bool
> diff --git a/src/properties.c b/src/properties.c
> index 4366034..a579479 100644
> --- a/src/properties.c
> +++ b/src/properties.c
> @@ -84,6 +84,8 @@ Atom prop_gestures = 0;
> Atom prop_capabilities = 0;
> Atom prop_resolution = 0;
> Atom prop_area = 0;
> +Atom prop_touch_button_area = 0;
> +Atom prop_touch_button_sense = 0;
>
> static Atom
> InitAtom(DeviceIntPtr dev, char *name, int format, int nvalues, int *values)
> @@ -274,6 +276,10 @@ InitDeviceProperties(LocalDevicePtr local)
> values[2] = para->area_top_edge;
> values[3] = para->area_bottom_edge;
> prop_area = InitAtom(local->dev, SYNAPTICS_PROP_AREA, 32, 4, values);
> +
> + prop_touch_button_area = InitAtom(local->dev, SYNAPTICS_PROP_TOUCH_BUTTON_AREA, 8, 1, ¶->touch_button_area);
> +
> + prop_touch_button_sense = InitAtom(local->dev, SYNAPTICS_PROP_TOUCH_BUTTON_SENSE, 8, 1, ¶->touch_button_sense);
> }
>
> int
> @@ -490,6 +496,7 @@ SetProperty(DeviceIntPtr dev, Atom property, XIPropertyValuePtr prop,
> return BadValue;
>
> para->touchpad_off = off;
> +
> } else if (property == prop_guestmouse)
> {
> if (prop->size != 1 || prop->format != 8 || prop->type != XA_INTEGER)
> @@ -642,6 +649,18 @@ SetProperty(DeviceIntPtr dev, Atom property, XIPropertyValuePtr prop,
> para->area_right_edge = area[1];
> para->area_top_edge = area[2];
> para->area_bottom_edge = area[3];
> + } else if (property == prop_touch_button_area)
> + {
> + if (prop->size != 1 || prop->format != 8 || prop->type != XA_INTEGER)
> + return BadMatch;
> +
> + para->touch_button_area = *(CARD8*)prop->data;
> + } else if (property == prop_touch_button_sense)
> + {
> + if (prop->size != 1 || prop->format != 8 || prop->type != XA_INTEGER)
> + return BadMatch;
> +
> + para->touch_button_sense = *(BOOL*)prop->data;
> }
>
> return Success;
> diff --git a/src/synaptics.c b/src/synaptics.c
> index 091dbe1..f701074 100644
> --- a/src/synaptics.c
> +++ b/src/synaptics.c
> @@ -532,6 +532,8 @@ static void set_default_parameters(LocalDevicePtr local)
> pars->tap_and_drag_gesture = xf86SetBoolOption(opts, "TapAndDragGesture", TRUE);
> pars->resolution_horiz = xf86SetIntOption(opts, "HorizResolution", horizResolution);
> pars->resolution_vert = xf86SetIntOption(opts, "VertResolution", vertResolution);
> + pars->touch_button_area = xf86SetIntOption(opts, "TouchButtonArea", 20);
> + pars->touch_button_sense = xf86SetBoolOption(opts, "TouchButtonSense", FALSE);
>
> /* Warn about (and fix) incorrectly configured TopEdge/BottomEdge parameters */
> if (pars->top_edge > pars->bottom_edge) {
> @@ -2052,6 +2054,56 @@ HandleClickWithFingers(SynapticsParameters *para, struct SynapticsHwState *hw)
> }
> }
>
> +/* clickpad event handling */
> +static void
> +HandleClickpad(LocalDevicePtr local, struct SynapticsHwState *hw)
> +{
> + SynapticsPrivate *priv = (SynapticsPrivate *) (local->private);
> + SynapticsParameters *para = &priv->synpara;
> + int width = priv->maxx - priv->minx;
> + int height = priv->maxy - priv->miny;
> + int button_area_y, x_vscroll;
> + 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;
^ space missing
> + right_button_x = width * 3 / 5 + priv->minx;
> + /* button area */
> + button_area_y = priv->maxy - height * para->touch_button_area / 100;
> + /* allow position change for the v-scroll area exceptionally */
> + x_vscroll = priv->maxx - width / 15;
huh? what does this do? why don't you just use the right edge for this?
also, you're hardcoding the width as 1/15 from the right - what happens if
someone changes the edge settings?
> +
> + if (hw->y >= button_area_y) {
> + /* button area */
> + if (!para->touch_button_sense) {
> + if (hw->left ||
> + /* in h- or v-scrolling area, allow sensing */
> + (!para->scroll_edge_horiz &&
> + (hw->x < x_vscroll || !para->scroll_edge_vert)))
> + hw->z = 0; /* don't move pointer */
> + }
> + /* 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;
> + }
> + } 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;
> +}
>
> /*
> * React on changes in the hardware state. This function is called every time
> @@ -2102,6 +2154,10 @@ HandleState(LocalDevicePtr local, struct SynapticsHwState *hw)
> if (para->touchpad_off == 1)
> return delay;
>
> + /* Clickpad handling for button area */
> + if (priv->is_clickpad && para->touch_button_area > 0)
> + HandleClickpad(local, hw);
> +
> /* Treat the first two multi buttons as up/down for now. */
> hw->up |= hw->multi[0];
> hw->down |= hw->multi[1];
> diff --git a/src/synapticsstr.h b/src/synapticsstr.h
> index bd19c79..54afea6 100644
> --- a/src/synapticsstr.h
> +++ b/src/synapticsstr.h
> @@ -160,6 +160,8 @@ typedef struct _SynapticsParameters
> unsigned int resolution_horiz; /* horizontal resolution of touchpad in units/mm */
> unsigned int resolution_vert; /* vertical resolution of touchpad in units/mm */
> int area_left_edge, area_right_edge, area_top_edge, area_bottom_edge; /* area coordinates absolute */
> + int touch_button_area; /* touch button (clickpad) area in percent */
> + Bool touch_button_sense; /* touch button (clickpad) area sensing */
> } SynapticsParameters;
>
>
> @@ -232,6 +234,8 @@ typedef struct _SynapticsPrivateRec
> Bool has_double; /* double click detected for this device */
> Bool has_triple; /* triple click detected for this device */
> Bool has_pressure; /* device reports pressure */
> + 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;
> diff --git a/tools/synclient.c b/tools/synclient.c
> index 316ae2c..c4a458b 100644
> --- a/tools/synclient.c
> +++ b/tools/synclient.c
> @@ -143,6 +143,8 @@ static struct Parameter params[] = {
> {"AreaRightEdge", PT_INT, 0, 10000, SYNAPTICS_PROP_AREA, 32, 1},
> {"AreaTopEdge", PT_INT, 0, 10000, SYNAPTICS_PROP_AREA, 32, 2},
> {"AreaBottomEdge", PT_INT, 0, 10000, SYNAPTICS_PROP_AREA, 32, 3},
> + {"TouchButtonArea", PT_INT, 0, 100, SYNAPTICS_PROP_TOUCH_BUTTON_AREA, 8, 0},
> + {"TouchButtonSense", PT_BOOL, 0, 1, SYNAPTICS_PROP_TOUCH_BUTTON_SENSE, 8, 0},
> { NULL, 0, 0, 0, 0 }
> };
>
> --
> 1.7.0.4
Cheers,
Peter
More information about the xorg-devel
mailing list