[PATCH] adb-mouse: convert to use QemuInputHandler

Mark Cave-Ayland posted 1 patch 2 months, 2 weeks ago
There is a newer version of this series
hw/input/adb-mouse.c | 56 ++++++++++++++++++++++++++++++++++++--------
1 file changed, 46 insertions(+), 10 deletions(-)
[PATCH] adb-mouse: convert to use QemuInputHandler
Posted by Mark Cave-Ayland 2 months, 2 weeks ago
Update the ADB mouse implementation to use QemuInputHandler instead of the
legacy qemu_add_mouse_event_handler() function.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/input/adb-mouse.c | 56 ++++++++++++++++++++++++++++++++++++--------
 1 file changed, 46 insertions(+), 10 deletions(-)

diff --git a/hw/input/adb-mouse.c b/hw/input/adb-mouse.c
index 144a0ccce7..7aa36caf2f 100644
--- a/hw/input/adb-mouse.c
+++ b/hw/input/adb-mouse.c
@@ -38,6 +38,7 @@ struct MouseState {
     ADBDevice parent_obj;
     /*< private >*/
 
+    QemuInputHandlerState *hs;
     int buttons_state, last_buttons_state;
     int dx, dy, dz;
 };
@@ -51,17 +52,52 @@ struct ADBMouseClass {
     DeviceRealize parent_realize;
 };
 
-static void adb_mouse_event(void *opaque,
-                            int dx1, int dy1, int dz1, int buttons_state)
+#define ADB_MOUSE_BUTTON_LEFT   0x01
+#define ADB_MOUSE_BUTTON_RIGHT  0x02
+
+static void adb_mouse_handle_event(DeviceState *dev, QemuConsole *src,
+                                   InputEvent *evt)
 {
-    MouseState *s = opaque;
+    MouseState *s = (MouseState *)dev;
+    InputMoveEvent *move;
+    InputBtnEvent *btn;
+    static const int bmap[INPUT_BUTTON__MAX] = {
+        [INPUT_BUTTON_LEFT]   = ADB_MOUSE_BUTTON_LEFT,
+        [INPUT_BUTTON_RIGHT]  = ADB_MOUSE_BUTTON_RIGHT,
+    };
+
+    switch (evt->type) {
+    case INPUT_EVENT_KIND_REL:
+        move = evt->u.rel.data;
+        if (move->axis == INPUT_AXIS_X) {
+            s->dx += move->value;
+        } else if (move->axis == INPUT_AXIS_Y) {
+            s->dy += move->value;
+        }
+        break;
+
+    case INPUT_EVENT_KIND_BTN:
+        btn = evt->u.btn.data;
+        if (bmap[btn->button]) {
+            if (btn->down) {
+                s->buttons_state |= bmap[btn->button];
+            } else {
+                s->buttons_state &= ~bmap[btn->button];
+            }
+        }
+        break;
 
-    s->dx += dx1;
-    s->dy += dy1;
-    s->dz += dz1;
-    s->buttons_state = buttons_state;
+    default:
+        /* keep gcc happy */
+        break;
+    }
 }
 
