input: change pointer screen crossing behaviour for multiple ScreenRecs

miPointerSetPosition traditionally took coordinates on a per-screen basis,
triggering a screen switch when these went out-of-bounds. For absolute
devices, this prevented screen crossing in the negative x/y direction.

This patch changes the event generation patch to handle screen coordinates
in a desktop range (i.e. all screens together). Screen switches are
triggered when these coordinates are not on the current screen.

This unifies the pointer behaviour of single ScreenRec multihead and
multiple ScreenRecs multihead in that the cursor by default moves about the
whole screen rather than be confined to one single screen. The
transformation matrix may then be used to actually confine the cursor to the
screen again.

Note: fill_pointer_events has to deal with several different coordinate
systems. Make sure you read the comment before trying to understand the code.

Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>

4 files changed, 117 insertions, 36 deletions

diff --git a/dix/devices.c b/dix/devices.c
index 64557aaf1..787940524 100644
--- a/dix/devices.c
+++ b/dix/devices.c
@@ -612,6 +612,7 @@ CorePointerProc(DeviceIntPtr pDev, int what)
     int i = 0;
     Atom btn_labels[NBUTTONS] = {0};
     Atom axes_labels[NAXES] = {0};
+    ScreenPtr scr = screenInfo.screens[0];
 
     switch (what) {
     case DEVICE_INIT:
@@ -638,10 +639,11 @@ CorePointerProc(DeviceIntPtr pDev, int what)
                    pDev->name);
             return BadAlloc; /* IPDS only fails on allocs */
         }
-        pDev->valuator->axisVal[0] = screenInfo.screens[0]->width / 2;
-        pDev->last.valuators[0] = pDev->valuator->axisVal[0];
-        pDev->valuator->axisVal[1] = screenInfo.screens[0]->height / 2;
-        pDev->last.valuators[1] = pDev->valuator->axisVal[1];
+        /* axisVal is per-screen, last.valuators is desktop-wide */
+        pDev->valuator->axisVal[0] = scr->width / 2;
+        pDev->last.valuators[0] = pDev->valuator->axisVal[0] + scr->x;
+        pDev->valuator->axisVal[1] = scr->height / 2;
+        pDev->last.valuators[1] = pDev->valuator->axisVal[1] + scr->y;
         break;
 
     case DEVICE_CLOSE:
diff --git a/dix/getevents.c b/dix/getevents.c
index 548cc8b81..cd6260b2e 100644
--- a/dix/getevents.c
+++ b/dix/getevents.c
@@ -295,7 +295,7 @@ updateSlaveDeviceCoords(DeviceIntPtr master, DeviceIntPtr pDev)
     int i;
     DeviceIntPtr lastSlave;
 
-    /* master->last.valuators[0]/[1] is in screen coords and the actual
+    /* master->last.valuators[0]/[1] is in desktop-wide coords and the actual
      * position of the pointer */
     pDev->last.valuators[0] = master->last.valuators[0];
     pDev->last.valuators[1] = master->last.valuators[1];
@@ -757,8 +757,8 @@ accelPointer(DeviceIntPtr dev, ValuatorMask* valuators, CARD32 ms)
  * device's coordinate range.
  *
  * @param dev The device to scale for.
- * @param[in, out] mask The mask in sceen coordinates, modified in place to
- * contain device coordinate range.
+ * @param[in, out] mask The mask in desktop coordinates, modified in place
+ * to contain device coordinate range.
  */
 static void
 scale_from_screen(DeviceIntPtr dev, ValuatorMask *mask)
