Re: PVFB wheel events (z-axis)

[Markus Armbruster <armbru@redhat.com>]

Pat Campbell <plc@novell.com> 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@novell.com>
>>
>>     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.

[...]