* [PATCH] ui/sdl: Mouse event optimization @ 2024-10-25 6:52 Lei Huang 2024-10-25 9:30 ` BALATON Zoltan 0 siblings, 1 reply; 10+ messages in thread From: Lei Huang @ 2024-10-25 6:52 UTC (permalink / raw) To: marcandre.lureau Cc: qemu-devel, pierre-eric.pelloux-prayer, ken.xue, lei.huang, Lei Huang Use a convergence factor to make the VM's input global coordinates more closely approach the global coordinates of DOM0. Change-Id: I2c3f12f1fe7dfb9306d1fc40c4fd4d299937f4c6 Signed-off-by: Lei Huang <Lei.Huang@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..8f504dd8727 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 dom0 side. + *The goal of this entire process is to align the global coordinates of + *the VM with those of dom0 using dx and dy. The current approach aims + *for precise calibration in once attempt; however, because guest_x + *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 dom0's 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 | + * virtio-gpu <------ drmModeMoveCursor <------ compositor <------- | + * (guest_x,guest_y) calculates guest_x and + * guest_y dy using dx and dy + */ +#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; } qemu_input_queue_rel(scon->dcl.con, INPUT_AXIS_X, dx); qemu_input_queue_rel(scon->dcl.con, INPUT_AXIS_Y, dy); -- 2.45.2 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] ui/sdl: Mouse event optimization 2024-10-25 6:52 [PATCH] ui/sdl: Mouse event optimization Lei Huang @ 2024-10-25 9:30 ` BALATON Zoltan 2024-10-29 6:08 ` Lei Huang 0 siblings, 1 reply; 10+ messages in thread From: BALATON Zoltan @ 2024-10-25 9:30 UTC (permalink / raw) To: Lei Huang Cc: marcandre.lureau, qemu-devel, pierre-eric.pelloux-prayer, ken.xue On Fri, 25 Oct 2024, Lei Huang wrote: > Use a convergence factor to make the VM's input > global coordinates more closely approach the global > coordinates of DOM0. Dom0 is some Xen terminology. Do you mean "host" which is more often used in QEMU? > Change-Id: I2c3f12f1fe7dfb9306d1fc40c4fd4d299937f4c6 > Signed-off-by: Lei Huang <Lei.Huang@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..8f504dd8727 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 dom0 side. > + *The goal of this entire process is to align the global coordinates of > + *the VM with those of dom0 using dx and dy. The current approach aims > + *for precise calibration in once attempt; however, because guest_x "in one attempt" also add a space after the * at the beginning of lines. > + *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 dom0's 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 | > + * virtio-gpu <------ drmModeMoveCursor <------ compositor <------- | > + * (guest_x,guest_y) calculates guest_x and > + * guest_y dy using dx and dy > + */ What about other display devices than virtio-gpu? Does this work with those or do they need some update? If this is independent of graphics device maybe add a note that virtio-gpu is an example and could be any graphics device. Regards, BALATON Zoltan > +#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; > } > qemu_input_queue_rel(scon->dcl.con, INPUT_AXIS_X, dx); > qemu_input_queue_rel(scon->dcl.con, INPUT_AXIS_Y, dy); > -- > 2.45.2 > > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] ui/sdl: Mouse event optimization 2024-10-25 9:30 ` BALATON Zoltan @ 2024-10-29 6:08 ` Lei Huang 2024-10-29 12:51 ` BALATON Zoltan 0 siblings, 1 reply; 10+ messages in thread From: Lei Huang @ 2024-10-29 6:08 UTC (permalink / raw) To: balaton Cc: marcandre.lureau, qemu-devel, pierre-eric.pelloux-prayer, ken.xue, Lei.Huang > On Fri, 25 Oct 2024, Lei Huang wrote: > > Use a convergence factor to make the VM's input > > global coordinates more closely approach the global > > coordinates of DOM0. > > Dom0 is some Xen terminology. Do you mean "host" which is more often used > in QEMU? Yes, I will change it to host > > > Change-Id: I2c3f12f1fe7dfb9306d1fc40c4fd4d299937f4c6 > > Signed-off-by: Lei Huang <Lei.Huang@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..8f504dd8727 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 dom0 side. > > + *The goal of this entire process is to align the global coordinates of > > + *the VM with those of dom0 using dx and dy. The current approach aims > > + *for precise calibration in once attempt; however, because guest_x > > "in one attempt" also add a space after the * at the beginning of lines. okay, I will change it > > > + *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 dom0's 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 | > > + * virtio-gpu <------ drmModeMoveCursor <------ compositor <------- | > > + * (guest_x,guest_y) calculates guest_x and > > + * guest_y dy using dx and dy > > + */ > > What about other display devices than virtio-gpu? Does this work with > those or do they need some update? If this is independent of graphics > device maybe add a note that virtio-gpu is an example and could be any > graphics device. Yes,this applies to any device using SDL where the VM utilizes cursor plane acceleration; virtio-gpu is just an example. I will add it to the explanation. > > Regards, > BALATON Zoltan > > > +#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; > > } > > qemu_input_queue_rel(scon->dcl.con, INPUT_AXIS_X, dx); > > qemu_input_queue_rel(scon->dcl.con, INPUT_AXIS_Y, dy); > > -- > > 2.45.2 > > > > > > > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] ui/sdl: Mouse event optimization 2024-10-29 6:08 ` Lei Huang @ 2024-10-29 12:51 ` BALATON Zoltan 2024-10-30 3:20 ` Lei Huang 0 siblings, 1 reply; 10+ messages in thread From: BALATON Zoltan @ 2024-10-29 12:51 UTC (permalink / raw) To: Lei Huang Cc: marcandre.lureau, qemu-devel, pierre-eric.pelloux-prayer, ken.xue On Tue, 29 Oct 2024, Lei Huang wrote: >> On Fri, 25 Oct 2024, Lei Huang wrote: >>> Use a convergence factor to make the VM's input >>> global coordinates more closely approach the global >>> coordinates of DOM0. >> >> Dom0 is some Xen terminology. Do you mean "host" which is more often used >> in QEMU? > > Yes, I will change it to host > >> >>> Change-Id: I2c3f12f1fe7dfb9306d1fc40c4fd4d299937f4c6 >>> Signed-off-by: Lei Huang <Lei.Huang@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..8f504dd8727 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 dom0 side. >>> + *The goal of this entire process is to align the global coordinates of >>> + *the VM with those of dom0 using dx and dy. The current approach aims >>> + *for precise calibration in once attempt; however, because guest_x >> >> "in one attempt" also add a space after the * at the beginning of lines. > > okay, I will change it > >> >>> + *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 dom0's 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 | >>> + * virtio-gpu <------ drmModeMoveCursor <------ compositor <------- | >>> + * (guest_x,guest_y) calculates guest_x and >>> + * guest_y dy using dx and dy >>> + */ >> >> What about other display devices than virtio-gpu? Does this work with >> those or do they need some update? If this is independent of graphics >> device maybe add a note that virtio-gpu is an example and could be any >> graphics device. > > Yes,this applies to any device using SDL where the VM utilizes cursor plane acceleration; > virtio-gpu is just an example. I will add it to the explanation. You could change the figure in the comment to say instead of virtio-gpu display device (e.g. virtio-gpu) Which may be enough to clearify this. Regards, BALATON Zoltan ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] ui/sdl: Mouse event optimization 2024-10-29 12:51 ` BALATON Zoltan @ 2024-10-30 3:20 ` Lei Huang 2024-10-30 3:43 ` [PATCH v2] " Lei Huang 0 siblings, 1 reply; 10+ messages in thread From: Lei Huang @ 2024-10-30 3:20 UTC (permalink / raw) To: balaton Cc: marcandre.lureau, qemu-devel, pierre-eric.pelloux-prayer, ken.xue, Lei.Huang >On Tue, 29 Oct 2024, Lei Huang wrote: >>> On Fri, 25 Oct 2024, Lei Huang wrote: >>>> Use a convergence factor to make the VM's input >>>> global coordinates more closely approach the global >>>> coordinates of DOM0. >>> >>> Dom0 is some Xen terminology. Do you mean "host" which is more often used >>> in QEMU? >> >> Yes, I will change it to host >> >>> >>>> Change-Id: I2c3f12f1fe7dfb9306d1fc40c4fd4d299937f4c6 >>>> Signed-off-by: Lei Huang <Lei.Huang@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..8f504dd8727 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 dom0 side. >>>> + *The goal of this entire process is to align the global coordinates of >>>> + *the VM with those of dom0 using dx and dy. The current approach aims >>>> + *for precise calibration in once attempt; however, because guest_x >>> >>> "in one attempt" also add a space after the * at the beginning of lines. >> >> okay, I will change it >> >>> >>>> + *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 dom0's 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 | >>>> + * virtio-gpu <------ drmModeMoveCursor <------ compositor <------- | >>>> + * (guest_x,guest_y) calculates guest_x and >>>> + * guest_y dy using dx and dy >>>> + */ >>> >>> What about other display devices than virtio-gpu? Does this work with >>> those or do they need some update? If this is independent of graphics >>> device maybe add a note that virtio-gpu is an example and could be any >>> graphics device. >> >> Yes,this applies to any device using SDL where the VM utilizes cursor plane acceleration; >> virtio-gpu is just an example. I will add it to the explanation. > >You could change the figure in the comment to say instead of virtio-gpu > > display device >(e.g. virtio-gpu) > >Which may be enough to clearify this. okay, got it, thank you very much. Additionally, I noticed in GTK's implementation, it resolves this issue by using the gdk_window_get_root_coords method in the gd_mouse_set function. however, I could not find a similar method in SDL2, so I am not sure if there are better solutions available. Neverthless, the current modification dose indeed fix the mouse drift issue on the VM side. > >Regards, >BALATON Zoltan ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2] ui/sdl: Mouse event optimization 2024-10-30 3:20 ` Lei Huang @ 2024-10-30 3:43 ` Lei Huang 2024-10-30 13:38 ` BALATON Zoltan 0 siblings, 1 reply; 10+ messages in thread From: Lei Huang @ 2024-10-30 3:43 UTC (permalink / raw) To: balaton Cc: marcandre.lureau, qemu-devel, pierre-eric.pelloux-prayer, ken.xue, Lei.Huang 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.Huang@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 + * 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 + */ +#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; } qemu_input_queue_rel(scon->dcl.con, INPUT_AXIS_X, dx); qemu_input_queue_rel(scon->dcl.con, INPUT_AXIS_Y, dy); -- 2.45.2 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2] ui/sdl: Mouse event optimization 2024-10-30 3:43 ` [PATCH v2] " Lei Huang @ 2024-10-30 13:38 ` BALATON Zoltan 2024-10-31 8:13 ` Lei Huang 0 siblings, 1 reply; 10+ messages in thread From: BALATON Zoltan @ 2024-10-30 13:38 UTC (permalink / raw) To: Lei Huang Cc: marcandre.lureau, qemu-devel, pierre-eric.pelloux-prayer, ken.xue, Howard Spoelstra 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.Huang@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); > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] ui/sdl: Mouse event optimization 2024-10-30 13:38 ` BALATON Zoltan @ 2024-10-31 8:13 ` Lei Huang 2024-10-31 12:50 ` BALATON Zoltan 0 siblings, 1 reply; 10+ messages in thread From: Lei Huang @ 2024-10-31 8:13 UTC (permalink / raw) To: balaton Cc: marcandre.lureau, qemu-devel, pierre-eric.pelloux-prayer, ken.xue, Lei.Huang, hsp.cat7 >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.Huang@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. Oh, okay, got it > >> + * 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.) okay, thanks, got it > >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. yes, it is 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? It's not the reason of guest_x += x;. I previously tried removing guest_x += x; and only letting sdl_mouse_warp update guest_x, but the issue still persists. The behavior of different virtual machines varies. In the Ubuntu VM, the cursor jitters violently within a small range; for example, when moving a folder, it rapidly flickers back and forth. On Android 14, it can barely be used, though individual coordinates may deviate significantly, but they can be quickly corrected. However, on Android 15, it is completely unusable. The reason for the issue in Android 15 is that the initial global coordinates of Android are random values, not (0, 0). During the second calibration, it is possible to calculate negative values, which are forced to be set to 0, resulting in a continuous loop. The occurrence of negative values is an extreme situation. > >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); >> ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] ui/sdl: Mouse event optimization 2024-10-31 8:13 ` Lei Huang @ 2024-10-31 12:50 ` BALATON Zoltan 2024-11-01 1:24 ` Lei Huang 0 siblings, 1 reply; 10+ messages in thread From: BALATON Zoltan @ 2024-10-31 12:50 UTC (permalink / raw) To: Lei Huang Cc: marcandre.lureau, qemu-devel, pierre-eric.pelloux-prayer, ken.xue, hsp.cat7 On Thu, 31 Oct 2024, Lei Huang wrote: >> 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.Huang@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. > > Oh, okay, got it > >> >>> + * 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.) > > okay, thanks, got it > >> >> 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. > > yes, it is correct. Then maybe it's a good opportunity to simplify this now. >> 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? > > It's not the reason of guest_x += x;. I previously tried removing guest_x += x; > and only letting sdl_mouse_warp update guest_x, but the issue still persists. > > The behavior of different virtual machines varies. In the Ubuntu VM, the cursor > jitters violently within a small range; for example, when moving a folder, it rapidly > flickers back and forth. On Android 14, it can barely be used, though individual coordinates > may deviate significantly, but they can be quickly corrected. However, on Android 15, it is > completely unusable. The reason for the issue in Android 15 is that the initial global > coordinates of Android are random values, not (0, 0). During the second calibration, > it is possible to calculate negative values, which are forced to be set to 0, resulting in > a continuous loop. The occurrence of negative values is an extreme situation. I still don't understand the reason for this behaviour. I thought it's probably caused by different pointer acceleration settings on the host and guest so a move by some value moves the host and guest pointer by different amounts. QEMU could get the host settings but knows nothing about the guest so this probably can't be fixed other than trying to converge the values. The question is why 3 is a good value for CONVERGENCE_FACTOR and is there a better value or can it be calculated somehow. But if we don't know and this fixes the issue it's likely good enough, I'm just trying to understand the issue better. Regards, BALATON Zoltan ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] ui/sdl: Mouse event optimization 2024-10-31 12:50 ` BALATON Zoltan @ 2024-11-01 1:24 ` Lei Huang 0 siblings, 0 replies; 10+ messages in thread From: Lei Huang @ 2024-11-01 1:24 UTC (permalink / raw) To: balaton Cc: marcandre.lureau, qemu-devel, pierre-eric.pelloux-prayer, ken.xue, Lei.Huang, hsp.cat7 >On Thu, 31 Oct 2024, Lei Huang wrote: >>> 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.Huang@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. >> >> Oh, okay, got it >> >>> >>>> + * 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.) >> >> okay, thanks, got it >> >>> >>> 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. >> >> yes, it is correct. > >Then maybe it's a good opportunity to simplify this now. > >>> 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? >> >> It's not the reason of guest_x += x;. I previously tried removing guest_x += x; >> and only letting sdl_mouse_warp update guest_x, but the issue still persists. >> >> The behavior of different virtual machines varies. In the Ubuntu VM, the cursor >> jitters violently within a small range; for example, when moving a folder, it rapidly >> flickers back and forth. On Android 14, it can barely be used, though individual coordinates >> may deviate significantly, but they can be quickly corrected. However, on Android 15, it is >> completely unusable. The reason for the issue in Android 15 is that the initial global >> coordinates of Android are random values, not (0, 0). During the second calibration, >> it is possible to calculate negative values, which are forced to be set to 0, resulting in >> a continuous loop. The occurrence of negative values is an extreme situation. > >I still don't understand the reason for this behaviour. I thought it's >probably caused by different pointer acceleration settings on the host and >guest so a move by some value moves the host and guest pointer by >different amounts. QEMU could get the host settings but knows nothing >about the guest so this probably can't be fixed other than trying to >converge the values. The question is why 3 is a good value for >CONVERGENCE_FACTOR and is there a better value or can it be calculated >somehow. But if we don't know and this fixes the issue it's likely good >enough, I'm just trying to understand the issue better. This is partly because mouse events are reported whenever there is a change, but feedback occurs every 16ms. If the VM refreshes at 60Hz, the reason for the value of 3 is that a larger value will make the VM appear smoother, but it will ultimately lead to a 3-pixel input event discrepancy between the VM and the host. Therefore, considering all these factors, the value of 3 is chosen. Under normal circumstances, dx and dy should vary within 20 pixels because these are relative coordinates, and the mouse cannot move too far. However, due to issues with the coordinates reported by the VM, the calculated dx and dy can change by hundreds of pixels. Such large fluctuations are more likely to trigger the extreme situations mentioned earlier, leading to a vicious cycle. > >Regards, >BALATON Zoltan ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2024-11-01 1:26 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-10-25 6:52 [PATCH] ui/sdl: Mouse event optimization Lei Huang 2024-10-25 9:30 ` BALATON Zoltan 2024-10-29 6:08 ` Lei Huang 2024-10-29 12:51 ` BALATON Zoltan 2024-10-30 3:20 ` Lei Huang 2024-10-30 3:43 ` [PATCH v2] " Lei Huang 2024-10-30 13:38 ` BALATON Zoltan 2024-10-31 8:13 ` Lei Huang 2024-10-31 12:50 ` BALATON Zoltan 2024-11-01 1:24 ` Lei Huang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).