[PATCH libinput] touchpad: make the hysteresis dependent on physical distance

[Peter Hutterer]

On Mon, Jun 15, 2015 at 10:56:21AM -0700, Bill Spitzak wrote:
> My idea is that there still would be an indication of "fake resolution",
> but when that is turned on it also fills in the resolution values with this
> value as a guess. Then anything that fails to test the fake resolution flag
> would at least tend to produce stuff the same size as this does. It does
> seem possible to grep through the code for references to the fake
> resolution and see if they are all using a similar value.
>
> 
> What I am unsure of is if it is ok to write over the resolution fields when
> fake resolution is turned on. Is there some number in there that needs to
> be preserved?
> 
yeah, that's what I thought you wanted to do. leave the fake_resolution
bool in place (that tells you whether the res is accurate).

we currently take devices with a resolution of 0 and force-set it to 1 so
that divisions don't blow up. so the only numbers in there are 0 (bad), 1
(fake resolution) or the actual resolution.
see evdev_fix_abs_resolution(), you can use that to pass in the calculated
resolution.

Cheers,
   Peter

> On Sun, Jun 14, 2015 at 5:41 PM, Peter Hutterer <peter.hutterer at who-t.net>
> wrote:
> 
> > please fix your email client to not drop CC's on reply.
> >
> > On Sat, Jun 13, 2015 at 09:30:35AM -0700, Bill Spitzak wrote:
> > > Can you set the "fake" resolution so that you get the same answer without
> > > having to check if it is fake?
> > >
> > > It looks like setting tp->device->abs.absinfo_x/y->resolution to
> > diagonal *
> > > 2 / DEFAULT_HYSTERESIS_MARGIN_DENOMINATOR would avoid the if statement,
> > and
> > > I would probably make everything that uses this resolution value "work"
> > (in
> > > that they all scale the same).
> >
> > yes and no, we have a bunch of things that rely on resolution or otherwise
> > guesswork (software button area for example). so the approach is a bit more
> > complex, but setting a resolution based on the diagonal is an interesting
> > approach.
> > Have a look into that if you can please, see if the numbers work out.
> >
> > Cheers,
> >    Peter
> >
> > >
> > > >>On 12-06-15 08:09, Peter Hutterer wrote:
> > > >>
> > > >>>Some touchpads, e.g. the Cyapa in the Acer c720 have a small axis
> > range
> > > >>>([0, 870], [0, 470]), so the diagonal/magic value yields a hysteresis
> > > >>>margin
> > > >>>of 1 device unit. On that device, that's one-tenth of a millimeter,
> > > >>>causing
> > > >>>pointer motion just by holding the finger.
> > > >>>
> > > >>>For touchpads that provide a physical resolution, set the hysteresis
> > axes
> > > >>>to
> > > >>>0.5mm and do away with the magic factor.
> > > >>>
> > > >>>https://bugzilla.redhat.com/show_bug.cgi?id=1230441
> > > >>>
> > > >>>Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> > > >>>
> > > >>
> > > >>Yes I think this is a good idea:
> > > >>
> > > >>Reviewed-by: Hans de Goede <hdegoede at redhat.com>
> > > >>
> > > >>Can you also ask the reporter of:
> > > >>
> > > >>https://bugs.freedesktop.org/show_bug.cgi?id=90590
> > > >>
> > > >>To test libinput/master once you've pushed this? It will likely
> > > >>help with his problem too.
> > > >>
> > > >>Regards,
> > > >>
> > > >>Hans
> > > >>
> > > >>
> > > >>
> > > >>  ---
> > > >>>   src/evdev-mt-touchpad.c | 16 ++++++++++++----
> > > >>>   1 file changed, 12 insertions(+), 4 deletions(-)
> > > >>>
> > > >>>diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c
> > > >>>index 8c28ff7..e9870ad 100644
> > > >>>--- a/src/evdev-mt-touchpad.c
> > > >>>+++ b/src/evdev-mt-touchpad.c
> > > >>>@@ -1495,10 +1495,18 @@ tp_init(struct tp_dispatch *tp,
> > > >>>                                                        EV_ABS,
> > > >>>
> > ABS_MT_DISTANCE);
> > > >>>
> > > >>>-       tp->hysteresis_margin.x =
> > > >>>-               diagonal / DEFAULT_HYSTERESIS_MARGIN_DENOMINATOR;
> > > >>>-       tp->hysteresis_margin.y =
> > > >>>-               diagonal / DEFAULT_HYSTERESIS_MARGIN_DENOMINATOR;
> > > >>>+       if (device->abs.fake_resolution) {
> > > >>>+               tp->hysteresis_margin.x =
> > > >>>+                       diagonal /
> > DEFAULT_HYSTERESIS_MARGIN_DENOMINATOR;
> > > >>>+               tp->hysteresis_margin.y =
> > > >>>+                       diagonal /
> > DEFAULT_HYSTERESIS_MARGIN_DENOMINATOR;
> > > >>>+       } else {
> > > >>>+               int res_x = tp->device->abs.absinfo_x->resolution,
> > > >>>+                   res_y = tp->device->abs.absinfo_y->resolution;
> > > >>>+
> > > >>>+               tp->hysteresis_margin.x = res_x/2;
> > > >>>+               tp->hysteresis_margin.y = res_y/2;
> > > >>>+       }
> > > >>>
> > > >>>         if (tp_init_accel(tp, diagonal) != 0)
> > > >>>                 return -1;
> > > >>>
> > > >>>  _______________________________________________
> > > >>wayland-devel mailing list
> > > >>wayland-devel at lists.freedesktop.org
> > > >>http://lists.freedesktop.org/mailman/listinfo/wayland-devel
> > > >>
> > > >
> > >
> > >
> > > _______________________________________________
> > > wayland-devel mailing list
> > > wayland-devel at lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/wayland-devel
> > >
> >