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 709600 - Screen mapping incorrect when Cintiq connected to tablet PC
Screen mapping incorrect when Cintiq connected to tablet PC
Status: RESOLVED FIXED
Product: gnome-settings-daemon
Classification: Core
Component: wacom
unspecified
Other Linux
: Normal normal
: ---
Assigned To: Joaquim Rocha
gnome-settings-daemon-maint
: 624880 652247 702952 720722 723930 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2013-10-07 22:19 UTC by Jason Gerecke
Modified: 2014-02-14 01:39 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Fix reported bug (1.56 KB, patch)
2013-10-07 22:47 UTC, Jason Gerecke
none Details | Review
wacom: Map ISD digitizers only to integrated displays (1.41 KB, patch)
2013-10-07 23:38 UTC, Bastien Nocera
none Details | Review
wacom: Map ISD digitizers only to integrated displays (1.35 KB, patch)
2013-10-08 00:12 UTC, Jason Gerecke
none Details | Review
wacom: Map ISD digitizers only to integrated displays (1.32 KB, patch)
2013-10-08 05:43 UTC, Bastien Nocera
reviewed Details | Review
wacom: Map ISD digitizers only to integrated displays (1.58 KB, patch)
2013-11-29 18:24 UTC, Carlos Garnacho
none Details | Review
wacom: Attempt to match EDID product with screen-tablet wacom devices (1.63 KB, patch)
2013-11-29 18:24 UTC, Carlos Garnacho
none Details | Review
common: Add GsdDeviceMapper (38.00 KB, patch)
2014-02-05 19:07 UTC, Carlos Garnacho
none Details | Review
xrandr: Use GsdDeviceMap to rotate touscreens (10.76 KB, patch)
2014-02-05 19:07 UTC, Carlos Garnacho
accepted-commit_now Details | Review
wacom: Use GsdDeviceMapper (9.55 KB, patch)
2014-02-05 19:07 UTC, Carlos Garnacho
none Details | Review
common: Add GsdDeviceMapper (44.83 KB, patch)
2014-02-07 22:34 UTC, Carlos Garnacho
none Details | Review
wacom: Use GsdDeviceMapper (11.40 KB, patch)
2014-02-07 22:35 UTC, Carlos Garnacho
none Details | Review
wacom: Use GsdDeviceMapper (11.37 KB, patch)
2014-02-10 14:51 UTC, Carlos Garnacho
accepted-commit_now Details | Review
common: Add GsdDeviceMapper (38.30 KB, patch)
2014-02-10 15:22 UTC, Carlos Garnacho
needs-work Details | Review
common: Add GsdDeviceMapper (39.91 KB, patch)
2014-02-11 23:13 UTC, Carlos Garnacho
needs-work Details | Review
xrandr: Use GsdDeviceMapper to rotate touscreens (11.17 KB, patch)
2014-02-11 23:13 UTC, Carlos Garnacho
committed Details | Review
wacom: Use GsdDeviceMapper (11.32 KB, patch)
2014-02-11 23:13 UTC, Carlos Garnacho
committed Details | Review
common: Add GsdDeviceMapper (40.19 KB, patch)
2014-02-12 14:04 UTC, Carlos Garnacho
committed Details | Review

Description Jason Gerecke 2013-10-07 22:19:28 UTC
g-s-d tries to determine the correct screen mapping by binding tablets to any display with a "WAC" EDID. This makes sense for Cinitq devices, but not for tablet PCs. If a Cintiq is connected to a tablet PC, the heuristic causes the ISD digitizer to map to the Cintiq instead of remaining on the integrated screen.
Comment 1 Jason Gerecke 2013-10-07 22:47:53 UTC
Created attachment 256674 [details] [review]
Fix reported bug

