[PATCH xf86-input-libinput] Always delay hotplugging subdevices

Peter Hutterer peter.hutterer at who-t.net
Thu Aug 18 05:33:28 UTC 2016


Avoid creating new devices from within the input thread which was the case for
tablet tools. It requires a lot more care about locking and has a potential to
mess up things.

Instead, schedule a WorkProc and buffer all events until we have the device
created. Once that's done, replay the event sequence so far. If the device
comes into proximity and out again before we manage to create the new device
we just ditch the whole sequence and wait for the next proximity in.

Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
---
 src/xf86libinput.c | 259 +++++++++++++++++++++++++++++++++++++++++------------
 1 file changed, 202 insertions(+), 57 deletions(-)

diff --git a/src/xf86libinput.c b/src/xf86libinput.c
index 1ecbc41..9b9ba12 100644
--- a/src/xf86libinput.c
+++ b/src/xf86libinput.c
@@ -102,6 +102,12 @@ struct xf86libinput_device {
 	struct xorg_list unclaimed_tablet_tool_list;
 };
 
+struct xf86libinput_tablet_tool_event_queue {
+	bool need_to_queue;
+	struct libinput_event_tablet_tool *events[128];
+	size_t nevents;
+};
+
 struct xf86libinput_tablet_tool {
 	struct xorg_list node;
 	struct libinput_tablet_tool *tool;
@@ -162,20 +168,22 @@ struct xf86libinput {
 	bool allow_mode_group_updates;
 };
 
-enum hotplug_when {
-	HOTPLUG_LATER,
-	HOTPLUG_NOW,
+enum event_cleanup {
+	EVENT_KEEP,
+	EVENT_DESTROY,
 };
 
-static DeviceIntPtr
+static void
 xf86libinput_create_subdevice(InputInfoPtr pInfo,
 			      uint32_t capabilities,
-			      enum hotplug_when,
 			      XF86OptionPtr extra_opts);
 static inline void
 update_mode_prop(InputInfoPtr pInfo,
 		 struct libinput_event_tablet_pad *event);
 
+static enum event_cleanup
+xf86libinput_handle_event(struct libinput_event *event);
+
 static inline int
 use_server_fd(const InputInfoPtr pInfo) {
 	return pInfo->fd > -1 && (pInfo->flags & XI86_SERVER_FD);
@@ -1386,28 +1394,110 @@ xf86libinput_pick_device(struct xf86libinput_device *shared_device,
 	return NULL;
 }
 
-static void
+static inline void
+xf86libinput_tool_replay_events(struct xf86libinput_tablet_tool_event_queue *queue)
+{
+	size_t i;
+
+	for (i = 0; i < queue->nevents; i++) {
+		struct libinput_event *event;
+		event = libinput_event_tablet_tool_get_base_event(queue->events[i]);
+		xf86libinput_handle_event(event);
+		queue->events[i] = NULL;
+		/* we're not queueing anymore so we expect handle_event to
+		   libinput_event_destroy() */
+	}
+	queue->nevents = 0;
+}
+
+/* Return true if we queued, false otherwise */
+static inline bool
+xf86libinput_tool_queue_event(struct libinput_event_tablet_tool *event)
+{
+	struct libinput_event *e;
+	struct libinput_tablet_tool *tool;
+	struct xf86libinput_tablet_tool_event_queue *queue;
+
+	tool = libinput_event_tablet_tool_get_tool(event);
+	if (!tool)
+		return true;
+
+	queue = libinput_tablet_tool_get_user_data(tool);
+	if (!queue)
+		return false;
+
+	if (!queue->need_to_queue) {
+		if (queue->nevents > 0) {
+			libinput_tablet_tool_set_user_data(tool, NULL);
+			xf86libinput_tool_replay_events(queue);
+			free(queue);
+		}
+
+		return false;
+	}
+
+	/* We got the prox out while still queuing, just ditch the whole
+	 * series of events and the event queue with it. */
+	if (libinput_event_tablet_tool_get_proximity_state(event) ==
+	    LIBINPUT_TABLET_TOOL_PROXIMITY_STATE_OUT) {
+		int i;
+
+		for (i = 0; i < queue->nevents; i++) {
+			e = libinput_event_tablet_tool_get_base_event(queue->events[i]);
+			libinput_event_destroy(e);
+		}
+		libinput_tablet_tool_set_user_data(tool, NULL);
+		free(queue);
+
+		/* we destroy the event here but return true
+		 * to make sure the event looks like it got queued and the
+		 * caller doesn't destroy it for us
+		 */
+		e = libinput_event_tablet_tool_get_base_event(event);
+		libinput_event_destroy(e);
+		return true;
+	}
+
+	if (queue->nevents == ARRAY_SIZE(queue->events)) {
+		e = libinput_event_tablet_tool_get_base_event(event);
+		libinput_event_destroy(e);
+		return true;
+	}
+
+	queue->events[queue->nevents++] = event;
+	return true;
+}
+
+static enum event_cleanup
 xf86libinput_handle_tablet_tip(InputInfoPtr pInfo,
 			       struct libinput_event_tablet_tool *event)
 {
 	enum libinput_tablet_tool_tip_state state;
 	const BOOL is_absolute = TRUE;
 
+	if (xf86libinput_tool_queue_event(event))
+		return EVENT_KEEP;
+
 	state = libinput_event_tablet_tool_get_tip_state(event);
 
 	xf86PostButtonEventP(pInfo->dev,
 			     is_absolute, 1,
 			     state == LIBINPUT_TABLET_TOOL_TIP_DOWN ? 1 : 0,
 			     0, 0, NULL);
+
+	return EVENT_DESTROY;
 }
 
-static void
+static enum event_cleanup
 xf86libinput_handle_tablet_button(InputInfoPtr pInfo,
 				  struct libinput_event_tablet_tool *event)
 {
 	enum libinput_button_state state;
 	uint32_t button, b;
 
+	if (xf86libinput_tool_queue_event(event))
+		return EVENT_KEEP;
+
 	button = libinput_event_tablet_tool_get_button(event);
 	state = libinput_event_tablet_tool_get_button_state(event);
 
@@ -1418,9 +1508,11 @@ xf86libinput_handle_tablet_button(InputInfoPtr pInfo,
 			     b,
 			     state == LIBINPUT_BUTTON_STATE_PRESSED ? 1 : 0,
 			     0, 0, NULL);
+
+	return EVENT_DESTROY;
 }
 
-static void
+static enum event_cleanup
 xf86libinput_handle_tablet_axis(InputInfoPtr pInfo,
 				struct libinput_event_tablet_tool *event)
 {
@@ -1430,6 +1522,9 @@ xf86libinput_handle_tablet_axis(InputInfoPtr pInfo,
 	struct libinput_tablet_tool *tool;
 	double value;
 
+	if (xf86libinput_tool_queue_event(event))
+		return EVENT_KEEP;
+
 	value = libinput_event_tablet_tool_get_x_transformed(event,
 							TABLET_AXIS_MAX);
 	valuator_mask_set_double(mask, 0, value);
@@ -1477,13 +1572,15 @@ xf86libinput_handle_tablet_axis(InputInfoPtr pInfo,
 		default:
 			xf86IDrvMsg(pInfo, X_ERROR,
 				    "Invalid rotation axis on tool\n");
-			return;
+			return EVENT_DESTROY;
 		}
 
 		valuator_mask_set_double(mask, valuator, value);
 	}
 
 	xf86PostMotionEventM(dev, Absolute, mask);
+
+	return EVENT_DESTROY;
 }
 
 static inline const char *
@@ -1507,47 +1604,35 @@ tool_type_to_str(enum libinput_tablet_tool_type type)
 	return str;
 }
 
-static void
-xf86libinput_handle_tablet_proximity(InputInfoPtr pInfo,
-				     struct libinput_event_tablet_tool *event)
+static inline void
+xf86libinput_create_tool_subdevice(InputInfoPtr pInfo,
+				   struct libinput_event_tablet_tool *event)
 {
 	struct xf86libinput *driver_data = pInfo->private;
 	struct xf86libinput_device *shared_device = driver_data->shared_device;
-	struct xf86libinput *dev = pInfo->private;
-	struct libinput_tablet_tool *tool;
 	struct xf86libinput_tablet_tool *t;
+	struct xf86libinput_tablet_tool_event_queue *queue;
+	struct libinput_tablet_tool *tool;
 	uint64_t serial, tool_id;
 	XF86OptionPtr options = NULL;
-	DeviceIntPtr pDev = pInfo->dev;
 	char name[64];
-	ValuatorMask *mask = driver_data->valuators;
-	double x, y;
-	BOOL in_prox;
-
-	x = libinput_event_tablet_tool_get_x_transformed(event, TABLET_AXIS_MAX);
-	y = libinput_event_tablet_tool_get_y_transformed(event, TABLET_AXIS_MAX);
-	valuator_mask_set_double(mask, 0, x);
-	valuator_mask_set_double(mask, 1, y);
-
-	tool = libinput_event_tablet_tool_get_tool(event);
-	serial = libinput_tablet_tool_get_serial(tool);
-	tool_id = libinput_tablet_tool_get_tool_id(tool);
-
-	xorg_list_for_each_entry(dev,
-				 &shared_device->device_list,
-				 shared_device_link) {
-		if (dev->tablet_tool &&
-		    libinput_tablet_tool_get_serial(dev->tablet_tool) == serial &&
-		    libinput_tablet_tool_get_tool_id(dev->tablet_tool) == tool_id) {
-			pDev = dev->pInfo->dev;
-			goto out;
-		}
-	}
 
 	t = calloc(1, sizeof *t);
 	if (!t)
 		return;
 
+	queue = calloc(1, sizeof(*queue));
+	if (!queue) {
+		free(t);
+		return;
+	}
+	queue->need_to_queue = true;
+	queue->nevents = 0;
+
+	tool = libinput_event_tablet_tool_get_tool(event);
+	serial = libinput_tablet_tool_get_serial(tool);
+	tool_id = libinput_tablet_tool_get_tool_id(tool);
+
 	t->tool = libinput_tablet_tool_ref(tool);
 	xorg_list_append(&t->node, &shared_device->unclaimed_tablet_tool_list);
 
@@ -1562,12 +1647,70 @@ xf86libinput_handle_tablet_proximity(InputInfoPtr pInfo,
 		     (uint32_t)serial) > strlen(pInfo->name))
 		options = xf86ReplaceStrOption(options, "Name", name);
 
-	pDev = xf86libinput_create_subdevice(pInfo, CAP_TABLET_TOOL, HOTPLUG_NOW, options);
+	libinput_tablet_tool_set_user_data(tool, queue);
+	xf86libinput_tool_queue_event(event);
+
+	xf86libinput_create_subdevice(pInfo, CAP_TABLET_TOOL, options);
+}
+
+static inline DeviceIntPtr
+xf86libinput_find_device_for_tool(InputInfoPtr pInfo,
+				  struct libinput_tablet_tool *tool)
+{
+	struct xf86libinput *dev = pInfo->private;
+	struct xf86libinput *driver_data = pInfo->private;
+	struct xf86libinput_device *shared_device = driver_data->shared_device;
+	uint64_t serial = libinput_tablet_tool_get_serial(tool);
+	uint64_t tool_id = libinput_tablet_tool_get_tool_id(tool);
+
+	xorg_list_for_each_entry(dev,
+				 &shared_device->device_list,
+				 shared_device_link) {
+		if (dev->tablet_tool &&
+		    libinput_tablet_tool_get_serial(dev->tablet_tool) == serial &&
+		    libinput_tablet_tool_get_tool_id(dev->tablet_tool) == tool_id) {
+			return dev->pInfo->dev;
+		}
+	}
+
+	return NULL;
+}
+
+static enum event_cleanup
+xf86libinput_handle_tablet_proximity(InputInfoPtr pInfo,
+				     struct libinput_event_tablet_tool *event)
+{
+	struct xf86libinput *driver_data = pInfo->private;
+	struct libinput_tablet_tool *tool;
+	DeviceIntPtr pDev;
+	ValuatorMask *mask = driver_data->valuators;
+	double x, y;
+	BOOL in_prox;
+
+	tool = libinput_event_tablet_tool_get_tool(event);
+	pDev = xf86libinput_find_device_for_tool(pInfo, tool);
 
-out:
 	in_prox = libinput_event_tablet_tool_get_proximity_state(event) ==
 				LIBINPUT_TABLET_TOOL_PROXIMITY_STATE_IN;
+
+	if (pDev == NULL && in_prox) {
+		xf86libinput_create_tool_subdevice(pInfo, event);
+		return EVENT_KEEP;
+	}
+
+	if (xf86libinput_tool_queue_event(event))
+		return EVENT_KEEP;
+
+	BUG_RETURN_VAL(pDev == NULL, EVENT_DESTROY);
+
+	x = libinput_event_tablet_tool_get_x_transformed(event, TABLET_AXIS_MAX);
+	y = libinput_event_tablet_tool_get_y_transformed(event, TABLET_AXIS_MAX);
+	valuator_mask_set_double(mask, 0, x);
+	valuator_mask_set_double(mask, 1, y);
+
 	xf86PostProximityEventM(pDev, in_prox, mask);
+
+	return EVENT_DESTROY;
 }
 
 static void
@@ -1646,12 +1789,13 @@ xf86libinput_handle_tablet_pad_ring(InputInfoPtr pInfo,
 	xf86PostMotionEventM(dev, Absolute, mask);
 }
 
-static void
+static enum event_cleanup
 xf86libinput_handle_event(struct libinput_event *event)
 {
 	struct libinput_device *device;
 	enum libinput_event_type type;
 	InputInfoPtr pInfo;
+	int event_handling = EVENT_DESTROY;
 
 	type = libinput_event_get_type(event);
 	device = libinput_event_get_device(event);
@@ -1659,7 +1803,7 @@ xf86libinput_handle_event(struct libinput_event *event)
 					 event);
 
 	if (!pInfo || !pInfo->dev->public.on)
-		return;
+		goto out;
 
 	switch (type) {
 		case LIBINPUT_EVENT_NONE:
@@ -1705,19 +1849,19 @@ xf86libinput_handle_event(struct libinput_event *event)
 		case LIBINPUT_EVENT_GESTURE_PINCH_END:
 			break;
 		case LIBINPUT_EVENT_TABLET_TOOL_AXIS:
-			xf86libinput_handle_tablet_axis(pInfo,
+			event_handling = xf86libinput_handle_tablet_axis(pInfo,
 							libinput_event_get_tablet_tool_event(event));
 			break;
 		case LIBINPUT_EVENT_TABLET_TOOL_BUTTON:
-			xf86libinput_handle_tablet_button(pInfo,
+			event_handling = xf86libinput_handle_tablet_button(pInfo,
 							  libinput_event_get_tablet_tool_event(event));
 			break;
 		case LIBINPUT_EVENT_TABLET_TOOL_PROXIMITY:
-			xf86libinput_handle_tablet_proximity(pInfo,
+			event_handling = xf86libinput_handle_tablet_proximity(pInfo,
 							     libinput_event_get_tablet_tool_event(event));
 			break;
 		case LIBINPUT_EVENT_TABLET_TOOL_TIP:
-			xf86libinput_handle_tablet_tip(pInfo,
+			event_handling = xf86libinput_handle_tablet_tip(pInfo,
 						       libinput_event_get_tablet_tool_event(event));
 			break;
 		case LIBINPUT_EVENT_TABLET_PAD_BUTTON:
@@ -1733,6 +1877,9 @@ xf86libinput_handle_event(struct libinput_event *event)
 							     libinput_event_get_tablet_pad_event(event));
 			break;
 	}
+
+out:
+	return event_handling;
 }
 
 static void
@@ -1754,8 +1901,8 @@ xf86libinput_read_input(InputInfoPtr pInfo)
 	}
 
 	while ((event = libinput_get_event(libinput))) {
-		xf86libinput_handle_event(event);
-		libinput_event_destroy(event);
+		if (xf86libinput_handle_event(event) == EVENT_DESTROY)
+			libinput_event_destroy(event);
 	}
 }
 
@@ -2439,10 +2586,9 @@ xf86libinput_hotplug_device_cb(ClientPtr client, pointer closure)
 	return TRUE;
 }
 
