Specify MODIFIERS to be sent for any state change.

Updates protocol and API documentation to specify that the modifiers
event should be sent by the EIS implementation every time the modifier
or group state changes, including when the change is triggered by key
events on the emulated keyboard.

The previous approach of expecting the client to track modifier state
using xkb_state_update_key() for injected keys resulted in multiple
opportunities for the client and server to get out of sync (both due to
unavoidable race conditions and due the client not having access to the
complete state used by the server to calculate state changes), with no
way for the client to ensure it had a correct modifier map short of
unbinding and rebinding the seat.

The new approach allows the client to track state solely by applying
modifiers events with xkb_state_update_mask(), simplifying client
implementation. Because the event is sent for all changes, the client
can use ei_connection.sync / ei_ping() to ensure that it has received
the latest state incorporating all key requests sent prior to the sync
request (along with any externally-caused modifier state changes that
may have occured up to the time the sync message was received).

Part-of: <https://gitlab.freedesktop.org/libinput/libei/-/merge_requests/318>

proto/protocol.xml

@@ -1361,12 +1361,33 @@
       <description summary="Modifier change event">
         Notification that the EIS implementation has changed group or modifier
         states on this device, but not necessarily in response to an
-        ei_keyboard.key event. Future ei_keyboard.key requests must take the
-        new group or modifier state into account.
+        ei_keyboard.key event or request. Future ei_keyboard.key requests must
+        take the new group and modifier state into account.
 
-        This event should not be sent in response to ei_keyboard.key events
-        that change the group or modifier state according to the keymap. The
-        client is expected to track such group or modifier states on its own.
+        This event should be sent any time the modifier state or effective group
+        has changed, whether caused by an ei_keyboard.key event in accordance
+        with the keymap, indirectly due to further handling of an
+        ei_keyboard.key event (e.g., because it triggered a keyboard shortcut
+        that then changed the state), or caused by an unrelated an event (e.g.,
+        input from a different keyboard, or a group change triggered by a layout
+        selection widget).
+
+        For receiver clients, modifiers events will always be properly ordered
+        with received key events, so each key event should be interpreted using
+        the most recently-received modifier state. The server should send this
+        event immediately following the ei_device.frame event for the key press
+        that caused the change. If the state change impacts multiple keyboards,
+        this event should be sent for all of them.
+        
+        For sender clients, the modifiers event is not inherently synchronized
+        with key requests, but the client may send an ei_connection.sync request
+        when synchronization is required. When the corresponding
+        ei_callback.done event is received, all key requests sent prior to the
+        sync request are guaranteed to have been processed, and any
+        directly-resulting modifiers events are guaranteed to have been
+        received. Note, however, that it is still possible for
+        indirectly-triggered state changes, such as via a keyboard shortcut not
+        encoded in the keymap, to be reported after the done event.
 
         A client must assume that all modifiers are lifted when it
         receives an ei_device.paused event. The EIS implementation
@@ -1377,6 +1398,12 @@
         be processed immediately by the client.
 
         This event is only sent for devices with an ei_keyboard.keymap.
+
+        Note: A previous version of the documentation instead specified that
+        this event should not be sent in response to ei_keyboard.key events
+        that change the group or modifier state according to the keymap.
+        However, this complicated client implementation and resulted in
+        situations where the client state could get out of sync with the server.
       </description>
       <arg name="serial" type="uint32" summary="this event's serial number"/>
       <arg name="depressed" type="uint32" summary="depressed modifiers"/>

src/libei.h

@@ -414,11 +414,35 @@ enum ei_event_type {
 	 * ei_event_keyboard_get_xkb_mods_locked(), and
 	 * ei_event_keyboard_get_xkb_group().
 	 *
-	 * This event is sent in response to an external modifier state
-	 * change. Where the client triggers a modifier state change in
-	 * response to ei_device_keyboard_key(), no such event is sent.
+	 * This event is sent in response to any modifier state or effective
+	 * group change, including where the change is triggered by a client
+	 * call to ei_device_keyboard_key().
+	 *
+	 * For receiver clients, this will always be properly ordered with
+	 * EI_EVENT_KEYBOARD_KEY events, so each key event should be
+	 * interpreted should using the most recently received  modifier
+	 * state.
+	 *
+	 * For sender clients, the this event is not inherently synchronized
+	 * with calls to ei_device_keyboard_key(), but the client may call
+	 * ei_ping() when synchronization is required. When the corresponding
+	 * EI_EVENT_PONG event is received, all key events sent prior to the
+	 * sync request are guaranteed to have been processed, and any
+	 * directly-resulting modifiers events are guaranteed to have been
+	 * received. Note, however, that it is still possible for
+	 * indirectly-triggered state changes, such as via a keyboard
+	 * shortcut not encoded in the keymap, to be reported after the done
+	 * event.
 	 *
 	 * This event may arrive while a device is paused.
+	 *
+	 * Note: It was previously specified that a where a sender client
+	 * triggers a modifier state change in response to
+	 * ei_device_keyboard_key(), no MODIFIERS event would be sent.
+	 * Clients were expected to mix calls to xkb_state_update_key() and
+	 * xkb_state_update_mask() to track the state with libxkbcommon,
+	 * which could lead to disagreements between the client and server as
+	 * to the current state.
 	 */
 	EI_EVENT_KEYBOARD_MODIFIERS,
 
@@ -1995,7 +2019,20 @@ ei_event_keyboard_get_xkb_mods_locked(struct ei_event *event);
  * @ingroup libei-receiver
  *
  * For an event of type @ref EI_EVENT_KEYBOARD_MODIFIERS, get the
- * logical group state.
+ * current effective group.
+ *
+ * This may be passed to xkb_state_update_mask() as either
+ * depressed_layout (effectively pretending the user is holding down some
+ * key for this group at all times) or locked_layout (treating it as a
+ * layout the user has switched to through some mechanism), but never
+ * both at the same time. The other two layout arguments must be set to
+ * zero.
+ *
+ * Note: Because the client only knows the current effective group and
+ * not the combination of state from which it was calculated, any attempt
+ * to predict how future key presses will impact the group state will
+ * necessarily be unreliable.
+ *
  * See ei_device_keyboard_get_keymap() for the corresponding keymap.
  */
 uint32_t

src/libeis.h

@@ -1399,6 +1399,22 @@ eis_device_keyboard_get_keymap(struct eis_device *device);
  * @ingroup libeis-device
  *
  * Notify the client of the current XKB modifier state.
+ *
+ * This should be called every time the modifier state or current
+ * effective group changes.
+ *
+ * When the state changes due to an incoming
+ * EIS_EVENT_KEYBOARD_KEY (for sender clients), this method should be
+ * called when the corresponding EIS_EVENT_FRAME is processed, before
+ * processing any subsequent events.
+ *
+ * When the state changes due to a key press with a receiver client, this
+ * method should be called immediately after the corresponding call to
+ * eis_device_frame(). If the change impacts multiple keyboards, this
+ * method should be called for all of them.
+ *
+ * For changes caused by other factors, this method should be called for
+ * all affected keyboards at the point the change occurs.
  */
 void
 eis_device_keyboard_send_xkb_modifiers(struct eis_device *device,