Re: [PATCH v2 1/5] ps2: Initial horizontal scroll support

On Wed, Dec 22, 2021 at 5:27 AM Dmitry Petrov <dpetroff@gmail.com> wrote:

v2:
- Patch is split into a sequence
- value is clamped to 31 for horizontal scroll

This patch introduces horizontal scroll support for the ps/2
mouse.

The patch is based on the previous work
by Brad Jorsch done in 2010
but never merge, see
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=579968

The patch series doesn't form a thread. You should also add a cover letter. This way, tools like patchew/patchwork can handle it correctly.

This change adds support for horizontal scroll to ps/2 mouse device
code. The code is implemented to match the logic of linux kernel
which is used as a reference.

Signed-off-by: Dmitry Petrov <dpetroff@gmail.com>

I just realized that ps2.c isn't reused by vmmouse.

What's really the point in adding horizontal scroll to ps2?

A VM without absolute pointer is not really acceptable or working these days. Especially on Wayland with the QEMU GTK display. (fwiw, spice-gtk and rdw do implement pointer contraints & relative extensions so it's kinda working ok).

Have you looked at adding virtio-input-hid support instead? That should be more straightforward.

---
hw/input/ps2.c | 57 +++++++++++++++++++++++++++++++++++++++++++-------
qapi/ui.json | 2 +-
2 files changed, 50 insertions(+), 9 deletions(-)

diff --git a/hw/input/ps2.c b/hw/input/ps2.c
index 9376a8f4ce..6236711e1b 100644
--- a/hw/input/ps2.c
+++ b/hw/input/ps2.c
@@ -123,6 +123,7 @@ typedef struct {
int mouse_dx; /* current values, needed for 'poll' mode */
int mouse_dy;
int mouse_dz;
+ int mouse_dw;
uint8_t mouse_buttons;
} PS2MouseState;

@@ -715,7 +716,7 @@ static int ps2_mouse_send_packet(PS2MouseState *s)
/* IMPS/2 and IMEX send 4 bytes, PS2 sends 3 bytes */
const int needed = s->mouse_type ? 4 : 3;
unsigned int b;
- int dx1, dy1, dz1;
+ int dx1, dy1, dz1, dw1;

if (PS2_QUEUE_SIZE - s->common.queue.count < needed) {
return 0;
@@ -724,6 +725,7 @@ static int ps2_mouse_send_packet(PS2MouseState *s)
dx1 = s->mouse_dx;
dy1 = s->mouse_dy;
dz1 = s->mouse_dz;
+ dw1 = s->mouse_dw;
/* XXX: increase range to 8 bits ? */
if (dx1 > 127)
dx1 = 127;
@@ -740,6 +742,9 @@ static int ps2_mouse_send_packet(PS2MouseState *s)
/* extra byte for IMPS/2 or IMEX */
switch(s->mouse_type) {
default:
+ /* Just ignore the wheels if not supported */
+ s->mouse_dz = 0;
+ s->mouse_dw = 0;
break;
case 3:
if (dz1 > 127)
@@ -747,13 +752,41 @@ static int ps2_mouse_send_packet(PS2MouseState *s)
else if (dz1 < -127)
dz1 = -127;
ps2_queue_noirq(&s->common, dz1 & 0xff);
+ s->mouse_dz -= dz1;
+ s->mouse_dw = 0;
break;
case 4:
- if (dz1 > 7)
- dz1 = 7;
- else if (dz1 < -7)
- dz1 = -7;
- b = (dz1 & 0x0f) | ((s->mouse_buttons & 0x18) << 1);
+ /*
+ * This matches what the Linux kernel expects for exps/2 in
+ * drivers/input/mouse/psmouse-base.c. Note, if you happen to
+ * press/release the 4th or 5th buttons at the same moment as a
+ * horizontal wheel scroll, those button presses will get lost. I'm not
+ * sure what to do about that, since by this point we don't know
+ * whether those buttons actually changed state.
+ */
+ if (dw1 != 0) {
+ if (dw1 > 31) {
+ dw1 = 31;
+ } else if (dw1 < -31) {
+ dw1 = -31;
+ }
+
+ /*
+ * linux kernel expects first 6 bits to represent the value
+ * for horizontal scroll
+ */
+ b = (dw1 & 0x3f) | 0x40;
+ s->mouse_dw -= dw1;
+ } else {
+ if (dz1 > 7) {
+ dz1 = 7;
+ } else if (dz1 < -7) {
+ dz1 = -7;
+ }
+
+ b = (dz1 & 0x0f) | ((s->mouse_buttons & 0x18) << 1);
+ s->mouse_dz -= dz1;
+ }
ps2_queue_noirq(&s->common, b);
break;
}
@@ -764,7 +797,6 @@ static int ps2_mouse_send_packet(PS2MouseState *s)
/* update deltas */
s->mouse_dx -= dx1;
s->mouse_dy -= dy1;
- s->mouse_dz -= dz1;

return 1;
}
@@ -806,6 +838,12 @@ static void ps2_mouse_event(DeviceState *dev, QemuConsole *src,
} else if (btn->button == INPUT_BUTTON_WHEEL_DOWN) {
s->mouse_dz++;
}
+
+ if (btn->button == INPUT_BUTTON_WHEEL_RIGHT) {
+ s->mouse_dw--;
+ } else if (btn->button == INPUT_BUTTON_WHEEL_LEFT) {
+ s->mouse_dw++;
+ }
} else {
s->mouse_buttons &= ~bmap[btn->button];
}
@@ -833,8 +871,10 @@ static void ps2_mouse_sync(DeviceState *dev)
/* if not remote, send event. Multiple events are sent if
too big deltas */
while (ps2_mouse_send_packet(s)) {
- if (s->mouse_dx == 0 && s->mouse_dy == 0 && s->mouse_dz == 0)
+ if (s->mouse_dx == 0 && s->mouse_dy == 0
+ && s->mouse_dz == 0 && s->mouse_dw == 0) {
break;
+ }
}
}
}
@@ -1036,6 +1076,7 @@ static void ps2_mouse_reset(void *opaque)
s->mouse_dx = 0;
s->mouse_dy = 0;
s->mouse_dz = 0;
+ s->mouse_dw = 0;
s->mouse_buttons = 0;
}

diff --git a/qapi/ui.json b/qapi/ui.json
index d7567ac866..9dac1bf548 100644
--- a/qapi/ui.json
+++ b/qapi/ui.json
@@ -905,7 +905,7 @@
##
{ 'enum' : 'InputButton',
'data' : [ 'left', 'middle', 'right', 'wheel-up', 'wheel-down', 'side',
- 'extra' ] }
+ 'extra', 'wheel-left', 'wheel-right' ] }

##
# @InputAxis:
--
2.32.0

From:	Dmitry Petrov
Subject:	Re: [PATCH v2 1/5] ps2: Initial horizontal scroll support
Date:	Wed, 22 Dec 2021 16:10:49 +0100