Created attachment 136637 [details] [review] Proposed patch I'm working on a driver for my Chinese tablet, and while at it, I've found that proximity events aren't generated at all by the evdev Xorg driver. The reason is that the call to xf86PostProximityEvent(pInfo->dev, which, 0, 0) seems ignored by X core. It won't emit a proximity event without any valuators. That's how it looks: xinput test -proximity "Huion Tablet HA60-F400 Pen" motion a[0]=8683 a[1]=27178 motion a[0]=8672 a[1]=27158 motion a[1]=27137 motion a[0]=8658 a[1]=27113 motion a[1]=27091 So, I've patched evdev driver so that proper valuator values are provided. After that everything works as designed: xinput test -proximity "Huion HA60-F400 Tablet Pen" proximity in a[0]=9432 a[1]=16417 motion a[0]=9432 a[1]=16417 ... motion a[0]=8683 a[1]=16073 proximity out a[0]=8683 a[1]=16073 Without proximity events being generated there are various bad things happening with software. For example, GIMP won't allow you to use the mouse in the drawing area as soon as you enable the tablet (it thinks stylus is hovering over the tablet so you can't even scroll the picture with the wheel).
Comment on attachment 136637 [details] [review] Proposed patch Review of attachment 136637 [details] [review]: ----------------------------------------------------------------- Confirmed, there's a comment in the X server saying that while we internally generate a proximity event, it'll eventually fail XI1 conversion and is dropped. So this needs fixing. For the patch itself, seem my review comment. Please also post this as proper git-formatted patch so we can apply it with the right authorship, etc. Thanks. ::: xf86-input-evdev-2.10.5.orig/src/evdev.c @@ +909,5 @@ > + * on proximity in old_vals should be valid. > + */ > + xf86PostProximityEventM(pInfo->dev, which, > + valuator_mask_size(pEvdev->abs_vals) ? pEvdev->abs_vals : > + pEvdev->old_vals); looking at EvdevProcessProximityState(), the valuators to add to the proximity event should always be in pEvdev->prox, we don't have to guess whether it's abs_vals or old_vals
Created attachment 136670 [details] [review] Proposed patch, properly formatted
I've reformatted the patch with git. Also see my answer to your review.
sorry, I don't see any answer to the review, did that get submitted?
Here's a direct link https://bugs.freedesktop.org/page.cgi?id=splinter.html&bug=104562&attachment=136637
Comment on attachment 136637 [details] [review] Proposed patch Review of attachment 136637 [details] [review]: ----------------------------------------------------------------- ::: xf86-input-evdev-2.10.5.orig/src/evdev.c @@ +909,5 @@ > + * on proximity in old_vals should be valid. > + */ > + xf86PostProximityEventM(pInfo->dev, which, > + valuator_mask_size(pEvdev->abs_vals) ? pEvdev->abs_vals : > + pEvdev->old_vals); Unfortunately, that will not work. I started from looking at prox, but it is being cleared every time proximity state changes (and that's right the moment when we must generate a proximity event). See the code: /* We're about to go into/out of proximity but have no abs events * within the EV_SYN. Use the last coordinates we have. */ for (i = 0; i < valuator_mask_size(pEvdev->prox); i++) if (!valuator_mask_isset(pEvdev->abs_vals, i) && valuator_mask_isset(pEvdev->prox, i)) valuator_mask_set(pEvdev->abs_vals, i, valuator_mask_get(pEvdev->prox, i)); valuator_mask_zero(pEvdev->prox); ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ I've added some debug prints into EvdevPostProximityEvents, and here's what happens when I bring the stylus to/away from the tablet a few times: EvdevPostProximityEvents, which=1 -> 0 valuators in pEvdev->prox: -> 2 valuators in pEvdev->abs_vals: 0: 22629 1: 15528 -> 2 valuators in pEvdev->old_vals: 0: 22629 1: 15528 EvdevPostProximityEvents, which=0 -> 0 valuators in pEvdev->prox: -> 0 valuators in pEvdev->abs_vals: -> 2 valuators in pEvdev->old_vals: 0: 22611 1: 15462 EvdevPostProximityEvents, which=1 -> 0 valuators in pEvdev->prox: -> 2 valuators in pEvdev->abs_vals: 0: 22467 1: 7007 -> 2 valuators in pEvdev->old_vals: 0: 22467 1: 7007 EvdevPostProximityEvents, which=0 -> 0 valuators in pEvdev->prox: -> 0 valuators in pEvdev->abs_vals: -> 2 valuators in pEvdev->old_vals: 0: 21692 1: 6550 EvdevPostProximityEvents, which=1 -> 0 valuators in pEvdev->prox: -> 2 valuators in pEvdev->abs_vals: 0: 3524 1: 30395 -> 2 valuators in pEvdev->old_vals: 0: 3524 1: 30395 EvdevPostProximityEvents, which=0 -> 0 valuators in pEvdev->prox: -> 0 valuators in pEvdev->abs_vals: -> 2 valuators in pEvdev->old_vals: 0: 4254 1: 29264 So, in fact, we could always use old_vals, but I have left abs_vals as a extra guarantee.
Any comments?
sorry, ETIME and this fell under the radar. I tried your patch but unfortunately it doesn't work for Wacom tablets. For those, both abs_vals and old_vals is 0 but the mask is set, thus forcing the 0 values. This is probably caused by the Wacom tablets sending a 0/0 event like this on proximity out: Event: time 1527486679.056698, type 3 (EV_ABS), code 0 (ABS_X), value 0 Event: time 1527486679.056698, type 3 (EV_ABS), code 1 (ABS_Y), value 0 Event: time 1527486679.056698, type 3 (EV_ABS), code 25 (ABS_DISTANCE), value 0 Event: time 1527486679.056698, type 3 (EV_ABS), code 26 (ABS_TILT_X), value 0 Event: time 1527486679.056698, type 3 (EV_ABS), code 27 (ABS_TILT_Y), value 0 Event: time 1527486679.056698, type 1 (EV_KEY), code 320 (BTN_TOOL_PEN), value 0 Event: time 1527486679.056698, type 3 (EV_ABS), code 40 (ABS_MISC), value 0 Event: time 1527486679.056698, type 4 (EV_MSC), code 0 (MSC_SERIAL), value -1719661677 Event: time 1527486679.056698, -------------- SYN_REPORT ------------ I fixed this (I think) and sent the patches for some extra review to the list, see https://patchwork.freedesktop.org/patch/225733/ and your patch, slightly modified, here https://patchwork.freedesktop.org/patch/225734/ We can just use old_vals instead of abs_vals, that's the only change I made to your patch (and the commit msg notes this now) Please give those a test, thanks.
Pushed as 2fb95783e04eaf74fa8d051abd94341622a7e01e because I needed the next release :) And you said above that we could always use old_vals anyway. Thanks for the fix, sorry about the delay.
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.