After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 588649 - extended input events sent to widgets that didn't select for them
extended input events sent to widgets that didn't select for them
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Widget: Other
2.17.x
Other All
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
csw
Depends on:
Blocks:
 
 
Reported: 2009-07-15 12:57 UTC by Tom Jaeger
Modified: 2009-10-05 20:21 UTC
See Also:
GNOME target: ---
GNOME version: 2.27/2.28


Attachments
Offset coordinates (24.42 KB, image/png)
2009-09-02 20:40 UTC, Josh Andler
  Details
Fix x/y coordinates for extended events. (1.18 KB, patch)
2009-09-21 17:38 UTC, Carlos Garnacho
committed Details | Review
Block extended events to windows that didn't select them. (950 bytes, patch)
2009-09-21 17:39 UTC, Carlos Garnacho
committed Details | Review
Always report XInput events to the grab window if any. (1.63 KB, patch)
2009-09-21 17:39 UTC, Carlos Garnacho
committed Details | Review
step in the right direction... (1.04 KB, patch)
2009-09-23 00:30 UTC, Tom Jaeger
none Details | Review
Slightly improved patch (5.40 KB, patch)
2009-09-23 08:15 UTC, Tom Jaeger
none Details | Review
proposed patch (14.27 KB, patch)
2009-09-24 22:37 UTC, Tom Jaeger
none Details | Review
revised patch (13.38 KB, patch)
2009-09-25 09:19 UTC, Tom Jaeger
none Details | Review
revised patch (15.72 KB, patch)
2009-09-25 15:27 UTC, Tom Jaeger
none Details | Review
Use strstr instead of g_strrstr. (1.15 KB, patch)
2009-09-29 06:44 UTC, Tom Jaeger
none Details | Review
Refactor _gdk_input_other_event (1.67 KB, patch)
2009-09-29 06:48 UTC, Tom Jaeger
committed Details | Review
XInput allows up to 255 buttons (3.02 KB, patch)
2009-09-29 06:48 UTC, Tom Jaeger
none Details | Review
Report XInput button motion events until all buttons are released. (926 bytes, patch)
2009-09-29 06:48 UTC, Tom Jaeger
committed Details | Review
Always select all XInput motion events (2.33 KB, patch)
2009-09-29 06:48 UTC, Tom Jaeger
committed Details | Review
Filter out events that the current window didn't select for (3.19 KB, patch)
2009-09-29 06:48 UTC, Tom Jaeger
committed Details | Review
Keep track of axis values (7.58 KB, patch)
2009-09-29 06:48 UTC, Tom Jaeger
none Details | Review
Select for DeviceStateNotify (2.67 KB, patch)
2009-09-29 06:49 UTC, Tom Jaeger
none Details | Review
Interpret min_value == max_value correctly (2.43 KB, patch)
2009-09-29 06:49 UTC, Tom Jaeger
none Details | Review
0001-Keep-track-of-axis-values.patch (3.93 KB, patch)
2009-09-29 21:04 UTC, Tom Jaeger
committed Details | Review
0002-Select-for-DeviceStateNotify.patch (2.33 KB, patch)
2009-09-29 21:07 UTC, Tom Jaeger
committed Details | Review
0003-Interpret-min_value-max_value-correctly.patch (2.17 KB, patch)
2009-09-29 21:18 UTC, Tom Jaeger
committed Details | Review

Description Tom Jaeger 2009-07-15 12:57:54 UTC
Please describe the problem:
If an application uses extended input devices (e.g. xournal), even widgets that don't care about those events apparently get them anyway.  Since XInput devices might use different coordinate systems this leads to all sorts of breakage, such as scrollbars and menus not working.

Steps to reproduce:
1. Start xournal
2. Make sure Options/Use XInput is enabled
3. Try using the menus


Actual results:


Expected results:


Does this happen every time?
Yes

