When show-cursor is on, most of the time Windows VM draws the cursor by
itself and hands it over to Qemu as a separate resource. However,
sometimes, Windows OS gives control of the cursor to applications like
Notepad. In such cases a software cursor which is part of the overall
framebuffer is drawn by the application. Windows intimates the indirect
display driver (IDD) about this change through a flag and IDD is in turn
responsible for communicating with the hypervisor (Qemu) to hide the HW
cursor. This show/hide cursor can happen any time dynamically.

Unfortunately, it seems like Qemu doesn't expect this change to happen
dynamically. The code in virtio-gpu.c was written such that
update_cursor would first call dpy_cursor_define if the cursor shape has
changed and this is not a simple move operation (which indeed is the
case when moving to a SW cursor) and then call dpy_mouse_set.
dpy_cursor_define calls toolkits like GTK but in addition to creating
the cursor content, it also calls gdk_window_set_cursor thereby setting
the cursor. It is therefore, the right function to either show or hide
the cursor but there was no code present to hide the cursor. Changing
the cursor visibility in dpy_mouse_set has two issues. First,
dpy_cursor_define already decided to display the cursor so showing the
cursor in the previous call only to hide it in dpy_mouse_set doesn't
sound right. Second, dpy_mouse_set skips the rest of the code if we
are in absolute mode so adding this code there wouldn't work in that
mode.

Qemu makes the decision of whether to show or hide the cursor by
observing the cursor->resource_id flag in virtio-gpu.c as is evident
from the lines
	dpy_mouse_set(s->con, cursor->pos.x, cursor->pos.y,
                  cursor->resource_id ? 1 : 0);
The last parameter is considered the "visible" parameter in gdk code.
Therefore, in this patch we continue with the same model. Instead of
changing the function prototype and changing a dozen plus files, we are
adding the visible field in QEMUCursor data structure and passing
it from virtio-gpu to the GTK code where we check if the cursor is
visible or not. If not, we simply call gdk_window_set_cursor with NULL
parameter, thereby hiding the cursor. Once Windows VM switches back to
the HW cursor, then IDD would again provide a new resource_id to Qemu
and we can start displaying it once more.

Signed-off-by: Satyeshwar Singh <satyeshwar.singh@intel.com>
Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
Cc: Vivek Kasireddy <vivek.kasireddy@intel.com>
Cc: Dongwon Kim <dongwon.kim@intel.com>
---
 hw/display/virtio-gpu.c | 1 +
 include/ui/console.h    | 1 +
 ui/gtk.c                | 5 +++++
 3 files changed, 7 insertions(+)

diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 1c1ee230b3..1ae9be605b 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -98,6 +98,7 @@ static void update_cursor(VirtIOGPU *g, struct virtio_gpu_update_cursor *cursor)
 
         s->current_cursor->hot_x = cursor->hot_x;
         s->current_cursor->hot_y = cursor->hot_y;
