Merge branch 'wip/stop-emulating-unwind' into 'main'

Properly reset devices on pause and track state during stop_emulating

See merge request libinput/libei!368

proto/protocol.xml

@@ -612,6 +612,12 @@
 
         It is a protocol violation to send this request for a client
         of an ei_handshake.context_type other than sender.
+
+        It is up to the EIS implementation to reset the device state when a
+        stop_emulating event is received. The recommendation is that the device
+        is set to a neutral state such that all touches, buttons, keys are logically up.
+        A client should send the corresponding events before stop_emulating
+        to avoid any ambiguity on event interpretation.
       </description>
       <arg name="last_serial" type="uint32" summary="the last serial sent by the EIS implementation"/>
     </request>
@@ -1214,6 +1220,10 @@
 
         It is a protocol violation to send this request for a client
         of an ei_handshake.context_type other than sender.
+
+        A client should send a ei_button.button release event before
+        ei_device.stop_emulating to avoid any ambiguity on interpretation
+        of button events.
       </description>
       <arg name="button" type="uint32" summary="button code"/>
       <arg name="state" type="uint32" enum="button_state"/>
@@ -1290,6 +1300,10 @@
 
         It is a protocol violation to send this request for a client
         of an ei_handshake.context_type other than sender.
+
+        A client should send a ei_key.key release event before
+        ei_device.stop_emulating to avoid any ambiguity on interpretation
+        of key events.
       </description>
       <arg name="key" type="uint32" summary="the key code"/>
       <arg name="state" type="uint32" enum="key_state" summary="logical state of the key"/>
@@ -1442,6 +1456,10 @@
 
         It is a protocol violation to send a touch down in the same
         frame as a touch motion or touch up.
+
+        A client should send a ei_touch.up or ei_touch.cancel event before
+        ei_device.stop_emulating to avoid any ambiguity on interpretation of touch
+        events.
       </description>
       <arg name="touchid" type="uint32" summary="a unique touch id to identify this touch"/>
       <arg name="x" type="float" summary="touch x coordinate in logical pixels"/>

src/libei.h

@@ -396,13 +396,24 @@ enum ei_event_type {
 
 	/**
 	 * Any events sent from this device will be discarded until the next
-	 * resume. The state of a device is not expected to change between
-	 * pause/resume - for any significant state changes the server is
-	 * expected to remove the device instead.
+	 * resume.
+	 *
+	 * Pausing a device resets the logical state of the device to neutral.
+	 * This includes:
+	 * - any buttons or keys logically down are released
+	 * - any modifiers logically down are released
+	 * - any touches logically down are released
+	 *
+	 * Sender clients must wait until @ref EI_EVENT_DEVICE_RESUMED
+	 * before sending events.
 	 */
 	EI_EVENT_DEVICE_PAUSED,
+
 	/**
 	 * The client may send events.
+	 *
+	 * Once resumed, a sender client may call ei_device_start_emulating()
+	 * and begin emulating events.
 	 */
 	EI_EVENT_DEVICE_RESUMED,
 

src/libeis-device.c

@@ -25,6 +25,7 @@
 #include "config.h"
 
 #include <errno.h>
+#include <stdint.h>
 
 #include "util-macros.h"
 #include "util-bits.h"
@@ -428,6 +429,13 @@ client_msg_button(struct eis_button *button, uint32_t btn, uint32_t state)
 				       "Button event for non-button device");
 	}
 
+	if (btn >= KEY_CNT)
+		return brei_result_new(EIS_CONNECTION_DISCONNECT_REASON_PROTOCOL,
+				       "Button event for invalid button %x (KEY_CNT is %#x)", btn, KEY_CNT);
+
+	if (!eis_device_update_key_button_state(device, btn, state))
+		return NULL;
+
 	if (device->state == EIS_DEVICE_STATE_EMULATING) {
 		eis_queue_pointer_button_event(device, btn, !!state);
 		return NULL;
@@ -598,6 +606,13 @@ client_msg_keyboard_key(struct eis_keyboard *keyboard, uint32_t key, uint32_t st
 				       "Key event for non-keyboard device");
 	}
 
