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 679077 - The buttons mapping on the pad are lost as soon as left-handed is set/unset
The buttons mapping on the pad are lost as soon as left-handed is set/unset
Status: RESOLVED FIXED
Product: gnome-settings-daemon
Classification: Core
Component: wacom
3.5.x
Other Linux
: Normal major
: ---
Assigned To: gnome-settings-daemon-maint
gnome-settings-daemon-maint
Depends on:
Blocks:
 
 
Reported: 2012-06-28 16:46 UTC by Olivier Fourdan
Modified: 2012-07-02 17:02 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Proposed patch (1.06 KB, patch)
2012-06-28 16:48 UTC, Olivier Fourdan
accepted-commit_now Details | Review

Description Olivier Fourdan 2012-06-28 16:46:58 UTC
On a tablet with a pad device, changing the left-handed settings causes all buttons on the pad to stop working.
Comment 1 Olivier Fourdan 2012-06-28 16:48:17 UTC
Created attachment 217549 [details] [review]
Proposed patch

Proposed fix, rotation does (and should) not apply to the pad device.
Comment 2 Peter Hutterer 2012-06-29 03:05:00 UTC
Review of attachment 217549 [details] [review]:

ACK, but this patch just works around it - what actually unsets the buttons when the rotation is triggered?
Comment 3 Olivier Fourdan 2012-06-29 11:19:54 UTC
(In reply to comment #2)
> Review of attachment 217549 [details] [review]:
> 
> ACK, but this patch just works around it - what actually unsets the buttons
> when the rotation is triggered?

g-s-d doesn't seem to be the problem here, I disabled all but the Wacom plugin and the buttons are still reset when changing orientation of the tablet, so that rules out all other plugins (including the mouse plugin).
Comment 4 Olivier Fourdan 2012-06-29 11:31:29 UTC
Can we get some detail on why this patch is rejected now although it was previously approved in comment #2?
Comment 5 Bastien Nocera 2012-06-29 11:44:13 UTC
It wasn't approved, Peter reviewed it. I'd rather the driver was fixed instead.
Comment 6 Olivier Fourdan 2012-06-29 11:56:47 UTC
Note that patch is probably still needed also because Peter posted a patch on Linuxwacom-devel to remove the rotation property on the pad, so without this patch g-s-d would try to set a non-existing property.
Comment 7 Bastien Nocera 2012-06-29 12:04:19 UTC
Then a comment to that effect in the patch would be appreciated.
Comment 8 Bastien Nocera 2012-06-29 12:05:22 UTC
Comment on attachment 217549 [details] [review]
Proposed patch

Don't forget to change the commit message to match the effects with Peter's patch.
Comment 9 Olivier Fourdan 2012-06-29 12:39:10 UTC
Thanks, committed with the following message instead:

    wacom: do not rotate pad devices
    
    as the rotation property is to be removed on pad devices
    in the driver.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=679077
Comment 10 Peter Hutterer 2012-07-02 02:04:38 UTC
(In reply to comment #5)
> I'd rather the driver was fixed instead.

The driver sends the same button numbers regardless of rotation. These events are grabbed by g-s-d and converted to XTest key events, respective to their mapping, so if that mapping is lost, it's somewhere inside g-s-d.
Comment 11 Olivier Fourdan 2012-07-02 09:36:44 UTC
(In reply to comment #10)
> (In reply to comment #5)
> > I'd rather the driver was fixed instead.
> 
> The driver sends the same button numbers regardless of rotation. These events
> are grabbed by g-s-d and converted to XTest key events, respective to their
> mapping, so if that mapping is lost, it's somewhere inside g-s-d.

Err nope, I am really not seeing this in the code.

Button mapping is set in reset_pad_buttons() which calls XSetDeviceButtonMapping()

reset_pad_buttons() is called on pad devices from set_wacom_settings()

set_wacom_settings() is called from device_added_cb() which is the callback for newly added devices.

The call stack is as follow:

  device_added_cb()
   +-> set_wacom_settings()
        +-> reset_pad_buttons()
             +-> XSetDeviceButtonMapping()

Reverting this patch from attachment 217549 [details] [review] and running gnome-settings-daemon within gdb with a breakpoint on XSetDeviceButtonMapping() shows that XSetDeviceButtonMapping() is not called from g-s-d (from any plugin) while rotation is applied yet the button mapping is lost.
Comment 12 Olivier Fourdan 2012-07-02 09:57:17 UTC
Actually it seems g-s-d loses the passive grab it installed on the root win for those buttons.
Comment 13 Olivier Fourdan 2012-07-02 14:41:09 UTC
Debugging from the XServer shows the passive grab is lost because the device is closed:

Breakpoint 3, DeletePassiveGrab (value=0x1df8050, id=1105199549) at grabs.c:314
314	{
(gdb) bt
  • #0 DeletePassiveGrab
    at grabs.c line 314
  • #1 doFreeResource
    at resource.c line 549
  • #2 FreeResource
    at resource.c line 579
  • #3 DeleteDeviceEvents
    at closedev.c line 105
  • #4 ProcXCloseDevice
    at closedev.c line 158
  • #5 Dispatch
    at dispatch.c line 428
  • #6 main
    at main.c line 288
  • #4 ProcXCloseDevice
    at closedev.c line 158
  • #0 XCloseDevice
    at XCloseDev.c line 68
  • #1 wacom_set_property
    at gsd-wacom-manager.c line 172
  • #2 set_rotation
    at gsd-wacom-manager.c line 188
  • #3 wacom_settings_changed
    at gsd-wacom-manager.c line 764

Comment 14 Olivier Fourdan 2012-07-02 14:45:41 UTC
Argh, the CSS in bugzilla.gnome.org is hiding most of my previous comment 13 ...

What that comment was saying is that the passive grab is removed because the device is closed by gnome-settings-daemon and the device is closed from wacom_set_property()
Comment 15 Bastien Nocera 2012-07-02 17:02:20 UTC
(In reply to comment #14)
> Argh, the CSS in bugzilla.gnome.org is hiding most of my previous comment 13

Click on the link, and you'll get the full trace, eg.
https://bugzilla.gnome.org/page.cgi?id=trace.html&trace_id=230461

> What that comment was saying is that the passive grab is removed because the
> device is closed by gnome-settings-daemon and the device is closed from
> wacom_set_property()