On Sat, Sep 27, 2014 at 10:08:39PM +0200, Éric Brunet wrote: > When dealing with a device identified as a tablet, both Qt and gtk seem to > ignore the position as reported by the X server, but rather compute > themselves the position from the raw valuator values (In order to achieve > better precision, I imagine). This usually works well because most > tablets use the wacom driver and wacom always report all the valuators at > each event, even the unchanged ones. However, evdev reports only the > valuators that have changed, and it is a source of problems for several > programs when used with a tablet, see bugs > https://bugs.freedesktop.org/show_bug.cgi?id=82250 > and https://bugs.freedesktop.org/show_bug.cgi?id=82181 > > This patch makes evdev always report all the valuators for devices with > absolute positionning, which fixes the problems I reported. > > Signed-off-by: Éric Brunet <[email protected]> > --- > src/evdev.c | 33 +++++++++++++++++++++++++++------ > 1 file changed, 27 insertions(+), 6 deletions(-) > > diff --git a/src/evdev.c b/src/evdev.c > index 8eb749a..9fa17b5 100644 > --- a/src/evdev.c > +++ b/src/evdev.c > @@ -941,11 +941,26 @@ static void EvdevPostQueuedEvents(InputInfoPtr pInfo) > pEvdev->queue[i].val)) > break; > > - if (pEvdev->abs_queued && pEvdev->in_proximity) { > - xf86PostButtonEvent(pInfo->dev, Absolute, pEvdev- > >queue[i].detail.key,
fwiw, please fix your email client, it mangled the patch as seen here. > - pEvdev->queue[i].val, 0, 0); > - > - } else > + /* > + * We try to report all the valuators with the button events > + * because both Qt ang gtk seem to expect them to be present > + * for tablets and similar devices. In fact, the wacom driver > + * always sends all the valuator, so evdev should do the same. > + * > + * However, we only do this if > + * a) the device uses absolute values (because sending a > + * relative valuator several time would be akin to moving > the > + * cursor twice) > + * b) the device is in proximity (because we don't trust > position > + * otherwise) > + */ > + if ((pEvdev->flags & EVDEV_ABSOLUTE_EVENTS) && > + !(pEvdev->flags & EVDEV_RELATIVE_MODE) && > + pEvdev->in_proximity && pEvdev->vals) > + xf86PostButtonEventM(pInfo->dev, Absolute, pEvdev- > >queue[i].detail.key, > + pEvdev->queue[i].val, pEvdev->vals); > + else > + /* Either relative or not in proximity */ > xf86PostButtonEvent(pInfo->dev, Relative, pEvdev- > >queue[i].detail.key, > pEvdev->queue[i].val, 0, 0); > break; > @@ -993,7 +1008,13 @@ EvdevProcessSyncEvent(InputInfoPtr pInfo, struct > input_event *ev) > /* don't reset the touchMask */ > } > > - if (pEvdev->vals) > + /* > + * Do not reset pEvdev->vals if the device uses absolute events. That > + * way, all the valuators (even those that didn't change) are always > + * reported > + */ > + if (pEvdev->vals && > + (pEvdev->flags & (EVDEV_RELATIVE_EVENTS|EVDEV_RELATIVE_MODE)) ) > valuator_mask_zero(pEvdev->vals); > pEvdev->num_queue = 0; > pEvdev->abs_queued = 0; ok, this patch looked good at first, so I applied it locally. Unfortunately, it breaks a couple of tests in XIT, specifically EvdevMixedDeviceTest.AbsXYAndRelScroll/*. http://cgit.freedesktop.org/xorg/test/xorg-integration-tests/tree/tests/input/evdev.cpp#n1925 These tests set up a device with absolute axes and scroll wheels, then post a scroll wheel event followed by a abs movement event. The variations are whether IgnoreAbsoluteAxes or IgnoreRelativeAxes is set, i.e. whether we expect the abs event to come through or not. It breaks in all cases absolute axes are not ignored (including the default case, so easy enough to test w/o config). Here's the problem: if smooth scrolling is enabled, the wheel events are set as relative valuators in the mask. so on the first wheel event, the mask is [0, 0, 1, ...], on the second wheel event the mask is [0, 0, -1, ..]. the device is in absolute mode, so the mask isn't reset after events (scroll wheels on their own don't set the device to relative, they are special). Now you get the abs x/y events and the mask is [70, 70, -1, ...]. The server interprets the -1 as another scroll event. long story short, if you ever get a scroll wheel event on those devices, you keep scrolling. That's a lot of fun, but arguably a regression :) So the mask reset needs to be a bit smarter but from a quick skim of the code it looks like that's a general problem anyway. if the REL_WHEEL is sent in the same EV_SYN frame as the ABS_X event, both rel_queued and abs_queued are set and the mask is submitted twice to the server (once as rel, once as abs mask) and we get duplicate events. So congratulations I guess: you found a bug and won the chance to fix that too ;) sorry about that. Cheers, Peter PS: first patch of this pair pushed, thanks _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
