xkb: post-fix PointerKeys button events with a DeviceChangedEvent.

commit 14327858391ebe929b806efb53ad79e789361883
    xkb: release XTEST pointer buttons on physical releases. (#28808)
revealed a bug with the XTEST/PointerKeys interaction.

Events resulting from PointerKeys are injected into the event processing
stream, not appended to the event queue. The events generated for the fake
button press include a DeviceChangedEvent (DCE), a raw button event and the
button event itself. The DCE causes the master to switch classes to the
attached XTEST pointer device.

Once the fake button is processed, normal event processing continues with
events in the EQ. The master still contains the XTEST classes, causing some
events to be dropped if e.g. the number of valuators of the event in the
queue exceeds the XTEST device's number of valuators.

Example: the EQ contains the following events, processed one-by-one, left to
right.

[DCE (dev)][Btn down][Btn up][Motion][Motion][...]
                  ^ XkbFakeDeviceButton injects [DCE (XTEST)][Btn up]

Thus the event sequence processed looks like this:

[DCE (dev)][Btn down][Btn up][DCE (XTEST)][Btn up][Motion][Motion][...]

The first DCE causes the master to switch to the device. The button up event
injects a DCE to the XTEST device, causing the following Motion events to be
processed with the master still being on XTEST classes.

This patch post-fixes the injected event sequence with a DCE to restore the
classes of the original slave device, resulting in an event sequence like
this:
[DCE (dev)][Btn down][Btn up][DCE (XTEST)][Btn up][DCE (dev)][Motion][Motion]

Note that this is a simplified description. The event sequence injected by
the PointerKeys code is injected for the master device only and the matching
slave device that caused the injection has already finished processing on
the slave. Furthermore, the injection happens as part of the the XKB layer,
before the unwrapping of the processInputProc takes us into the DIX where
the DCE is actually handled.

Bug reproducible with a device that reports more than 2 valuators. Simply
cause button releases on the device and wait for a "too many valuators"
warning message.

Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Acked-by: Daniel Stone <daniel@fooishbar.org>
Reviewed-by: Keith Packard <keithp@keithp.com>

3 files changed, 68 insertions, 38 deletions

diff --git a/dix/getevents.c b/dix/getevents.c
index a9b6e82c3..20bcf7e8a 100644
--- a/dix/getevents.c
+++ b/dix/getevents.c
@@ -652,8 +652,8 @@ clipValuators(DeviceIntPtr pDev, int first_valuator, int num_valuators,
  *        events if a DCCE was generated.
  * @return The updated @events pointer.
  */
-static EventListPtr
-updateFromMaster(EventListPtr events, DeviceIntPtr dev, int type, int *num_events)
+EventListPtr
+UpdateFromMaster(EventListPtr events, DeviceIntPtr dev, int type, int *num_events)
 {
     DeviceIntPtr master;
 
@@ -929,7 +929,7 @@ GetKeyboardValuatorEvents(EventList *events, DeviceIntPtr pDev, int type,
 
     num_events = 1;
 
-    events = updateFromMaster(events, pDev, DEVCHANGE_KEYBOARD_EVENT, &num_events);
+    events = UpdateFromMaster(events, pDev, DEVCHANGE_KEYBOARD_EVENT, &num_events);
 
     /* Handle core repeating, via press/release/press/release. */
     if (type == KeyPress && key_is_down(pDev, key_code, KEY_POSTED)) {
@@ -1091,7 +1091,7 @@ GetPointerEvents(EventList *events, DeviceIntPtr pDev, int type, int buttons,
         (type == MotionNotify && num_valuators <= 0))
         return 0;
 
-    events = updateFromMaster(events, pDev, DEVCHANGE_POINTER_EVENT, &num_events);
+    events = UpdateFromMaster(events, pDev, DEVCHANGE_POINTER_EVENT, &num_events);
 
     raw = (RawDeviceEvent*)events->event;
     events++;
@@ -1206,7 +1206,7 @@ GetProximityEvents(EventList *events, DeviceIntPtr pDev, int type,
         (num_valuators + first_valuator) > pDev->valuator->numAxes)
         return 0;
 
-    events = updateFromMaster(events, pDev, DEVCHANGE_POINTER_EVENT, &num_events);
+    events = UpdateFromMaster(events, pDev, DEVCHANGE_POINTER_EVENT, &num_events);
 
     event = (DeviceEvent *) events->event;
     init_event(pDev, event, GetTimeInMillis());
diff --git a/include/input.h b/include/input.h
index 55b1537f6..ffb1c33fa 100644
--- a/include/input.h
+++ b/include/input.h
@@ -439,6 +439,12 @@ extern void CreateClassesChangedEvent(EventListPtr event,
                                       DeviceIntPtr master,
                                       DeviceIntPtr slave,
                                       int type);
+extern EventListPtr UpdateFromMaster(
+    EventListPtr events,
+    DeviceIntPtr pDev,
+    int type,
+    int *num_events);
+
 extern _X_EXPORT int GetPointerEvents(
     EventListPtr events,
     DeviceIntPtr pDev,
diff --git a/xkb/xkbActions.c b/xkb/xkbActions.c
index eea3d4adf..c36dba0b5 100644
--- a/xkb/xkbActions.c
+++ b/xkb/xkbActions.c
@@ -1332,35 +1332,53 @@ xkbStateNotify	sn;
     return;
 }
 
+/*
+ * The event is injected into the event processing, not the EQ. Thus,
+ * ensure that we restore the master after the event sequence to the
+ * original set of classes. Otherwise, the master remains on the XTEST
+ * classes and drops events that don't fit into the XTEST layout (e.g.
+ * events with more than 2 valuators).
+ *
+ * FIXME: EQ injection in the processing stage is not designed for, so this
+ * is a rather awkward hack. The event list returned by GetPointerEvents()
+ * and friends is always prefixed with a DCE if the last _posted_ device was
+ * different. For normal events, this sequence then resets the master during
+ * the processing stage. Since we inject the PointerKey events in the
+ * processing stage though, we need to manually reset to restore the
+ * previous order, because the events already in the EQ must be sent for the
+ * right device.
+ * So we post-fix the event list we get from GPE with a DCE back to the
+ * previous slave device.
+ *
+ * First one on drinking island wins!
+ */
 static void
-XkbFakePointerMotion(DeviceIntPtr dev, unsigned flags,int x,int y)
+InjectPointerKeyEvents(DeviceIntPtr dev, int type, int button, int flags, int num_valuators, int *valuators)
 {
+    ScreenPtr           pScreen;
     EventListPtr        events;
     int                 nevents, i;
-    DeviceIntPtr        ptr;
-    ScreenPtr           pScreen;
+    DeviceIntPtr        ptr, mpointer, lastSlave;
     Bool                saveWait;
-    int                 gpe_flags = 0;
 
-    if (IsMaster(dev))
-        ptr = GetXTestDevice(GetMaster(dev, MASTER_POINTER));
-    else if (!dev->u.master)
+    if (IsMaster(dev)) {
+        mpointer = GetMaster(dev, MASTER_POINTER);
+        lastSlave = mpointer->u.lastSlave;
+        ptr = GetXTestDevice(mpointer);
+    } else if (!dev->u.master)
         ptr = dev;
     else
         return;
 
-    if (flags & XkbSA_MoveAbsoluteX || flags & XkbSA_MoveAbsoluteY)
-        gpe_flags = POINTER_ABSOLUTE;
-    else
-        gpe_flags = POINTER_RELATIVE;
 
-    events = InitEventList(GetMaximumEventsNum());
+    events = InitEventList(GetMaximumEventsNum() + 1);
     OsBlockSignals();
     pScreen = miPointerGetScreen(ptr);
     saveWait = miPointerSetWaitForUpdate(pScreen, FALSE);
-    nevents = GetPointerEvents(events, ptr,
-                               MotionNotify, 0,
-                               gpe_flags, 0, 2, (int[]){x, y});
+    nevents = GetPointerEvents(events, ptr, type, button, flags, 0,
+                               num_valuators, valuators);
+    if (IsMaster(dev) && (lastSlave && lastSlave != ptr))
+        UpdateFromMaster(&events[nevents], lastSlave, DEVCHANGE_POINTER_EVENT, &nevents);
     miPointerSetWaitForUpdate(pScreen, saveWait);
     OsReleaseSignals();
 
@@ -1368,13 +1386,29 @@ XkbFakePointerMotion(DeviceIntPtr dev, unsigned flags,int x,int y)
         mieqProcessDeviceEvent(ptr, (InternalEvent*)events[i].event, NULL);
 
     FreeEventList(events, GetMaximumEventsNum());
+
+}
+
+static void
+XkbFakePointerMotion(DeviceIntPtr dev, unsigned flags,int x,int y)
+{
+    int                 gpe_flags = 0;
+
+    /* ignore attached SDs */
+    if (!IsMaster(dev) && GetMaster(dev, MASTER_POINTER) != NULL)
+        return;
+
+    if (flags & XkbSA_MoveAbsoluteX || flags & XkbSA_MoveAbsoluteY)
+        gpe_flags = POINTER_ABSOLUTE;
+    else
+        gpe_flags = POINTER_RELATIVE;
+
+    InjectPointerKeyEvents(dev, MotionNotify, 0, gpe_flags, 2, (int[]){x, y});
 }
 
 void
 XkbFakeDeviceButton(DeviceIntPtr dev,Bool press,int button)
 {
-    EventListPtr        events;
-    int                 nevents, i;
     DeviceIntPtr        ptr;
 
     /* If dev is a slave device, and the SD is attached, do nothing. If we'd
@@ -1385,24 +1419,14 @@ XkbFakeDeviceButton(DeviceIntPtr dev,Bool press,int button)
      * if dev is a floating slave, post through the device itself.
      */
 
-    if (IsMaster(dev))
-        ptr = GetXTestDevice(GetMaster(dev, MASTER_POINTER));
-    else if (!dev->u.master)
+    if (IsMaster(dev)) {
+        DeviceIntPtr mpointer = GetMaster(dev, MASTER_POINTER);
+        ptr = GetXTestDevice(mpointer);
+    } else if (!dev->u.master)
         ptr = dev;
     else
         return;
 
-    events = InitEventList(GetMaximumEventsNum());
-    OsBlockSignals();
-    nevents = GetPointerEvents(events, ptr,
-                               press ? ButtonPress : ButtonRelease, button,
-                               0 /* flags */, 0 /* first */,
-                               0 /* num_val */, NULL);
-    OsReleaseSignals();
-
-
-    for (i = 0; i < nevents; i++)
-        mieqProcessDeviceEvent(ptr, (InternalEvent*)events[i].event, NULL);
-
-    FreeEventList(events, GetMaximumEventsNum());
+    InjectPointerKeyEvents(dev, press ? ButtonPress : ButtonRelease,
+                           button, 0, 0, NULL);
 }