-static DeviceIntPtr
+static void
 xf86libinput_create_subdevice(InputInfoPtr pInfo,
 			      uint32_t capabilities,
-			      enum hotplug_when when,
 			      XF86OptionPtr extra_options)
 {
 	struct xf86libinput *driver_data = pInfo->private;
@@ -2483,18 +2629,14 @@ xf86libinput_create_subdevice(InputInfoPtr pInfo,
 
 	hotplug = calloc(1, sizeof(*hotplug));
 	if (!hotplug)
-		return NULL;
+		return;
 
 	hotplug->input_options = iopts;
 	hotplug->attrs = DuplicateInputAttributes(pInfo->attrs);
 
 	xf86IDrvMsg(pInfo, X_INFO, "needs a virtual subdevice\n");
-	if (when == HOTPLUG_LATER)
-		QueueWorkProc(xf86libinput_hotplug_device_cb, serverClient, hotplug);
-	else
-		return xf86libinput_hotplug_device(hotplug);
 
-	return NULL;
+	QueueWorkProc(xf86libinput_hotplug_device_cb, serverClient, hotplug);
 }
 
 static inline uint32_t
@@ -2519,6 +2661,7 @@ claim_tablet_tool(InputInfoPtr pInfo)
 {
 	struct xf86libinput *driver_data = pInfo->private;
 	struct xf86libinput_device *shared_device = driver_data->shared_device;
+	struct xf86libinput_tablet_tool_event_queue *queue;
 	struct xf86libinput_tablet_tool *t;
 	uint64_t serial, tool_id;
 
@@ -2531,6 +2674,9 @@ claim_tablet_tool(InputInfoPtr pInfo)
 		if (libinput_tablet_tool_get_serial(t->tool) == serial &&
 		    libinput_tablet_tool_get_tool_id(t->tool) == tool_id) {
 			driver_data->tablet_tool = t->tool;
+			queue = libinput_tablet_tool_get_user_data(t->tool);
+			if (queue)
+				queue->need_to_queue = false;
 			xorg_list_del(&t->node);
 			free(t);
 			return TRUE;
@@ -2662,7 +2808,6 @@ xf86libinput_pre_init(InputDriverPtr drv,
 		driver_data->capabilities &= ~CAP_KEYBOARD;
 		xf86libinput_create_subdevice(pInfo,
 					      CAP_KEYBOARD,
-					      HOTPLUG_LATER,
 					      NULL);
 	}
 
-- 
2.7.4



More information about the xorg-devel mailing list