Re: [PATCH] hw/char: suppress sunmouse events with no changes

[Mark Cave-Ayland]

On 20/08/2024 08:34, Richard Henderson wrote:

> On 8/20/24 09:18, Carl Hauser wrote:
> > @@ -959,6 +960,15 @@ static void sunmouse_event(void *opaque,
> >       int ch;
> >
> >       trace_escc_sunmouse_event(dx, dy, buttons_state);
> > +
> > +    /* Don't send duplicate events without motion */
> > +    if (dx == 0 &&
> > +        dy == 0 &&
> > +        (s->sunmouse_prev_state ^ buttons_state) == 0) {
>
> Were you intending to mask vs MOUSE_EVENT_*BUTTON?
> Otherwise this is just plain equality.
>
> > diff --git a/include/hw/char/escc.h b/include/hw/char/escc.h
> > index 5669a5b811..bc5ba4f564 100644
> > --- a/include/hw/char/escc.h
> > +++ b/include/hw/char/escc.h
> > @@ -46,6 +46,7 @@ typedef struct ESCCChannelState {
> >       uint8_t rx, tx;
> >       QemuInputHandlerState *hs;
> >       char *sunkbd_layout;
> > +    int sunmouse_prev_state;
>
> This adds new state that must be migrated.
>
> While the patch is relatively simple, I do wonder if this code could be improved by 
> converting away from the legacy mouse interface to qemu_input_handler_register. 
> Especially if that might help avoid needing to add migration state that isn't 
> "really" part of the device.
>
> Mark?

Ooof I didn't even realise that qemu_add_mouse_event_handler() was legacy - is that 
documented anywhere at all?

At first glance (e.g. 
https://gitlab.com/qemu-project/qemu/-/blob/master/hw/input/ps2.c?ref_type=heads#L789) 
it appears that movement and button events are handled separately which I think would 
solve the problem.

I can try and put something together quickly for Carl to test and improve if that 
helps, although I'm quite tied up with client work and life in general right now(!).


ATB,

Mark.