From 9537afb13f2750d22350b7441570332ae60e4860 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Tue, 30 Aug 2011 13:37:31 +1000 Subject: [PATCH 01/16] dix: fill out root_x/y for keyboard events Switching screens relies on rootx/y to be set to the correct value. Note: though we technically take a mask for GetKeyboardEvents we don't actually handle it properly to move the pointer as required (and generate motion events if needed). Signed-off-by: Peter Hutterer Reviewed-by: Daniel Stone --- dix/getevents.c | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/dix/getevents.c b/dix/getevents.c index c42971592..b81562a86 100644 --- a/dix/getevents.c +++ b/dix/getevents.c @@ -859,6 +859,15 @@ queueEventList(DeviceIntPtr device, InternalEvent *events, int nevents) mieqEnqueue(device, &events[i]); } +static void +event_set_root_coordinates(DeviceEvent* event, double x, double y) +{ + event->root_x = trunc(x); + event->root_y = trunc(y); + event->root_x_frac = x - trunc(x); + event->root_y_frac = y - trunc(y); +} + /** * Generate internal events representing this keyboard event and enqueue * them on the event queue. @@ -956,6 +965,13 @@ GetKeyboardEvents(InternalEvent *events, DeviceIntPtr pDev, int type, set_valuators(pDev, event, &mask); + if (!IsFloating(pDev)) { + DeviceIntPtr master = GetMaster(pDev, MASTER_POINTER); + event_set_root_coordinates(event, + master->last.valuators[0], + master->last.valuators[1]); + } + return num_events; } @@ -1158,10 +1174,7 @@ fill_pointer_events(InternalEvent *events, DeviceIntPtr pDev, int type, } /* root_x and root_y must be in screen co-ordinates */ - event->root_x = trunc(screenx); - event->root_y = trunc(screeny); - event->root_x_frac = screenx - trunc(screenx); - event->root_y_frac = screeny - trunc(screeny); + event_set_root_coordinates(event, screenx, screeny); if (flags & POINTER_EMULATED) { raw->flags = XIPointerEmulated; From 535b3789be3a7b43b5d9026e2b5150521d91e32b Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Wed, 31 Aug 2011 14:15:02 +1000 Subject: [PATCH 02/16] dix: warn about keyboard events with valuator masks We don't actually handle the mask correctly. They're clipped and dropped into the event but that's about it. I don't think we did since 1.4, let's warn the user if this happens. Signed-off-by: Peter Hutterer Reviewed-by: Daniel Stone --- dix/getevents.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/dix/getevents.c b/dix/getevents.c index b81562a86..ebf265377 100644 --- a/dix/getevents.c +++ b/dix/getevents.c @@ -919,6 +919,11 @@ GetKeyboardEvents(InternalEvent *events, DeviceIntPtr pDev, int type, (key_code < 8 || key_code > 255)) return 0; + if (mask_in && valuator_mask_size(mask_in) > 1) { + ErrorF("[dix] the server does not handle valuator masks with " + "keyboard events. This is a bug. You may fix it.\n"); + } + num_events = 1; events = UpdateFromMaster(events, pDev, DEVCHANGE_KEYBOARD_EVENT, &num_events); From d0a7cd759d4741a1ae118d579c90704410cde244 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Fri, 2 Sep 2011 09:53:02 +1000 Subject: [PATCH 03/16] dix: NewCurrentScreen must work on pointers where possible When a screen switch is triggered by PointerKeys, the device for NewCurrentScreen is the keyboard. Submitting pointer events for this keyboard (without valuators) has no effect as GPE ignores the event. Force the dequeuing through the XTest device attached to this device. Signed-off-by: Peter Hutterer Reviewed-by: Daniel Stone --- dix/events.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/dix/events.c b/dix/events.c index 4e21c2df2..0f5b042e3 100644 --- a/dix/events.c +++ b/dix/events.c @@ -3360,7 +3360,11 @@ WindowHasNewCursor(WindowPtr pWin) void NewCurrentScreen(DeviceIntPtr pDev, ScreenPtr newScreen, int x, int y) { - SpritePtr pSprite = pDev->spriteInfo->sprite; + DeviceIntPtr ptr; + SpritePtr pSprite; + + ptr = IsFloating(pDev) ? pDev : GetXTestDevice(GetMaster(pDev, MASTER_POINTER)); + pSprite = ptr->spriteInfo->sprite; pSprite->hotPhys.x = x; pSprite->hotPhys.y = y; @@ -3372,15 +3376,15 @@ NewCurrentScreen(DeviceIntPtr pDev, ScreenPtr newScreen, int x, int y) pSprite->screen = newScreen; /* Make sure we tell the DDX to update its copy of the screen */ if(pSprite->confineWin) - XineramaConfineCursorToWindow(pDev, + XineramaConfineCursorToWindow(ptr, pSprite->confineWin, TRUE); else - XineramaConfineCursorToWindow(pDev, screenInfo.screens[0]->root, TRUE); + XineramaConfineCursorToWindow(ptr, screenInfo.screens[0]->root, TRUE); /* if the pointer wasn't confined, the DDX won't get told of the pointer warp so we reposition it here */ if(!syncEvents.playingEvents) (*pSprite->screen->SetCursorPosition)( - pDev, + ptr, pSprite->screen, pSprite->hotPhys.x + screenInfo.screens[0]->x - pSprite->screen->x, @@ -3390,7 +3394,7 @@ NewCurrentScreen(DeviceIntPtr pDev, ScreenPtr newScreen, int x, int y) } else #endif if (newScreen != pSprite->hotPhys.pScreen) - ConfineCursorToWindow(pDev, newScreen->root, TRUE, FALSE); + ConfineCursorToWindow(ptr, newScreen->root, TRUE, FALSE); } #ifdef PANORAMIX From 911e7368bf9c00d327e994a6f7a1d8d8f9b83c72 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Tue, 30 Aug 2011 16:58:04 -0400 Subject: [PATCH 04/16] Move pointOnScreen to inpututils.c We need this from other files too. Signed-off-by: Peter Hutterer Reviewed-by: Daniel Stone --- dix/events.c | 11 ++--------- dix/inpututils.c | 8 ++++++++ include/input.h | 2 ++ 3 files changed, 12 insertions(+), 9 deletions(-) diff --git a/dix/events.c b/dix/events.c index 0f5b042e3..f87d2bbf9 100644 --- a/dix/events.c +++ b/dix/events.c @@ -524,13 +524,6 @@ SyntheticMotion(DeviceIntPtr dev, int x, int y) { #ifdef PANORAMIX static void PostNewCursor(DeviceIntPtr pDev); -static Bool -pointOnScreen(ScreenPtr pScreen, int x, int y) -{ - return x >= pScreen->x && x < pScreen->x + pScreen->width && - y >= pScreen->y && y < pScreen->y + pScreen->height; -} - static Bool XineramaSetCursorPosition( DeviceIntPtr pDev, @@ -550,13 +543,13 @@ XineramaSetCursorPosition( x += screenInfo.screens[0]->x; y += screenInfo.screens[0]->y; - if(!pointOnScreen(pScreen, x, y)) + if(!point_on_screen(pScreen, x, y)) { FOR_NSCREENS(i) { if(i == pScreen->myNum) continue; - if(pointOnScreen(screenInfo.screens[i], x, y)) + if(point_on_screen(screenInfo.screens[i], x, y)) { pScreen = screenInfo.screens[i]; break; diff --git a/dix/inpututils.c b/dix/inpututils.c index 0a3d3d8b4..eeae2a74f 100644 --- a/dix/inpututils.c +++ b/dix/inpututils.c @@ -37,6 +37,7 @@ #include "xkbstr.h" #include "inpututils.h" #include "eventstr.h" +#include "scrnintstr.h" /* Check if a button map change is okay with the device. * Returns -1 for BadValue, as it collides with MappingBusy. */ @@ -619,6 +620,13 @@ void init_device_event(DeviceEvent *event, DeviceIntPtr dev, Time ms) event->sourceid = dev->id; } +Bool +point_on_screen(ScreenPtr pScreen, int x, int y) +{ + return x >= pScreen->x && x < pScreen->x + pScreen->width && + y >= pScreen->y && y < pScreen->y + pScreen->height; +} + /** * Delete the element with the key from the list, freeing all memory * associated with the element.. diff --git a/include/input.h b/include/input.h index 6ba1ab200..b7de5ca3d 100644 --- a/include/input.h +++ b/include/input.h @@ -608,4 +608,6 @@ extern _X_EXPORT const char* input_option_get_value(const InputOption *opt); extern _X_EXPORT void input_option_set_key(InputOption *opt, const char* key); extern _X_EXPORT void input_option_set_value(InputOption *opt, const char* value); +extern _X_HIDDEN Bool point_on_screen(ScreenPtr pScreen, int x, int y); + #endif /* INPUT_H */ From 765ef69295ddc473640c96f1b4f54e0b8bfc670e Mon Sep 17 00:00:00 2001 From: Max Schwarz Date: Tue, 4 Oct 2011 22:06:08 +0200 Subject: [PATCH 05/16] dix: fix inverted handling of legacy scroll button events This bug led to inverted scrolling axes with drivers that support smooth scrolling axes but send legacy button events. Signed-off-by: Max Schwarz Reviewed-by: Peter Hutterer Signed-off-by: Peter Hutterer --- dix/getevents.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/dix/getevents.c b/dix/getevents.c index ebf265377..97c3937b9 100644 --- a/dix/getevents.c +++ b/dix/getevents.c @@ -1330,21 +1330,22 @@ GetPointerEvents(InternalEvent *events, DeviceIntPtr pDev, int type, double val, adj; int axis; + /* Up is negative on valuators, down positive */ switch (buttons) { case 4: - adj = 1.0; + adj = -1.0; axis = v_scroll_axis; break; case 5: - adj = -1.0; + adj = 1.0; axis = v_scroll_axis; break; case 6: - adj = 1.0; + adj = -1.0; axis = h_scroll_axis; break; case 7: - adj = -1.0; + adj = 1.0; axis = h_scroll_axis; break; default: From f4ca19ce3ab91a9c8ad9de60f7dc95466f21f589 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Fri, 29 Jul 2011 10:56:44 +1000 Subject: [PATCH 06/16] dix: copy the source ID into the RawEvent (#34240) X.Org Bug 34240 Signed-off-by: Peter Hutterer Reviewed-by: Daniel Stone --- dix/eventconvert.c | 1 + 1 file changed, 1 insertion(+) diff --git a/dix/eventconvert.c b/dix/eventconvert.c index f9aafa5d1..189cb85d0 100644 --- a/dix/eventconvert.c +++ b/dix/eventconvert.c @@ -667,6 +667,7 @@ eventToRawEvent(RawDeviceEvent *ev, xEvent **xi) raw->length = bytes_to_int32(len - sizeof(xEvent)); raw->detail = ev->detail.button; raw->deviceid = ev->deviceid; + raw->sourceid = ev->sourceid; raw->valuators_len = vallen; raw->flags = ev->flags; From 7074ec87bdf81699df172619aea7aae1ad4ec3c6 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Fri, 30 Sep 2011 10:47:00 +1000 Subject: [PATCH 07/16] dix: document transformAbsolute Signed-off-by: Peter Hutterer Reviewed-by: Jamey Sharp Reviewed-by: Daniel Stone --- dix/getevents.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/dix/getevents.c b/dix/getevents.c index 97c3937b9..e47835565 100644 --- a/dix/getevents.c +++ b/dix/getevents.c @@ -1020,6 +1020,14 @@ transform(struct pixman_f_transform *m, double *x, double *y) *y = p.v[1]; } +/** + * Apply the device's transformation matrix to the valuator mask and replace + * the scaled values in mask. This transformation only applies to valuators + * 0 and 1, others will be untouched. + * + * @param dev The device the valuators came from + * @param[in,out] mask The valuator mask. + */ static void transformAbsolute(DeviceIntPtr dev, ValuatorMask *mask) { From 959d18c3765e447897a8cfd358e9ee645df595d9 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Fri, 30 Sep 2011 10:50:51 +1000 Subject: [PATCH 08/16] dix: fix missing verb in comment Signed-off-by: Peter Hutterer Reviewed-by: Jamey Sharp Reviewed-by: Daniel Stone --- dix/getevents.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/dix/getevents.c b/dix/getevents.c index e47835565..cf82cbf71 100644 --- a/dix/getevents.c +++ b/dix/getevents.c @@ -760,8 +760,9 @@ accelPointer(DeviceIntPtr dev, ValuatorMask* valuators, CARD32 ms) * miPointerSetPosition() and then scale back into device coordinates (if * needed). miPSP will change x/y if the screen was crossed. * - * The coordinates provided are always absolute. The parameter mode whether - * it was relative or absolute movement that landed us at those coordinates. + * The coordinates provided are always absolute. The parameter mode + * specifies whether it was relative or absolute movement that landed us at + * those coordinates. * * @param dev The device to be moved. * @param mode Movement mode (Absolute or Relative) From b966362ccf0fe6fdd44f4d778d47e3677f55f11b Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Mon, 3 Oct 2011 12:18:20 +1000 Subject: [PATCH 09/16] dix: rename moveAbsolute to clipAbsolute Let's be honest about what it does. moveRelative accumulates delta _and_ clips in some cases, so that one can keep it's name. Signed-off-by: Peter Hutterer Reviewed-by: Jamey Sharp Reviewed-by: Daniel Stone --- dix/getevents.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dix/getevents.c b/dix/getevents.c index cf82cbf71..6a812fbed 100644 --- a/dix/getevents.c +++ b/dix/getevents.c @@ -693,7 +693,7 @@ UpdateFromMaster(InternalEvent* events, DeviceIntPtr dev, int type, int *num_eve * @param mask Valuator data for this event. */ static void -moveAbsolute(DeviceIntPtr dev, ValuatorMask *mask) +clipAbsolute(DeviceIntPtr dev, ValuatorMask *mask) { int i; @@ -1146,7 +1146,7 @@ fill_pointer_events(InternalEvent *events, DeviceIntPtr pDev, int type, } transformAbsolute(pDev, &mask); - moveAbsolute(pDev, &mask); + clipAbsolute(pDev, &mask); } else { if (flags & POINTER_ACCELERATE) accelPointer(pDev, &mask, ms); From bccff533184a051b614a26304ce77ad30bede5e0 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Mon, 3 Oct 2011 12:19:21 +1000 Subject: [PATCH 10/16] dix: moveRelative modifies parameter in-place, say so. Signed-off-by: Peter Hutterer Reviewed-by: Jamey Sharp Reviewed-by: Daniel Stone --- dix/getevents.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dix/getevents.c b/dix/getevents.c index 6a812fbed..992669313 100644 --- a/dix/getevents.c +++ b/dix/getevents.c @@ -713,7 +713,7 @@ clipAbsolute(DeviceIntPtr dev, ValuatorMask *mask) * Move the device's pointer by the values given in @valuators. * * @param dev The device whose pointer is to be moved. - * @param mask Valuator data for this event. + * @param[in,out] mask Valuator data for this event, modified in-place. */ static void moveRelative(DeviceIntPtr dev, ValuatorMask *mask) From b059e06e19ac9417ceeb8be58c1c91b159291865 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Mon, 29 Aug 2011 12:36:26 +1000 Subject: [PATCH 11/16] dix: don't allow keyboard devices to submit motion or button events. GPE unconditionally dereferences pDev->valuator if a mask is present. This shouldn't really happen but if it does, don't crash, just ignore the events with an error. Signed-off-by: Peter Hutterer Reviewed-by: Jamey Sharp Reviewed-by: Daniel Stone --- dix/getevents.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/dix/getevents.c b/dix/getevents.c index 992669313..98d8cf0f1 100644 --- a/dix/getevents.c +++ b/dix/getevents.c @@ -1099,6 +1099,11 @@ fill_pointer_events(InternalEvent *events, DeviceIntPtr pDev, int type, switch (type) { case MotionNotify: + if (!pDev->valuator) + { + ErrorF("[dix] motion events from device %d without valuators\n", pDev->id); + return 0; + } if (!mask_in || valuator_mask_num_valuators(mask_in) <= 0) return 0; break; @@ -1106,6 +1111,11 @@ fill_pointer_events(InternalEvent *events, DeviceIntPtr pDev, int type, case ButtonRelease: if (!pDev->button || !buttons) return 0; + if (mask_in && valuator_mask_size(mask_in) > 0 && !pDev->valuator) + { + ErrorF("[dix] button event with valuator from device %d without valuators\n", pDev->id); + return 0; + } break; default: return 0; From 967bc25da221a69c8fc390253465145ce534fcb9 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Mon, 3 Oct 2011 11:42:08 +1000 Subject: [PATCH 12/16] dix: move screen- to device coordinate scaling to separate function No functional changes. Signed-off-by: Peter Hutterer Reviewed-by: Jamey Sharp Reviewed-by: Daniel Stone --- dix/getevents.c | 50 +++++++++++++++++++++++++++++++------------------ 1 file changed, 32 insertions(+), 18 deletions(-) diff --git a/dix/getevents.c b/dix/getevents.c index 98d8cf0f1..3ef7a5cc5 100644 --- a/dix/getevents.c +++ b/dix/getevents.c @@ -752,6 +752,37 @@ accelPointer(DeviceIntPtr dev, ValuatorMask* valuators, CARD32 ms) dev->valuator->accelScheme.AccelSchemeProc(dev, valuators, ms); } +/** + * Scale from absolute screen coordinates to absolute coordinates in the + * 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. + */ +static void +scale_from_screen(DeviceIntPtr dev, ValuatorMask *mask) +{ + double scaled; + ScreenPtr scr = miPointerGetScreen(dev); + + if (valuator_mask_isset(mask, 0)) + { + scaled = rescaleValuatorAxis(valuator_mask_get_double(mask, 0), + NULL, dev->valuator->axes + 0, + scr->width); + valuator_mask_set_double(mask, 0, scaled); + } + if (valuator_mask_isset(mask, 1)) + { + scaled = rescaleValuatorAxis(valuator_mask_get_double(mask, 1), + NULL, dev->valuator->axes + 1, + scr->height); + valuator_mask_set_double(mask, 1, scaled); + } +} + + /** * If we have HW cursors, this actually moves the visible sprite. If not, we * just do all the screen crossing, etc. @@ -1136,24 +1167,7 @@ fill_pointer_events(InternalEvent *events, DeviceIntPtr pDev, int type, if (flags & POINTER_ABSOLUTE) { if (flags & POINTER_SCREEN) /* valuators are in screen coords */ - { - double scaled; - - if (valuator_mask_isset(&mask, 0)) - { - scaled = rescaleValuatorAxis(valuator_mask_get_double(&mask, 0), - NULL, pDev->valuator->axes + 0, - scr->width); - valuator_mask_set_double(&mask, 0, scaled); - } - if (valuator_mask_isset(&mask, 1)) - { - scaled = rescaleValuatorAxis(valuator_mask_get_double(&mask, 1), - NULL, pDev->valuator->axes + 1, - scr->height); - valuator_mask_set_double(&mask, 1, scaled); - } - } + scale_from_screen(pDev, &mask); transformAbsolute(pDev, &mask); clipAbsolute(pDev, &mask); From 88dfe5366d9855e0ebf8bbff74967b793ede57d1 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Mon, 3 Oct 2011 12:37:28 +1000 Subject: [PATCH 13/16] dix: drop screen argument from positionSprite We can just get this in the function, no effective functional changes. Also return the screen to the caller. Though we don't use it yet, we will in a follow-up patch. Signed-off-by: Peter Hutterer Reviewed-by: Jamey Sharp Reviewed-by: Daniel Stone --- dix/getevents.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/dix/getevents.c b/dix/getevents.c index 3ef7a5cc5..4206ca9e1 100644 --- a/dix/getevents.c +++ b/dix/getevents.c @@ -797,20 +797,20 @@ scale_from_screen(DeviceIntPtr dev, ValuatorMask *mask) * * @param dev The device to be moved. * @param mode Movement mode (Absolute or Relative) - * @param scr Screen the device's sprite is currently on. * @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. */ -static void -positionSprite(DeviceIntPtr dev, int mode, ScreenPtr scr, ValuatorMask *mask, +static ScreenPtr +positionSprite(DeviceIntPtr dev, int mode, ValuatorMask *mask, double *screenx, double *screeny) { int isx, isy; /* screen {x, y}, in int */ double x, y; + ScreenPtr scr = miPointerGetScreen(dev); if (!dev->valuator || dev->valuator->numAxes < 2) - return; + return scr; if (valuator_mask_isset(mask, 0)) x = valuator_mask_get_double(mask, 0); @@ -859,6 +859,8 @@ positionSprite(DeviceIntPtr dev, int mode, ScreenPtr scr, ValuatorMask *mask, valuator_mask_set_double(mask, 0, x); if (valuator_mask_isset(mask, 1)) valuator_mask_set_double(mask, 1, y); + + return scr; } /** @@ -1124,7 +1126,6 @@ fill_pointer_events(InternalEvent *events, DeviceIntPtr pDev, int type, DeviceEvent *event; RawDeviceEvent *raw; double screenx = 0.0, screeny = 0.0; - ScreenPtr scr = miPointerGetScreen(pDev); ValuatorMask mask; switch (type) @@ -1180,7 +1181,7 @@ fill_pointer_events(InternalEvent *events, DeviceIntPtr pDev, int type, if ((flags & POINTER_NORAW) == 0) set_raw_valuators(raw, &mask, raw->valuators.data); - positionSprite(pDev, (flags & POINTER_ABSOLUTE) ? Absolute : Relative, scr, + positionSprite(pDev, (flags & POINTER_ABSOLUTE) ? Absolute : Relative, &mask, &screenx, &screeny); updateHistory(pDev, &mask, ms); From 81cfe44b1ed0de84ad1941fe2ca74bebef3fc58d Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Mon, 3 Oct 2011 12:49:49 +1000 Subject: [PATCH 14/16] mi: return the screen from miPointerSetPosition miPointerSetPosition may switch screens. Always return the screen the sprite is on instead of relying on callers to call miPointerGetScreen(). Signed-off-by: Peter Hutterer Reviewed-by: Jamey Sharp Reviewed-by: Daniel Stone --- dix/getevents.c | 3 +-- mi/mipointer.c | 14 +++++++------- mi/mipointer.h | 2 +- 3 files changed, 9 insertions(+), 10 deletions(-) diff --git a/dix/getevents.c b/dix/getevents.c index 4206ca9e1..bb1f5c96d 100644 --- a/dix/getevents.c +++ b/dix/getevents.c @@ -833,8 +833,7 @@ positionSprite(DeviceIntPtr dev, int mode, ValuatorMask *mask, * screenx back into device co-ordinates. */ isx = trunc(*screenx); isy = trunc(*screeny); - miPointerSetPosition(dev, mode, &isx, &isy); - scr = miPointerGetScreen(dev); + scr = miPointerSetPosition(dev, mode, &isx, &isy); if (isx != trunc(*screenx)) { *screenx -= trunc(*screenx) - isx; diff --git a/mi/mipointer.c b/mi/mipointer.c index 670f63b6e..4901d139b 100644 --- a/mi/mipointer.c +++ b/mi/mipointer.c @@ -574,7 +574,7 @@ miPointerMoveNoEvent (DeviceIntPtr pDev, ScreenPtr pScreen, * @param[in,out] y The y coordinate in screen coordinates (in regards to total * desktop size) */ -void +ScreenPtr miPointerSetPosition(DeviceIntPtr pDev, int mode, int *x, int *y) { miPointerScreenPtr pScreenPriv; @@ -584,12 +584,12 @@ miPointerSetPosition(DeviceIntPtr pDev, int mode, int *x, int *y) miPointerPtr pPointer; if (!pDev || !pDev->coreEvents) - return; + return NULL; pPointer = MIPOINTER(pDev); pScreen = pPointer->pScreen; if (!pScreen) - return; /* called before ready */ + return NULL; /* called before ready */ if (*x < 0 || *x >= pScreen->width || *y < 0 || *y >= pScreen->height) { @@ -622,11 +622,11 @@ miPointerSetPosition(DeviceIntPtr pDev, int mode, int *x, int *y) if (pScreen->ConstrainCursorHarder) pScreen->ConstrainCursorHarder(pDev, pScreen, mode, x, y); - if (pPointer->x == *x && pPointer->y == *y && - pPointer->pScreen == pScreen) - return; + if (pPointer->x != *x || pPointer->y != *y || + pPointer->pScreen != pScreen) + miPointerMoveNoEvent(pDev, pScreen, *x, *y); - miPointerMoveNoEvent(pDev, pScreen, *x, *y); + return pScreen; } /** diff --git a/mi/mipointer.h b/mi/mipointer.h index c4265f9d8..35428df32 100644 --- a/mi/mipointer.h +++ b/mi/mipointer.h @@ -131,7 +131,7 @@ extern _X_EXPORT void miPointerGetPosition( /* Moves the cursor to the specified position. May clip the co-ordinates: * x and y are modified in-place. */ -extern _X_EXPORT void miPointerSetPosition( +extern _X_EXPORT ScreenPtr miPointerSetPosition( DeviceIntPtr pDev, int mode, int *x, From 3b36fd1b49030ead44358945f62e5abe7f4609ce Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Mon, 3 Oct 2011 13:10:53 +1000 Subject: [PATCH 15/16] mi: switch miPointerSetPosition to take doubles Don't switch between doubles and ints in the caller, instead take doubles in miPointerSetPosition and do the conversion there. For full feature we should change everything down from here for doubles too. Functional change: previously we'd restore the remainder regardless of screen switching/confinement (despite what the comment said). Now, screen changing or cursor constraints will cause the remainder be clipped off. This should happen for cursor constraints but arguably not for screen crossing. This also corrects a currently wrong comment about miPointerSetPosition's input coordinates. Signed-off-by: Peter Hutterer Reviewed-by: Daniel Stone --- dix/getevents.c | 27 ++++++++++++--------------- mi/mipointer.c | 47 +++++++++++++++++++++++++++++------------------ mi/mipointer.h | 4 ++-- 3 files changed, 43 insertions(+), 35 deletions(-) diff --git a/dix/getevents.c b/dix/getevents.c index bb1f5c96d..ade3ca132 100644 --- a/dix/getevents.c +++ b/dix/getevents.c @@ -805,8 +805,8 @@ static ScreenPtr positionSprite(DeviceIntPtr dev, int mode, ValuatorMask *mask, double *screenx, double *screeny) { - int isx, isy; /* screen {x, y}, in int */ double x, y; + double tmpx, tmpy; ScreenPtr scr = miPointerGetScreen(dev); if (!dev->valuator || dev->valuator->numAxes < 2) @@ -827,25 +827,22 @@ positionSprite(DeviceIntPtr dev, int mode, ValuatorMask *mask, *screeny = rescaleValuatorAxis(y, dev->valuator->axes + 1, NULL, scr->height); + tmpx = *screenx; + tmpy = *screeny; /* miPointerSetPosition takes care of crossing screens for us, as well as - * clipping to the current screen. In the event we actually change screen, - * we just drop the float component on the floor, then convert from - * screenx back into device co-ordinates. */ - isx = trunc(*screenx); - isy = trunc(*screeny); - scr = miPointerSetPosition(dev, mode, &isx, &isy); - if (isx != trunc(*screenx)) - { - *screenx -= trunc(*screenx) - isx; + * clipping to the current screen. */ + scr = miPointerSetPosition(dev, mode, screenx, screeny); + + /* If we were constrained, rescale x/y from the screen coordinates so + * the device valuators reflect the correct position. For screen + * crossing this doesn't matter much, the coords would be 0 or max. + */ + if (tmpx != *screenx) x = rescaleValuatorAxis(*screenx, NULL, dev->valuator->axes + 0, scr->width); - } - if (isy != trunc(*screeny)) - { - *screeny -= trunc(*screeny) - isy; + if (tmpy != *screeny) y = rescaleValuatorAxis(*screeny, NULL, dev->valuator->axes + 1, scr->height); - } /* Update the MD's co-ordinates, which are always in screen space. */ if (!IsMaster(dev) || !IsFloating(dev)) { diff --git a/mi/mipointer.c b/mi/mipointer.c index 4901d139b..55e4081f2 100644 --- a/mi/mipointer.c +++ b/mi/mipointer.c @@ -569,17 +569,16 @@ miPointerMoveNoEvent (DeviceIntPtr pDev, ScreenPtr pScreen, * * @param pDev The device to move * @param mode Movement mode (Absolute or Relative) - * @param[in,out] x The x coordinate in screen coordinates (in regards to total - * desktop size) - * @param[in,out] y The y coordinate in screen coordinates (in regards to total - * desktop size) + * @param[in,out] screenx The x coordinate in screen coordinates + * @param[in,out] screeny The y coordinate in screen coordinates */ ScreenPtr -miPointerSetPosition(DeviceIntPtr pDev, int mode, int *x, int *y) +miPointerSetPosition(DeviceIntPtr pDev, int mode, double *screenx, double *screeny) { miPointerScreenPtr pScreenPriv; ScreenPtr pScreen; ScreenPtr newScreen; + int x, y; miPointerPtr pPointer; @@ -591,13 +590,16 @@ miPointerSetPosition(DeviceIntPtr pDev, int mode, int *x, int *y) if (!pScreen) return NULL; /* called before ready */ - if (*x < 0 || *x >= pScreen->width || *y < 0 || *y >= pScreen->height) + x = trunc(*screenx); + y = trunc(*screeny); + + if (x < 0 || x >= pScreen->width || y < 0 || y >= pScreen->height) { pScreenPriv = GetScreenPrivate (pScreen); if (!pPointer->confined) { newScreen = pScreen; - (*pScreenPriv->screenFuncs->CursorOffScreen) (&newScreen, x, y); + (*pScreenPriv->screenFuncs->CursorOffScreen) (&newScreen, &x, &y); if (newScreen != pScreen) { pScreen = newScreen; @@ -610,21 +612,30 @@ miPointerSetPosition(DeviceIntPtr pDev, int mode, int *x, int *y) } } /* Constrain the sprite to the current limits. */ - if (*x < pPointer->limits.x1) - *x = pPointer->limits.x1; - if (*x >= pPointer->limits.x2) - *x = pPointer->limits.x2 - 1; - if (*y < pPointer->limits.y1) - *y = pPointer->limits.y1; - if (*y >= pPointer->limits.y2) - *y = pPointer->limits.y2 - 1; + if (x < pPointer->limits.x1) + x = pPointer->limits.x1; + if (x >= pPointer->limits.x2) + x = pPointer->limits.x2 - 1; + if (y < pPointer->limits.y1) + y = pPointer->limits.y1; + if (y >= pPointer->limits.y2) + y = pPointer->limits.y2 - 1; if (pScreen->ConstrainCursorHarder) - pScreen->ConstrainCursorHarder(pDev, pScreen, mode, x, y); + pScreen->ConstrainCursorHarder(pDev, pScreen, mode, &x, &y); - if (pPointer->x != *x || pPointer->y != *y || + if (pPointer->x != x || pPointer->y != y || pPointer->pScreen != pScreen) - miPointerMoveNoEvent(pDev, pScreen, *x, *y); + miPointerMoveNoEvent(pDev, pScreen, x, 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 + * crossings */ + if (x != trunc(*screenx)) + *screenx = x; + if (y != trunc(*screeny)) + *screeny = y; return pScreen; } diff --git a/mi/mipointer.h b/mi/mipointer.h index 35428df32..45abb5b56 100644 --- a/mi/mipointer.h +++ b/mi/mipointer.h @@ -134,8 +134,8 @@ extern _X_EXPORT void miPointerGetPosition( extern _X_EXPORT ScreenPtr miPointerSetPosition( DeviceIntPtr pDev, int mode, - int *x, - int *y); + double *x, + double *y); extern _X_EXPORT void miPointerUpdateSprite( DeviceIntPtr pDev); From 32b289e46cc2d5ec32ff0c4ba5bbfbf602afb388 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Mon, 3 Oct 2011 13:58:01 +1000 Subject: [PATCH 16/16] dix: move MD last.valuator update into fill_pointer_events Don't update the MD where it's not expected, positionSprite should really just do that - position the sprite. Signed-off-by: Peter Hutterer Reviewed-by: Daniel Stone --- dix/getevents.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/dix/getevents.c b/dix/getevents.c index ade3ca132..874189f57 100644 --- a/dix/getevents.c +++ b/dix/getevents.c @@ -844,12 +844,6 @@ positionSprite(DeviceIntPtr dev, int mode, ValuatorMask *mask, y = rescaleValuatorAxis(*screeny, NULL, dev->valuator->axes + 1, scr->height); - /* Update the MD's co-ordinates, which are always in screen space. */ - if (!IsMaster(dev) || !IsFloating(dev)) { - DeviceIntPtr master = GetMaster(dev, MASTER_POINTER); - master->last.valuators[0] = *screenx; - master->last.valuators[1] = *screeny; - } if (valuator_mask_isset(mask, 0)) valuator_mask_set_double(mask, 0, x); @@ -1189,6 +1183,13 @@ fill_pointer_events(InternalEvent *events, DeviceIntPtr pDev, int type, pDev->last.valuators[i] = valuator_mask_get_double(&mask, i); } + /* Update the MD's co-ordinates, which are always in screen space. */ + if (!IsMaster(pDev) || !IsFloating(pDev)) { + DeviceIntPtr master = GetMaster(pDev, MASTER_POINTER); + master->last.valuators[0] = screenx; + master->last.valuators[1] = screeny; + } + event = &events->device_event; init_device_event(event, pDev, ms);