Bug 21832 - Corrupt mouse motion events send pointer to top-left of screen
Corrupt mouse motion events send pointer to top-left of screen
Status: RESOLVED FIXED
Product: xorg
Classification: Unclassified
Component: Server/Input/Core
git
x86 (IA32) Linux (All)
: medium normal
Assigned To: Peter Hutterer
Xorg Project Team
http://bugs.debian.org/528994
:
: 21891 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2009-05-20 07:10 UTC by Julien Cristau
Modified: 2009-05-27 15:56 UTC (History)
2 users (show)

See Also:
i915 platform:
i915 features:


Attachments
Patch to ignore REL, ABS events when driver not configured for them (890 bytes, patch)
2009-05-20 21:52 UTC, Derek Upham
no flags Details | Splinter Review
Git-format patch to ignore REL, ABS events when driver not configured for them (1.73 KB, patch)
2009-05-21 00:28 UTC, Derek Upham
no flags Details | Splinter Review
Derek's patch amended with Peter's suggestion from comment #6 (2.10 KB, patch)
2009-05-21 05:38 UTC, Julien Cristau
no flags Details | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Julien Cristau 2009-05-20 07:10:12 UTC
[Forwarded from Debian bug#528994, original report by Derek Upham <sand@blarg.net>]

Using xserver based on 1.6-branch (a9f85dc), and evdev 2.2.2.

Quoting Derek:
"With my upgrade to xserver-xorg-core 2:1.6.1.901-2, normal mouse
activity consistently causes my mouse pointer to move to the top-left
corner of my display.  All I need to do is combine mouse motion with
button presses.  An easy way to reproduce this is to use middle-mouse
scrolling in Firefox: start moving the mouse down, and while moving
press the middle mouse button.  The mouse pointer moves to position
+0+0, taking focus with it.  This doesn't happen with every stroke,
but within a couple of strokes it does.

The problem is occuring with a Contour Mouse "Perfit Mouse Optical".
I have been unable to reproduce the problem with a Logitech WingMan
mouse.  (Both are USB mice.)  This is independent of my window
manager; I can reproduce the problem running just Xterm.  It is
happening whether I use the mousedrv or evdev input driver.  I'm
running a single-head configuration."

"I have managed to work my way back up the failure chain, to the point
where GDB can stop the server before the mouse moves.  The failure is
associated with a motion event of {0,0} and is_absolute of 1.

(gdb) bt
#0  xf86PostMotionEventP (device=0x84a6308, is_absolute=1, first_valuator=0, num_valuators=2, valuators=0xbfc6af7c)
    at ../../../../hw/xfree86/common/xf86Xinput.c:741
#1  0xb7a6a263 in ?? () from /usr/lib/xorg/modules/input//evdev_drv.so
#2  0x084a6308 in ?? ()
#3  0x00000001 in ?? ()
#4  0x00000000 in ?? ()
(gdb) p *valuators@num_valuators
$77 = {0, 0}
"

According to Peter, "i think it's to do with the last.valuators on the master device not getting updated properly if num_valuators is 0".
Comment 1 Derek Upham 2009-05-20 21:22:04 UTC
Found the problem.  My mouse is sending EV_ABS with event code 40:

event type=EV_KEY, code=272, value=1
event type=EV_ABS, code=40, value=0
event type=EV_REL, code=0, value=2
event type=EV_REL, code=1, value=5
event type=EV_SYN, code=0, value=0

Why?  Who knows!  Every time there's a

  event type=EV_ABS, code=40, value=0

though it's followed by

  event type=EV_ABS, code=40, value=2

so perhaps there is some Deeper Meaning.  Checking input.h shows that code 40 means ABS_MISC, which provides no additional information.

The real problem appears to be that the current code doesn't support mixed relative and absolute devices:

    /* We don't allow relative and absolute axes on the same device. Reason
       Reason being that some devices (MS Optical Desktop 2000) register both
       rel and abs axes for x/y.
       The abs axes register min/max, this min/max then also applies to the
       relative device (the mouse) and caps it at 0..255 for both axis.
       So unless you have a small screen, you won't be enjoying it much.

        FIXME: somebody volunteer to fix this.
     */
    if (pEvdev->flags & EVDEV_RELATIVE_EVENTS)
	EvdevAddRelClass(device);
    else if (pEvdev->flags & EVDEV_ABSOLUTE_EVENTS)
        EvdevAddAbsClass(device);

Except that it does support them mixed, sort of.  EvdevProcessEvent will happily accept EV_ABS events, attempt to stuff them in the "vals" array, and set the "abs" flag.

        case EV_ABS:
            if (ev->code > ABS_MAX)
                break;
            pEvdev->vals[pEvdev->axis_map[ev->code]] = value;
            if (ev->code == ABS_X)
                abs |= ABS_X_VALUE;
            else if (ev->code == ABS_Y)
                abs |= ABS_Y_VALUE;
            else
                abs |= ABS_VALUE;
            break;

Then with the next EV_SYN we process the data as if they were absolute in the first place, and call xf86PostMotionEventP() with "is_absolute" set to TRUE.

So the driver queries the mouse for the relative and absolute events it will generate (and my mouse correctly reports that it will send ABS_MISC).  Then it initializes just the EV_REL portion.  Then it accepts and processes EV_ABS events that it never set itself up to handle.  The minimal solution appears to be:

   * Process EV_REL events in EvdevProcessEvent if and only if the EVDEV_RELATIVE_EVENTS flag is set.  Ignore them otherwise.

   * Process EV_ABS events in EvdevProcessEvent if and only if the EVDEV_ABSOLUTE_EVENTS flag is set _and_ the EVDEV_RELATIVE_EVENTS flag is not set.  Ignore them otherwise.

Comment 2 Derek Upham 2009-05-20 21:52:28 UTC
Created attachment 26047 [details] [review]
Patch to ignore REL, ABS events when driver not configured for them

This patch fixes the problem on my machine.
Comment 3 Peter Hutterer 2009-05-20 22:08:47 UTC
+  if (EVDEV_RELATIVE_EVENTS || ! (pEvdev->flags & EVDEV_ABSOLUTE_EVENTS))

the first bit doesn't make sense, shouldn't that be (.. flags & RELATIVE..)?
Also, the second condition should be enough. If not, then we need to change
the flag setting code.

please remove the space between the ! and the condition too.

Other than that, looks good. Please reattach the modified version as a
git-formatted patch (see
http://wiki.x.org/wiki/Development/Documentation/SubmittingPatches)
Comment 4 Derek Upham 2009-05-21 00:28:51 UTC
Created attachment 26050 [details] [review]
Git-format patch to ignore REL, ABS events when driver not configured for them

Yeah, the first bit is broken.  But it does need both parts.  Look at the initialization code:

    if (pEvdev->flags & EVDEV_RELATIVE_EVENTS)
        EvdevAddRelClass(device);
    else if (pEvdev->flags & EVDEV_ABSOLUTE_EVENTS)
        EvdevAddAbsClass(device);

It's completely legal for a device to support neither relative nor absolute events.  The ABS handlers won't get set up if EVDEV_RELATIVE_EVENTS is set, and they won't get set up if EVDEV_ABSOLUTE_EVENTS is not set.  We should ignore the ABS events in either situation.

Added git-format patch.
Comment 5 Julien Cristau 2009-05-21 00:44:09 UTC
> --- Comment #4 from Derek Upham <sand@blarg.net>  2009-05-21 00:28:51 PST ---
> Created an attachment (id=26050)
>  --> (http://bugs.freedesktop.org/attachment.cgi?id=26050)
> Git-format patch to ignore REL, ABS events when driver not configured for them

ack, looks good to me.
Comment 6 Peter Hutterer 2009-05-21 05:21:05 UTC
> Yeah, the first bit is broken.  But it does need both parts.  Look at the
> initialization code:
> 
>     if (pEvdev->flags & EVDEV_RELATIVE_EVENTS)
>         EvdevAddRelClass(device);
>     else if (pEvdev->flags & EVDEV_ABSOLUTE_EVENTS)
>         EvdevAddAbsClass(device);

You are right. This makes me wonder though if we should just unset the
EVDEV_ABSOLUTE_EVENTS flag at this point in the code. This way we require a
single condition in your patch and we only have one point where this hack is
needed (and documented). In the future, when we manage to figure out how to
support both types of axes there's only one point in the code we have to
worry about.
Comment 7 Julien Cristau 2009-05-21 05:38:17 UTC
Created attachment 26063 [details] [review]
Derek's patch amended with Peter's suggestion from comment #6
Comment 8 Derek Upham 2009-05-21 09:36:34 UTC
We shouldn't unset the flag there.  We should change EvdevProbe so that it never enters the current

  if (has_abs_axes) { ... }

block if we have already set up for relative events. That way the device is never in an inconsistent state.  The function is already doing something similar for pure scroll wheels, near the bottom.

The simplest approach would be to never enter the for (...) {...} loop that tests bits if "has_rel_axes" is set.  Or we have special code after the loop that checks "has_rel_axes", sets "has_abs_axes" to false, and then sends an X_INFO message: "already set up for relative motion, ignoring absolute motion events".
Comment 9 Peter Hutterer 2009-05-21 15:25:21 UTC
I disagree, simply for the reason that it would split the hack over two different locations again, making it harder to identify and fix in the future. If we were to skip the has_abs part, then we'd have to have a comment there to refer to the other location and vice versa.

You're right that we should have X_INFO output, I'll add that to your patch before the flag is unset.
Comment 10 Peter Hutterer 2009-05-23 04:01:16 UTC
*** Bug 21891 has been marked as a duplicate of this bug. ***
Comment 11 Peter Hutterer 2009-05-27 15:56:15 UTC
Pushed as 0a3657d2ee62f4086e9687218cb33835ba61a0b3. Thanks for the patch.