Bug 82250

install evemu and run evemu-record against the device. Then try to reproduce the issue, ideally with the shortest recording possible. http://www.freedesktop.org/wiki/Evemu has more info.

attach the recording here and I can try to reproduce

Created attachment 104355 [details]
evemu-record for mypaint

Created attachment 104356 [details]
screenshot of mypaint

I have been playing with evemu-record on the mypaint program.

Here is what I am drawing: going from top to bottom, I draw 4 horizontal lines from left to right. Between two lines, I raise the pen. Between the third and fourth line, I put the pen on the menu bar.

Here is what appears on the screen (see screenshot)
  when I first touch the screen, a straight is drawn from some random point (bottom right) to where I touch the screen. I can then draw my first line. When I start the second line, a straight line is drawn from the end of the first line to the beginning of the second. Idem between second and third line. After the third line, when I touch the menu bar, a line is drawn between the end of the third line and the point I touch on the menubar. Another such line is drawn when I start the fourth line.

In replaying the record, I don't see the first line. I tried many times, I don't seem to be able to record the first events after launching evemu-record. I had already noticed that after launching X the first pen events where going through the touchscreen device, and then all the pen or touch events were nicely separated, going through their respective interfaces (very strange, I know. The pen and the touchscreen are two distnct interfaces of the same usb device). It seems that simply running evemu-record does something to the pen/touch device causing again some pen events going through the touch interface. As a consequence, I cannot record the first pen events after running evemu-record, and the first recorded event is the last point of the first horizontal line. When replaying, I see a straight line coming from some random point to that last point of the first line, and then everything goes as before: straight lines between the end of a line and the beginning of the next, straight lines, going to the menu batr, etc. This is not reproducible. When I tried to do that yesterday, I had record that showed no artefact at replay time whereas there were some artefacts when recording. I don't know why it is different today.

Just to make sure, please also attach your Xorg.log. are you sure you're using evdev, not wacom?

evemu sits below/on the side of X. It records kernel events, the same that evdev would see but to the kernel it is just another process. So short of a kernel bug evemu cannot change the event sequence that X sees. Event recordings aren't always 100% correct, but the few corner cases shouldn't trigger on your device, they're multi-touch related.

Created attachment 104612 [details]
Xorg's log

> Just to make sure, please also attach your Xorg.log. are you sure you're using > evdev, not wacom?

No, it's really evdev.

> evemu sits below/on the side of X. It records kernel events, the same that 
> evdev would see but to the kernel it is just another process. So short of a 
> kernel bug evemu cannot change the event sequence that X sees. 
> aren't always 100% correct, but the few corner cases shouldn't trigger on your > device, they're multi-touch related.

I understand that, and I wonder what is happening. Maybe the emulation device is recognized differently (tablet vs touchscreen vs mouse, or I don't know what ?)
Maybe the application is confused by the fact that the same usb device has two distinct interface, maybe there is some state in the application  associated to the pen which is not associated to the emulation device, I don't know.

Anyway, thanks for your time.

I digged through the sources of gdk. It turns out that when parsing a X event, gdk uses the values of the valuators to compute (x,y) position rather than using the (x,y) position returned by X; see function _gdk_input_common_other_event and the way it calls gdk_input_translate_coordinates. When one (or both) valuators are missing, the previous data (stored in gdkdev->axis_data) is reused.

With my pen, when touching the screen, a ButtonPress event is generated with the correct position (X core kept track) but with no valuator. gdk thus returns the coordinates of the last time it knew about the pen position. This is why, when clicking on the canvas of a gtk drawing program after selecting a tool, the canvas receives the ButtonPress event but with coordinates of the tool icon (outside the canvas) !

So, as with Qt (see bug 82181), gtk expects X to send all the valuators with each X event, and with my pen, X does not do that. I am not sure where the blame lies. Should toolkits avoid using the valuators or, at least, should they validate them? Should Xcore always send all available valuators to applications?
Should evdev send to XCore all available valuators ? 

For info, I filed a bug with gtk:
https://bugzilla.gnome.org/show_bug.cgi?id=735003

evdev has, to my knowledge, never sent valuators with button events. It's a bit of a grey area as to whether we should or not, but we do send motion events before every button event (that's behaviour we can never change). So simply said, if the button event doesn't have valuators and GTK thinks they're 0 that's a GTK bug.

I strongly suspect the reason you get the extra valuators with the finger events is that the button event here is emulated from a touch event. But that too may have values missing, so I think it's only chance you haven't seen that with the finger yet.

