[PATCH libinput v3] tablet: Use separate tool objects for tools without serials

Stephen Chandler Paul thatslyude at gmail.com
Thu Aug 7 16:00:52 PDT 2014


With tablets that don't support serial numbers, we can't guarantee that the tool
objects are unique. Because of this, this can give clients the false impression
that a tool without a serial number is being shared between tablets when it very
well might not be. So we keep tools without serial numbers in a list that's
local to the tablet they belong to, and keep tools with serials in a list that's
local to the libinput context.

Signed-off-by: Stephen Chandler Paul <thatslyude at gmail.com>
---

Changes (forgot to make these with the last patch, sorry!):
	- Add comment to explain why we use a separate tool list in the code
	- Remove leftover include

 src/evdev-tablet.c | 41 ++++++++++++++++++++-----
 src/evdev-tablet.h |  3 ++
 test/tablet.c      | 87 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 123 insertions(+), 8 deletions(-)

diff --git a/src/evdev-tablet.c b/src/evdev-tablet.c
index 31dd8d7..15f0663 100644
--- a/src/evdev-tablet.c
+++ b/src/evdev-tablet.c
@@ -259,17 +259,36 @@ tablet_process_misc(struct tablet_dispatch *tablet,
 }
 
 static struct libinput_tool *
-tablet_get_tool(struct libinput *li,
+tablet_get_tool(struct tablet_dispatch *tablet,
 		enum libinput_tool_type type,
 		uint32_t serial)
 {
 	struct libinput_tool *tool = NULL, *t;
+	struct list *tool_list;
 
-	/* Check if we already have the tool in our list of tools */
-	list_for_each(t, &li->tool_list, link) {
-		if (type == t->type && serial == t->serial) {
-			tool = t;
-			break;
+	if (serial) {
+		tool_list = &tablet->device->base.seat->libinput->tool_list;
+
+		/* Check if we already have the tool in our list of tools */
+		list_for_each(t, tool_list, link) {
+			if (type == t->type && serial == t->serial) {
+				tool = t;
+				break;
+			}
+		}
+	} else {
+		/* We can't guarantee that tools without serial numbers are
+		 * unique, so we keep them local to the tablet that they come
+		 * into proximity of instead of storing them in the global tool
+		 * list */
+		tool_list = &tablet->tool_list;
+
+		/* Same as above, but don't bother checking the serial number */
+		list_for_each(t, tool_list, link) {
+			if (type == t->type) {
+				tool = t;
+				break;
+			}
 		}
 	}
 
@@ -283,7 +302,7 @@ tablet_get_tool(struct libinput *li,
 			.refcount = 1,
 		};
 
-		list_insert(&li->tool_list, &tool->link);
+		list_insert(tool_list, &tool->link);
 	}
 
 	return tool;
@@ -382,7 +401,7 @@ tablet_flush(struct tablet_dispatch *tablet,
 	     uint32_t time)
 {
 	struct libinput_tool *tool =
-		tablet_get_tool(device->base.seat->libinput,
+		tablet_get_tool(tablet,
 				tablet->current_tool_type,
 				tablet->current_tool_serial);
 
@@ -473,6 +492,11 @@ tablet_destroy(struct evdev_dispatch *dispatch)
 {
 	struct tablet_dispatch *tablet =
 		(struct tablet_dispatch*)dispatch;
+	struct libinput_tool *tool, *tmp;
+
+	list_for_each_safe(tool, tmp, &tablet->tool_list, link) {
+		libinput_tool_unref(tool);
+	}
 
 	free(tablet);
 }
@@ -490,6 +514,7 @@ tablet_init(struct tablet_dispatch *tablet,
 	tablet->device = device;
 	tablet->status = TABLET_NONE;
 	tablet->current_tool_type = LIBINPUT_TOOL_NONE;
+	list_init(&tablet->tool_list);
 
 	tablet_mark_all_axes_changed(tablet, device);
 
diff --git a/src/evdev-tablet.h b/src/evdev-tablet.h
index 1b53d20..adc3aa4 100644
--- a/src/evdev-tablet.h
+++ b/src/evdev-tablet.h
@@ -49,6 +49,9 @@ struct tablet_dispatch {
 	unsigned char changed_axes[NCHARS(LIBINPUT_TABLET_AXIS_CNT)];
 	double axes[LIBINPUT_TABLET_AXIS_CNT];
 
+	/* Only used for tablets that don't report serial numbers */
+	struct list tool_list;
+
 	struct button_state button_state;
 	struct button_state prev_button_state;
 
diff --git a/test/tablet.c b/test/tablet.c
index 0cd6357..2de0989 100644
--- a/test/tablet.c
+++ b/test/tablet.c
@@ -681,6 +681,91 @@ START_TEST(pad_buttons_ignored)
 }
 END_TEST
 
+START_TEST(tools_with_serials)
+{
+	struct libinput *li = litest_create_context();
+	struct litest_device *dev[2];
+	struct libinput_tool *tool[2];
+	struct libinput_event *event;
+	int i;
+
+	for (i = 0; i < 2; i++) {
+		dev[i] = litest_add_device_with_overrides(li,
+							  LITEST_WACOM_INTUOS,
+							  NULL,
+							  NULL,
+							  NULL,
+							  NULL);
+
+		litest_event(dev[i], EV_KEY, BTN_TOOL_PEN, 1);
+		litest_event(dev[i], EV_MSC, MSC_SERIAL, 100);
+		litest_event(dev[i], EV_SYN, SYN_REPORT, 0);
+
+		libinput_dispatch(li);
+		while ((event = libinput_get_event(li))) {
+			if (libinput_event_get_type(event) ==
+			    LIBINPUT_EVENT_TABLET_PROXIMITY_IN) {
+				struct libinput_event_tablet *t =
+					libinput_event_get_tablet_event(event);
+
+				tool[i] = libinput_event_tablet_get_tool(t);
+			}
+
+			libinput_event_destroy(event);
+		}
+	}
+
+	/* We should get the same object for both devices */
+	ck_assert_ptr_eq(tool[0], tool[1]);
+
+	litest_delete_device(dev[0]);
+	litest_delete_device(dev[1]);
+	libinput_unref(li);
+}
+END_TEST
+
+START_TEST(tools_without_serials)
+{
+	struct libinput *li = litest_create_context();
+	struct litest_device *dev[2];
+	struct libinput_tool *tool[2];
+	struct libinput_event *event;
+	int i;
+
+	for (i = 0; i < 2; i++) {
+		dev[i] = litest_add_device_with_overrides(li,
+							  LITEST_WACOM_ISDV4,
+							  NULL,
+							  NULL,
+							  NULL,
+							  NULL);
+
+		litest_event(dev[i], EV_KEY, BTN_TOOL_PEN, 1);
+		litest_event(dev[i], EV_SYN, SYN_REPORT, 0);
+
+		libinput_dispatch(li);
+		while ((event = libinput_get_event(li))) {
+			if (libinput_event_get_type(event) ==
+			    LIBINPUT_EVENT_TABLET_PROXIMITY_IN) {
+				struct libinput_event_tablet *t =
+					libinput_event_get_tablet_event(event);
+
+				tool[i] = libinput_event_tablet_get_tool(t);
+			}
+
+			libinput_event_destroy(event);
+		}
+	}
+
+	/* We should get different tool objects for each device */
+	ck_assert_ptr_ne(tool[0], tool[1]);
+
+	litest_delete_device(dev[0]);
+	litest_delete_device(dev[1]);
+	libinput_unref(li);
+}
+END_TEST
+
 int
 main(int argc, char **argv)
 {
@@ -688,6 +773,8 @@ main(int argc, char **argv)
 	litest_add("tablet:tool_serial", tool_serial, LITEST_TABLET | LITEST_TOOL_SERIAL, LITEST_ANY);
 	litest_add("tablet:tool_serial", serial_changes_tool, LITEST_TABLET | LITEST_TOOL_SERIAL, LITEST_ANY);
 	litest_add("tablet:tool_serial", invalid_serials, LITEST_TABLET | LITEST_TOOL_SERIAL, LITEST_ANY);
+	litest_add_no_device("tablet:tool_serial", tools_with_serials);
+	litest_add_no_device("tablet:tool_serial", tools_without_serials);
 	litest_add("tablet:proximity", proximity_out_clear_buttons, LITEST_TABLET, LITEST_ANY);
 	litest_add("tablet:proximity", proximity_in_out, LITEST_TABLET, LITEST_ANY);
 	litest_add("tablet:proximity", bad_distance_events, LITEST_TABLET | LITEST_DISTANCE, LITEST_ANY);
-- 
1.8.5.5



More information about the wayland-devel mailing list