On Wed, 30 Oct 2024, Lei Huang wrote:
Use a convergence factor to make the VM's input
global coordinates more closely approach the global
coordinates of host.
Change-Id: I2c3f12f1fe7dfb9306d1fc40c4fd4d299937f4c6
Signed-off-by: Lei Huang <lei.hu...@amd.com>
---
ui/sdl2.c | 32 ++++++++++++++++++++++++++++++--
1 file changed, 30 insertions(+), 2 deletions(-)
diff --git a/ui/sdl2.c b/ui/sdl2.c
index bd4f5a9da14..ea3fd74dd63 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -303,6 +303,34 @@ static void sdl_mouse_mode_change(Notifier *notify, void
*data)
}
}
+/*
+ * guest_x and guest_y represent the global coordinates on the VM side,
+ * while x and y represent the global coordinates on the host side.
+ * The goal of this entire process is to align the global coordinates of
+ * the VM with those of host using dx and dy. The current approach aims
+ * for precise calibration in once attempt; however, because guest_x
There's still another one here. You can say "in one attempt" or "at once"
but combining the two is not correct.
+ * and guest_y are non-zero values, they are not accurate values when
+ * they are counted out to become negative. Therefore, achieving perfect
+ * alignment in one attempt is impossible. Since the same calibration method
+ * is used each time, repeated attempts cannot achieve alignment either.
+ * By introducing a convergence factor, guest_x and guest_y can be made to
+ * approach host x and y indefinitely.
+ *
+ * QEMU (dx,dy) VM
+ * calculates dx and dy using guest_x and guest_y ----> input driver
+ * ^ |
+ * | |
+ * | V
+ * | update
+ * | guest_x,guest_y input dispatcher --->
WindowManager
+ * | | |
+ * | | |
+ * | libdrm V |
+ * display device <------ drmModeMoveCursor <------ compositor <------- |
+ * (guest_x,guest_y) calculates guest_x and
+ * guest_y dy using dx and dy
Maybe adding (e.g. virtio-gpu) below display device would make it clearer.
Also under compositor there's "guest_y dy" where the dy seems to be left
there by mistake from some editing or I don't get this sentence. (Did
checkpatch.pl complain about too long lines? Maybe you could shorten the
arrows a bit and wrap the text under QEMU in two lines to try to fit in 80
chars.)
Sorry that I can only comment on the comments and not the actual change
but I've cc'd Howard who I think saw this issue before so may be able to
give it a test.
+ */
+#define CONVERGENCE_FACTOR 3
static void sdl_send_mouse_event(struct sdl2_console *scon, int dx, int dy,
int x, int y, int state)
{
@@ -331,8 +359,8 @@ static void sdl_send_mouse_event(struct sdl2_console *scon,
int dx, int dy,
y -= guest_y;
guest_x += x;
guest_y += y;
- dx = x;
- dy = y;
+ dx = x / CONVERGENCE_FACTOR;
+ dy = y / CONVERGENCE_FACTOR;
Looking at this calculation I don't quite get what's intended here and I
think this could be simplified. Originally in 47c03744b37 it seems the
qemu_input_queue_rel() was called with the modified value of x and y but
then afbc0dd6498 have introduced dx, dy. After that changing x and y seems
unnecessary as they are locals never used after this calculation. If I try
to expand these equations I get:
x = x - guest_x;
guest_x = guest_x + x - guest_x;
So isn't this equivalent to just:
dx = x - guest_x;
guest_x = x;
which seems to make more sense but I don't know if it's correct.
Then this patch takes the third of dx to avoid it overshooting the desired
value. The question is what causes this overshoot and can we calculate the
actual value of it to compensate for it in one calculation? Isn't it the
other line that sets guest_x to x that needs some correction?
Regards,
BALATON Zoltan
}
qemu_input_queue_rel(scon->dcl.con, INPUT_AXIS_X, dx);
qemu_input_queue_rel(scon->dcl.con, INPUT_AXIS_Y, dy);
