[PATCH] ui/gtk: Fix relative mouse with multiple monitors

[PATCH] ui/gtk: Fix relative mouse with multiple monitors

[Dennis Wölfing]

To handle relative mouse input the event handler needs to move the mouse
away from the screen edges. Failing to do so results in the mouse
getting stuck at invisible walls. However the current implementation for
this is broken on hosts with multiple monitors.

With multiple monitors the mouse can be located outside of the current
monitor which is not handled by the current code. Also the monitor
itself might be located at coordinates different from (0, 0).

Signed-off-by: Dennis Wölfing <denniswoelfing@gmx.de>
---
 ui/gtk.c | 21 +++++++++------------
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/ui/gtk.c b/ui/gtk.c
index 98046f577b..5258532b19 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -865,33 +865,30 @@ static gboolean gd_motion_event(GtkWidget *widget, GdkEventMotion *motion,
         GdkWindow *win = gtk_widget_get_window(widget);
         GdkMonitor *monitor = gdk_display_get_monitor_at_window(dpy, win);
         GdkRectangle geometry;
-        int screen_width, screen_height;

         int x = (int)motion->x_root;
         int y = (int)motion->y_root;

         gdk_monitor_get_geometry(monitor, &geometry);
-        screen_width = geometry.width;
-        screen_height = geometry.height;

         /* In relative mode check to see if client pointer hit
-         * one of the screen edges, and if so move it back by
+         * one of the monitor edges, and if so move it back by
          * 200 pixels. This is important because the pointer
          * in the server doesn't correspond 1-for-1, and so
          * may still be only half way across the screen. Without
          * this warp, the server pointer would thus appear to hit
          * an invisible wall */
-        if (x == 0) {
-            x += 200;
+        if (x <= geometry.x) {
+            x = geometry.x + 200;
         }
-        if (y == 0) {
-            y += 200;
+        if (y <= geometry.y) {
+            y = geometry.y + 200;
         }
-        if (x == (screen_width - 1)) {
-            x -= 200;
+        if (x - geometry.x >= (geometry.width - 1)) {
+            x = geometry.x + (geometry.width - 1) - 200;
         }
-        if (y == (screen_height - 1)) {
-            y -= 200;
+        if (y - geometry.y >= (geometry.height - 1)) {
+            y = geometry.y + (geometry.height - 1) - 200;
         }

         if (x != (int)motion->x_root || y != (int)motion->y_root) {
--
2.32.0

Re: [PATCH] ui/gtk: Fix relative mouse with multiple monitors

[Dennis Wölfing]

Ping
https://lore.kernel.org/qemu-devel/20210629132410.286813-1-denniswoelfing@gmx.de

On 29.06.21 15:24, Dennis Wölfing wrote:
> To handle relative mouse input the event handler needs to move the mouse
> away from the screen edges. Failing to do so results in the mouse
> getting stuck at invisible walls. However the current implementation for
> this is broken on hosts with multiple monitors.
>
> With multiple monitors the mouse can be located outside of the current
> monitor which is not handled by the current code. Also the monitor
> itself might be located at coordinates different from (0, 0).
>
> Signed-off-by: Dennis Wölfing <denniswoelfing@gmx.de>
> ---
>   ui/gtk.c | 21 +++++++++------------
>   1 file changed, 9 insertions(+), 12 deletions(-)
>
> diff --git a/ui/gtk.c b/ui/gtk.c
> index 98046f577b..5258532b19 100644
> --- a/ui/gtk.c
> +++ b/ui/gtk.c
> @@ -865,33 +865,30 @@ static gboolean gd_motion_event(GtkWidget *widget, GdkEventMotion *motion,
>           GdkWindow *win = gtk_widget_get_window(widget);
>           GdkMonitor *monitor = gdk_display_get_monitor_at_window(dpy, win);
>           GdkRectangle geometry;
> -        int screen_width, screen_height;
>
>           int x = (int)motion->x_root;
>           int y = (int)motion->y_root;
>
>           gdk_monitor_get_geometry(monitor, &geometry);
> -        screen_width = geometry.width;
> -        screen_height = geometry.height;
>
>           /* In relative mode check to see if client pointer hit
> -         * one of the screen edges, and if so move it back by
> +         * one of the monitor edges, and if so move it back by
>            * 200 pixels. This is important because the pointer
>            * in the server doesn't correspond 1-for-1, and so
>            * may still be only half way across the screen. Without
>            * this warp, the server pointer would thus appear to hit
>            * an invisible wall */
> -        if (x == 0) {
> -            x += 200;
> +        if (x <= geometry.x) {
> +            x = geometry.x + 200;
>           }
> -        if (y == 0) {
> -            y += 200;
> +        if (y <= geometry.y) {
> +            y = geometry.y + 200;
>           }
> -        if (x == (screen_width - 1)) {
> -            x -= 200;
> +        if (x - geometry.x >= (geometry.width - 1)) {
> +            x = geometry.x + (geometry.width - 1) - 200;
>           }
> -        if (y == (screen_height - 1)) {
> -            y -= 200;
> +        if (y - geometry.y >= (geometry.height - 1)) {
> +            y = geometry.y + (geometry.height - 1) - 200;
>           }
>
>           if (x != (int)motion->x_root || y != (int)motion->y_root) {
> --
> 2.32.0
>

Re: [PATCH for 6.1] ui/gtk: Fix relative mouse with multiple monitors

[Dennis Wölfing]

Ping 2
I'd like to get this bugfix into 6.1.

On 07.07.21 13:02, Dennis Wölfing wrote:
> Ping
> https://lore.kernel.org/qemu-devel/20210629132410.286813-1-denniswoelfing@gmx.de 
> 
> 
> On 29.06.21 15:24, Dennis Wölfing wrote:
>> To handle relative mouse input the event handler needs to move the mouse
>> away from the screen edges. Failing to do so results in the mouse
>> getting stuck at invisible walls. However the current implementation for
>> this is broken on hosts with multiple monitors.
>>
>> With multiple monitors the mouse can be located outside of the current
>> monitor which is not handled by the current code. Also the monitor
>> itself might be located at coordinates different from (0, 0).
>>
>> Signed-off-by: Dennis Wölfing <denniswoelfing@gmx.de>
>> ---
>>   ui/gtk.c | 21 +++++++++------------
>>   1 file changed, 9 insertions(+), 12 deletions(-)
>>
>> diff --git a/ui/gtk.c b/ui/gtk.c
>> index 98046f577b..5258532b19 100644
>> --- a/ui/gtk.c
>> +++ b/ui/gtk.c
>> @@ -865,33 +865,30 @@ static gboolean gd_motion_event(GtkWidget 
>> *widget, GdkEventMotion *motion,
>>           GdkWindow *win = gtk_widget_get_window(widget);
>>           GdkMonitor *monitor = gdk_display_get_monitor_at_window(dpy, 
>> win);
>>           GdkRectangle geometry;
>> -        int screen_width, screen_height;
>>
>>           int x = (int)motion->x_root;
>>           int y = (int)motion->y_root;
>>
>>           gdk_monitor_get_geometry(monitor, &geometry);
>> -        screen_width = geometry.width;
>> -        screen_height = geometry.height;
>>
>>           /* In relative mode check to see if client pointer hit
>> -         * one of the screen edges, and if so move it back by
>> +         * one of the monitor edges, and if so move it back by
>>            * 200 pixels. This is important because the pointer
>>            * in the server doesn't correspond 1-for-1, and so
>>            * may still be only half way across the screen. Without
>>            * this warp, the server pointer would thus appear to hit
>>            * an invisible wall */
>> -        if (x == 0) {
>> -            x += 200;
>> +        if (x <= geometry.x) {
>> +            x = geometry.x + 200;
>>           }
>> -        if (y == 0) {
>> -            y += 200;
>> +        if (y <= geometry.y) {
>> +            y = geometry.y + 200;
>>           }
>> -        if (x == (screen_width - 1)) {
>> -            x -= 200;
>> +        if (x - geometry.x >= (geometry.width - 1)) {
>> +            x = geometry.x + (geometry.width - 1) - 200;
>>           }
>> -        if (y == (screen_height - 1)) {
>> -            y -= 200;
>> +        if (y - geometry.y >= (geometry.height - 1)) {
>> +            y = geometry.y + (geometry.height - 1) - 200;
>>           }
>>
>>           if (x != (int)motion->x_root || y != (int)motion->y_root) {
>> -- 
>> 2.32.0
>>

Re: [PATCH] ui/gtk: Fix relative mouse with multiple monitors

[Marc-André Lureau]

[-- Attachment #1: Type: text/plain, Size: 3262 bytes --]

Hi Dennis

On Tue, Jun 29, 2021 at 5:26 PM Dennis Wölfing <denniswoelfing@gmx.de>
wrote:

> To handle relative mouse input the event handler needs to move the mouse
> away from the screen edges. Failing to do so results in the mouse
> getting stuck at invisible walls. However the current implementation for
> this is broken on hosts with multiple monitors.
>
> With multiple monitors the mouse can be located outside of the current
> monitor which is not handled by the current code. Also the monitor
> itself might be located at coordinates different from (0, 0).
>
> Signed-off-by: Dennis Wölfing <denniswoelfing@gmx.de>
>

Looks reasonable to me. In spice-gtk we have slightly different code, we
wrap at the middle of the monitor instead (
https://gitlab.freedesktop.org/spice/spice-gtk/-/blob/master/src/spice-widget.c#L1214),
what do you think?

And also, spice-gtk has special cases for w32 and wayland, which behave
differently.

Gtk4 is also different, as device_warp() is gone (it will have to handle it
specifically again for the different platforms:
https://gitlab.gnome.org/malureau/rdw/-/blob/master/rdw/src/display.rs#L812)



---
>  ui/gtk.c | 21 +++++++++------------
>  1 file changed, 9 insertions(+), 12 deletions(-)
>
> diff --git a/ui/gtk.c b/ui/gtk.c
> index 98046f577b..5258532b19 100644
> --- a/ui/gtk.c
> +++ b/ui/gtk.c
> @@ -865,33 +865,30 @@ static gboolean gd_motion_event(GtkWidget *widget,
> GdkEventMotion *motion,
>          GdkWindow *win = gtk_widget_get_window(widget);
>          GdkMonitor *monitor = gdk_display_get_monitor_at_window(dpy, win);
>          GdkRectangle geometry;
> -        int screen_width, screen_height;
>
>          int x = (int)motion->x_root;
>          int y = (int)motion->y_root;
>
>          gdk_monitor_get_geometry(monitor, &geometry);
> -        screen_width = geometry.width;
> -        screen_height = geometry.height;
>
>          /* In relative mode check to see if client pointer hit
> -         * one of the screen edges, and if so move it back by
> +         * one of the monitor edges, and if so move it back by
>           * 200 pixels. This is important because the pointer
>           * in the server doesn't correspond 1-for-1, and so
>           * may still be only half way across the screen. Without
>           * this warp, the server pointer would thus appear to hit
>           * an invisible wall */
> -        if (x == 0) {
> -            x += 200;
> +        if (x <= geometry.x) {
> +            x = geometry.x + 200;
>          }
> -        if (y == 0) {
> -            y += 200;
> +        if (y <= geometry.y) {
> +            y = geometry.y + 200;
>          }
> -        if (x == (screen_width - 1)) {
> -            x -= 200;
> +        if (x - geometry.x >= (geometry.width - 1)) {
> +            x = geometry.x + (geometry.width - 1) - 200;
>          }
> -        if (y == (screen_height - 1)) {
> -            y -= 200;
> +        if (y - geometry.y >= (geometry.height - 1)) {
> +            y = geometry.y + (geometry.height - 1) - 200;
>          }
>
>          if (x != (int)motion->x_root || y != (int)motion->y_root) {
> --
> 2.32.0
>
>
>

-- 
Marc-André Lureau

[-- Attachment #2: Type: text/html, Size: 4475 bytes --]

Re: [PATCH] ui/gtk: Fix relative mouse with multiple monitors

[Dennis Wölfing]

On 19.07.21 13:31, Marc-André Lureau wrote:
> Hi Dennis
>
> On Tue, Jun 29, 2021 at 5:26 PM Dennis Wölfing <denniswoelfing@gmx.de> wrote:
>
> > To handle relative mouse input the event handler needs to move the mouse
> > away from the screen edges. Failing to do so results in the mouse
> > getting stuck at invisible walls. However the current implementation for
> > this is broken on hosts with multiple monitors.
> >
> > With multiple monitors the mouse can be located outside of the current
> > monitor which is not handled by the current code. Also the monitor
> > itself might be located at coordinates different from (0, 0).
> >
> > Signed-off-by: Dennis Wölfing <denniswoelfing@gmx.de>
>
>
> Looks reasonable to me. In spice-gtk we have slightly different code, we
> wrap at the middle of the monitor instead
> (https://gitlab.freedesktop.org/spice/spice-gtk/-/blob/master/src/spice-widget.c#L1214),
> what do you think?

Indeed, warping to the center of the monitor allows for simpler code and
means that it will take longer until the cursors hits a border again.
I'll send a v2 that warps to the middle of the monitor.

> And also, spice-gtk has special cases for w32 and wayland, which behave
> differently.

I just tested QEMU on a Windows host and relative mouse input is quite
broken both before and after this patch: As soon as the cursor leaves
the window QEMU no longer receives mouse events until the cursor is
moved back into the window. I haven't tested it with Wayland.
So we should perhaps have similar special cases as in spice-gtk. However
I'm not comfortable with writing a patch to fix this as I am not
familiar with the Windows and Wayland APIs.

> Gtk4 is also different, as device_warp() is gone (it will have to handle
> it specifically again for the different platforms:
> https://gitlab.gnome.org/malureau/rdw/-/blob/master/rdw/src/display.rs#L812)

That is unfortunate. The code will need to be rewritten then when it
updated for Gtk4.