Other information:
I've tested this on a git master X Server, but I suspect it's also true on older servers.  I'm using gtk from Ubuntu Karmic.  A similar problem with scrollbars (the windows' contents jump around when dragging the scroll bar with the pointer in the window) has been present since at least 2.16
Comment 1 Josh Andler 2009-09-02 20:29:20 UTC
I can confirm this too.
Comment 2 Josh Andler 2009-09-02 20:36:23 UTC
I should also add that this has been tested against GIMP, Inkscape, and MyPaint as well. The typical symptoms tend to be that if you have the extended input capabilities configured, the on-canvas coordinates are offset by whatever additional widgets are to the top or left of the canvas. So, in Inkscape, the the more toolbars you have at the top will invisibly offset the input from the cursor on the canvas by the same height.
Comment 3 Josh Andler 2009-09-02 20:40:22 UTC
Created attachment 142353 [details]
Offset coordinates

This attachment shows the offset (visible by the arrow coordinates on the ruler) in relation to 0,0 on Inkscape's canvas. Note that there are three toolbars above canvas and the toolbox on the left side, as well as the rulers. This is a cropped screenshot fyi.

A more in-depth discussion about the issue can be found at:
https://bugs.launchpad.net/bugs/410267
Comment 4 Carlos Garnacho 2009-09-03 09:06:12 UTC
This seems to be due to events coming from input devices not being translated by _gdk_windowing_got_event(), so coordinates/window translation doesn't happen.

I will try to work on it on my xi2 branch so this specific fix can be backported to master.
Comment 5 Alexander Larsson 2009-09-03 12:17:31 UTC
Yeah, the input events handling is quite lame, but then it was pretty lame initially. However, i though i fixed this by always making the window native if you request input events on it, maybe that got broken somehow?

Of course, ideally we should make the input event handling not so lame.
Comment 6 Denis Auroux 2009-09-04 23:37:27 UTC
I'd like to point out that fixing the coordinates/window translation would be good, but is not a sufficient step as far as I'm concerned. (Xournal works around coordinate/window translation issues when XInput events are sent to its main drawing area). From Xournal's perspective it would be much better if widgets just received core events and no extended input events. 

Most XInput device drivers out there send extended events with completely meaningless (x,y) coordinates. Even those that do send meaningful (x,y) coordinates will still get mangled by GTK+ if the device resolution gets reconfigured while the application is running (think: tablet PC screen rotation). Xournal's workaround for the device reconfigure problem is to set the input device axes in GDK_AXIS_IGNORE mode and calculate the window-relative coordinates of XInput events manually; when a device's axes are set to GDK_AXIS_IGNORE, event coordinate translation within GDK does not happen, and there's no way that a widget that has no XInput awareness would know what to do with the event.

Anyway: anything would be better than the current situation, but my vote would be to stop sending extended input events to widgets that weren't meant to handle them, and just make sure they receive the corresponding core pointer event instead. (Note: in recent GTK+, core pointer events tend to get discarded silently whenever they match with an extended input event from an enabled device -- in my view this is a misfeature, I'd rather receive both and be able to handle whichever one I like or even both if I wish to).

Thanks in advance.
Denis
Comment 7 Denis Auroux 2009-09-06 18:53:28 UTC
Two additional related problems in the GTK+ 2.17 extended event code:

1. a pretty bad crasher: clicking in a combo box with an enabled XInput device results in a segfault in gdk_input_translate_coordinates()  (e.g.: in GIMP, the combo box that controls the zoom level at the bottom of the drawing window; also, in Xournal, the combo box that controls layers). This is due to gdk_input_translate_coordinates() being called to process extended events on a GdkWindow for which impl_window->input_window is NULL. The same happens with other widgets too (e.g. the popup generated when moving selected text within a GtkTextView, again assuming the GtkTextView resides within an xinput-aware window).

2. if gdk_input_set_extension_events() is called to disable extension events while one is being processed, the interface becomes unresponsive to *all* pointer events, as display->ignore_core_events remains stubbornly set to 1... the code should check and reset display->ignore_core_events when a window stops being aware of extended events.

