[PATCH 20/42] dix: handle DIX-submitted touch events
Peter Hutterer
peter.hutterer at who-t.net
Tue Dec 20 17:14:08 PST 2011
On Mon, Dec 19, 2011 at 06:33:06PM -0800, Chase Douglas wrote:
> On Dec 14, 2011, at 7:01 PM, Peter Hutterer <peter.hutterer at who-t.net> wrote:
> > The DIX needs to submit touch events for e.g. TouchEnd after an
> > acceptance/rejection. These have the TOUCH_CLIENT_ID flag set.
> >
> > Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> > ---
> > dix/getevents.c | 77 ++++++++++++++++++++++++++++++++++++++++--------------
> > 1 files changed, 57 insertions(+), 20 deletions(-)
> >
> > diff --git a/dix/getevents.c b/dix/getevents.c
> > index 4038309..819880c 100644
> > --- a/dix/getevents.c
> > +++ b/dix/getevents.c
> > @@ -1703,7 +1703,10 @@ GetTouchEvents(InternalEvent *events, DeviceIntPtr dev, uint32_t ddx_touchid,
> > int i;
> > int num_events = 0;
> > RawDeviceEvent *raw;
> > - DDXTouchPointInfoPtr ti;
> > + union touch {
> > + TouchPointInfoPtr dix_ti;
> > + DDXTouchPointInfoPtr ti;
> > + } touchpoint;
> > int need_rawevent = TRUE;
> > Bool emulate_pointer = FALSE;
> >
> > @@ -1711,15 +1714,39 @@ GetTouchEvents(InternalEvent *events, DeviceIntPtr dev, uint32_t ddx_touchid,
> > return 0;
> >
> > /* Find and/or create the DDX touch info */
> > - ti = TouchFindByDDXID(dev, ddx_touchid, (type == XI_TouchBegin));
> > - if (!ti)
> > +
> > + if (flags & TOUCH_CLIENT_ID) /* A DIX-submitted TouchEnd */
> > {
> > - ErrorF("[dix] %s: unable to %s touch point %x\n", dev->name,
> > - type == XI_TouchBegin ? "begin" : "find", ddx_touchid);
> > - return 0;
> > + touchpoint.dix_ti = TouchFindByClientID(dev, ddx_touchid);
> > + BUG_WARN(!touchpoint.dix_ti);
> > +
> > + if (!touchpoint.dix_ti)
> > + return 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;
> > + }
>
> I don't see why this is required. We're also inconsistent because we
> require x and y, but we don't copy any other valuators later on in this
> function for dix submitted events. If x and y are required, why not copy
> everything?
we mainly need x/y for sprite positioning and valuator scaling.
(abs events should have valuator information in any case, some (older)
clients can't easily deal with them) It's either requiring the DIX to submit
this information or adding hacks here to get the right data out.
There's a bit of history as well, when I added the requirement for x/y on
emulated TouchEnd events, GetTouchEvents didn't have access to
touchpoint.dix_ti->valuators yet. We can clean this up, but then we also
need the code to alloc/free mask_in here on demand. in short, we're mostly
just moving the code around and this is stuff I'd like to clean up later
when we got the features in and we're stabilising and polishing.
>
> > +
> > + need_rawevent = FALSE;
>
> Ahh, the answer to my question from patch 19 :).
>
> > + } else /* a DDX-submitted touch */
> > + {
> > + touchpoint.ti = TouchFindByDDXID(dev, ddx_touchid, (type == XI_TouchBegin));
> > + if (!touchpoint.ti)
> > + {
> > + ErrorF("[dix] %s: unable to %s touch point %x\n", dev->name,
> > + type == XI_TouchBegin ? "begin" : "find", ddx_touchid);
> > + return 0;
> > + }
> > }
> >
> > - emulate_pointer = ti->emulate_pointer;
> > + if (!(flags & TOUCH_CLIENT_ID))
> > + emulate_pointer = touchpoint.ti->emulate_pointer;
> > + else
> > + emulate_pointer = !!(flags & TOUCH_POINTER_EMULATED);
> >
> > if (!IsMaster(dev))
> > events = UpdateFromMaster(events, dev, DEVCHANGE_POINTER_EVENT, &num_events);
> > @@ -1731,7 +1758,7 @@ GetTouchEvents(InternalEvent *events, DeviceIntPtr dev, uint32_t ddx_touchid,
> > raw = &events->raw_event;
> > events++;
> > num_events++;
> > - init_raw(dev, raw, ms, type, ti->client_id);
> > + init_raw(dev, raw, ms, type, touchpoint.ti->client_id);
> > set_raw_valuators(raw, &mask, raw->valuators.data_raw);
> > }
> >
> > @@ -1739,6 +1766,12 @@ GetTouchEvents(InternalEvent *events, DeviceIntPtr dev, uint32_t ddx_touchid,
> > num_events++;
> >
> > init_event(dev, event, 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:
> > @@ -1765,17 +1798,19 @@ 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 */
> > - TouchEndDDXTouch(dev, ti);
> > + if (!(flags & TOUCH_CLIENT_ID))
> > + TouchEndDDXTouch(dev, touchpoint.ti);
> > break;
> > default:
> > return 0;
> > }
> > -
> > - if (!valuator_mask_isset(&mask, 0))
> > - valuator_mask_set_double(&mask, 0, valuator_mask_get_double(ti->valuators, 0));
> > - if (!valuator_mask_isset(&mask, 1))
> > - valuator_mask_set_double(&mask, 1, valuator_mask_get_double(ti->valuators, 1));
> > -
> > + if (!(flags & TOUCH_CLIENT_ID))
> > + {
> > + if (!valuator_mask_isset(&mask, 0))
> > + valuator_mask_set_double(&mask, 0, valuator_mask_get_double(touchpoint.ti->valuators, 0));
> > + if (!valuator_mask_isset(&mask, 1))
> > + valuator_mask_set_double(&mask, 1, valuator_mask_get_double(touchpoint.ti->valuators, 1));
> > + }
>
> Above, we guarantee that the x and y axes were set for dix submitted
> touches. It wouldn't be harmful to recheck if the valuators were set.
> Thus, imo it would be easier to read and understand without the check for
> a ddx submitted touch event here.
touchpoint.ti and touchpoint.dix_ti are different and we mustn't access the
latter for DDX-submitted touchpoints. Removing the condition relies to much
on the implicit assumption that 0/1 is always set for dix-submitted events.
or we force the two to be binary compatible. Right now they only need to be for
the client_id element and I've got a patch to remove requirement, it's a
leftover from earlier versions.
> > /* Get our screen event co-ordinates (root_x/root_y/event_x/event_y):
> > * these come from the touchpoint in Absolute mode, or the sprite in
> > @@ -1783,10 +1818,12 @@ GetTouchEvents(InternalEvent *events, DeviceIntPtr dev, uint32_t ddx_touchid,
> > if (t->mode == XIDirectTouch) {
> > transformAbsolute(dev, &mask);
> >
> > - for (i = 0; i < valuator_mask_size(&mask); i++) {
> > - if (valuator_mask_isset(&mask, i))
> > - valuator_mask_set_double(ti->valuators, i,
> > - valuator_mask_get_double(&mask, i));
> > + if (!(flags & TOUCH_CLIENT_ID)) {
> > + for (i = 0; i < valuator_mask_size(&mask); i++) {
> > + if (valuator_mask_isset(&mask, i))
> > + valuator_mask_set_double(touchpoint.ti->valuators, i,
> > + valuator_mask_get_double(&mask, i));
> > + }
>
> Why not copy the rest of the valuators, leaving the code a bit simpler to
> understand?
see above.
fwiw, this should use valuator_mask_fetch_double() for easier reading, I've
replaced this part with the code below.
diff --git a/dix/getevents.c b/dix/getevents.c
index 486c40a..b60ddc0 100644
--- a/dix/getevents.c
+++ b/dix/getevents.c
@@ -1849,9 +1849,9 @@ GetTouchEvents(InternalEvent *events, DeviceIntPtr dev, uint32_t ddx_touchid,
if (!(flags & TOUCH_CLIENT_ID)) {
for (i = 0; i < valuator_mask_size(&mask); i++) {
- if (valuator_mask_isset(&mask, i))
- valuator_mask_set_double(touchpoint.ti->valuators, i,
- valuator_mask_get_double(&mask, i));
+ double val;
+ if (valuator_mask_fetch_double(&mask, i, &val))
+ valuator_mask_set_double(touchpoint.ti->valuators, i, val);
}
}
Cheers,
Peter
More information about the xorg-devel
mailing list