The attached patch fixes this bug by changing the heuristic so that ISD digitizers only map to integrated displays and external digitizers search for a "WAC" EDID. (note: not yet compile-tested; my GNOME dev environment is months out of date...)
Comment 2 Bastien Nocera 2013-10-07 23:38:33 UTC
Created attachment 256675 [details] [review]
wacom: Map ISD digitizers only to integrated displays

ISD digitisers are built into the system that they're being used with
and as such should only ever be mapped to an integrated display. The
old logic preferentially maps to a display with the "WAC" EDID, which
will be an issue if a Cintiq is connected to a tablet PC.
Comment 3 Bastien Nocera 2013-10-07 23:39:41 UTC
This one keeps the fallback, if the cintiq screen is off (but the digitizer still on).
Comment 4 Jason Gerecke 2013-10-08 00:12:58 UTC
Created attachment 256676 [details] [review]
wacom: Map ISD digitizers only to integrated displays

ISD digitisers are built into the system that they're being used with
and as such should only ever be mapped to an integrated display. The
old logic preferentially maps to a display with the "WAC" EDID, which
will be an issue if a Cintiq is connected to a tablet PC.
Comment 5 Jason Gerecke 2013-10-08 00:14:41 UTC
Removes the unnecessary 2nd call to 'find_output_by_edid' in attachment 256675 [details] [review].
Comment 6 Bastien Nocera 2013-10-08 05:43:46 UTC
Created attachment 256681 [details] [review]
wacom: Map ISD digitizers only to integrated displays

ISD digitisers are built into the system that they're being used with
and as such should only ever be mapped to an integrated display. The
old logic preferentially maps to a display with the "WAC" EDID, which
will be an issue if a Cintiq is connected to a tablet PC.
Comment 7 Bastien Nocera 2013-10-08 05:47:52 UTC
With the fallback, really. I shouldn't update patches at 2AM.
Comment 8 Bastien Nocera 2013-10-21 20:32:50 UTC
Any news on testing this?
Comment 9 Jason Gerecke 2013-10-22 02:05:57 UTC
Afraid not. Its been near the bottom of my to-do list, even moreso with jhbuild generally disliking me. I'll see if I can find some time to wrangle with the build system and get you a test result.
Comment 10 Carlos Garnacho 2013-11-25 17:59:04 UTC
Comment on attachment 256681 [details] [review]
wacom: Map ISD digitizers only to integrated displays

I've tricked g-s-d underneath to trigger this with my x220t and my not-really-a-cintiq second monitor, the patch indeed works for the case.

Although I would suggest taking this out of find_output_by_heuristic() and putting it into find_output() itself, as it's not really an heuristic anymore, plus it makes the "mapping tablet to heuristically-found display" warning go from frequent to gone in this laptop :)
Comment 11 Carlos Garnacho 2013-11-29 18:24:53 UTC
Created attachment 263150 [details] [review]
wacom: Map ISD digitizers only to integrated displays

ISD digitisers are built into the system that they're being used with
and as such should only ever be mapped to an integrated display. The
old logic preferentially maps to a display with the "WAC" EDID, which
will be an issue if a Cintiq is connected to a tablet PC.
Comment 12 Carlos Garnacho 2013-11-29 18:24:57 UTC
Created attachment 263151 [details] [review]
wacom: Attempt to match EDID product with screen-tablet wacom devices

Before sorting out to the heuristic methods; if a wacom device is a
screen tablet, look for a display with a product name equal to the
device name's. This does work with a Cintiq 12WX, so it should be
safe to assume this happens too on devices with sane EDIDs.
Comment 13 Jason Gerecke 2013-12-05 22:12:37 UTC
I like the idea behind the second patch, but as its written I don't think its doing anything useful. The EDID product names appear to be very generic and wouldn't match in find_output_by_edid using the logic you have in place. My 12WX at least has the EDID product name "Cintiq", rather than the needed "Cintiq 12WX".