Denis
Comment 8 Juan Sebastián Marulanda Pulido 2009-09-20 18:44:37 UTC
It¿'s been over a month since this issue was commented, I'd like to know what has been done. My tablet can't be used properly yet.
Comment 9 Carlos Garnacho 2009-09-21 17:38:46 UTC
Created attachment 143621 [details] [review]
Fix x/y coordinates for extended events.

Extended events were being reported to virtual windows without taking into
account the position of the virtual window inside the native one.
Comment 10 Carlos Garnacho 2009-09-21 17:39:10 UTC
Created attachment 143622 [details] [review]
Block extended events to windows that didn't select them.

Now XSelectExtensionEvent() is called on the native window, so there
may be virtual windows inside that shouldn't receive extended events.
Comment 11 Carlos Garnacho 2009-09-21 17:39:23 UTC
Created attachment 143623 [details] [review]
Always report XInput events to the grab window if any.

The grab window should be the first option to send events to, else we may
get unpaired events, making display->ignore_core_events go crazy.
Comment 12 Alexander Larsson 2009-09-21 18:09:18 UTC
Those looked good to me, so i commited them all. Does this fix everyones problem?
Comment 13 Tom Jaeger 2009-09-22 00:05:40 UTC
Thanks, absolute devices work perfectly now.  However, I'm still having trouble with relative devices: They don't work at all -- at least on xserver master.  Could someone test this on a 1.6 xserver? I have a feeling this is due to device coordinates being reported differently between server versions.
Comment 14 Carlos Garnacho 2009-09-22 09:25:41 UTC
(In reply to comment #13)
> Thanks, absolute devices work perfectly now.  However, I'm still having trouble
> with relative devices: They don't work at all -- at least on xserver master. 
> Could someone test this on a 1.6 xserver? I have a feeling this is due to
> device coordinates being reported differently between server versions.

In general I see the same behavior than non-csw GTK+ versions with a fairly recent Xorg git build, could you try running apps with GDK_NATIVE_WINDOWS=1 or linking to your system libg[dt]k+-2.0.so? That should help distinguish where the bug is
Comment 15 Tom Jaeger 2009-09-22 15:56:30 UTC
>       x_scale = gdk_screen_get_width (gdk_drawable_get_screen (window)) / device_width;
>       y_scale = gdk_screen_get_height (gdk_drawable_get_screen (window)) / device_height;
 
This is wrong.  If the device is relative, axis_min == axis_max == -1, so we devide by zero.  I haven't tested this, but I think scale should be 1 in this case.
Comment 16 Alexander Larsson 2009-09-22 18:14:02 UTC
Tom: The pre-csw code does exactly the same thing though...
Comment 17 Tom Jaeger 2009-09-23 00:20:05 UTC
(In reply to comment #16)
> Tom: The pre-csw code does exactly the same thing though...

I don't know how we've gotten away with this for so long, but this is clearly not correct.  Coordinate translation should be functionally equivalent to how the X server translates SD coordinate systems to MD ones (rescaleValuatorAxis in dix/getevents.c).
Comment 18 Tom Jaeger 2009-09-23 00:30:56 UTC
Created attachment 143764 [details] [review]
step in the right direction...

This fixes one of the issues with the current code, namely the fact that devices are allowed to set max_value == min_value == -1.

Recent versions of evdev send only the valuators that have changed since the last event, i.e. first_axis may be non-zero and axes_count may be less then num_axes.  In this case axis_data[i] represents the (i+first_axis)th valuator.
Comment 19 Tom Jaeger 2009-09-23 05:44:54 UTC
I've posted some more information about the problem here:

http://mail.gnome.org/archives/gtk-devel-list/2009-September/msg00055.html

I've also noticed another problem: with ENABLE_XINPUT_BUGFIX (which basically takes over coordinate translation from gtk) disabled, coordinates are off after scrolling.
Comment 20 Denis Auroux 2009-09-23 05:51:01 UTC
Tom, in reply to your last comment: the issue you observe with ENABLE_XINPUT_BUGFIX disabled is a bug in xournal's code, not in GTK+.
I'll fix it very soon in the CVS.

Denis
Comment 21 Tom Jaeger 2009-09-23 05:59:31 UTC
(In reply to comment #20)
> Tom, in reply to your last comment: the issue you observe with
> ENABLE_XINPUT_BUGFIX disabled is a bug in xournal's code, not in GTK+.
> I'll fix it very soon in the CVS.
> 
> Denis

Thanks, I see.  Sorry about the noise.
Comment 22 Alexander Larsson 2009-09-23 07:24:31 UTC
Tom: I don't see how that patch can help? If width == 0 then we would have divided by zero before we reach the changed code, no?
Comment 23 Tom Jaeger 2009-09-23 07:29:59 UTC
(In reply to comment #22)
> Tom: I don't see how that patch can help? If width == 0 then we would have
> divided by zero before we reach the changed code, no?

Yes, but that's fine since we're not actually using x_scale.
Comment 24 Tom Jaeger 2009-09-23 08:15:51 UTC
Created attachment 143770 [details] [review]
Slightly improved patch

We still get crazy coordinates (I think this is due to XQueryDeviceState returning garbage on master), but at least I can draw in gimp with my track point now.

TODO: Bounds checks, always select motion events, and select and interpret DeviceState events.
Comment 25 Alexander Larsson 2009-09-24 09:57:40 UTC
Tom: How is that fine? doesn't the divide by zero cause a SIGFPE?
Comment 26 Tom Jaeger 2009-09-24 22:37:21 UTC
Created attachment 143944 [details] [review]
proposed patch

This patch implements strategy (b) from my mailing list post and should make gtk+ work with recent builds of the evdev driver.

Caveat: there seems to be a bug in xserver master (not sure about earlier versions) that prevents DeviceStatusNotify events from being sent.  This means that if you Alt+Tab into your window without and then click before moving the mouse, you'll get wrong coordinates for your click.  I think this is something we can live with.
Comment 27 Alexander Larsson 2009-09-25 08:35:12 UTC
You seem to be leaking axis_data.

If you want to correctly emulate button motion, don't you need to select for all button events too.

Emulation of GDK_POINTER_MOTION_HINT_MASK is a quite complex thing, and I'm not sure that the code there really does this right.

Emulation of GDK_BUTTON1_MOTION_MASK etc is wrong, I think. It doesn't only depend on the first button pressed. You need to keep track of the full bitmask of which buttons are down.
Comment 28 Tom Jaeger 2009-09-25 09:06:04 UTC
(In reply to comment #27)
> You seem to be leaking axis_data.
The existing code seems to be already leaking axes, so I thought I was fine.

> If you want to correctly emulate button motion, don't you need to select for
> all button events too.
Button events are always automatically selected (cf _gdk_input_select_events).

> Emulation of GDK_POINTER_MOTION_HINT_MASK is a quite complex thing, and I'm not
> sure that the code there really does this right.
To be honest, I'm not even sure what it is supposed to do.  My code just treats it as if it was GDK_POINTER_MOTION_MASK.

> Emulation of GDK_BUTTON1_MOTION_MASK etc is wrong, I think. It doesn't only
> depend on the first button pressed. You need to keep track of the full bitmask
> of which buttons are down.
I always assumed ButtonXMotion events behaved the same as grabs, which stay active until the last button is released, but I'll have to test that to make sure.
Comment 29 Tom Jaeger 2009-09-25 09:19:48 UTC
Created attachment 143972 [details] [review]
revised patch

You were correct.  ButtonXMask only cares about the logical state of button X.  This actually simplifies things a little bit.
Comment 30 Alexander Larsson 2009-09-25 10:11:35 UTC
axes is freed in gdkinput.c:gdk_device_dispose()

GDK_POINTER_MOTION_HINT_MASK is a hint that is combined with the other motion masks to limit the amount of motion events sent:

• PointerMotionHintMask
  If PointerMotionHintMask is selected in combination with one or more of the 
  above masks, the X server is free to send only one MotionNotify event (with 
  the is_hint member of the XPointerMovedEvent structure set to NotifyHint) to 
  the client for the event window, until either the key or button state changes,
  the pointer leaves the event window, or the client calls XQueryPointer or 
  XGetMotionEvents. The server still may send MotionNotify events without 
  is_hint set to NotifyHint.

Rather than doing anything that is not a full implementation of this i recommend to just strip out and ignore that flag. Its optional after all.
Comment 31 Alexander Larsson 2009-09-25 10:17:31 UTC
+      if ((gdkdev->button_state & 1 << 1) && (priv->extension_events & GDK_BUTTON1_MOTION_MASK))

Surely button state mask for button "1 << 0" (== 1), not "1 << 1" (== 2)
Comment 32 Tom Jaeger 2009-09-25 15:27:40 UTC
Created attachment 144007 [details] [review]
revised patch

(In reply to comment #30)
> axes is freed in gdkinput.c:gdk_device_dispose()
> 
> GDK_POINTER_MOTION_HINT_MASK is a hint that is combined with the other motion
> masks to limit the amount of motion events sent:
> 
> [...]
> 
> Rather than doing anything that is not a full implementation of this i
> recommend to just strip out and ignore that flag. Its optional after all.

That seems fair, especially since the functionality is gone in XI2 anyway.


(In reply to comment #31)
> +      if ((gdkdev->button_state & 1 << 1) && (priv->extension_events &
> GDK_BUTTON1_MOTION_MASK))
> 
> Surely button state mask for button "1 << 0" (== 1), not "1 << 1" (== 2)
No, this is the way the original code worked (gdkdev->button_state |= 1 << xdbe->button).  But you raise a good point:  XI allows up to 255 buttons, so button_state was to small to hold all the buttons anyway.
Comment 33 Alexander Larsson 2009-09-28 08:42:31 UTC
I don't understand how this can work with relative devices. 
I suppose those generally send zero for every axis except the ones the changed, and then only the delta since last event in those directions, right?

If so, how can we just take this value and add the x/y_offset (basically converting this delta to absolute root coords). Shouldn't we keep track of the
last position, and add the delta to that? And, why are we always treating relative devices like GDK_MODE_SCREEN? That doesn't seem right either, why can't?

Or, are we assuming that apps should know that a device is relative and expect events to work in a totally different way? Wouldn't it make more sense to have a relative mode then? Anyway, if we're doing this, should we really add x/y_offset to the relative axis data?
Comment 34 Tom Jaeger 2009-09-28 16:27:53 UTC
My code was based on my experience with X server 1.7.  I should have known that older X servers behave totally differently.  X server 1.6 does indeed send relative data in axis_data, so we have no choice but to make gtk's behavior dependent on the X server version.  If we get correct DeviceStateNotify events (I haven't tested this yet), we can indeed do what you suggest and just increment axis values by the delta.  But if we don't -- we're basically screwed and our best bet will be to use the outdated x_root and y_root values.

I figured GDK_MODE_WINDOW didn't make sense for relative devices, but maybe I'm wrong, this will be an easy fix once we've dealt with the more critical issues.

A realtive mode would be nice to have, but we'll have to wait for XI2 raw events (which also require grabs IIRC) for that since even the data that xserver-1.6 uses is subject to clipping.

I guess the only good news in all of this is that we're not breaking anything that used to work before.
Comment 35 Alexander Larsson 2009-09-28 17:28:20 UTC
I still don't understand
If you get a relative event with dx, dy we report an event at 
x = x_offset + dx;
y = y_offset + dy;

Where x/y_offset is the offset from the top left corner to the root window origin.

So, say the event window is 100x100 at position 100,100 in root window coords. Then we get an event with dx = 10, and dy = 0.

Given the above equations we report an event with coords x = -90 y = -100.

We then get another event with dx = 0 and dy = 10, so we report an event with coordinates x = -100 y = -90.

How is this useful, or am i missing something here?
Comment 36 Tom Jaeger 2009-09-29 06:41:33 UTC
You are correct on xserver-1.6, which sends events containing relative (clipped) valuators for relative devices.  xserver-1.7 always sends absolute coordinates, even for relative devices, so my code was working fine there.

Unfortunately, neither xserver version seems to send the appriopriate DeviceStateNotify events.

I have things working now under both xserver versions.  The trick is to realize that x_root and y_root always contain the state of the valuators (converted to screen coordinates) before the event took place.  If things don't match, use XDeviceQueryState to fetch updated valuator information for the device.
Comment 37 Tom Jaeger 2009-09-29 06:44:52 UTC
Created attachment 144235 [details] [review]
Use strstr instead of g_strrstr.

Signed-off-by: Thomas Jaeger <ThJaeger@gmail.com>
Comment 38 Tom Jaeger 2009-09-29 06:46:03 UTC
Sorry, please ignore the last attachment. git-bz didn't do what I expected.
Comment 39 Tom Jaeger 2009-09-29 06:48:27 UTC
Created attachment 144236 [details] [review]
Refactor _gdk_input_other_event

Signed-off-by: Thomas Jaeger <ThJaeger@gmail.com>
Comment 40 Tom Jaeger 2009-09-29 06:48:32 UTC
Created attachment 144237 [details] [review]
XInput allows up to 255 buttons

Signed-off-by: Thomas Jaeger <ThJaeger@gmail.com>
Comment 41 Tom Jaeger 2009-09-29 06:48:37 UTC
Created attachment 144238 [details] [review]
Report XInput button motion events until all buttons are released.

Signed-off-by: Thomas Jaeger <ThJaeger@gmail.com>
Comment 42 Tom Jaeger 2009-09-29 06:48:43 UTC
Created attachment 144239 [details] [review]
Always select all XInput motion events

Signed-off-by: Thomas Jaeger <ThJaeger@gmail.com>
Comment 43 Tom Jaeger 2009-09-29 06:48:49 UTC
Created attachment 144240 [details] [review]
Filter out events that the current window didn't select for

Signed-off-by: Thomas Jaeger <ThJaeger@gmail.com>
Comment 44 Tom Jaeger 2009-09-29 06:48:56 UTC
Created attachment 144241 [details] [review]
Keep track of axis values

Signed-off-by: Thomas Jaeger <ThJaeger@gmail.com>
Comment 45 Tom Jaeger 2009-09-29 06:49:01 UTC
Created attachment 144242 [details] [review]
Select for DeviceStateNotify

Signed-off-by: Thomas Jaeger <ThJaeger@gmail.com>
Comment 46 Tom Jaeger 2009-09-29 06:49:07 UTC
Created attachment 144243 [details] [review]
Interpret min_value == max_value correctly
Comment 47 Alexander Larsson 2009-09-29 08:45:23 UTC
+  if (hypot(x_root - x_screen, y_root - y_screen) > 1.0)

There is generally no need to actually do the square-root when doing a comparison like this.

Also, i'd like to have a lot more comments about why we do things this way, since its not very obvious with version checks, comparisons to root_x/y and whatnot.

rescale_valuator_axis has some weird non-gtk+-style indentation
Comment 48 Alexander Larsson 2009-09-29 08:47:48 UTC
Also, is gdk_input_get_axes right for relative devices?
man XQueryDeviceState says:

Valuators on the device report 0 if they are reporting relative information, and the current value if they are reporting  absolute information.

But you pass in absolute=TRUE
Comment 49 Carlos Garnacho 2009-09-29 11:47:07 UTC
am I right thinking in this as a "new feature"? AFAICS relative devices weren't ever meant to work properly before, and the patches are somewhat disruptive with the XInput2 work.

I've just filled bug #596725 in to track the XInput2 changes, perhaps this bug should be marked as dependent on that one?
Comment 50 Alexander Larsson 2009-09-29 12:47:28 UTC
Attachment 144236 [details] pushed as abf7742 - Refactor _gdk_input_other_event
Attachment 144237 [details] pushed as 2635fb7 - XInput allows up to 255 buttons
Attachment 144238 [details] pushed as acdecb6 - Report XInput button motion events until all buttons are released.
Attachment 144239 [details] pushed as 51f2a99 - Always select all XInput motion events
Attachment 144240 [details] pushed as 67728ac - Filter out events that the current window didn't select for
Comment 51 Alexander Larsson 2009-09-29 12:50:58 UTC
I commited the initial stuff, as that seemed obviously correct and the event filtering seems important.

I'm more hesitant with the relative events stuff. Its complex, version dependent and it never worked before, so I'm hesitant to land this when we're planning such a large rewrite with the xi2 stuff.
Comment 52 Tom Jaeger 2009-09-29 21:00:10 UTC
(In reply to comment #51)
> I commited the initial stuff, as that seemed obviously correct and the event
> filtering seems important.
> 
> I'm more hesitant with the relative events stuff. Its complex, version
> dependent and it never worked before, so I'm hesitant to land this when we're
> planning such a large rewrite with the xi2 stuff.

Let's drop the patch in comment #44 then, especially since relative devices will work fine on xserver-1.7 either way.
Comment 53 Tom Jaeger 2009-09-29 21:04:40 UTC
Created attachment 144297 [details] [review]
0001-Keep-track-of-axis-values.patch

This patch is still necessary to get correct behavior with the new evdev behavior.  There are absolute devices out there that are handled by evdev, and the current code will send wrong coordinates whenever there is horizontal or vertical motion.
Comment 54 Tom Jaeger 2009-09-29 21:07:28 UTC
Created attachment 144299 [details] [review]
0002-Select-for-DeviceStateNotify.patch

This patch doesn't have any practical implications, since I haven't actually managed to get the server send DeviceStateNotify events, but it's technically needed to get correct coordinates.
Comment 55 Tom Jaeger 2009-09-29 21:18:34 UTC
Created attachment 144303 [details] [review]
0003-Interpret-min_value-max_value-correctly.patch

I've rewritten this patch to make it less intrusive.  It only touches codepaths that were previously dividing by zero, so it's clearly safe and as an added benefit makes relative devices work on xserver-1.7.
Comment 56 Tom Jaeger 2009-09-29 21:29:39 UTC
(In reply to comment #49)
> am I right thinking in this as a "new feature"? AFAICS relative devices weren't
> ever meant to work properly before, and the patches are somewhat disruptive
> with the XInput2 work.
> 
> I've just filled bug #596725 in to track the XInput2 changes, perhaps this bug
> should be marked as dependent on that one?

It's more than just relative devices.  The biggest problem was that we now have devices (plenty of them) that don't send all their valuators in each event.  You'll face the same problem on the xi2 branch with absolute axes:  the valuator state needs to be cached on the client side since you can't just send zero if the device didn't report that particular valuator in the event.
Comment 57 Alexander Larsson 2009-09-30 07:28:01 UTC
I commited these. I guess we can mark this bug as fixed now? And open a new one about relative devices?
Comment 58 Denis Auroux 2009-10-03 22:55:42 UTC
Unfortunately, the new behavior for relative devices in 2.18.1 is IMHO a regression. Previously a GTK+ application that listens to a relative device would receive events with coordinates (inf,inf) and it would be easy to discard those events or request correct coordinates by another mechanism. Now the events arrive at coordinates close to the upper-left corner of the root window (converted to window coordinates), which is much harder to weed out as "obviously wrong".

I'll open it as a new bug, since this is no longer the same topic.

Denis
Comment 59 Alexander Larsson 2009-10-05 20:21:27 UTC
please do so, I'm closing this one since its getting unwieldly and the core issue is fixed.