OTOH, including x/y data with button presses should be fairly trivial, so if you can come up with a patch that does that and doesn't break any of my tests, then I'd be willing to merge it. It still doesn't cover the whole issue though, there is the odd case where pressure may change but x/y doesn't, in which case we only forward pressure. That _could_ be an issue.

I filed a bug with gtk, and another with Qt. I haven't received any comment yet.

I have tried to make a patch; when applied, all my problems with okular, xournal and gimp disappear. I still have some issues with mypaint, but I am not alone there: if I understand well, mypaint more or less expects the pressure to be at 0 when the pen leaves the tablet/screen, and does not work well when this assumption isn't met (as with my device). So let us just say that mypaint expect a more professional device than my simple pen.

There are two things about my patch where I am really not sure:

1) in EvdevProcessSyncEvent, all the "Post" function are called with a table of valuators (num_v, first_v v). In the current version, this table is empty. My patch populates v with values from either vals or old_vals so that, now, all the Post functions have the full set of valuators. For my pen to work, it would be sufficient to send a populated v array to EvdevPostQueuedEvents and one could continue to send an empty array to the other functions. I decided to send the full v array to everybody; is it the right choice ?

2) I don't understand the logic in EvdevPostQueuedEvent, line 944
            if (pEvdev->abs_queued && pEvdev->in_proximity) 
                        xf86PostButtonEventP(...,Absolute,...)
            else        xf86PostButtonEvent(...,Relative,...)
what does it mean for the X server for a button event to be absolute or relative ? And why does it depend on whether the pen is in proximity or not ? I changed the test  into
            if (pEvdev->flags & EVDEV_ABSOLUTE_EVENTS) 
but I might have missed something important. Would it be very wrong to drop the test entirely and always send "Absolute" Button events ?

I am a complete beginner in programming X; don't hesitate to criticize my patch...

Created attachment 105054 [details] [review]
Patch to always send all the valuators on motion or button press

(In reply to comment #10)
> if I understand well, mypaint more or less expects the
> pressure to be at 0 when the pen leaves the tablet/screen, and does not work
> well when this assumption isn't met (as with my device). So let us just say
> that mypaint expect a more professional device than my simple pen.

hmm, I guess this could be classified as either a kernel bug or an evdev bug. Please file a separate bug for "reset pressure to 0 when tablet device goes out of touch". Definitely needs to be fixed.


> There are two things about my patch where I am really not sure:
> 
> 1) in EvdevProcessSyncEvent, all the "Post" function are called with a table
> of valuators (num_v, first_v v). In the current version, this table is
> empty. My patch populates v with values from either vals or old_vals so
> that, now, all the Post functions have the full set of valuators. For my pen
> to work, it would be sufficient to send a populated v array to
> EvdevPostQueuedEvents and one could continue to send an empty array to the
> other functions. I decided to send the full v array to everybody; is it the
> right choice ?

This is a leftover from switching from valuator arrays with first/num to valuator masks. I think it'd be best to switch to valuator masks instead, that's were we currently keep all the information and it's a more verbose but nicer interface. The valuator array can probably be dropped, I think I started with that patch once but then got pulled off to something else


> 2) I don't understand the logic in EvdevPostQueuedEvent, line 944
>             if (pEvdev->abs_queued && pEvdev->in_proximity) 
>                         xf86PostButtonEventP(...,Absolute,...)
>             else        xf86PostButtonEvent(...,Relative,...)
> what does it mean for the X server for a button event to be absolute or
> relative ? And why does it depend on whether the pen is in proximity or not
> ? I changed the test  into
>             if (pEvdev->flags & EVDEV_ABSOLUTE_EVENTS) 
> but I might have missed something important. Would it be very wrong to drop
> the test entirely and always send "Absolute" Button events ?

yeah. simple example would be a mouse: if you get REL_X/REL_Y and BTN_LEFT in the same SYN_REPORT frame you can't use the deltas as absolute coordinates. And, more importantly, for relative coordinates you'll need to send the motion event first and then a button event without deltas, otherwise you'll move twice for the same hardware event.

As for the weird condition: it's an admittedly odd check to avoid sending the wrong events. If there is abs_queued events but the tool isn't in proximity we don't post axes and the position may be wrong. So in that case sending the button event as relative event won't move the pointer, we can't just use the current x/y values because they may be wrong. Could do with a comment, I guess :)

Comment on attachment 105054 [details] [review]
Patch to always send all the valuators on motion or button press

Review of attachment 105054 [details] [review]:
-----------------------------------------------------------------