EDID product names for other Wacom displays I have handy seem to be similarly generic:
Cintiq 24HDT: "Cintiq"
Cintiq 21UX: "WACOM"
Cintiq 12WX "Cintiq"
DTU-2231: N/A
Comment 14 Carlos Garnacho 2013-12-06 18:42:07 UTC
Hmm, thanks for that list, I find it very funky that the 12WX I have here has a more specific EDID product name... Seeing the situation, I think there's several levels of confidence if we continue going this way:

- not-so-heuristic checks like the one in your patch
- an exact match on product name is next (still prone to failure if 2 identical tablets are there, but...)
- a product name substring match comes after
- just matching vendor name comes last

I would propose doing the following:

- per-device, fetch all possible matching outputs, sorted by confidence
- traverse through devices from most to least confidence gotten.
- pick the output(s) with most confidence
  * if there's one, take it as good
  * if there's more than one with the same degree, pick one and warn
  * always set a hint on outputs so we know which ones already have an in-display tablet assigned in order to ignore these on later matches
- if there was no match, check for any second output, as any in-display tablet left here is unlikely to be on the primary monitor. Also warn.
- if there's no second monitor, then pick primary output and warn

With something like this, it could still break if there's 2 too similar outputs (24HDT and 12WX, or 21UX and DTU-2231), for that I guess we could also keep a gsettings key for edid like in other devices, so there's a way to mean it...

I'll start working on this
Comment 15 Carlos Garnacho 2014-02-05 19:07:20 UTC
Created attachment 268200 [details] [review]
common: Add GsdDeviceMapper

This is a per-screen object that's to be used across plugins,
it on one hand keeps track of all outputs and their position/rotation,
and on the other hand it is offered by plugins GdkDevices to handle,
so those devices are mapped to the corresponding device.

The primary source to know the corresponding output is the
configuration, if a device has a configured display in GSettings
that's currently present, that will be the one the device maps to.

When a device has no settings, or an unconfigured display, multiple
things are checked in order to find out a good guess.
- System-integrated devices go to the builtin output
- Several EDID checks are done for Wacom devices with a display
  built-in.
- Already attached devices are checked on outputs, so a single
  output doesn't hoard multiple pen/eraser/touch devices.

If an output is found on some or other way, the device will be
mapped to that output, and follow it if the display configuration
changes.
Comment 16 Carlos Garnacho 2014-02-05 19:07:25 UTC
Created attachment 268201 [details] [review]
xrandr: Use GsdDeviceMap to rotate touscreens

The Xrandr module was previously in charge of rotating touchscreens,
although it did so in a limited number of cases (eg. only through fn-F7,
not through the g-c-c panel), and didn't do anything to map touchscreens
to the right output if multiple monitors are present, so touchscreens
would be left mapped to the whole span of monitors.

Fix this by using GsdDeviceMapper, so touchscreen mapping is managed by
that object. The Xrandr module still gets to manage what/whether
touchscreens get added to the mapper.
Comment 17 Carlos Garnacho 2014-02-05 19:07:29 UTC
Created attachment 268202 [details] [review]
wacom: Use GsdDeviceMapper

Offload device mapping to this object, that has more ellaborate
methods to find out a compelling candidate to map an input device to.
Comment 18 Carlos Garnacho 2014-02-07 22:34:56 UTC
Created attachment 268456 [details] [review]
common: Add GsdDeviceMapper

This is a per-screen object that's to be used across plugins,
it on one hand keeps track of all outputs and their position/rotation,
and on the other hand it is offered by plugins GdkDevices to handle,
so those devices are mapped to the corresponding device.

The primary source to know the corresponding output is the
configuration, if a device has a configured display in GSettings
that's currently present, that will be the one the device maps to.

When a device has no settings, or an unconfigured display, multiple
things are checked in order to find out a good guess.
- System-integrated devices go to the builtin output
- Several EDID checks are done for Wacom devices with a display
  built-in.
- Already attached devices are checked on outputs, so a single
  output doesn't hoard multiple pen/eraser/touch devices.

