Hi Chase! I thoroughly sympathize with removing code duplication, especially once it's gotten complicated like this, but please, no macro magic...
After trying a bunch of alternatives with GCC 4.4.5 on x86-64 to see whether it would generate sane code, it looks to me like the right answer is a getValuatorValue static function implementing the right-hand side of the assignment, and leaving the assignment itself written out explicitly in the switch. A less important complaint: It wasn't obvious to me on initial review that it was safe to remove the assignment "dev = NULL". It turns out that it is safe because dixLookupDevice ensures it's NULL on failure, and the loop only runs if num_valuators > 0, in which case dixLookupDevice has certainly been called first. But perhaps some short comment is in order? (It also took me a while to notice that there were no break statements in the switch, which led to a few minutes of wtfs...) Jamey On Mon, Mar 28, 2011 at 10:53 AM, Chase Douglas <[email protected]> wrote: > This allows for masked valuators to be handled properly in XI 1.x > events. Any unset valuators in the device event are set to the last > known value when transmitted on the wire through XI 1.x valuator events. > > Fixes https://bugs.launchpad.net/ubuntu/+source/xorg-server/+bug/736500 > > Signed-off-by: Chase Douglas <[email protected]> > --- > dix/eventconvert.c | 24 +++++++++++++----------- > 1 files changed, 13 insertions(+), 11 deletions(-) > > diff --git a/dix/eventconvert.c b/dix/eventconvert.c > index 9fce447..edecf6c 100644 > --- a/dix/eventconvert.c > +++ b/dix/eventconvert.c > @@ -358,27 +358,30 @@ countValuators(DeviceEvent *ev, int *first) > return num_valuators; > } > > -#define set_valuator_value(xv, ev, num) \ > - xv->valuator##num = (ev)->valuators.data[(xv)->first_valuator + num]; > +#define set_valuator_value(xv, ev, dev, num) \ > + if (BitIsOn((ev)->valuators.mask, (xv)->first_valuator + num)) \ > + xv->valuator##num = (ev)->valuators.data[(xv)->first_valuator + > num]; \ > + else \ > + xv->valuator##num = \ > + (dev)->valuator->axisVal[(xv)->first_valuator + num]; > static int > getValuatorEvents(DeviceEvent *ev, deviceValuator *xv) > { > int i; > int state = 0; > int first_valuator, num_valuators; > + DeviceIntPtr dev; > > > num_valuators = countValuators(ev, &first_valuator); > if (num_valuators > 0) > { > - DeviceIntPtr dev = NULL; > dixLookupDevice(&dev, ev->deviceid, serverClient, DixUseAccess); > /* State needs to be assembled BEFORE the device is updated. */ > state = (dev && dev->key) ? > XkbStateFieldFromRec(&dev->key->xkbInfo->state) : 0; > state |= (dev && dev->button) ? (dev->button->state) : 0; > } > > - /* FIXME: non-continuous valuator data in internal events*/ > for (i = 0; i < num_valuators; i += 6, xv++) { > xv->type = DeviceValuator; > xv->first_valuator = first_valuator + i; > @@ -388,17 +391,17 @@ getValuatorEvents(DeviceEvent *ev, deviceValuator *xv) > > switch (xv->num_valuators) { > case 6: > - set_valuator_value(xv, ev, 5); > + set_valuator_value(xv, ev, dev, 5); > case 5: > - set_valuator_value(xv, ev, 4); > + set_valuator_value(xv, ev, dev, 4); > case 4: > - set_valuator_value(xv, ev, 3); > + set_valuator_value(xv, ev, dev, 3); > case 3: > - set_valuator_value(xv, ev, 2); > + set_valuator_value(xv, ev, dev, 2); > case 2: > - set_valuator_value(xv, ev, 1); > + set_valuator_value(xv, ev, dev, 1); > case 1: > - set_valuator_value(xv, ev, 0); > + set_valuator_value(xv, ev, dev, 0); > } > > if (i + 6 < num_valuators) > @@ -407,7 +410,6 @@ getValuatorEvents(DeviceEvent *ev, deviceValuator *xv) > > return (num_valuators + 5) / 6; > } > -#undef set_valuator_value > > > static int > -- > 1.7.4.1 > > _______________________________________________ > [email protected]: X.Org development > Archives: http://lists.x.org/archives/xorg-devel > Info: http://lists.x.org/mailman/listinfo/xorg-devel > _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