+static const QemuInputHandler adb_mouse_handler = {
+    .name  = "QEMU ADB Mouse",
+    .mask  = INPUT_EVENT_MASK_BTN | INPUT_EVENT_MASK_REL,
+    .event = adb_mouse_handle_event,
+};
 
 static int adb_mouse_poll(ADBDevice *d, uint8_t *obuf)
 {
@@ -94,10 +130,10 @@ static int adb_mouse_poll(ADBDevice *d, uint8_t *obuf)
     dx &= 0x7f;
     dy &= 0x7f;
 
-    if (!(s->buttons_state & MOUSE_EVENT_LBUTTON)) {
+    if (!(s->buttons_state & ADB_MOUSE_BUTTON_LEFT)) {
         dy |= 0x80;
     }
-    if (!(s->buttons_state & MOUSE_EVENT_RBUTTON)) {
+    if (!(s->buttons_state & ADB_MOUSE_BUTTON_RIGHT)) {
         dx |= 0x80;
     }
 
@@ -236,7 +272,7 @@ static void adb_mouse_realizefn(DeviceState *dev, Error **errp)
 
     amc->parent_realize(dev, errp);
 
-    qemu_add_mouse_event_handler(adb_mouse_event, s, 0, "QEMU ADB Mouse");
+    s->hs = qemu_input_handler_register((DeviceState *)s, &adb_mouse_handler);
 }
 
 static void adb_mouse_initfn(Object *obj)
-- 
2.39.2
Re: [PATCH] adb-mouse: convert to use QemuInputHandler
Posted by Philippe Mathieu-Daudé 2 months, 2 weeks ago
Hi Mark,

On 4/9/24 12:40, Mark Cave-Ayland wrote:
> Update the ADB mouse implementation to use QemuInputHandler instead of the
> legacy qemu_add_mouse_event_handler() function.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>   hw/input/adb-mouse.c | 56 ++++++++++++++++++++++++++++++++++++--------
>   1 file changed, 46 insertions(+), 10 deletions(-)
> 
> diff --git a/hw/input/adb-mouse.c b/hw/input/adb-mouse.c
> index 144a0ccce7..7aa36caf2f 100644
> --- a/hw/input/adb-mouse.c
> +++ b/hw/input/adb-mouse.c
> @@ -38,6 +38,7 @@ struct MouseState {
>       ADBDevice parent_obj;
>       /*< private >*/
>   
> +    QemuInputHandlerState *hs;
>       int buttons_state, last_buttons_state;
>       int dx, dy, dz;
>   };
> @@ -51,17 +52,52 @@ struct ADBMouseClass {
>       DeviceRealize parent_realize;
>   };
>   
> -static void adb_mouse_event(void *opaque,
> -                            int dx1, int dy1, int dz1, int buttons_state)
> +#define ADB_MOUSE_BUTTON_LEFT   0x01
> +#define ADB_MOUSE_BUTTON_RIGHT  0x02
> +
> +static void adb_mouse_handle_event(DeviceState *dev, QemuConsole *src,
> +                                   InputEvent *evt)
>   {
> -    MouseState *s = opaque;
> +    MouseState *s = (MouseState *)dev;
> +    InputMoveEvent *move;
> +    InputBtnEvent *btn;
> +    static const int bmap[INPUT_BUTTON__MAX] = {
> +        [INPUT_BUTTON_LEFT]   = ADB_MOUSE_BUTTON_LEFT,
> +        [INPUT_BUTTON_RIGHT]  = ADB_MOUSE_BUTTON_RIGHT,
> +    };
> +
> +    switch (evt->type) {
> +    case INPUT_EVENT_KIND_REL:
> +        move = evt->u.rel.data;
> +        if (move->axis == INPUT_AXIS_X) {
> +            s->dx += move->value;
> +        } else if (move->axis == INPUT_AXIS_Y) {
> +            s->dy += move->value;
> +        }
> +        break;
> +
> +    case INPUT_EVENT_KIND_BTN:
> +        btn = evt->u.btn.data;
> +        if (bmap[btn->button]) {
> +            if (btn->down) {
> +                s->buttons_state |= bmap[btn->button];
> +            } else {
> +                s->buttons_state &= ~bmap[btn->button];
> +            }
> +        }
> +        break;
>   
> -    s->dx += dx1;
> -    s->dy += dy1;
> -    s->dz += dz1;
> -    s->buttons_state = buttons_state;
> +    default:
> +        /* keep gcc happy */
> +        break;
> +    }
>   }
>   
> +static const QemuInputHandler adb_mouse_handler = {
> +    .name  = "QEMU ADB Mouse",
> +    .mask  = INPUT_EVENT_MASK_BTN | INPUT_EVENT_MASK_REL,
> +    .event = adb_mouse_handle_event,

Don't we need adb_mouse_handle_sync()?

> +};
>   
>   static int adb_mouse_poll(ADBDevice *d, uint8_t *obuf)
>   {
> @@ -94,10 +130,10 @@ static int adb_mouse_poll(ADBDevice *d, uint8_t *obuf)
>       dx &= 0x7f;
>       dy &= 0x7f;
>   
> -    if (!(s->buttons_state & MOUSE_EVENT_LBUTTON)) {
> +    if (!(s->buttons_state & ADB_MOUSE_BUTTON_LEFT)) {
>           dy |= 0x80;
>       }
> -    if (!(s->buttons_state & MOUSE_EVENT_RBUTTON)) {
> +    if (!(s->buttons_state & ADB_MOUSE_BUTTON_RIGHT)) {
>           dx |= 0x80;
>       }
>   
> @@ -236,7 +272,7 @@ static void adb_mouse_realizefn(DeviceState *dev, Error **errp)
>   
>       amc->parent_realize(dev, errp);
>   
> -    qemu_add_mouse_event_handler(adb_mouse_event, s, 0, "QEMU ADB Mouse");
> +    s->hs = qemu_input_handler_register((DeviceState *)s, &adb_mouse_handler);

Simply:

        s->hs = qemu_input_handler_register(dev, &adb_mouse_handler);

>   }
>   
>   static void adb_mouse_initfn(Object *obj)

Waiting for clarification on QemuInputHandler::sync, otherwise LGTM.

Regards,

Phil.
Re: [PATCH] adb-mouse: convert to use QemuInputHandler
Posted by Mark Cave-Ayland 2 months, 2 weeks ago
On 07/09/2024 06:40, Philippe Mathieu-Daudé wrote:

> Hi Mark,
> 
> On 4/9/24 12:40, Mark Cave-Ayland wrote:
>> Update the ADB mouse implementation to use QemuInputHandler instead of the
>> legacy qemu_add_mouse_event_handler() function.
>>
>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>> ---
>>   hw/input/adb-mouse.c | 56 ++++++++++++++++++++++++++++++++++++--------
>>   1 file changed, 46 insertions(+), 10 deletions(-)
>>
>> diff --git a/hw/input/adb-mouse.c b/hw/input/adb-mouse.c
>> index 144a0ccce7..7aa36caf2f 100644
>> --- a/hw/input/adb-mouse.c
>> +++ b/hw/input/adb-mouse.c
>> @@ -38,6 +38,7 @@ struct MouseState {
>>       ADBDevice parent_obj;
>>       /*< private >*/
>> +    QemuInputHandlerState *hs;
>>       int buttons_state, last_buttons_state;
>>       int dx, dy, dz;
>>   };
>> @@ -51,17 +52,52 @@ struct ADBMouseClass {
>>       DeviceRealize parent_realize;
>>   };
>> -static void adb_mouse_event(void *opaque,
>> -                            int dx1, int dy1, int dz1, int buttons_state)
>> +#define ADB_MOUSE_BUTTON_LEFT   0x01
>> +#define ADB_MOUSE_BUTTON_RIGHT  0x02
>> +
>> +static void adb_mouse_handle_event(DeviceState *dev, QemuConsole *src,
>> +                                   InputEvent *evt)
>>   {
>> -    MouseState *s = opaque;
>> +    MouseState *s = (MouseState *)dev;
>> +    InputMoveEvent *move;
>> +    InputBtnEvent *btn;
>> +    static const int bmap[INPUT_BUTTON__MAX] = {
>> +        [INPUT_BUTTON_LEFT]   = ADB_MOUSE_BUTTON_LEFT,
>> +        [INPUT_BUTTON_RIGHT]  = ADB_MOUSE_BUTTON_RIGHT,
>> +    };
>> +
>> +    switch (evt->type) {
>> +    case INPUT_EVENT_KIND_REL:
>> +        move = evt->u.rel.data;
>> +        if (move->axis == INPUT_AXIS_X) {
>> +            s->dx += move->value;
>> +        } else if (move->axis == INPUT_AXIS_Y) {
>> +            s->dy += move->value;
>> +        }
>> +        break;
>> +
>> +    case INPUT_EVENT_KIND_BTN:
>> +        btn = evt->u.btn.data;
>> +        if (bmap[btn->button]) {
>> +            if (btn->down) {
>> +                s->buttons_state |= bmap[btn->button];
>> +            } else {
>> +                s->buttons_state &= ~bmap[btn->button];
>> +            }
>> +        }
>> +        break;
>> -    s->dx += dx1;
>> -    s->dy += dy1;
>> -    s->dz += dz1;
>> -    s->buttons_state = buttons_state;
>> +    default:
>> +        /* keep gcc happy */
>> +        break;
>> +    }
>>   }
>> +static const QemuInputHandler adb_mouse_handler = {
>> +    .name  = "QEMU ADB Mouse",
>> +    .mask  = INPUT_EVENT_MASK_BTN | INPUT_EVENT_MASK_REL,
>> +    .event = adb_mouse_handle_event,
> 
> Don't we need adb_mouse_handle_sync()?

I'm not convinced that implementing .sync would work well here, since unlike e.g. 
PS/2 where async mouse events are sent over the serial port, an ADB mouse is 
constantly polled by the host via the adb_mouse_poll() callback.

You may be able to add a .sync function that generates the next set of mouse 
coordinates to be returned by adb_mouse_poll(), but then you still have to wait for 
the next ADB packet to be sent back to the host which feels like it would introduce 
extra latency.

>> +};
>>   static int adb_mouse_poll(ADBDevice *d, uint8_t *obuf)
>>   {
>> @@ -94,10 +130,10 @@ static int adb_mouse_poll(ADBDevice *d, uint8_t *obuf)
>>       dx &= 0x7f;
>>       dy &= 0x7f;
>> -    if (!(s->buttons_state & MOUSE_EVENT_LBUTTON)) {
>> +    if (!(s->buttons_state & ADB_MOUSE_BUTTON_LEFT)) {
>>           dy |= 0x80;
>>       }
>> -    if (!(s->buttons_state & MOUSE_EVENT_RBUTTON)) {
>> +    if (!(s->buttons_state & ADB_MOUSE_BUTTON_RIGHT)) {
>>           dx |= 0x80;
>>       }
>> @@ -236,7 +272,7 @@ static void adb_mouse_realizefn(DeviceState *dev, Error **errp)
>>       amc->parent_realize(dev, errp);
>> -    qemu_add_mouse_event_handler(adb_mouse_event, s, 0, "QEMU ADB Mouse");
>> +    s->hs = qemu_input_handler_register((DeviceState *)s, &adb_mouse_handler);
> 
> Simply:
> 
>         s->hs = qemu_input_handler_register(dev, &adb_mouse_handler);

Ah yes, that should work here.

>>   }
>>   static void adb_mouse_initfn(Object *obj)
> 
> Waiting for clarification on QemuInputHandler::sync, otherwise LGTM.

Hopefully explained above :)  I'll send a v2 later this evening.


ATB,

Mark.