[PATCH] dix: only transform valuators when we need them.

[Peter Hutterer]

On Mon, Apr 25, 2011 at 12:58:17PM -0400, Chase Douglas wrote:
> On 04/21/2011 03:35 AM, Peter Hutterer wrote:
> > Unconditionally drop the valuators back into the mask when they were there
> > in the first place. Otherwise, sending identical coordinates from the driver
> > on a translated device causes the valuator mask to be alternatively
> > overwritten with the translated value or left as-is. This leads to the
> > device jumping around between the translated and the original position.
> > 
> > The same could be achieved with a valuator_mask_unset() combination.
> > 
> > Testcase:
> > xsetwacom set "device name" MapToOutput VGA1
> > Then press a button on the device, cursor jumps between the two positions.
> > 
> > Introduced in 31737fff08ec19b394837341d5e358ec401f5cd8
> > 
> > Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> > ---
> >  dix/getevents.c |    5 +++--
> >  1 files changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/dix/getevents.c b/dix/getevents.c
> > index 0fa8046..7afd330 100644
> > --- a/dix/getevents.c
> > +++ b/dix/getevents.c
> > @@ -1065,9 +1065,10 @@ transformAbsolute(DeviceIntPtr dev, ValuatorMask *mask)
> >  
> >      pixman_f_transform_point(&dev->transform, &p);
> >  
> > -    if (lround(p.v[0]) != dev->last.valuators[0])
> > +    if (valuator_mask_isset(mask, 0))
> >          valuator_mask_set(mask, 0, lround(p.v[0]));
> > -    if (lround(p.v[1]) != dev->last.valuators[1])
> > +
> > +    if (valuator_mask_isset(mask, 1))
> >          valuator_mask_set(mask, 1, lround(p.v[1]));
> >  }
> >  
> 
> Why don't we change this to:
> 
> if (lround(p.v[0]) != dev->last.valuators[0])
>     valuator_mask_set(mask, 0, lround(p.v[0]));
> else
>     valuator_mask_unset(mask, 0);
> 
> The proposed fix will cause valuators to be sent with repeated values if
> they haven't actually changed.

I think this would generally need a bigger patch set than just the above
(which was my first attempt at the patch, see also the commit message) to be
addressed properly.

We don't have any guidelines for drivers regarding sending identical
coordinates and the server does a pretty poor job in filtering them,
especially for core events.
https://bugs.freedesktop.org/show_bug.cgi?id=23985

Plus the need for continuous valuator ranges in XI1 also means that even
when unsetting valuators in the generation path, they may (should!) get
added later anyway.

At least in the Wacom driver we need the coordinates to not be filtered when
identical, for the clients' sake. the specific use-case here is sending a
motion event followed by a button event. They have identical coordinates,
but if a client only listens to button events and the coordinates are
filtered, they cannot get the valuator values from the button event only.

Cheers,
  Peter