The resolution of wacom touch screen devices is different from the screen resolution, which results in incorrect pointer movement as described in [1] and [2]. This is caused by calls to GetTouchEvents during touch event processing, which has unintended side effects such as moving the sprite or setting the last position valuators. The touch code calls GetTouchEvents in two places: (1) When replaying events and (2) when generating TouchEnd events that don't correspond to a user action. In case (1) the only purpose of this call is to generate a possible DCCE event. But since GetTouchEvents operates under the assumption that the device is a slave device, no event is generated for master devices. Patch 3 factors out the generation of the DCCE event (fixing the issue just described) and then replays the actual event. A remaining problem is that the DCCE event is passed to DeliverTouchEvents, which ignores the event as far as I can tell. In case (2), a touch event needs to be generated from a TouchPointInfo. GetTouchEvents already has lots code conditional on whether it is called from the driver or from the touch processing code, which could be extended further to handle this case correctly, but at this point, it seems simpler to split it into two separate functions, which is what patch 4 does.
I've been testing these patches for the last few days and I haven't noticed any additional issues. Tom [1] https://bugs.freedesktop.org/show_bug.cgi?id=55477 [2] http://lists.x.org/archives/xorg-devel/2012-October/033914.html
>From ba12ba280990a89e85a2e2516f855f4fca42956e Mon Sep 17 00:00:00 2001 From: Thomas Jaeger <[email protected]> Date: Sat, 13 Oct 2012 22:39:27 -0400 Subject: [PATCH 1/5] remove init_event The function is identical to init_device_event from inpututils.c with the first two arguments swapped. Signed-off-by: Thomas Jaeger <[email protected]> --- dix/getevents.c | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/dix/getevents.c b/dix/getevents.c index 71d83c4..492579d 100644 --- a/dix/getevents.c +++ b/dix/getevents.c @@ -164,17 +164,6 @@ key_autorepeats(DeviceIntPtr pDev, int key_code) } static void -init_event(DeviceIntPtr dev, DeviceEvent *event, Time ms) -{ - memset(event, 0, sizeof(DeviceEvent)); - event->header = ET_Internal; - event->length = sizeof(DeviceEvent); - event->time = ms; - event->deviceid = dev->id; - event->sourceid = dev->id; -} - -static void init_touch_ownership(DeviceIntPtr dev, TouchOwnershipEvent *event, Time ms) { memset(event, 0, sizeof(TouchOwnershipEvent)); @@ -1914,7 +1903,7 @@ GetTouchEvents(InternalEvent *events, DeviceIntPtr dev, uint32_t ddx_touchid, event = &events->device_event; num_events++; - init_event(dev, event, ms); + init_device_event(event, dev, ms); /* if submitted for master device, get the sourceid from there */ if (flags & TOUCH_CLIENT_ID) { event->sourceid = touchpoint.dix_ti->sourceid; -- 1.7.10.4
>From 975304f3fe106a79c7834daba409b9481320ca2f Mon Sep 17 00:00:00 2001 From: Thomas Jaeger <[email protected]> Date: Sat, 13 Oct 2012 22:43:26 -0400 Subject: [PATCH 2/5] Update the MD's position when a touch event is received Signed-off-by: Thomas Jaeger <[email protected]> --- dix/getevents.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/dix/getevents.c b/dix/getevents.c index 492579d..6c592ec 100644 --- a/dix/getevents.c +++ b/dix/getevents.c @@ -1993,6 +1993,14 @@ GetTouchEvents(InternalEvent *events, DeviceIntPtr dev, uint32_t ddx_touchid, if (emulate_pointer) storeLastValuators(dev, &mask, 0, 1, devx, devy); + /* Update the MD's co-ordinates, which are always in desktop space. */ + if (emulate_pointer && !IsMaster(dev) && !IsFloating(dev)) { + DeviceIntPtr master = GetMaster(dev, MASTER_POINTER); + + master->last.valuators[0] = screenx; + master->last.valuators[1] = screeny; + } + event->root = scr->root->drawable.id; event_set_root_coordinates(event, screenx, screeny); -- 1.7.10.4
>From 287a2a6495c039147b4449ca66672dbb4d60ad68 Mon Sep 17 00:00:00 2001 From: Thomas Jaeger <[email protected]> Date: Sat, 13 Oct 2012 22:51:24 -0400 Subject: [PATCH 3/5] Don't use GetTouchEvents when replaying events GetTouchEvents has plenty of side effects such as moving the pointer or updating the master device, which we don't want to happen when replaying. The only reason for calling it was to generate a DCCE event, but GetTouchEvents doesn't even do that right (we might need a DCCE event even when replaying a master event, or clients could interpret valuator data incorrectly). This discussion is moot at the moment anyway, since DeliverTouchEvents doesn't appear to deliver DCCE events. Signed-off-by: Thomas Jaeger <[email protected]> --- dix/touch.c | 61 ++++++++++++++++++++++++------------------------------- include/input.h | 3 +++ 2 files changed, 30 insertions(+), 34 deletions(-) diff --git a/dix/touch.c b/dix/touch.c index 497ad7d..34ed240 100644 --- a/dix/touch.c +++ b/dix/touch.c @@ -463,45 +463,14 @@ TouchEventHistoryPush(TouchPointInfoPtr ti, const DeviceEvent *ev) void TouchEventHistoryReplay(TouchPointInfoPtr ti, DeviceIntPtr dev, XID resource) { - InternalEvent *tel; - ValuatorMask *mask; - int i, nev; - int flags; + int i; if (!ti->history) return; - tel = InitEventList(GetMaximumEventsNum()); - mask = valuator_mask_new(0); - - valuator_mask_set_double(mask, 0, ti->history[0].valuators.data[0]); - valuator_mask_set_double(mask, 1, ti->history[0].valuators.data[1]); - - flags = TOUCH_CLIENT_ID | TOUCH_REPLAYING; - if (ti->emulate_pointer) - flags |= TOUCH_POINTER_EMULATED; - /* Generate events based on a fake touch begin event to get DCCE events if - * needed */ - /* FIXME: This needs to be cleaned up */ - nev = GetTouchEvents(tel, dev, ti->client_id, XI_TouchBegin, flags, mask); - for (i = 0; i < nev; i++) { - /* Send saved touch begin event */ - if (tel[i].any.type == ET_TouchBegin) { - DeviceEvent *ev = &ti->history[0]; - ev->flags |= TOUCH_REPLAYING; - DeliverTouchEvents(dev, ti, (InternalEvent*)ev, resource); - } - else {/* Send DCCE event */ - tel[i].any.time = ti->history[0].time; - DeliverTouchEvents(dev, ti, tel + i, resource); - } - } - - valuator_mask_free(&mask); - FreeEventList(tel, GetMaximumEventsNum()); + TouchDeliverDeviceClassesChangedEvent(ti, ti->history[0].time, resource); - /* First event was TouchBegin, already replayed that one */ - for (i = 1; i < ti->history_elements; i++) { + for (i = 0; i < ti->history_elements; i++) { DeviceEvent *ev = &ti->history[i]; ev->flags |= TOUCH_REPLAYING; @@ -509,6 +478,30 @@ TouchEventHistoryReplay(TouchPointInfoPtr ti, DeviceIntPtr dev, XID resource) } } +void +TouchDeliverDeviceClassesChangedEvent(TouchPointInfoPtr ti, Time time, + XID resource) +{ + DeviceIntPtr dev; + int num_events = 0; + InternalEvent dcce; + + dixLookupDevice(&dev, ti->sourceid, serverClient, DixWriteAccess); + + if (!dev) + return; + + /* UpdateFromMaster generates at most one event */ + UpdateFromMaster(&dcce, dev, DEVCHANGE_POINTER_EVENT, &num_events); + BUG_WARN(num_events > 1); + + if (num_events) { + dcce.any.time = time; + // FIXME: This doesn't do anything! + DeliverTouchEvents(dev, ti, &dcce, resource); + } +} + Bool TouchBuildDependentSpriteTrace(DeviceIntPtr dev, SpritePtr sprite) { diff --git a/include/input.h b/include/input.h index 5747f3c..6d46f5d 100644 --- a/include/input.h +++ b/include/input.h @@ -579,6 +579,9 @@ extern int TouchListenerAcceptReject(DeviceIntPtr dev, TouchPointInfoPtr ti, int listener, int mode); extern int TouchAcceptReject(ClientPtr client, DeviceIntPtr dev, int mode, uint32_t touchid, Window grab_window, XID *error); +extern void TouchDeliverDeviceClassesChangedEvent(TouchPointInfoPtr ti, + Time time, XID resource); + /* misc event helpers */ extern Mask GetEventMask(DeviceIntPtr dev, xEvent *ev, InputClientsPtr clients); -- 1.7.10.4
>From 638754745e9e14b657a730242df3ebf632ca8f85 Mon Sep 17 00:00:00 2001 From: Thomas Jaeger <[email protected]> Date: Sat, 13 Oct 2012 23:08:27 -0400 Subject: [PATCH 4/5] Don't use GetTouchEvents in EmitTouchEnd As before GetTouchEvents causes unwanted side effects. Add a new function GetDixTouchEnd, which generates a touch event from the touch point. We fill in the event's screen coordinates from the MD's current sprite position. Signed-off-by: Thomas Jaeger <[email protected]> --- Xi/exevents.c | 18 ++++-------------- dix/getevents.c | 31 +++++++++++++++++++++++++++++++ include/input.h | 5 +++++ 3 files changed, 40 insertions(+), 14 deletions(-) diff --git a/Xi/exevents.c b/Xi/exevents.c index 4cbeb37..dea536f 100644 --- a/Xi/exevents.c +++ b/Xi/exevents.c @@ -1066,24 +1066,14 @@ ActivateEarlyAccept(DeviceIntPtr dev, TouchPointInfoPtr ti) static void EmitTouchEnd(DeviceIntPtr dev, TouchPointInfoPtr ti, int flags, XID resource) { - InternalEvent *tel = InitEventList(GetMaximumEventsNum()); - ValuatorMask *mask = valuator_mask_new(2); - int i, nev; - - valuator_mask_set_double(mask, 0, - valuator_mask_get_double(ti->valuators, 0)); - valuator_mask_set_double(mask, 1, - valuator_mask_get_double(ti->valuators, 1)); + InternalEvent event; flags |= TOUCH_CLIENT_ID; if (ti->emulate_pointer) flags |= TOUCH_POINTER_EMULATED; - nev = GetTouchEvents(tel, dev, ti->client_id, XI_TouchEnd, flags, mask); - for (i = 0; i < nev; i++) - DeliverTouchEvents(dev, ti, tel + i, resource); - - valuator_mask_free(&mask); - FreeEventList(tel, GetMaximumEventsNum()); + TouchDeliverDeviceClassesChangedEvent(ti, GetTimeInMillis(), resource); + GetDixTouchEnd(&event, dev, ti, flags); + DeliverTouchEvents(dev, ti, &event, resource); } /** diff --git a/dix/getevents.c b/dix/getevents.c index 6c592ec..c48bbdb 100644 --- a/dix/getevents.c +++ b/dix/getevents.c @@ -2021,6 +2021,37 @@ GetTouchEvents(InternalEvent *events, DeviceIntPtr dev, uint32_t ddx_touchid, return num_events; } +void +GetDixTouchEnd(InternalEvent *ievent, DeviceIntPtr dev, TouchPointInfoPtr ti, + uint32_t flags) +{ + ScreenPtr scr = dev->spriteInfo->sprite->hotPhys.pScreen; + DeviceEvent *event = &ievent->device_event; + CARD32 ms = GetTimeInMillis(); + + BUG_WARN(!dev->enabled); + + init_device_event(event, dev, ms); + + event->sourceid = ti->sourceid; + event->type = ET_TouchEnd; + + event->root = scr->root->drawable.id; + + /* Get screen event coordinates from the sprite. Is this really the best + * we can do? */ + event_set_root_coordinates(event, + dev->spriteInfo->sprite->hotPhys.x, + dev->spriteInfo->sprite->hotPhys.y); + event->touchid = ti->client_id; + event->flags = flags; + + if (flags & TOUCH_POINTER_EMULATED) { + event->flags |= TOUCH_POINTER_EMULATED; + event->detail.button = 1; + } +} + /** * Synthesize a single motion event for the core pointer. * diff --git a/include/input.h b/include/input.h index 6d46f5d..3ad2cb3 100644 --- a/include/input.h +++ b/include/input.h @@ -465,6 +465,11 @@ extern int GetTouchOwnershipEvents(InternalEvent *events, TouchPointInfoPtr ti, uint8_t mode, XID resource, uint32_t flags); +extern void GetDixTouchEnd(InternalEvent *ievent, + DeviceIntPtr dev, + TouchPointInfoPtr ti, + uint32_t flags); + extern _X_EXPORT int GetProximityEvents(InternalEvent *events, DeviceIntPtr pDev, int type, const ValuatorMask *mask); -- 1.7.10.4
>From 590410e453bf952bd5a20089072793029eedef12 Mon Sep 17 00:00:00 2001 From: Thomas Jaeger <[email protected]> Date: Sat, 13 Oct 2012 23:18:50 -0400 Subject: [PATCH 5/5] Simplify GetTouchEvents With only one callee left, we are free to assume that !(flags & TOUCH_CLIENT_ID) Signed-off-by: Thomas Jaeger <[email protected]> --- dix/getevents.c | 66 ++++++++++++++----------------------------------------- 1 file changed, 17 insertions(+), 49 deletions(-) diff --git a/dix/getevents.c b/dix/getevents.c index c48bbdb..7d42f56 100644 --- a/dix/getevents.c +++ b/dix/getevents.c @@ -1832,10 +1832,7 @@ GetTouchEvents(InternalEvent *events, DeviceIntPtr dev, uint32_t ddx_touchid, int i; int num_events = 0; RawDeviceEvent *raw; - union touch { - TouchPointInfoPtr dix_ti; - DDXTouchPointInfoPtr ti; - } touchpoint; + DDXTouchPointInfoPtr ti; int need_rawevent = TRUE; Bool emulate_pointer = FALSE; int client_id = 0; @@ -1854,37 +1851,15 @@ GetTouchEvents(InternalEvent *events, DeviceIntPtr dev, uint32_t ddx_touchid, /* Find and/or create the DDX touch info */ - if (flags & TOUCH_CLIENT_ID) { /* A DIX-submitted TouchEnd */ - touchpoint.dix_ti = TouchFindByClientID(dev, ddx_touchid); - BUG_RETURN_VAL(!touchpoint.dix_ti, 0); - - if (!mask_in || - !valuator_mask_isset(mask_in, 0) || - !valuator_mask_isset(mask_in, 1)) { - ErrorF - ("[dix] dix-submitted events must have x/y valuator information.\n"); - return 0; - } - - need_rawevent = FALSE; - client_id = touchpoint.dix_ti->client_id; - } - else { /* a DDX-submitted touch */ - - touchpoint.ti = - TouchFindByDDXID(dev, ddx_touchid, (type == XI_TouchBegin)); - if (!touchpoint.ti) { - ErrorFSigSafe("[dix] %s: unable to %s touch point %u\n", dev->name, - type == XI_TouchBegin ? "begin" : "find", ddx_touchid); - return 0; - } - client_id = touchpoint.ti->client_id; + ti = TouchFindByDDXID(dev, ddx_touchid, (type == XI_TouchBegin)); + if (!ti) { + ErrorFSigSafe("[dix] %s: unable to %s touch point %u\n", dev->name, + type == XI_TouchBegin ? "begin" : "find", ddx_touchid); + return 0; } + client_id = ti->client_id; - if (!(flags & TOUCH_CLIENT_ID)) - emulate_pointer = touchpoint.ti->emulate_pointer; - else - emulate_pointer = ! !(flags & TOUCH_POINTER_EMULATED); + emulate_pointer = ti->emulate_pointer; if (!IsMaster(dev)) events = @@ -1904,11 +1879,6 @@ GetTouchEvents(InternalEvent *events, DeviceIntPtr dev, uint32_t ddx_touchid, num_events++; init_device_event(event, dev, ms); - /* if submitted for master device, get the sourceid from there */ - if (flags & TOUCH_CLIENT_ID) { - event->sourceid = touchpoint.dix_ti->sourceid; - /* TOUCH_CLIENT_ID implies norawevent */ - } switch (type) { case XI_TouchBegin: @@ -1933,20 +1903,20 @@ GetTouchEvents(InternalEvent *events, DeviceIntPtr dev, uint32_t ddx_touchid, event->type = ET_TouchEnd; /* We can end the DDX touch here, since we don't use the active * field below */ - if (!(flags & TOUCH_CLIENT_ID)) - TouchEndDDXTouch(dev, touchpoint.ti); + TouchEndDDXTouch(dev, ti); break; default: return 0; } - if (t->mode == XIDirectTouch && !(flags & TOUCH_CLIENT_ID)) { + + if (t->mode == XIDirectTouch) { if (!valuator_mask_isset(&mask, 0)) valuator_mask_set_double(&mask, 0, - valuator_mask_get_double(touchpoint.ti-> + valuator_mask_get_double(ti-> valuators, 0)); if (!valuator_mask_isset(&mask, 1)) valuator_mask_set_double(&mask, 1, - valuator_mask_get_double(touchpoint.ti-> + valuator_mask_get_double(ti-> valuators, 1)); } @@ -1956,13 +1926,11 @@ GetTouchEvents(InternalEvent *events, DeviceIntPtr dev, uint32_t ddx_touchid, if (t->mode == XIDirectTouch) { transformAbsolute(dev, &mask); - if (!(flags & TOUCH_CLIENT_ID)) { - for (i = 0; i < valuator_mask_size(&mask); i++) { - double val; + for (i = 0; i < valuator_mask_size(&mask); i++) { + double val; - if (valuator_mask_fetch_double(&mask, i, &val)) - valuator_mask_set_double(touchpoint.ti->valuators, i, val); - } + if (valuator_mask_fetch_double(&mask, i, &val)) + valuator_mask_set_double(ti->valuators, i, val); } clipAbsolute(dev, &mask); -- 1.7.10.4
_______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