@@ -768,14 +768,16 @@ scale_from_screen(DeviceIntPtr dev, ValuatorMask *mask)
 
     if (valuator_mask_isset(mask, 0))
     {
-        scaled = rescaleValuatorAxis(valuator_mask_get_double(mask, 0),
+        scaled = valuator_mask_get_double(mask, 0) + scr->x;
+        scaled = rescaleValuatorAxis(scaled,
                                      NULL, dev->valuator->axes + 0,
                                      0, scr->width);
         valuator_mask_set_double(mask, 0, scaled);
     }
     if (valuator_mask_isset(mask, 1))
     {
-        scaled = rescaleValuatorAxis(valuator_mask_get_double(mask, 1),
+        scaled = valuator_mask_get_double(mask, 1) + scr->y;
+        scaled = rescaleValuatorAxis(scaled,
                                      NULL, dev->valuator->axes + 1,
                                      0, scr->height);
         valuator_mask_set_double(mask, 1, scaled);
@@ -793,16 +795,21 @@ scale_from_screen(DeviceIntPtr dev, ValuatorMask *mask)
  *
  * The coordinates provided are always absolute. The parameter mode
  * specifies whether it was relative or absolute movement that landed us at
- * those coordinates.
+ * those coordinates. see fill_pointer_events for information on coordinate
+ * systems.
  *
  * @param dev The device to be moved.
  * @param mode Movement mode (Absolute or Relative)
- * @param mask Mask of axis values for this event
- * @param screenx Screen x coordinate the sprite is on after the update.
- * @param screeny Screen y coordinate the sprite is on after the update.
+ * @param[in,out] mask Mask of axis values for this event, returns the
+ * per-screen device coordinates after confinement
+ * @param[out] devx x desktop-wide coordinate in device coordinate system
+ * @param[out] devy y desktop-wide coordinate in device coordinate system
+ * @param[out] screenx x coordinate in desktop coordinate system
+ * @param[out] screeny y coordinate in desktop coordinate system
  */
 static ScreenPtr
 positionSprite(DeviceIntPtr dev, int mode, ValuatorMask *mask,
+               double *devx, double *devy,
                double *screenx, double *screeny)
 {
     double x, y;
@@ -821,16 +828,20 @@ positionSprite(DeviceIntPtr dev, int mode, ValuatorMask *mask,
     else
         y = dev->last.valuators[1];
 
-    /* scale x&y to screen */
+    /* scale x&y to desktop coordinates */
     *screenx = rescaleValuatorAxis(x, dev->valuator->axes + 0, NULL,
-                                   0, scr->width);
+                                   screenInfo.x, screenInfo.width);
     *screeny = rescaleValuatorAxis(y, dev->valuator->axes + 1, NULL,
-                                   0, scr->height);
+                                   screenInfo.y, screenInfo.height);
 
     tmpx = *screenx;
     tmpy = *screeny;
+    *devx = x;
+    *devy = y;
+
     /* miPointerSetPosition takes care of crossing screens for us, as well as
-     * clipping to the current screen. */
+     * clipping to the current screen. Coordinates returned are in desktop
+     * coord system */
     scr = miPointerSetPosition(dev, mode, screenx, screeny);
 
     /* If we were constrained, rescale x/y from the screen coordinates so
@@ -838,17 +849,24 @@ positionSprite(DeviceIntPtr dev, int mode, ValuatorMask *mask,
      * crossing this doesn't matter much, the coords would be 0 or max.
      */
     if (tmpx != *screenx)
-        x = rescaleValuatorAxis(*screenx, NULL, dev->valuator->axes + 0,
-                                0, scr->width);
-    if (tmpy != *screeny)
-        y = rescaleValuatorAxis(*screeny, NULL, dev->valuator->axes + 1,
-                                0, scr->height);
+        *devx = rescaleValuatorAxis(*screenx, NULL, dev->valuator->axes + 0,
+                                    screenInfo.x, screenInfo.width);
 
+    if (tmpy != *screeny)
+        *devy = rescaleValuatorAxis(*screeny, NULL, dev->valuator->axes + 1,
+                                    screenInfo.y, screenInfo.height);
 
-    if (valuator_mask_isset(mask, 0))
+    /* Recalculate the per-screen device coordinates */
+    if (valuator_mask_isset(mask, 0)) {
+        x = rescaleValuatorAxis(*screenx - scr->x, NULL, dev->valuator->axes + 0,
+                                0, scr->width);
         valuator_mask_set_double(mask, 0, x);
-    if (valuator_mask_isset(mask, 1))
+    }
+    if (valuator_mask_isset(mask, 1)) {
+        y = rescaleValuatorAxis(*screeny - scr->y, NULL, dev->valuator->axes + 1,
+                                0, scr->height);
         valuator_mask_set_double(mask, 1, y);
+    }
 
     return scr;
 }
@@ -1105,6 +1123,38 @@ QueuePointerEvents(DeviceIntPtr device, int type,
  *
  * Should not be called by anyone other than GetPointerEvents.
  *
+ * We use several different coordinate systems and need to switch between
+ * the three in fill_pointer_events, positionSprite and
+ * miPointerSetPosition. "desktop" refers to the width/height of all
+ * screenInfo.screens[n]->width/height added up. "screen" is ScreenRec, not
+ * output.
+ *
+ * Coordinate systems:
+ * - relative events have a mask_in in relative coordinates, mapped to
+ *   pixels. These events are mapped to the current position±delta.
+ * - absolute events have a mask_in in absolute device coordinates in
+ *   device-specific range. This range is mapped to the desktop.
+ * - POINTER_SCREEN absolute events (x86WarpCursor) are in screen-relative
+ *   screen coordinate range.
+ * - rootx/rooty in events must be be relative to the current screen's
+ *   origin (screen coordinate system)
+ * - XI2 valuators must be relative to the current screen's origin. On
+ *   the protocol the device min/max range maps to the current screen.
+ *
+ * For screen switching we need to get the desktop coordinates for each
+ * event, then map that to the respective position on each screen and
+ * position the cursor there.
+ * The device's last.valuator[] stores the last position in desktop-wide
+ * coordinates (in device range for slave devices, desktop range for master
+ * devices).
+ *
+ * screen-relative device coordinates requires scaling: A device coordinate
+ * x/y of range [n..m] that maps to positions Sx/Sy on Screen S must be
+ * rescaled to match Sx/Sy for [n..m]. In the simplest example, x of (m/2-1)
+ * is the last coordinate on the first screen and must be rescaled for the
+ * event to be m. XI2 clients that do their own coordinate mapping would
+ * otherwise interpret the position of the device elsewere to the cursor.
+ *
  * @return the number of events written into events.
  */
 static int
@@ -1115,8 +1165,10 @@ fill_pointer_events(InternalEvent *events, DeviceIntPtr pDev, int type,
     int num_events = 1, i;
     DeviceEvent *event;
     RawDeviceEvent *raw;
-    double screenx = 0.0, screeny = 0.0;
+    double screenx = 0.0, screeny = 0.0; /* desktop coordinate system */
+    double devx = 0.0, devy = 0.0; /* desktop-wide in device coords */
     ValuatorMask mask;
+    ScreenPtr scr;
 
     switch (type)
     {
@@ -1155,6 +1207,8 @@ fill_pointer_events(InternalEvent *events, DeviceIntPtr pDev, int type,
         set_raw_valuators(raw, &mask, raw->valuators.data_raw);
     }
 
+    /* valuators are in driver-native format (rel or abs) */
+
     if (flags & POINTER_ABSOLUTE)
     {
         if (flags & POINTER_SCREEN) /* valuators are in screen coords */
@@ -1168,22 +1222,34 @@ fill_pointer_events(InternalEvent *events, DeviceIntPtr pDev, int type,
         moveRelative(pDev, &mask);
     }
 
+    /* valuators are in device coordinate system in absolute coordinates */
+
     if ((flags & POINTER_NORAW) == 0)
         set_raw_valuators(raw, &mask, raw->valuators.data);
 
-    positionSprite(pDev, (flags & POINTER_ABSOLUTE) ? Absolute : Relative,
-                   &mask, &screenx, &screeny);
+    scr = positionSprite(pDev, (flags & POINTER_ABSOLUTE) ? Absolute : Relative,
+                         &mask, &devx, &devy, &screenx, &screeny);
+
+    /* screenx, screeny are in desktop coordinates,
+       mask is in device coordinates per-screen (the event data)
+       devx/devy is in device coordinate desktop-wide */
     updateHistory(pDev, &mask, ms);
 
     clipValuators(pDev, &mask);
 
-    for (i = 0; i < valuator_mask_size(&mask); i++)
+    /* store desktop-wide in last.valuators */
+    if (valuator_mask_isset(&mask, 0))
+        pDev->last.valuators[0] = devx;
+    if (valuator_mask_isset(&mask, 1))
+        pDev->last.valuators[1] = devy;
+
+    for (i = 2; i < valuator_mask_size(&mask); i++)
     {
         if (valuator_mask_isset(&mask, i))
             pDev->last.valuators[i] = valuator_mask_get_double(&mask, i);
     }
 
-    /* Update the MD's co-ordinates, which are always in screen space. */
+    /* Update the MD's co-ordinates, which are always in desktop space. */
     if (!IsMaster(pDev) || !IsFloating(pDev)) {
         DeviceIntPtr master = GetMaster(pDev, MASTER_POINTER);
         master->last.valuators[0] = screenx;
@@ -1209,8 +1275,8 @@ fill_pointer_events(InternalEvent *events, DeviceIntPtr pDev, int type,
         event->detail.button = buttons;
     }
 
-    /* root_x and root_y must be in screen co-ordinates */
-    event_set_root_coordinates(event, screenx, screeny);
+    /* root_x and root_y must be in per-screen co-ordinates */
+    event_set_root_coordinates(event, screenx - scr->x, screeny - scr->y);
 
     if (flags & POINTER_EMULATED) {
         raw->flags = XIPointerEmulated;
diff --git a/include/inputstr.h b/include/inputstr.h
index 9d4108ef5..1238f930b 100644
--- a/include/inputstr.h
+++ b/include/inputstr.h
@@ -534,8 +534,9 @@ typedef struct _DeviceIntRec {
     DeviceIntPtr        lastSlave;  /* last slave device used */
 
     /* last valuator values recorded, not posted to client;
-     * for slave devices, valuators is in device coordinates
-     * for master devices, valuators is in screen coordinates
+     * for slave devices, valuators is in device coordinates, mapped to the
+     * desktop
+     * for master devices, valuators is in desktop coordinates.
      * see dix/getevents.c
      * remainder supports acceleration
      */
diff --git a/mi/mipointer.c b/mi/mipointer.c
index 55e4081f2..998c86c15 100644
--- a/mi/mipointer.c
+++ b/mi/mipointer.c
@@ -569,8 +569,8 @@ miPointerMoveNoEvent (DeviceIntPtr pDev, ScreenPtr pScreen,
  *
  * @param pDev The device to move
  * @param mode Movement mode (Absolute or Relative)
- * @param[in,out] screenx The x coordinate in screen coordinates
- * @param[in,out] screeny The y coordinate in screen coordinates
+ * @param[in,out] screenx The x coordinate in desktop coordinates
+ * @param[in,out] screeny The y coordinate in desktop coordinates
  */
 ScreenPtr
 miPointerSetPosition(DeviceIntPtr pDev, int mode, double *screenx, double *screeny)
@@ -579,6 +579,7 @@ miPointerSetPosition(DeviceIntPtr pDev, int mode, double *screenx, double *scree
     ScreenPtr		pScreen;
     ScreenPtr		newScreen;
     int			x, y;
+    Bool		switch_screen = FALSE;
 
     miPointerPtr        pPointer; 
 
@@ -593,7 +594,14 @@ miPointerSetPosition(DeviceIntPtr pDev, int mode, double *screenx, double *scree
     x = trunc(*screenx);
     y = trunc(*screeny);
 
-    if (x < 0 || x >= pScreen->width || y < 0 || y >= pScreen->height)
+    switch_screen = !point_on_screen(pScreen, x, y);
+
+    /* Switch to per-screen coordinates for CursorOffScreen and
+     * Pointer->limits */
+    x -= pScreen->x;
+    y -= pScreen->y;
+
+    if (switch_screen)
     {
 	pScreenPriv = GetScreenPrivate (pScreen);
 	if (!pPointer->confined)
@@ -628,6 +636,10 @@ miPointerSetPosition(DeviceIntPtr pDev, int mode, double *screenx, double *scree
             pPointer->pScreen != pScreen)
         miPointerMoveNoEvent(pDev, pScreen, x, y);
 
+    /* Convert to desktop coordinates again */
+    x += pScreen->x;
+    y += pScreen->y;
+
     /* In the event we actually change screen or we get confined, we just
      * drop the float component on the floor
      * FIXME: only drop remainder for ConstrainCursorHarder, not for screen