If an output is found on some or other way, the device will be
mapped to that output, and follow it if the display configuration
changes.
Comment 19 Carlos Garnacho 2014-02-07 22:35:01 UTC
Created attachment 268457 [details] [review]
wacom: Use GsdDeviceMapper

Offload device mapping to this object, that has more ellaborate
methods to find out a compelling candidate to map an input device to.
Comment 20 Carlos Garnacho 2014-02-10 14:51:57 UTC
Created attachment 268679 [details] [review]
wacom: Use GsdDeviceMapper

Offload device mapping to this object, that has more ellaborate
methods to find out a compelling candidate to map an input device to.
Comment 21 Carlos Garnacho 2014-02-10 15:14:56 UTC
I think I should explain the changes a bit more in detail, GsdDeviceMapper is a per-screen object, it will know directly about all outputs, but will need explicit calls to gsd_device_mapper_add_input() to handle an input device.

When a device is handled by the device mapper, the output is decided as described in comment #14, which from extensive testing is proving robust here. It is worth noting that all output mapping and left/right handed mode in tablets are unified through the "Coordinate Transformation Matrix" property in XI(2) devices, other properties like "Wacom Rotation" or "Evdev Axis Inversion" are skipped (this might bring issues in testing, if those properties keep the older values from a previous g-s-d instance).

The changes in the wacom and xrandr modules ensure pretty much every pen/eraser/pad/touch-on-tablet/touchscreen goes through GsdDeviceMapper, so this also fixes other mapping bugs, especially on touchscreens.
Comment 22 Carlos Garnacho 2014-02-10 15:22:56 UTC
Created attachment 268685 [details] [review]
common: Add GsdDeviceMapper

This is a per-screen object that's to be used across plugins,
it on one hand keeps track of all outputs and their position/rotation,
and on the other hand it is offered by plugins GdkDevices to handle,
so those devices are mapped to the corresponding device.

The primary source to know the corresponding output is the
configuration, if a device has a configured display in GSettings
that's currently present, that will be the one the device maps to.

When a device has no settings, or an unconfigured display, multiple
things are checked in order to find out a good guess.
- System-integrated devices go to the builtin output
- Several EDID checks are done for Wacom devices with a display
  built-in.
- Already attached devices are checked on outputs, so a single
  output doesn't hoard multiple pen/eraser/touch devices.

If an output is found on some or other way, the device will be
mapped to that output, and follow it if the display configuration
changes.
Comment 23 Bastien Nocera 2014-02-10 15:57:34 UTC
Review of attachment 268685 [details] [review]:

I can't really parse the first paragraph of the commit message, could you reword it?
Also, when you use "device" I'd rather your said "GdkDevice" or "input device" depending on whether you're talking about the physical device, or the XInput/GdkDevice instances (eg. a Wacom tablet is a single device, but multiple GdkDevices).

"If an output is found on some or other way"
That should be "If an output is found one way or another", but you could probably reword that to "If an output is found for that GdkDevice".

The code is missing some comments in general.

::: plugins/common/gsd-device-mapper.c
@@ +33,3 @@
+
+enum {
+	INPUT_IS_SYSTEM_INTEGRATED = 1 << 0,

Can you add some comments/examples for each type of device listed here?

@@ +39,3 @@
+	INPUT_IS_ERASER		   = 1 << 4,
+	INPUT_IS_PAD		   = 1 << 5
+};

Name the flags here please.