+	if (key >= KEY_CNT)
+		return brei_result_new(EIS_CONNECTION_DISCONNECT_REASON_PROTOCOL,
+				       "Key event for invalid key %x (KEY_CNT is %#x)", key, KEY_CNT);
+
+	if (!eis_device_update_key_button_state(device, key, state))
+		return NULL;
+
 	if (device->state == EIS_DEVICE_STATE_EMULATING) {
 		eis_queue_keyboard_key_event(device, key, !!state);
 		return NULL;
@@ -627,6 +642,35 @@ eis_device_get_keyboard_interface(struct eis_device *device)
 	return &keyboard_interface;
 }
 
+/* Returns true and the position of the touch with the given ID, or
+ * false and the first position that is available
+ */
+static bool
+find_touch(struct eis_device *device, uint32_t touchid, size_t *index)
+{
+	ssize_t first_available = -1;
+	for (size_t i = 0; i < ARRAY_LENGTH(device->touch_state.down); i++) {
+		if (device->touch_state.down[i] != UINT64_MAX) {
+			if (device->touch_state.down[i] == touchid) {
+				if (index)
+					*index = i;
+				return true;
+			}
+		} else if (first_available < 0) {
+			first_available = i;
+		}
+	}
+
+	if (index) {
+		if (first_available < 0)
+			*index = EIS_MAX_TOUCHES;
+		else
+			*index = (size_t)first_available;
+	}
+
+	return false;
+}
+
 static struct brei_result *
 client_msg_touch_down(struct eis_touchscreen *touchscreen,
 		      uint32_t touchid, float x, float y)
@@ -641,6 +685,17 @@ client_msg_touch_down(struct eis_touchscreen *touchscreen,
 	}
 
 	if (device->state == EIS_DEVICE_STATE_EMULATING) {
+		size_t first_available;
+		if (find_touch(device, touchid, &first_available)) {
+			return brei_result_new(EIS_CONNECTION_DISCONNECT_REASON_PROTOCOL,
+					       "Touch down event for duplicated touch ID");
+		}
+
+		if (first_available >= EIS_MAX_TOUCHES)
+			return brei_result_new(EIS_CONNECTION_DISCONNECT_REASON_ERROR,
+						"Too many simultaneous touch events");
+
+		device->touch_state.down[first_available] = touchid;
 		eis_queue_touch_down_event(device, touchid, x, y);
 		return NULL;
 	}
@@ -662,13 +717,26 @@ client_msg_touch_motion(struct eis_touchscreen *touchscreen,
 	}
 
 	if (device->state == EIS_DEVICE_STATE_EMULATING) {
-		eis_queue_touch_motion_event(device, touchid, x, y);
+		/* Silently ignore motion for non-existing touches */
+		if (find_touch(device, touchid, NULL))
+			eis_queue_touch_motion_event(device, touchid, x, y);
 		return NULL;
 	}
 
 	return maybe_error_on_device_state(device, "touch motion");
 }
 
+static bool
+release_touch(struct eis_device *device, uint32_t touchid)
+{
+	size_t index;
+	bool rc = find_touch(device, touchid, &index);
+	if (rc)
+		device->touch_state.down[index] = UINT64_MAX;
+
+	return rc;
+}
+
 static struct brei_result *
 client_msg_touch_up(struct eis_touchscreen *touchscreen, uint32_t touchid)
 {
@@ -681,8 +749,11 @@ client_msg_touch_up(struct eis_touchscreen *touchscreen, uint32_t touchid)
 				       "Touch up event for non-touch device");
 	}
 
-	if (device->state == EIS_DEVICE_STATE_EMULATING) {
-		eis_queue_touch_up_event(device, touchid);
+	/* End the touch locally even if we're not emulating, but
+	 * silently ignore touch end/cancel for non-existing touches */
+	if (release_touch(device, touchid)) {
+		if (device->state == EIS_DEVICE_STATE_EMULATING)
+			eis_queue_touch_up_event(device, touchid);
 		return NULL;
 	}
 
@@ -707,8 +778,11 @@ client_msg_touch_cancel(struct eis_touchscreen *touchscreen, uint32_t touchid)
 				       "Touch cancel event for touchscreen version v1");
 	}
 