+        s->current_cursor->visible = cursor->resource_id ? 1 : 0;
 
         if (cursor->resource_id > 0) {
             vgc->update_cursor_data(g, s, cursor->resource_id);
diff --git a/include/ui/console.h b/include/ui/console.h
index a4a49ffc64..47c39ed405 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -161,6 +161,7 @@ typedef struct QEMUCursor {
     uint16_t            width, height;
     int                 hot_x, hot_y;
     int                 refcount;
+    int                 visible;
     uint32_t            data[];
 } QEMUCursor;
 
diff --git a/ui/gtk.c b/ui/gtk.c
index 810d7fc796..12a76de570 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -478,6 +478,11 @@ static void gd_cursor_define(DisplayChangeListener *dcl,
         return;
     }
 
+    if(!c->visible) {
+        gdk_window_set_cursor(gtk_widget_get_window(vc->gfx.drawing_area), NULL);
+        return;
+    }
+
     pixbuf = gdk_pixbuf_new_from_data((guchar *)(c->data),
                                       GDK_COLORSPACE_RGB, true, 8,
                                       c->width, c->height, c->width * 4,
-- 
2.33.1

Hi

On Sat, Feb 24, 2024 at 4:49 AM Satyeshwar Singh
<satyeshwar.singh@intel.com> wrote:
>
> When show-cursor is on, most of the time Windows VM draws the cursor by
> itself and hands it over to Qemu as a separate resource. However,
> sometimes, Windows OS gives control of the cursor to applications like
> Notepad. In such cases a software cursor which is part of the overall
> framebuffer is drawn by the application. Windows intimates the indirect
> display driver (IDD) about this change through a flag and IDD is in turn
> responsible for communicating with the hypervisor (Qemu) to hide the HW
> cursor. This show/hide cursor can happen any time dynamically.
>
> Unfortunately, it seems like Qemu doesn't expect this change to happen
> dynamically. The code in virtio-gpu.c was written such that
> update_cursor would first call dpy_cursor_define if the cursor shape has
> changed and this is not a simple move operation (which indeed is the
> case when moving to a SW cursor) and then call dpy_mouse_set.
> dpy_cursor_define calls toolkits like GTK but in addition to creating
> the cursor content, it also calls gdk_window_set_cursor thereby setting
> the cursor. It is therefore, the right function to either show or hide
> the cursor but there was no code present to hide the cursor. Changing
> the cursor visibility in dpy_mouse_set has two issues. First,
> dpy_cursor_define already decided to display the cursor so showing the
> cursor in the previous call only to hide it in dpy_mouse_set doesn't
> sound right. Second, dpy_mouse_set skips the rest of the code if we
> are in absolute mode so adding this code there wouldn't work in that
> mode.
>
> Qemu makes the decision of whether to show or hide the cursor by
> observing the cursor->resource_id flag in virtio-gpu.c as is evident
> from the lines
>         dpy_mouse_set(s->con, cursor->pos.x, cursor->pos.y,
>                   cursor->resource_id ? 1 : 0);
> The last parameter is considered the "visible" parameter in gdk code.
> Therefore, in this patch we continue with the same model. Instead of
> changing the function prototype and changing a dozen plus files, we are
> adding the visible field in QEMUCursor data structure and passing
> it from virtio-gpu to the GTK code where we check if the cursor is

You will need to review all usages of QEMUCursor and set visible =
true by default.

Whether it's better or not than modifying a dozen files with a new
"visible" argument, I can't say.

Also we should have consistent behaviour across all display backends,
not just GTK.

> visible or not. If not, we simply call gdk_window_set_cursor with NULL

You should use GtkDisplayState.null_cursor, there is an option to
still show the cursor, even when the guest makes it invisible.
(show-cursor=on)

Which guest software do you test with?

Both gtk and firefox with cursor none test will actually set a
"transparent" cursor, which we don't detect. Maybe we should also have
some check for this case to implement that "show-cursor" behaviour in
that case too.

> parameter, thereby hiding the cursor. Once Windows VM switches back to
> the HW cursor, then IDD would again provide a new resource_id to Qemu
> and we can start displaying it once more.
>
> Signed-off-by: Satyeshwar Singh <satyeshwar.singh@intel.com>
> Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
> Cc: Vivek Kasireddy <vivek.kasireddy@intel.com>
> Cc: Dongwon Kim <dongwon.kim@intel.com>
> ---
>  hw/display/virtio-gpu.c | 1 +
>  include/ui/console.h    | 1 +
>  ui/gtk.c                | 5 +++++
>  3 files changed, 7 insertions(+)
>
> diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
> index 1c1ee230b3..1ae9be605b 100644
> --- a/hw/display/virtio-gpu.c
> +++ b/hw/display/virtio-gpu.c
> @@ -98,6 +98,7 @@ static void update_cursor(VirtIOGPU *g, struct virtio_gpu_update_cursor *cursor)
>
>          s->current_cursor->hot_x = cursor->hot_x;
>          s->current_cursor->hot_y = cursor->hot_y;
> +        s->current_cursor->visible = cursor->resource_id ? 1 : 0;
>
>          if (cursor->resource_id > 0) {
>              vgc->update_cursor_data(g, s, cursor->resource_id);
> diff --git a/include/ui/console.h b/include/ui/console.h
> index a4a49ffc64..47c39ed405 100644
> --- a/include/ui/console.h
> +++ b/include/ui/console.h
> @@ -161,6 +161,7 @@ typedef struct QEMUCursor {
>      uint16_t            width, height;
>      int                 hot_x, hot_y;
>      int                 refcount;
> +    int                 visible;
>      uint32_t            data[];
>  } QEMUCursor;
>
> diff --git a/ui/gtk.c b/ui/gtk.c
> index 810d7fc796..12a76de570 100644
> --- a/ui/gtk.c
> +++ b/ui/gtk.c
> @@ -478,6 +478,11 @@ static void gd_cursor_define(DisplayChangeListener *dcl,
>          return;
>      }
>
> +    if(!c->visible) {
> +        gdk_window_set_cursor(gtk_widget_get_window(vc->gfx.drawing_area), NULL);
> +        return;
> +    }
> +
>      pixbuf = gdk_pixbuf_new_from_data((guchar *)(c->data),
>                                        GDK_COLORSPACE_RGB, true, 8,
>                                        c->width, c->height, c->width * 4,
> --
> 2.33.1
>