Hi
On Sat, Feb 24, 2024 at 4:49 AM Satyeshwar Singh
<satyeshwar.si...@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.si...@intel.com>
> Cc: Marc-André Lureau <marcandre.lur...@redhat.com>
> Cc: Vivek Kasireddy <vivek.kasire...@intel.com>
> Cc: Dongwon Kim <dongwon....@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
>
