Re: [PATCH] Input: vmmouse - add macros to enable vmmouse relative mode

[Zack Rusin]

On Fri, 2023-04-14 at 16:58 +0800, zongmin zhou wrote:
> On Fri, 2023-04-14 at 01:22 +0000, Zack Rusin wrote:
> > On Thu, 2023-04-13 at 16:56 +0800, Zongmin Zhou wrote:
> > > Add macros to enable request relative mode.
> > >
> > > Change the REL_Y value passed by input_report_rel function,
> > > to match the direction of mouse movement.
> >
> > Thanks for the patch, but in its current form it's a nack. First of
> > all we don't
> > want any defines in the driver code that affect compilation, it's
> > never going to be
> > tested or compiled in. Either a kconfig or a module parameter would
> > be acceptable
> > but that's only if you can actually explain what it is that you're
> > fixing. The
> > current single line description just mentions what the effect it has
> > (not completely
> > correctly either because for merged packets absolute will still be
> > x=x, y=y, but
> > relative will be x += dx, y -= dy) but not why it's done, what it's
> > fixing and how
> > to reproduce.
> >
> > z
> >
> Dear zack:
>
> Firstly,thanks for your reply.
>
> The reason I want to add macros to request different vmmouse
> modes(relative or absolute) is that the vmmouse drivers currently only
> supports request absolute mode.But in some case we want request
> relative mode so that Pointer acceleration feature can be used.(as I
> know,libinput module only support Pointer acceleration feature in
> relative mode.)
> So I think we can provide two vmmouse modes to facilitate the use of
> different needs.
> If need,I can change it to a kconfig or a module parameter.
>
> The reasons of fix for REL_Y value，are as follows:
> When I request relative vmmouse mode,and let mouse move up,the mouse
> pointer moved down instead.
> Similarly, when I move the mouse down, the mouse pointer moved up.
> it obviously with a wrong motion direction in y.
>
> Actually，I understand that the value of y here is the end calculation
> result of relative coordinate movement，the real calculation is in
> motion_event() of spice-gtk and  legacy_mouse_event() of qemu.
>
> Test scenario:
> 1) start virtual machine with qemu command "vmport=on"，also with spice
> protocal.
> 2) modify guest vmmouse drivers to request relative mode.
> 3) move the mouse,will observe the pointer freezed,it's because driver
> not match the condition 'status & VMMOUSE_RELATIVE_PACKET',can't find
> correct input device.need merge this patch in qemu:
> https://lore.kernel.org/all/20230413081526.2229916-1-zhouzongmin@xxxxxxxxxx/
> 4) after merge the patch in qemu,we can observe the issue of wrong
> motion direction in y.

Sounds like you have a bug in the monitor code to me. The mouse might request
relative mode, but that doesn't mean that it's going to be switched, it's a hint. On
enable we definitely want to request the default absolute mode.

Ultimately it's the delivery mechanism (i.e. whether the delivered event is
VMMOUSE_RELATIVE_PACKET or not) that defines what an event is. This sounds to me
like the monitor delivers VMMOUSE_RELATIVE_PACKET events but doesn't mark them as
such. You can confirm by putting some debugging output in the vmmouse_report_events.

z