@@ +101,3 @@
+
+enum {
+	PROP_SCREEN = 1

I prefer having a PROP_0 as the first item.

@@ +111,3 @@
+static guint signals[N_SIGNALS] = { 0 };
+
+#define NUM_ELEMS_MATRIX 9

Define that before and use it in the declaration of the rotation_matrices[].

@@ +255,3 @@
+
+		if (monitor_num == gdk_screen_get_monitor_at_point (mapper->screen,
+								    x, y))

No need for a linefeed here, we have wide screens.

@@ +267,3 @@
+	MappingHelper *helper;
+
+	helper = g_new0 (MappingHelper, 1);

Does it make sense to use slices for those?

@@ +325,3 @@
+	gchar **split;
+
+	name = gdk_device_get_name (input->device);

A few comments about what you're doing in this function wouldn't go amiss.

@@ +402,3 @@
+	gsize nvalues;
+
+	old = g_settings_get_value (settings, KEY_DISPLAY);

Why not g_settings_get_strv()?

::: plugins/common/gsd-device-mapper.h
@@ +39,3 @@
+
+GType		  gsd_device_mapper_get_type	      (void) G_GNUC_CONST;
+GsdDeviceMapper * gsd_device_mapper_get		      (GdkScreen       *screen);

We only support a single GdkScreen (with multiple monitors though), so a call that gets the GsdDeviceMapper for the default screen is probably best.

::: plugins/common/gsd-input-helper.c
@@ +578,3 @@
+
+const char *
+xdevice_get_tool_type (int deviceid)

get_wacom_tool_type() rather.

::: plugins/common/gsd-input-helper.h
@@ +83,3 @@
 
+const char * xdevice_get_tool_type (int                     deviceid);
+

Why the extra linefeeds?
Comment 24 Bastien Nocera 2014-02-10 16:00:01 UTC
Review of attachment 268201 [details] [review]:

"GsdDeviceMapper" in the commit subject.

Rest looks good!
Comment 25 Bastien Nocera 2014-02-10 16:07:03 UTC
Review of attachment 268679 [details] [review]:

"elaborate".

I'm not sure candidate screens get to be more "compelling" but rather they have "better heuristics".

Looks good otherwise.
Comment 26 Carlos Garnacho 2014-02-11 23:13:16 UTC
Created attachment 268862 [details] [review]
common: Add GsdDeviceMapper

This is an object attached to the default screen, meant to be used across
several g-s-d plugins. On one hand, it keeps track of monitors and their
layout/rotation/mode. On the other hand, g-s-d plugins tell GsdDeviceMapper
which GdkDevices will be handled by this object, so a monitor is chosen
for it applying what we know about the input device.

The primary source to know the corresponding monitor is the configuration,
if an input device has a configured display in GSettings, and the monitor
is currently present, that will be the one the input device will map to.

When an input device has no settings, or an unconfigured monitor, multiple
things are checked in order to find out a good guess for the GdkDevice.
- System-integrated input devices go to the built-in output
- Several EDID checks are done for Wacom devices with a built-in screen
- Sanity checks are done on outputs with screen-integrated input devices,
  if a monitor already has a GdkDevice with similar capabilities (eg. a
  system-integrated pen), this output will be punted, so there is no
  accumulation of input devices.

If an output is found on some or other way, the GdkDevice will be
mapped to that output, and follow it if the display configuration
changes, or device left/right handedness changes.
Comment 27 Carlos Garnacho 2014-02-11 23:13:23 UTC
Created attachment 268863 [details] [review]
xrandr: Use GsdDeviceMapper to rotate touscreens

The Xrandr module was previously in charge of rotating touchscreens,
although it did so in a limited number of cases (eg. only through fn-F7,
not through the g-c-c panel), and didn't do anything to map touchscreens
to the right output if multiple monitors are present, so touchscreens
would be left mapped to the whole span of monitors.

Fix this by using GsdDeviceMapper, so touchscreen mapping is managed by
that object. The Xrandr module still gets to manage what/whether
touchscreens get added to the mapper.
Comment 28 Carlos Garnacho 2014-02-11 23:13:27 UTC
Created attachment 268864 [details] [review]
wacom: Use GsdDeviceMapper

Offload device mapping to this object, that has more elaborate
heuristics to find out the (most suitable) monitor to map an
input device to.
Comment 29 Bastien Nocera 2014-02-12 11:40:27 UTC
Review of attachment 268863 [details] [review]:

Looks good.
Comment 30 Bastien Nocera 2014-02-12 11:49:10 UTC
Review of attachment 268864 [details] [review]:

Every call to:
g_object_get (device, "gdk-device", &gdk_device, NULL);
is leaking the device. It's defined as an object, so needs to be unref'ed.
Comment 31 Bastien Nocera 2014-02-12 12:03:36 UTC
Review of attachment 268862 [details] [review]:

> so a monitor is chosen for it applying what we know about the input device.

I don't understand that.

Rest looks good.

::: plugins/common/gsd-device-mapper.c
@@ +48,3 @@
+	INPUT_IS_ERASER		   = 1 << 4, /* eraser device, either touchscreen or tablet */
+	INPUT_IS_PAD		   = 1 << 5  /* pad device, most usually in tablets */
+};