-	if (device->state == EIS_DEVICE_STATE_EMULATING) {
-		eis_queue_touch_cancel_event(device, touchid);
+	/* End the touch locally even if we're not emulating, but
+	 * silently ignore touch end/cancel for non-existing touches */
+	if (release_touch(device, touchid)) {
+		if (device->state == EIS_DEVICE_STATE_EMULATING)
+			eis_queue_touch_cancel_event(device, touchid);
 		return NULL;
 	}
 
@@ -762,6 +836,10 @@ eis_seat_new_device(struct eis_seat *seat)
 
 	list_append(&seat->devices, &device->link);
 
+	for (size_t i = 0; i < ARRAY_LENGTH(device->touch_state.down); i++) {
+		device->touch_state.down[i] = UINT64_MAX;
+	}
+
 	return eis_device_ref(device);
 }
 
@@ -1447,6 +1525,12 @@ eis_device_pause(struct eis_device *device)
 
 	device->state = EIS_DEVICE_STATE_PAUSED;
 	eis_device_event_paused(device, eis_client_get_next_serial(client));
+
+	memset(device->key_button_state.down, 0, sizeof(device->key_button_state.down));
+
+	for (size_t i = 0; i < ARRAY_LENGTH(device->touch_state.down); i++) {
+		device->touch_state.down[i] = UINT64_MAX;
+	}
 }
 
 _public_ void

src/libeis-device.h

@@ -26,10 +26,16 @@
 
 #include "libeis.h"
 
+#include "util-bits.h"
 #include "util-object.h"
 #include "util-list.h"
 #include "brei-shared.h"
 
+#define KEY_MAX 0x2ffU
+#define KEY_CNT (KEY_MAX + 1)
+
+#define EIS_MAX_TOUCHES 16
+
 enum eis_device_state {
 	EIS_DEVICE_STATE_NEW,
 	EIS_DEVICE_STATE_AWAITING_READY,
@@ -75,6 +81,13 @@ struct eis_device {
 		bool x_is_cancelled, y_is_cancelled;
 	} scroll_state;
 
+	struct {
+		unsigned char down[NCHARS(KEY_CNT)];
+	} key_button_state;
+
+	struct {
+		uint64_t down[EIS_MAX_TOUCHES]; /* touch id */
+	} touch_state;
 };
 
 struct eis_touch {
@@ -118,6 +131,21 @@ OBJECT_DECLARE_GETTER(eis_device, button_interface, const struct eis_button_inte
 OBJECT_DECLARE_GETTER(eis_device, keyboard_interface, const struct eis_keyboard_interface *);
 OBJECT_DECLARE_GETTER(eis_device, touchscreen_interface, const struct eis_touchscreen_interface *);
 
+static inline bool
+eis_device_update_key_button_state(struct eis_device *device, uint32_t key_btn, uint32_t state)
+{
+	if (state) {
+		if (bit_is_set(device->key_button_state.down, key_btn))
+			return false;
+		set_bit(device->key_button_state.down, key_btn);
+	} else {
+		if (!bit_is_set(device->key_button_state.down, key_btn))
+			return false;
+		clear_bit(device->key_button_state.down, key_btn);
+	}
+	return true;
+}
+
 void
 eis_device_set_client_keymap(struct eis_device *device,
 		      enum eis_keymap_type type,

src/libeis.h

@@ -1349,11 +1349,12 @@ eis_device_remove(struct eis_device *device);
  * a number of events from a device after it has been paused and must
  * update its internal state accordingly.
  *
- * Pause/resume should only be used for short-term event delaying, a client
- * will expect that the device's state has not changed between pause and
- * resume. Where a device's state changes on the EIS implementation side (e.g.
- * buttons or keys are forcibly released), the device should be removed and
- * re-added as new device.
+ * Pausing a device resets the logical state of the device to neutral.
+ * This includes:
+ * - any buttons or keys logically down are released
+ * - any modifiers logically down are released
+ * - any touches logically down are released
+ * No events will be sent for these releases back to a neutral state.
  *
  * @param device A connected device
  */

test/test-ei-device.c

@@ -390,10 +390,20 @@ MUNIT_TEST(test_ei_device_button_button)
 		struct ei_device *device = peck_ei_get_default_pointer(peck);
 		ei_device_button_button(device, BTN_LEFT, true);
 		ei_device_frame(device, peck_ei_now(peck));
+
+		/* double press is quietly ignored */
+		ei_device_button_button(device, BTN_LEFT, true);
+		ei_device_frame(device, peck_ei_now(peck));
+
 		ei_device_button_button(device, BTN_RIGHT, true);
 		ei_device_frame(device, peck_ei_now(peck));
 		ei_device_button_button(device, BTN_RIGHT, false);
 		ei_device_frame(device, peck_ei_now(peck));
+
+		/* double release is quietly ignored */
+		ei_device_button_button(device, BTN_RIGHT, false);
+		ei_device_frame(device, peck_ei_now(peck));
+
 		ei_device_button_button(device, BTN_LEFT, false);
 		ei_device_frame(device, peck_ei_now(peck));
 	}
@@ -421,6 +431,7 @@ MUNIT_TEST(test_ei_device_button_button)
 		munit_assert_int(eis_event_button_get_button(lu), ==, BTN_LEFT);
 		munit_assert_false(eis_event_button_get_is_press(lu));
 
+		peck_assert_no_eis_events(eis);
 	}
 
 	return MUNIT_OK;
