Pat Campbell <plc@xxxxxxxxxx> writes:
> Markus Armbruster wrote:
>> I got questions on this changeset:
>>
>> changeset: 354:c3ff0b26f664
>> date: Mon Dec 10 13:52:47 2007 +0000
>>
>> Decode mouse event packet dz value and passes it as a wheel event into
>> the input stream.
>>
>> Signed-off-by: Pat Campbell <plc@xxxxxxxxxx>
>>
>> diff -r 7232a025140f -r c3ff0b26f664 drivers/xen/fbfront/xenkbd.c
>> --- a/drivers/xen/fbfront/xenkbd.c Mon Dec 10 13:51:57 2007 +0000
>> +++ b/drivers/xen/fbfront/xenkbd.c Mon Dec 10 13:52:47 2007 +0000
>> @@ -64,8 +64,13 @@ static irqreturn_t input_handler(int rq,
>> dev = info->ptr;
>> switch (event->type) {
>> case XENKBD_TYPE_MOTION:
>> - input_report_rel(dev, REL_X,
>> event->motion.rel_x);
>> - input_report_rel(dev, REL_Y,
>> event->motion.rel_y);
>> + if ( event->motion.rel_z == 1 ||
>> event->motion.rel_z == -1 ) {
>> + input_report_rel(dev, REL_WHEEL, 0 -
>> event->motion.rel_z);
>> + }
>> + else {
>> + input_report_rel(dev, REL_X,
>> event->motion.rel_x);
>> + input_report_rel(dev, REL_Y,
>> event->motion.rel_y);
>> + }
>>
>> I don't understand the conditional. Why is rel_z to be used *only*
>> when it's 1 or -1, and why are rel_x and rel_y to be used *only* when
>> rel_z isn't? That sure is a weird protocol, and it isn't documented
>> anywhere...
>>
> In my testing I never saw a case where the rel_x and rel_y were not zero
> or the abs_x and abs_y changed when a z event came thru. A small
> attempt to not flood the input stream with redundant data.
Assuming conditions you observed in your tests will hold elsehwere in
space or time is calling for trouble :)
> Probably for clarity should have been: (same for the abs case)
> if (event->motion.rel_z != 0)
> input_report_rel(dev, REL_WHEEL, 0 - event->motion.rel_z);
> input_report_rel(dev, REL_X, event->motion.rel_x);
> input_report_rel(dev, REL_Y, event->motion.rel_y);
Why use REL_WHEEL and not REL_Z?
Why suppress zero Z-axis motion, but not X/Y-axis?
>> break;
>> case XENKBD_TYPE_KEY:
>> dev = NULL;
>> @@ -81,8 +86,13 @@ static irqreturn_t input_handler(int rq,
>> event->key.keycode);
>> break;
>> case XENKBD_TYPE_POS:
>> - input_report_abs(dev, ABS_X, event->pos.abs_x);
>> - input_report_abs(dev, ABS_Y, event->pos.abs_y);
>> + if ( event->pos.abs_z == 1 || event->pos.abs_z
>> == -1 ) {
>> + input_report_rel(dev, REL_WHEEL, 0 -
>> event->pos.abs_z);
>> + }
>> + else {
>> + input_report_abs(dev, ABS_X,
>> event->pos.abs_x);
>> + input_report_abs(dev, ABS_Y,
>> event->pos.abs_y);
>> + }
>>
>> And why isn't this using REL_ABS?
>>
> I assume you meant to ask why not ABS_WHEEL. Xorg 6.9 evdev driver does
Yes.
> not decode ABS_WHEEL. Xorg 7.3 decodes both REL and ABS WHEEL but
> ABS_WHEEL requires extra xorg.conf input options. We get greater
> coverage by using REL_WHEEL and reduce the need to edit xorg.conf.
Okay, that's fair.
[...]
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|