Those enums/flags are still missing names.

@@ +56,3 @@
+	PRIO_EDID_MATCH_VENDOR,	 /* EDID vendor match, eg. "WAC" for Wacom */
+	N_OUTPUT_PRIORITIES
+};

Ditto.

@@ +274,3 @@
+	MappingHelper *helper;
+
+	helper = g_new0 (MappingHelper, 1);

g_slice?

@@ +466,3 @@
+	}
+
+	ptr = (guessed) ? &input->guessed_output : &input->output;

No need for brackets here, and you're already doing that a couple of lines before.

@@ +700,3 @@
+			if ((info->input->capabilities &
+			     (INPUT_IS_SYSTEM_INTEGRATED | INPUT_IS_SCREEN_INTEGRATED)) &&
+			    output_has_input_type (output, info->input->capabilities))

Could you split this into multiple conditions?

if (!(info->input->capabilities & (INPUT_IS...))
  continue;

::: plugins/common/gsd-input-helper.c
@@ +578,3 @@
+
+const char *
+get_wacom_tool_type (int deviceid)

I meant adding "wacom" in the middle of the function name, not truncating it :)
Sorry that wasn't clear.
Comment 32 Carlos Garnacho 2014-02-12 12:10:08 UTC
Thanks for the comments Bastien, I actually forgot to reply on some bits of the first round too (although most was applied)

(In reply to comment #23)
> @@ +267,3 @@
> +    MappingHelper *helper;
> +
> +    helper = g_new0 (MappingHelper, 1);
> 
> Does it make sense to use slices for those?

I didn't do this in the end... I don't think there's much benefit as this struct isn't going to be very often, nor very long in memory, and there would be once instance top at any given time. If this happened more often maybe could be worth to have this stack allocated OTOH.

(In reply to comment #30)
> Review of attachment 268864 [details] [review]:
> 
> Every call to:
> g_object_get (device, "gdk-device", &gdk_device, NULL);
> is leaking the device. It's defined as an object, so needs to be unref'ed.

AFAICS the GsdWacomDevice property has a pointer type:

        g_object_class_install_property (object_class, PROP_GDK_DEVICE,
                                         g_param_spec_pointer ("gdk-device", "gdk-device", "gdk-device",
                                                               G_PARAM_READWRITE | G_PARAM_CONSTRUCT_ONLY));

Could be worth changing that to being a GParamSpecObject property, although not getting an extra ref was convenient here :).
Comment 33 Carlos Garnacho 2014-02-12 14:04:54 UTC
Created attachment 268914 [details] [review]
common: Add GsdDeviceMapper

This is an object attached to the default screen, meant to be used across
several g-s-d plugins. On one hand, it keeps track of monitors and their
layout/rotation/mode. On the other hand, g-s-d plugins tell GsdDeviceMapper
which GdkDevices will be handled by this object, so a monitor is chosen
to map the GdkDevice to, applying what we know about the input device.

The primary source to know the corresponding monitor is the configuration,
if an input device has a configured display in GSettings, and the monitor
is currently present, that will be the one the input device will map to.

When an input device has no settings, or an unconfigured monitor, multiple
things are checked in order to find out a good guess for the GdkDevice.
- System-integrated input devices go to the built-in output
- Several EDID checks are done for Wacom devices with a built-in screen
- Sanity checks are done on outputs with screen-integrated input devices,
  if a monitor already has a GdkDevice with similar capabilities (eg. a
  system-integrated pen), this output will be punted, so there is no
  accumulation of input devices.

If an output is found on some or other way, the GdkDevice will be
mapped to that output, and follow it if the display configuration
changes, or device left/right handedness changes.
Comment 34 Carlos Garnacho 2014-02-12 14:40:45 UTC
(In reply to comment #31)
> Review of attachment 268862 [details] [review]:
> 
> > so a monitor is chosen for it applying what we know about the input device.
> 
> I don't understand that.
> 
> Rest looks good.
> 
> ::: plugins/common/gsd-device-mapper.c
> @@ +48,3 @@
> +    INPUT_IS_ERASER           = 1 << 4, /* eraser device, either touchscreen
> or tablet */
> +    INPUT_IS_PAD           = 1 << 5  /* pad device, most usually in tablets */
> +};
> 
> Those enums/flags are still missing names.

I added a typedef for those, and they are now "GSD_" prefixed too, event though it's all still internal to gsd-device-mapper.c

> @@ +466,3 @@
> +    }
> +
> +    ptr = (guessed) ? &input->guessed_output : &input->output;
> 
> No need for brackets here, and you're already doing that a couple of lines
> before.

