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 686022 - Strip/rings without a mode-switch are ignored by g-s-d/g-c-c
Strip/rings without a mode-switch are ignored by g-s-d/g-c-c
Status: RESOLVED FIXED
Product: gnome-settings-daemon
Classification: Core
Component: wacom
3.6.x
Other Linux
: Normal normal
: ---
Assigned To: Olivier Fourdan
gnome-settings-daemon-maint
Depends on:
Blocks:
 
 
Reported: 2012-10-12 12:25 UTC by Olivier Fourdan
Modified: 2012-10-19 13:57 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Proposed patch (4.07 KB, patch)
2012-10-16 16:38 UTC, Olivier Fourdan
accepted-commit_now Details | Review
Proposed patch for master (8.45 KB, patch)
2012-10-17 15:40 UTC, Olivier Fourdan
reviewed Details | Review

Description Olivier Fourdan 2012-10-12 12:25:21 UTC
The code in gsd-wacom-device.[ch] assumes that each touch{ring,strip} comes with a mode-switch button, and the touch{ring,strip} is added once dealing with the corresponding mode-switch button.

    http://git.gnome.org/browse/gnome-settings-daemon/tree/plugins/wacom/gsd-wacom-device.c#n1123

Some device such as the Cintiq 12WX do have 2 touch strips but no corresponding mode-switch button:

    http://linuxwacom.git.sourceforge.net/git/gitweb.cgi?p=linuxwacom/libwacom;a=blob;f=data/cintiq-12wx.tablet

For those devices, g-c-c/g-s-d don't allow to configure the touch{ring,strip} actions.
Comment 1 Bastien Nocera 2012-10-12 12:35:04 UTC
It's clearly a hardware problem.
Comment 2 Olivier Fourdan 2012-10-16 16:38:36 UTC
Created attachment 226570 [details] [review]
Proposed patch

(In reply to comment #1)
> It's clearly a hardware problem.

No, it's not, it's clearly a software issue. The code does not handle the older hardware which have a touchstrip/touchring but no corresponding modeswitch.

The code also assumes that if there is a touchstrip/touchring, there are multiple modes, which is not granted either, even without a modeswitch there is at least 1 mode (the one and only one).

The attached patch tries to address that issue. With this, the 2 touchstrips on the Cintiq 12WX are now configurable in GNOME.
Comment 3 Bastien Nocera 2012-10-16 16:45:08 UTC
Review of attachment 226570 [details] [review]:

Good enough for gnome-3-6, but you'll need a different one for master that doesn't say "strip mode #1" when there's only one mode.
Comment 4 Olivier Fourdan 2012-10-17 15:40:12 UTC
Created attachment 226650 [details] [review]
Proposed patch for master

(In reply to comment #3)
> Good enough for gnome-3-6, but you'll need a different one for master that
> doesn't say "strip mode #1" when there's only one mode.

Yeap, indeed that would break translations.

This other patch is for master, it does the things slightly differently, the (translatable) name is now "Left Ring", "Right Ring", "Left Touchstrip" or "Right Touchstrip" yet the ID remains "left-ring-mode-1", "right-ring-mode-1", "left-strip-mode-1" or "right-strip-mode-1" even if there is no multiple modes.

The reason behind this is backward compatibility with 3.6, so that people won't lose their strip buttons settings when upgrading to 3.8 when available.
Comment 5 Bastien Nocera 2012-10-17 17:01:44 UTC
Review of attachment 226650 [details] [review]:

Looks good apart from that one comment.

::: plugins/wacom/gsd-wacom-device.c
@@ +1209,3 @@
+	if (libwacom_has_ring2 (wacom_device) || libwacom_has_ring (wacom_device))
+		l = g_list_concat (l, gsd_wacom_device_add_ring_modes (wacom_device, settings_path, direction));
+	else if  (libwacom_get_num_strips (wacom_device) > 0)

Are we certain that a tablet cannot have both a ring and a strip?
Comment 6 Olivier Fourdan 2012-10-19 13:04:03 UTC
(In reply to comment #5)
> Review of attachment 226650 [details] [review]:
> 
> Looks good apart from that one comment.
> 
> ::: plugins/wacom/gsd-wacom-device.c
> @@ +1209,3 @@
> +    if (libwacom_has_ring2 (wacom_device) || libwacom_has_ring (wacom_device))
> +        l = g_list_concat (l, gsd_wacom_device_add_ring_modes (wacom_device,
> settings_path, direction));
> +    else if  (libwacom_get_num_strips (wacom_device) > 0)
> 
> Are we certain that a tablet cannot have both a ring and a strip?

Nope, you're right, there is no need for the "else", at least I don't see how removing the "else" would cause trouble.
Comment 7 Olivier Fourdan 2012-10-19 13:52:12 UTC
Pushed in both branches without the "else" statement.
Comment 8 Olivier Fourdan 2012-10-19 13:57:22 UTC
g-c-c updated as well, closing.