@@ -439,6 +450,15 @@ MUNIT_TEST(test_ei_device_keyboard_key)
 		struct ei_device *device = peck_ei_get_default_keyboard(peck);
 		ei_device_keyboard_key(device, KEY_Q, true);
 		ei_device_frame(device, peck_ei_now(peck));
+
+		/* Double press is quietly ignored */
+		ei_device_keyboard_key(device, KEY_Q, true);
+		ei_device_frame(device, peck_ei_now(peck));
+
+		ei_device_keyboard_key(device, KEY_Q, false);
+		ei_device_frame(device, peck_ei_now(peck));
+
+		/* Double release is quietly ignored */
 		ei_device_keyboard_key(device, KEY_Q, false);
 		ei_device_frame(device, peck_ei_now(peck));
 	}
@@ -1503,6 +1523,96 @@ MUNIT_TEST(test_ei_device_multitouch)
 	return MUNIT_OK;
 }
 
+MUNIT_TEST(test_ei_device_touch_up_after_paused)
+{
+	_unref_(peck) *peck = peck_new();
+	_unref_(ei_device) *device = NULL;
+	_unref_(eis_device) *eis_device = NULL;
+
+	peck_enable_eis_behavior(peck, PECK_EIS_BEHAVIOR_ACCEPT_ALL);
+	peck_enable_eis_behavior(peck, PECK_EIS_BEHAVIOR_ADD_TOUCH);
+	peck_enable_ei_behavior(peck, PECK_EI_BEHAVIOR_AUTODEVICES);
+	peck_dispatch_until_stable(peck);
+
+	_unref_(ei_touch) *t1 = NULL;
+	_unref_(ei_touch) *t2 = NULL;
+
+	with_client(peck) {
+		device = ei_device_ref(peck_ei_get_default_touch(peck));
+		t1 = ei_device_touch_new(device);
+		t2 = ei_device_touch_new(device);
+		ei_touch_down(t1, 1, 2);
+		ei_touch_down(t2, 3, 4);
+		ei_device_frame(device, peck_ei_now(peck));
+	}
+
+	peck_dispatch_until_stable(peck);
+
+	with_server(peck) {
+		_unref_(eis_event) *down1 = peck_eis_touch_down(eis, 1, 2);
+		_unref_(eis_event) *down2 = peck_eis_touch_down(eis, 3, 4);
+
+		peck_assert_no_eis_events(eis); /* drain the frame */
+
+		eis_device = eis_device_ref(eis_event_get_device(down1));
+		eis_device_pause(eis_device);
+	}
+
+	/* No ei dispatch here */
+	peck_dispatch_eis(peck);
+
+	with_client(peck) {
+		/* These events will arrive when the device is paused and
+		 * are discarded by EIS */
+		ei_touch_up(t1);
+		ei_touch_up(t2);
+		ei_device_frame(device, peck_ei_now(peck));
+		ei_touch_unref(t1);
+		ei_touch_unref(t2);
+	}
+
+	peck_dispatch_until_stable(peck);
+
+	with_client(peck) {
+		_unref_(ei_event) *pause = peck_ei_next_event(ei, EI_EVENT_DEVICE_PAUSED);
+	}
+
+	with_server(peck) {
+		/* touch up and empty frame were discarded */
+		peck_assert_no_eis_events(eis);
+		eis_device_resume(eis_device);
+	}
+
+	peck_dispatch_until_stable(peck);
+
+	with_client(peck) {
+		ei_device_start_emulating(device, 123);
+
+		/* The C API doesn't allow us to set a touch id
+		 * so we can't really test for the correct behavior.
+		 * All we can do is exercise most of the code by creating
+		 * new touches and hope.
+		 */
+		t1 = ei_device_touch_new(device);
+		t2 = ei_device_touch_new(device);
+		ei_touch_down(t1, 1, 2);
+		ei_touch_down(t2, 3, 4);
+		ei_device_frame(device, peck_ei_now(peck));
+	}
+
+	peck_dispatch_until_stable(peck);
+
+	with_server(peck) {
+		_unref_(eis_event) *down1 = peck_eis_touch_down(eis, 1, 2);
+		_unref_(eis_event) *down2 = peck_eis_touch_down(eis, 3, 4);
+		peck_assert_no_eis_events(eis); /* drain the frame */
+	}
+
+	peck_dispatch_until_stable(peck);
+
+	return MUNIT_OK;
+}
+
 #if HAVE_MEMFD_CREATE
 MUNIT_TEST(test_ei_keymap_invalid)
 {

test/test_protocol.py

@@ -1259,3 +1259,90 @@ class TestEiProtocol:
         else:
             ei.callback_roundtrip()
             assert status.disconnected is False
+
+    def test_touch_disconnect_on_duplicate_id(self, eis):
+        """
+        Ensure EIS disconnects us when we use a duplicate touch id
+        """
+
+        ei = eis.ei
+
+        @dataclass
+        class Status:
+            device: EiDevice = None
+            touchscreen: Optional[EiTouchscreen] = None
+            disconnected: bool = False
+            resumed: bool = False
+            serial: int = 0
+
+        status = Status()
+
+        def on_interface(device, object, name, version, new_objects):
+            logger.debug(
+                "new capability",
+                device=device,
+                object=object,
+                name=name,
+                version=version,
+            )
+            if name == InterfaceName.EI_TOUCHSCREEN:
+                assert status.touchscreen is None
+                status.touchscreen = new_objects["object"]
+
+        def on_device_resumed(device, serial):
+            status.resumed = True
+            status.serial = serial
+
+        def on_new_device(seat, device, version, new_objects):
+            logger.debug("new device", object=new_objects["device"])
+            status.device = new_objects["device"]
+            status.device.connect("Interface", on_interface)
+            status.device.connect("Resumed", on_device_resumed)
+
+        def on_new_object(o: Interface):
+            logger.debug("new object", object=o)
+            if o.name == InterfaceName.EI_SEAT:
+                ei.seat_fill_capability_masks(o)
+                o.connect("Device", on_new_device)
+
+        ei.context.connect("register", on_new_object)
+        ei.dispatch()
+
+        def on_disconnected(connection, last_serial, reason, explanation):
+            status.disconnected = True
+
+        def on_connection(setup, serial, id, version, new_objects={}):
+            connection = new_objects["connection"]
+            connection.connect("Disconnected", on_disconnected)
+
+        setup = ei.handshake
+        setup.connect("Connection", on_connection)
+        ei.init_default_sender_connection(interface_versions={"ei_touchscreen": 1})
+
+        ei.wait_for_seat()
+        seat = ei.seats[0]
+        ei.send(seat.Bind(seat.bind_mask([InterfaceName.EI_TOUCHSCREEN])))
+        ei.wait_for(lambda: status.touchscreen and status.resumed)
+
+        assert status.touchscreen is not None
+
+        ei.send(status.device.StartEmulating(status.serial, 123))
+        logger.debug("Sending touch events")
+        touchid = 1
+        touchscreen = status.touchscreen
+        device = status.device
+        ei.send(touchscreen.Down(touchid, 10, 20))
+        ei.send(device.Frame(status.serial, int(time.time())))
+        ei.send(touchscreen.Motion(touchid, 10, 25))
+        ei.send(device.Frame(status.serial, int(time.time())))
+
+        ei.send(touchscreen.Down(touchid, 10, 20))
+        try:
+            ei.send(device.Frame(status.serial, int(time.time())))
+        except BrokenPipeError:
+            pass
+
+        ei.dispatch()
+        ei.wait_for(lambda: status.disconnected)
+
+        assert status.disconnected is True