[Patch synaptics] add hysteresis-based noise cancellation
Peter Hutterer
peter.hutterer at who-t.net
Sun Feb 20 18:55:43 PST 2011
On Tue, Feb 15, 2011 at 12:24:39AM +0100, Simon Thum wrote:
> This adresses the noise motion coming from most touchpads. The reviews
> have been incorporated, the code now get fuzz from the kernel if that is
> set (which it usually isn't).
>
all 4 merged, though the ComputeDelta's patch has moved a bit.
Cheers,
Peter
> From 174882a675bf36b766f697958813ff0f8028a6ee Mon Sep 17 00:00:00 2001
> From: Simon Thum <simon.thum at gmx.de>
> Date: Sun, 6 Feb 2011 17:57:17 +0100
> Subject: [PATCH] Add hysteresis-based noise reduction
>
> This introduces hysteresis into the driver's processing. It significantly
> reduces noise motion, i.e. now the pad does no longer generate a stream of
> sub-pixel events when just holding the position with the finger down.
> Also, taking off the finger no longer generates additional motion,
> scrolling becomes flicker-free etc.
>
> The code makes use of "fuzz" from the kernel, if available. This has not
> been tested extensively, as an overwhelming majority of evdev touchpad
> drivers view 0 (zero) as a good value for fuzz, forcing userland into
> assuming "zero fuzz" means "make zero assumptions about fuzz", not
> "there is no fuzz". Until things improve, this is what we do.
>
> Anyway, the fuzz a.k.a. hysteresis can be set/overridden with options
> and properties, as documented.
>
> Signed-off-by: Simon Thum <simon.thum at gmx.de>
> ---
> include/synaptics-properties.h | 3 ++
> man/synaptics.man | 18 +++++++++++++
> src/eventcomm.c | 9 ++++++
> src/properties.c | 17 ++++++++++++
> src/synaptics.c | 55 ++++++++++++++++++++++++++++++++++------
> src/synapticsstr.h | 3 ++
> 6 files changed, 97 insertions(+), 8 deletions(-)
>
> diff --git a/include/synaptics-properties.h b/include/synaptics-properties.h
> index 9c6a2ee..bdb2112 100644
> --- a/include/synaptics-properties.h
> +++ b/include/synaptics-properties.h
> @@ -155,4 +155,7 @@
> /* 32 bit, 4 values, left, right, top, bottom */
> #define SYNAPTICS_PROP_AREA "Synaptics Area"
>
> +/* 32 Bit Integer, 2 values, horizontal hysteresis, vertical hysteresis */
> +#define SYNAPTICS_PROP_NOISE_CANCELLATION "Synaptics Noise Cancellation"
> +
> #endif /* _SYNAPTICS_PROPERTIES_H_ */
> diff --git a/man/synaptics.man b/man/synaptics.man
> index 3f1ca9d..16ae7f6 100644
> --- a/man/synaptics.man
> +++ b/man/synaptics.man
> @@ -222,6 +222,17 @@ Motion Factor"
> Greatest setting for pressure motion factor. Property: "Synaptics Pressure
> Motion Factor"
> .TP
> +.BI "Option \*qHorizHysteresis\*q \*q" integer \*q
> +The minimum horizontal HW distance required to generate motion events. Can be
> +specified as a percentage. Increase if noise motion is a problem for you. Zero
> +is disabled.
> +Default: 0.5 percent of the diagonal or (in case of evdev) the appropriate
> +"fuzz" as advertised by the device.
> +.TP
> +.BI "Option \*qVertHysteresis\*q \*q" integer \*q
> +The minimum vertical HW distance required to generate motion events. See
> +\fBHorizHysteresis\fR.
> +.TP
> .BI "Option \*qUpDownScrolling\*q \*q" boolean \*q
> If on, the up/down buttons generate button 4/5 events.
> .
> @@ -707,6 +718,13 @@ scrolling to circular scrolling. That is, if CornerCoasting is
> active, scrolling will stop, and circular scrolling will not start,
> when the finger leaves the corner.
>
> +.SS Noise cancellation
> +The synaptics has a built-in nose canellation based on hysteresis. This means
> +that incoming coordinates actually shift a box of predefined dimensions such
> +that it covers the incoming coordinate, and only the boxes own center is used
> +as input. Obviously, the smaller the box the better, but the likelyhood of
> +noise motion coming through also increases.
> +
> .SS Trackstick mode
> Trackstick emulation mode is entered when pressing the finger hard on
> the touchpad.
> diff --git a/src/eventcomm.c b/src/eventcomm.c
> index 4593bba..9357a26 100644
> --- a/src/eventcomm.c
> +++ b/src/eventcomm.c
> @@ -184,6 +184,12 @@ event_query_axis_ranges(InputInfoPtr pInfo)
> abs.minimum, abs.maximum);
> priv->minx = abs.minimum;
> priv->maxx = abs.maximum;
> + /*
> + * The kernel's fuzziness concept seems a bit weird, but it can more or
> + * less be applied as hysteresis directly, i.e. no factor here. Though,
> + /* we don't trust a zero fuzz as it probably is just a lazy value. */
> + if (abs.fuzz > 0)
> + priv->synpara.hyst_x = abs.fuzz;
> #if LINUX_VERSION_CODE > KERNEL_VERSION(2,6,30)
> priv->resx = abs.resolution;
> #endif
> @@ -198,6 +204,9 @@ event_query_axis_ranges(InputInfoPtr pInfo)
> abs.minimum, abs.maximum);
> priv->miny = abs.minimum;
> priv->maxy = abs.maximum;
> + /* don't trust a zero fuzz */
> + if (abs.fuzz > 0)
> + priv->synpara.hyst_y = abs.fuzz;
> #if LINUX_VERSION_CODE > KERNEL_VERSION(2,6,30)
> priv->resy = abs.resolution;
> #endif
> diff --git a/src/properties.c b/src/properties.c
> index 5400928..23b5a6a 100644
> --- a/src/properties.c
> +++ b/src/properties.c
> @@ -82,6 +82,7 @@ Atom prop_gestures = 0;
> Atom prop_capabilities = 0;
> Atom prop_resolution = 0;
> Atom prop_area = 0;
> +Atom prop_noise_cancellation = 0;
>
> static Atom
> InitAtom(DeviceIntPtr dev, char *name, int format, int nvalues, int *values)
> @@ -278,6 +279,12 @@ InitDeviceProperties(InputInfoPtr pInfo)
> values[2] = para->area_top_edge;
> values[3] = para->area_bottom_edge;
> prop_area = InitAtom(pInfo->dev, SYNAPTICS_PROP_AREA, 32, 4, values);
> +
> + values[0] = para->hyst_x;
> + values[1] = para->hyst_y;
> + prop_noise_cancellation = InitAtom(pInfo->dev,
> + SYNAPTICS_PROP_NOISE_CANCELLATION, 32, 2, values);
> +
> }
>
> int
> @@ -649,6 +656,16 @@ 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_noise_cancellation) {
> + INT32 *hyst;
> + if (prop->size != 2 || prop->format != 32 || prop->type != XA_INTEGER)
> + return BadMatch;
> +
> + hyst = (INT32*)prop->data;
> + if (hyst[0] < 0 || hyst[1] < 0)
> + return BadValue;
> + para->hyst_x = hyst[0];
> + para->hyst_y = hyst[1];
> }
>
> return Success;
> diff --git a/src/synaptics.c b/src/synaptics.c
> index 783bd64..d57a75a 100644
> --- a/src/synaptics.c
> +++ b/src/synaptics.c
> @@ -387,18 +387,19 @@ calculate_edge_widths(SynapticsPrivate *priv, int *l, int *r, int *t, int *b)
> * the log message.
> */
> static int set_percent_option(pointer options, const char* optname,
> - const int range, const int offset)
> + const int range, const int offset,
> + const int default_value)
> {
> int result;
> #if GET_ABI_MAJOR(ABI_XINPUT_VERSION) >= 11
> - int percent = xf86CheckPercentOption(options, optname, -1);
> + double percent = xf86CheckPercentOption(options, optname, -1);
>
> - if (percent != -1) {
> + if (percent >= 0.0) {
> percent = xf86SetPercentOption(options, optname, -1);
> result = percent/100.0 * range + offset;
> } else
> #endif
> - result = xf86SetIntOption(options, optname, 0);
> + result = xf86SetIntOption(options, optname, default_value);
>
> return result;
> }
> @@ -427,6 +428,7 @@ static void set_default_parameters(InputInfoPtr pInfo)
> int horizResolution = 1;
> int vertResolution = 1;
> int width, height, diag, range;
> + int horizHyst, vertHyst;
>
> /* read the parameters */
> if (priv->synshm)
> @@ -458,6 +460,10 @@ static void set_default_parameters(InputInfoPtr pInfo)
> edgeMotionMaxSpeed = diag * .080;
> accelFactor = 200.0 / diag; /* trial-and-error */
>
> + /* hysteresis, assume >= 0 is a detected value (e.g. evdev fuzz) */
> + horizHyst = pars->hyst_x >= 0 ? pars->hyst_x : diag * 0.005;
> + vertHyst = pars->hyst_y >= 0 ? pars->hyst_y : diag * 0.005;
> +
> range = priv->maxp - priv->minp;
>
> /* scaling based on defaults and a pressure of 256 */
> @@ -513,10 +519,13 @@ static void set_default_parameters(InputInfoPtr pInfo)
> pars->top_edge = xf86SetIntOption(opts, "TopEdge", t);
> pars->bottom_edge = xf86SetIntOption(opts, "BottomEdge", b);
>
> - pars->area_top_edge = set_percent_option(opts, "AreaTopEdge", height, priv->miny);
> - pars->area_bottom_edge = set_percent_option(opts, "AreaBottomEdge", height, priv->miny);
> - pars->area_left_edge = set_percent_option(opts, "AreaLeftEdge", width, priv->minx);
> - pars->area_right_edge = set_percent_option(opts, "AreaRightEdge", width, priv->minx);
> + pars->area_top_edge = set_percent_option(opts, "AreaTopEdge", height, priv->miny, 0);
> + pars->area_bottom_edge = set_percent_option(opts, "AreaBottomEdge", height, priv->miny, 0);
> + pars->area_left_edge = set_percent_option(opts, "AreaLeftEdge", width, priv->minx, 0);
> + pars->area_right_edge = set_percent_option(opts, "AreaRightEdge", width, priv->minx, 0);
> +
> + pars->hyst_x = set_percent_option(opts, "HorizHysteresis", width, 0, horizHyst);
> + pars->hyst_y = set_percent_option(opts, "VertHysteresis", height, 0, vertHyst);
>
> pars->finger_low = xf86SetIntOption(opts, "FingerLow", fingerLow);
> pars->finger_high = xf86SetIntOption(opts, "FingerHigh", fingerHigh);
> @@ -722,6 +731,8 @@ SynapticsPreInit(InputDriverPtr drv, InputInfoPtr pInfo, int flags)
> priv->tap_button = 0;
> priv->tap_button_state = TBS_BUTTON_UP;
> priv->touch_on.millis = 0;
> + priv->synpara.hyst_x = -1;
> + priv->synpara.hyst_y = -1;
>
> /* read hardware dimensions */
> ReadDevDimensions(pInfo);
> @@ -1715,6 +1726,26 @@ estimate_delta(double x0, double x1, double x2, double x3)
> return x0 * 0.3 + x1 * 0.1 - x2 * 0.1 - x3 * 0.3;
> }
>
> +/**
> + * Applies hysteresis. center is shifted such that it is in range with
> + * in by the margin again. The new center is returned.
> + * @param in the current value
> + * @param center the current center
> + * @param margin the margin to center in which no change is applied
> + * @return the new center (which might coincide with the previous)
> + */
> +static int hysteresis(int in, int center, int margin) {
> + int diff = in - center;
> + if (abs(diff) <= margin) {
> + diff = 0;
> + } else if (diff > margin) {
> + diff -= margin;
> + } else if (diff < -margin) {
> + diff += margin;
> + }
> + return center + diff;
> +}
> +
> static int
> ComputeDeltas(SynapticsPrivate *priv, const struct SynapticsHwState *hw,
> edge_type edge, int *dxP, int *dyP, Bool inside_area)
> @@ -2366,6 +2397,14 @@ HandleState(InputInfoPtr pInfo, struct SynapticsHwState *hw)
> if (para->touchpad_off == 1)
> return delay;
>
> + /* apply hysteresis before doing anything serious. This cancels
> + * out a lot of noise which might surface in strange phenomena
> + * like flicker in scrolling or noise motion. */
> + priv->hyst_center_x = hysteresis(hw->x, priv->hyst_center_x, para->hyst_x);
> + priv->hyst_center_y = hysteresis(hw->y, priv->hyst_center_y, para->hyst_y);
> + hw->x = priv->hyst_center_x;
> + hw->y = priv->hyst_center_y;
> +
> inside_active_area = is_inside_active_area(priv, hw->x, hw->y);
>
> /* now we know that these _coordinates_ aren't in the area.
> diff --git a/src/synapticsstr.h b/src/synapticsstr.h
> index 9ad8638..066b3f3 100644
> --- a/src/synapticsstr.h
> +++ b/src/synapticsstr.h
> @@ -160,6 +160,7 @@ 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 hyst_x, hyst_y; /* x and y width of hysteresis box */
> } SynapticsParameters;
>
>
> @@ -183,6 +184,8 @@ typedef struct _SynapticsPrivateRec
> Bool absolute_events; /* post absolute motion events instead of relative */
> SynapticsMoveHistRec move_hist[SYNAPTICS_MOVE_HISTORY]; /* movement history */
> int hist_index; /* Last added entry in move_hist[] */
> + int hyst_center_x; /* center x of hysteresis*/
> + int hyst_center_y; /* center y of hysteresis*/
> int scroll_y; /* last y-scroll position */
> int scroll_x; /* last x-scroll position */
> double scroll_a; /* last angle-scroll position */
> --
> 1.7.3.4
>
> From c992a92c0f39e755141eaf262b3f597908708764 Mon Sep 17 00:00:00 2001
> From: Simon Thum <simon.thum at gmx.de>
> Date: Thu, 10 Feb 2011 12:02:39 +0100
> Subject: [PATCH 1/3] add a few comments to ComputeDeltas()
>
> Signed-off-by: Simon Thum <simon.thum at gmx.de>
> ---
> src/synaptics.c | 12 +++++++++---
> 1 files changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/src/synaptics.c b/src/synaptics.c
> index d57a75a..b030127 100644
> --- a/src/synaptics.c
> +++ b/src/synaptics.c
> @@ -1746,6 +1746,9 @@ static int hysteresis(int in, int center, int margin) {
> return center + diff;
> }
>
> +/*
> + * Compute relative motion ('deltas') including edge motion xor trackstick.
> + */
> static int
> ComputeDeltas(SynapticsPrivate *priv, const struct SynapticsHwState *hw,
> edge_type edge, int *dxP, int *dyP, Bool inside_area)
> @@ -1779,9 +1782,11 @@ ComputeDeltas(SynapticsPrivate *priv, const struct SynapticsHwState *hw,
> !priv->vert_scroll_edge_on && !priv->horiz_scroll_edge_on &&
> !priv->vert_scroll_twofinger_on && !priv->horiz_scroll_twofinger_on &&
> !priv->circ_scroll_on && priv->prevFingers == hw->numFingers) {
> - /* FIXME: Wtf?? what's with 13? */
> - delay = MIN(delay, 13);
> - if (priv->count_packet_finger > 3) { /* min. 3 packets */
> + /* to create fluid edge motion, call back 'soon'
> + * even in the absence of new hardware events */
> + delay = 13;
> +
> + if (priv->count_packet_finger > 3) { /* min. 3 packets, see below */
> double tmpf;
> int x_edge_speed = 0;
> int y_edge_speed = 0;
> @@ -1794,6 +1799,7 @@ ComputeDeltas(SynapticsPrivate *priv, const struct SynapticsHwState *hw,
> dx = dx * dtime * para->trackstick_speed;
> dy = dy * dtime * para->trackstick_speed;
> } else if (moving_state == MS_TOUCHPAD_RELATIVE) {
> + /* HIST is full enough: priv->count_packet_finger > 3 */
> dx = estimate_delta(hw->x, HIST(0).x, HIST(1).x, HIST(2).x);
> dy = estimate_delta(hw->y, HIST(0).y, HIST(1).y, HIST(2).y);
>
> --
> 1.7.3.4
>
> From 9e51421675230cecab62d58013a7009160f2a9be Mon Sep 17 00:00:00 2001
> From: Simon Thum <simon.thum at gmx.de>
> Date: Thu, 10 Feb 2011 12:33:04 +0100
> Subject: [PATCH 2/3] reshuffle details on acceleration in the man page for increased readability
>
> Signed-off-by: Simon Thum <simon.thum at gmx.de>
> ---
> man/synaptics.man | 44 +++++++++++++++++++++++---------------------
> 1 files changed, 23 insertions(+), 21 deletions(-)
>
> diff --git a/man/synaptics.man b/man/synaptics.man
> index 16ae7f6..8d817ca 100644
> --- a/man/synaptics.man
> +++ b/man/synaptics.man
> @@ -608,8 +608,31 @@ HorizScrollDelta parameters.
> .
> To disable vertical or horizontal scrolling, set VertScrollDelta or
> HorizScrollDelta to zero.
> +
> +.SS Pressure motion
> +When pressure motion is activated, the cursor motion speed depends
> +on the pressure exerted on the touchpad (the more pressure exerted on
> +the touchpad, the faster the pointer).
> +.
> +More precisely the speed is first calculated according to MinSpeed,
> +MaxSpeed and AccelFactor, and then is multiplied by a sensitivity
> +factor.
> .
> .LP
> +The sensitivity factor can be adjusted using the PressureMotion
> +parameters.
> +.
> +If the pressure is below PressureMotionMinZ, PressureMotionMinFactor
> +is used, and if the pressure is greater than PressureMotionMaxZ,
> +PressureMotionMaxFactor is used.
> +.
> +By default, PressureMotionMinZ and PressureMotionMaxZ are equal to
> +EdgeMotionMinZ and EdgeMotionMaxZ.
> +.
> +For a pressure value between PressureMotionMinZ and
> +PressureMotionMaxZ, the factor is increased linearly.
> +.
> +.SS Edge motion
> When hitting an egde, movement can be automatically continued.
> .
> If EdgeMotionUseAlways is false, edge motion is only used when
> @@ -631,27 +654,6 @@ is used.
> For a pressure value between EdgeMotionMinZ and EdgeMotionMaxZ, the
> speed is increased linearly.
> .
> -.LP
> -When pressure motion is activated, the cursor motion speed depends
> -on the pressure exerted on the touchpad (the more pressure exerted on
> -the touchpad, the faster the pointer).
> -.
> -More precisely the speed is first calculated according to MinSpeed,
> -MaxSpeed and AccelFactor, and then is multiplied by a sensitivity
> -factor.
> -.
> -The sensitivity factor can be adjusted using the PressureMotion
> -parameters.
> -.
> -If the pressure is below PressureMotionMinZ, PressureMotionMinFactor
> -is used, and if the pressure is greater than PressureMotionMaxZ,
> -PressureMotionMaxFactor is used.
> -.
> -By default, PressureMotionMinZ and PressureMotionMaxZ are equal to
> -EdgeMotionMinZ and EdgeMotionMaxZ.
> -.
> -For a pressure value between PressureMotionMinZ and
> -PressureMotionMaxZ, the factor is increased linearly.
> .SS Middle button emulation
> Since most synaptics touchpad models don't have a button that
> corresponds to the middle button on a mouse, the driver can emulate
> --
> 1.7.3.4
>
> From 9623a1224ed4b5b7a63664e3efbc14b2a2331fa4 Mon Sep 17 00:00:00 2001
> From: Simon Thum <simon.thum at gmx.de>
> Date: Thu, 10 Feb 2011 13:30:07 +0100
> Subject: [PATCH 3/3] add some acceleration-related info to the man page
>
> Signed-off-by: Simon Thum <simon.thum at gmx.de>
> ---
> man/synaptics.man | 7 +++++++
> 1 files changed, 7 insertions(+), 0 deletions(-)
>
> diff --git a/man/synaptics.man b/man/synaptics.man
> index 8d817ca..0a35883 100644
> --- a/man/synaptics.man
> +++ b/man/synaptics.man
> @@ -608,6 +608,13 @@ HorizScrollDelta parameters.
> .
> To disable vertical or horizontal scrolling, set VertScrollDelta or
> HorizScrollDelta to zero.
> +.
> +.LP
> +Acceleration is mostly handled outside the driver, thus the driver will
> +translate MinSpeed into constant deceleration and adapt MaxSpeed at
> +startup time. This ensures you can user the other acceleration profiles, albeit
> +without pressure motion. However the numbers at runtime will likely be different
> +from any options you may have set.
>
> .SS Pressure motion
> When pressure motion is activated, the cursor motion speed depends
> --
> 1.7.3.4
>
More information about the xorg-devel
mailing list