Two general comments: we don't use // comments, only /* */. And make sure the whitespace is the same as in the surrounding code, same with { on newline or not. It's a bit of a mix that came over the years, try to stick to whatever the surrounding code has.

As said in the other comment, if you have abs events but the tool isn't in proximity, you can't rely on the axis information.

::: src_orig/evdev.c
@@ +486,5 @@
>          } else {
>              int val;
> +	    for(i = 0; i < pEvdev->num_vals; i++)
> +		if (valuator_mask_fetch(pEvdev->vals, i, &val))
> +                    valuator_mask_set(pEvdev->old_vals, i, val);

valuator_mask_copy() does that job. Also, I think it may be enough to just make sure x/y is in there, but you'll need to verify that with the apps that behave badly.

@@ +956,1 @@
>                  xf86PostButtonEventP(pInfo->dev, Absolute, pEvdev->queue[i].detail.key,

Probably best to use xf86PostButtonEventM here and just pass on the mask

Thanks for your comments; here is my second take on this patch.

1) I remove the passing of the empty valuator arrays (first_v,num_v,v) to
   all the EvdevPostSomething functions
2) EvdevPostQueuedEvents now tries to send down pEvdev->vals when appropriate
   for a button event. (I hope the test is now Ok)
3) Point 2) implies that pEvdev->vals must remain populated with the correct 
   values. Previous version of the patch juggled between vals and old_vals. This
   new version is simpler: I don't reset vals when in Absolute mode. (Again, I
   hope the test is Ok)

Some remarks: 
* I hope I didn't break relative devices or absolute devices emulating relative 
  devices; I cannot test these.
* Is there any reason why EvdevPostRelativeMotionEvents and
  EvdevPostAbsoluteMotionEvents are extern functions while 
  EvdevPostProximityEvents and EvdevPostQueuedEvents are static ?
* I think the "else" clause of "if (pEvdev->flags & EVDEV_RELATIVE_MODE)" in
  EvdevProcessValuators is useless: old_vals is only used if EVDEV_RELATIVE_MODE,
  there is no point of updating if we are not in that case.

I keep in mind that the pressure level not falling to 0 is another bug to be taken care of, but that will be later...

Created attachment 105101 [details] [review]
Second try on the previous patch

Comment on attachment 105101 [details] [review]
Second try on the previous patch

Review of attachment 105101 [details] [review]:
-----------------------------------------------------------------

::: src_orig/evdev.c
@@ -866,5 @@
>   * Post the relative motion events.
>   */
>  void
> -EvdevPostRelativeMotionEvents(InputInfoPtr pInfo, int num_v, int first_v,
> -                              int v[MAX_VALUATORS])

ACK to the patch dropping the unused parameters, but please split this out as two separate patches: one to drop the unused parameter, one to add the new functionality. This makes it a lot easier to review and test for regressions.
also, signed-off git formatted patches please, see http://www.x.org/wiki/Development/Documentation/SubmittingPatches/

@@ +947,5 @@
> +            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);

looks good and should work. I'll give it some testing once you re-submit as git-formatted patch (see above), please make sure the comment is indented as well though.

(In reply to comment #14)
> * Is there any reason why EvdevPostRelativeMotionEvents and
>   EvdevPostAbsoluteMotionEvents are extern functions while 
>   EvdevPostProximityEvents and EvdevPostQueuedEvents are static ?

check the git log, maybe they have been used from other files in the past so it may just be a leftover. Feel free to change them to static (separate patch though please).


> * I think the "else" clause of "if (pEvdev->flags & EVDEV_RELATIVE_MODE)" in
>   EvdevProcessValuators is useless: old_vals is only used if
> EVDEV_RELATIVE_MODE,
>   there is no point of updating if we are not in that case.

If I understand you correctly: a device can be switched from abs to rel mode at any time. I think there's a case where you switch a device from rel to abs and back to rel, and you get a cursor jump because old_vals wasn't updated while in abs mode. That could probably be avoided by zeroing old_vals when we switch the mode though.

late update: that patch was pushed as commit 605047613c534babf723f25597e8cc4be6758db0 and there's a few patches by you later in the repo, so I'm just going to guess that this is fixed now :)

Hi,
I have now the same problem. I checked the code there were changes around the originally patched parts. 
I'm not a newbie, but never played around X before, where could I find the git for this code?
I just downloaded the package sources from ubuntu.

Any help would be appreciated, as this is a really annoying bug...

Thanks a lot in advance !
Br, 
 Peter.

I think recent ubuntu uses libinput, are you sure you're on evdev? If not, please move this to a separate bug.

6 months later, let's close this again