Good catch, that line is just removed now.

> 
> @@ +700,3 @@
> +            if ((info->input->capabilities &
> +                 (INPUT_IS_SYSTEM_INTEGRATED | INPUT_IS_SCREEN_INTEGRATED)) &&
> +                output_has_input_type (output, info->input->capabilities))
> 
> Could you split this into multiple conditions?

Note this is if (a && b), I've nested the ifs and moved the comment within, so it's hopefully clearer.

> 
> 
> I meant adding "wacom" in the middle of the function name, not truncating it :)
> Sorry that wasn't clear.

oh :), changed that
Comment 35 Bastien Nocera 2014-02-13 13:04:14 UTC
Review of attachment 268914 [details] [review]:

Looks good!
Comment 36 Carlos Garnacho 2014-02-13 16:35:11 UTC
Attachment 268863 [details] pushed as 8e488c9 - xrandr: Use GsdDeviceMapper to rotate touscreens
Attachment 268864 [details] pushed as 0810de0 - wacom: Use GsdDeviceMapper
Attachment 268914 [details] pushed as bf2f0d5 - common: Add GsdDeviceMapper
Comment 37 Hashem Nasarat 2014-02-13 16:48:06 UTC
Carlos, you mentioned g-c-c in your commit. Do these commits fix this related bug?
https://bugzilla.gnome.org/show_bug.cgi?id=723930
Comment 38 Giovanni Campagna 2014-02-13 22:06:12 UTC
*** Bug 723930 has been marked as a duplicate of this bug. ***
Comment 39 Carlos Garnacho 2014-02-14 00:57:05 UTC
(In reply to comment #37)
> Carlos, you mentioned g-c-c in your commit. Do these commits fix this related
> bug?
> https://bugzilla.gnome.org/show_bug.cgi?id=723930

Indeed it was, thanks for noticing that one
Comment 40 Carlos Garnacho 2014-02-14 01:05:52 UTC
*** Bug 720722 has been marked as a duplicate of this bug. ***
Comment 41 Carlos Garnacho 2014-02-14 01:29:02 UTC
*** Bug 702952 has been marked as a duplicate of this bug. ***
Comment 42 Carlos Garnacho 2014-02-14 01:35:24 UTC
*** Bug 624880 has been marked as a duplicate of this bug. ***
Comment 43 Carlos Garnacho 2014-02-14 01:39:40 UTC
*** Bug 652247 has been marked as a duplicate of this bug. ***