From: "Marc-André Lureau" <marcandre.lureau@redhat.com>
To: Satyeshwar Singh <satyeshwar.singh@intel.com>
Cc: qemu-devel@nongnu.org,
Vivek Kasireddy <vivek.kasireddy@intel.com>,
Dongwon Kim <dongwon.kim@intel.com>
Subject: Re: [PATCH] virtio-gpu: Fix HW cursor visibility
Date: Wed, 28 Feb 2024 16:10:21 +0400 [thread overview]
Message-ID: <CAMxuvaysAxuchZAc-R+fAXfjtCOnKZhPDZbWHSEzgA-s5f5EkA@mail.gmail.com> (raw)
In-Reply-To: <20240224015123.1575580-1-satyeshwar.singh@intel.com>
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
>
prev parent reply other threads:[~2024-02-28 12:13 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-24 1:51 [PATCH] virtio-gpu: Fix HW cursor visibility Satyeshwar Singh
2024-02-28 12:10 ` Marc-André Lureau [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAMxuvaysAxuchZAc-R+fAXfjtCOnKZhPDZbWHSEzgA-s5f5EkA@mail.gmail.com \
--to=marcandre.lureau@redhat.com \
--cc=dongwon.kim@intel.com \
--cc=qemu-devel@nongnu.org \
--cc=satyeshwar.singh@intel.com \
--cc=vivek.kasireddy@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).