Re: [Linuxwacom-devel] [PATCH 3/3] Use absinfo.fuzz as default value for Suppress

[Jason G. <kil...@gm...>]

On Mon, Mar 25, 2013 at 6:39 PM, Peter Hutterer <pet...@wh...>wrote:

> Use the bigger of ABS_X/ABS_Y fuzz as the default value for suppress to
> avoid erroneous movements. The kernel-announced fuzz for I4 tablets is 4,
> so
> we get plenty of erroneous movement, specificially on the puck cursor.
>
> Signed-off-by: Peter Hutterer <pet...@wh...>
> ---
>  man/wacom.man | 8 +++++---
>  src/wcmUSB.c  | 6 ++++++
>  2 files changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/man/wacom.man b/man/wacom.man
> index 98dec37..622bf73 100644
> --- a/man/wacom.man
> +++ b/man/wacom.man
> @@ -88,9 +88,11 @@ This option is mandatory.
>  sets the position increment under which not to transmit coordinates.
>  This entry must be specified only in the first Wacom subsection if you
> have
>  multiple devices for one tablet. If you don't specify this entry, the
> default
> -value,  which is 2, will be used. To disable suppression, the entry
> should be
> -specified as 0.  When suppress is defined,  an event will be sent only
> when at
> -least one of the following conditions is met:
> +value will be used. This default value is retrieved from the kernel, or 2
> if
> +the kernel does not provide a fuzz value.
> +To disable suppression, the entry should be specified as 0.  When
> +suppress is defined,  an event will be sent only when at least one of the
> +following conditions is met:
>
>          the change between the current X coordinate and the previous one
> is
>  greater than suppress;
> diff --git a/src/wcmUSB.c b/src/wcmUSB.c
> index 980dac3..8ee0078 100644
> --- a/src/wcmUSB.c
> +++ b/src/wcmUSB.c
> @@ -534,6 +534,9 @@ int usbWcmGetRanges(InputInfoPtr pInfo)
>  #endif
>         }
>
> +       if (absinfo.fuzz > 0)
> +               common->wcmSuppress = absinfo.fuzz;
> +
>         /* max y */
>         if (ioctl(pInfo->fd, EVIOCGABS(ABS_Y), &absinfo) < 0)
>         {
> @@ -567,6 +570,9 @@ int usbWcmGetRanges(InputInfoPtr pInfo)
>  #endif
>         }
>
> +       if (absinfo.fuzz > 0)
> +               common->wcmSuppress = max(absinfo.fuzz,
> common->wcmSuppress);
> +
>         /* max finger strip X for tablets with Expresskeys
>          * or physical X for touch devices in hundredths of a mm */
>         if (ISBITSET(abs, ABS_RX) &&
> --
> 1.8.1.4
>
>
NAK. Three issues:

1) Correcting a noisy axis should be done by adjusting the in-kernel fuzz
value. Remember that the X driver isn't the only consumer of our data.

2) Suppress is a a sledgehammer and acts on every axis equally. Having the
suppress auto-detected to 4, for example, means that the touchring will
becomes quite insensitive to fine motion. If you must have the X driver
handle things, suppress should be rewritten to not be quite so global.

3) If suppress were rewritten to be more limited in scope, the proper
default value should be fuzz/2 (see input_defuzz_abs_event in
drivers/input/input.c)

Jason
---
Now instead of four in the eights place /
you’ve got three, ‘Cause you added one  /
(That is to say, eight) to the two,     /
But you can’t take seven from three,    /
So you look at the sixty-fours....