[PATCH v2 0/6] ui: rework -show-cursor option
Gerd Hoffmann (6): ui: add show-cursor option ui/gtk: implement show-cursor option ui/sdl: implement show-cursor option ui/cocoa: implement show-cursor option ui: wire up legacy -show-cursor option ui: deprecate legacy -show-cursor option include/sysemu/sysemu.h | 1 - ui/gtk.c | 8 +++++++- ui/sdl2.c | 28 ++++++++++++++++++++-------- vl.c | 6 ++++-- qapi/ui.json | 3 +++ qemu-deprecated.texi | 5 +++++ ui/cocoa.m | 4 ++++ 7 files changed, 43 insertions(+), 12 deletions(-) -- 2.18.1
When enabled, this forces showing the mouse cursor, i.e. do not allow the guest to hide it. Defaults to off. Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com> --- qapi/ui.json | 3 +++ 1 file changed, 3 insertions(+) diff --git a/qapi/ui.json b/qapi/ui.json index e04525d8b44b..b9df7fe7b5cb 100644 --- a/qapi/ui.json +++ b/qapi/ui.json @@ -1144,6 +1144,8 @@ # @type: Which DisplayType qemu should use. # @full-screen: Start user interface in fullscreen mode (default: off). # @window-close: Allow to quit qemu with window close button (default: on). +# @show-cursor: Force showing the mouse cursor (default: off). +# Since: 5.0 # @gl: Enable OpenGL support (default: off). # # Since: 2.12 @@ -1153,6 +1155,7 @@ 'base' : { 'type' : 'DisplayType', '*full-screen' : 'bool', '*window-close' : 'bool', + '*show-cursor' : 'bool', '*gl' : 'DisplayGLMode' }, 'discriminator' : 'type', 'data' : { 'gtk' : 'DisplayGTK', -- 2.18.1
Gerd Hoffmann <kraxel@redhat.com> writes:
When enabled, this forces showing the mouse cursor, i.e. do not allow the guest to hide it. Defaults to off.
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com> --- qapi/ui.json | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/qapi/ui.json b/qapi/ui.json index e04525d8b44b..b9df7fe7b5cb 100644 --- a/qapi/ui.json +++ b/qapi/ui.json @@ -1144,6 +1144,8 @@ # @type: Which DisplayType qemu should use. # @full-screen: Start user interface in fullscreen mode (default: off). # @window-close: Allow to quit qemu with window close button (default: on). +# @show-cursor: Force showing the mouse cursor (default: off). +# Since: 5.0 # @gl: Enable OpenGL support (default: off). # # Since: 2.12 @@ -1153,6 +1155,7 @@ 'base' : { 'type' : 'DisplayType', '*full-screen' : 'bool', '*window-close' : 'bool', + '*show-cursor' : 'bool', '*gl' : 'DisplayGLMode' }, 'discriminator' : 'type', 'data' : { 'gtk' : 'DisplayGTK',
Acked-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com> --- ui/gtk.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/ui/gtk.c b/ui/gtk.c index d18892d1de61..78b197ade4c1 100644 --- a/ui/gtk.c +++ b/ui/gtk.c @@ -247,6 +247,7 @@ static void gd_update_cursor(VirtualConsole *vc) { GtkDisplayState *s = vc->s; GdkWindow *window; + bool allow_hide_cursor = true; if (vc->type != GD_VC_GFX || !qemu_console_is_graphic(vc->gfx.dcl.con)) { @@ -257,8 +258,13 @@ static void gd_update_cursor(VirtualConsole *vc) return; } + if (s->opts->has_show_cursor && s->opts->show_cursor) { + allow_hide_cursor = false; + } + window = gtk_widget_get_window(GTK_WIDGET(vc->gfx.drawing_area)); - if (s->full_screen || qemu_input_is_absolute() || s->ptr_owner == vc) { + if (allow_hide_cursor && + (s->full_screen || qemu_input_is_absolute() || s->ptr_owner == vc)) { gdk_window_set_cursor(window, s->null_cursor); } else { gdk_window_set_cursor(window, NULL); -- 2.18.1
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com> --- ui/sdl2.c | 28 ++++++++++++++++++++-------- 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/ui/sdl2.c b/ui/sdl2.c index 9030f1c42efb..e18555b10a42 100644 --- a/ui/sdl2.c +++ b/ui/sdl2.c @@ -161,9 +161,15 @@ static void sdl_update_caption(struct sdl2_console *scon) } } -static void sdl_hide_cursor(void) +static void sdl_hide_cursor(struct sdl2_console *scon) { - if (!cursor_hide) { + bool allow_hide_cursor = true; + + if (scon->opts->has_show_cursor && scon->opts->show_cursor) { + allow_hide_cursor = false; + } + + if (!allow_hide_cursor) { return; } @@ -175,9 +181,15 @@ static void sdl_hide_cursor(void) } } -static void sdl_show_cursor(void) +static void sdl_show_cursor(struct sdl2_console *scon) { - if (!cursor_hide) { + bool allow_hide_cursor = true; + + if (scon->opts->has_show_cursor && scon->opts->show_cursor) { + allow_hide_cursor = false; + } + + if (!allow_hide_cursor) { return; } @@ -216,7 +228,7 @@ static void sdl_grab_start(struct sdl2_console *scon) SDL_WarpMouseInWindow(scon->real_window, guest_x, guest_y); } } else { - sdl_hide_cursor(); + sdl_hide_cursor(scon); } SDL_SetWindowGrab(scon->real_window, SDL_TRUE); gui_grab = 1; @@ -227,7 +239,7 @@ static void sdl_grab_end(struct sdl2_console *scon) { SDL_SetWindowGrab(scon->real_window, SDL_FALSE); gui_grab = 0; - sdl_show_cursor(); + sdl_show_cursor(scon); sdl_update_caption(scon); } @@ -658,7 +670,7 @@ static void sdl_mouse_warp(DisplayChangeListener *dcl, if (on) { if (!guest_cursor) { - sdl_show_cursor(); + sdl_show_cursor(scon); } if (gui_grab || qemu_input_is_absolute() || absolute_enabled) { SDL_SetCursor(guest_sprite); @@ -667,7 +679,7 @@ static void sdl_mouse_warp(DisplayChangeListener *dcl, } } } else if (gui_grab) { - sdl_hide_cursor(); + sdl_hide_cursor(scon); } guest_cursor = on; guest_x = x, guest_y = y; -- 2.18.1
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> --- ui/cocoa.m | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ui/cocoa.m b/ui/cocoa.m index fbb5b1b45f81..f7b323044582 100644 --- a/ui/cocoa.m +++ b/ui/cocoa.m @@ -125,6 +125,7 @@ typedef struct { NSWindow *normalWindow, *about_window; static DisplayChangeListener *dcl; static int last_buttons; +static int cursor_hide = 1; int gArgc; char **gArgv; @@ -1918,6 +1919,9 @@ static void cocoa_display_init(DisplayState *ds, DisplayOptions *opts) [(QemuCocoaAppController *)[[NSApplication sharedApplication] delegate] toggleFullScreen: nil]; }); } + if (opts->has_show_cursor && opts->show_cursor) { + cursor_hide = 0; + } dcl = g_malloc0(sizeof(DisplayChangeListener)); -- 2.18.1
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> --- include/sysemu/sysemu.h | 1 - vl.c | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h index 6358a324a711..7956e9054ade 100644 --- a/include/sysemu/sysemu.h +++ b/include/sysemu/sysemu.h @@ -41,7 +41,6 @@ extern const char *keyboard_layout; extern int win2k_install_hack; extern int alt_grab; extern int ctrl_grab; -extern int cursor_hide; extern int graphic_rotate; extern int no_shutdown; extern int old_param; diff --git a/vl.c b/vl.c index 7dcb0879c497..fd96d04578cd 100644 --- a/vl.c +++ b/vl.c @@ -168,7 +168,6 @@ int no_hpet = 0; int fd_bootchk = 1; static int no_reboot; int no_shutdown = 0; -int cursor_hide = 1; int graphic_rotate = 0; const char *watchdog; QEMUOptionRom option_rom[MAX_OPTION_ROMS]; @@ -3553,7 +3552,8 @@ int main(int argc, char **argv, char **envp) no_shutdown = 1; break; case QEMU_OPTION_show_cursor: - cursor_hide = 0; + dpy.has_show_cursor = true; + dpy.show_cursor = true; break; case QEMU_OPTION_uuid: if (qemu_uuid_parse(optarg, &qemu_uuid) < 0) { -- 2.18.1
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com> --- vl.c | 2 ++ qemu-deprecated.texi | 5 +++++ 2 files changed, 7 insertions(+) diff --git a/vl.c b/vl.c index fd96d04578cd..2f8b9753b49d 100644 --- a/vl.c +++ b/vl.c @@ -3552,6 +3552,8 @@ int main(int argc, char **argv, char **envp) no_shutdown = 1; break; case QEMU_OPTION_show_cursor: + warn_report("The -show-cursor option is deprecated, " + "use -display {sdl,gtk},show-cursor=on instead"); dpy.has_show_cursor = true; dpy.show_cursor = true; break; diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi index ea3e10bde398..c1444d1839bd 100644 --- a/qemu-deprecated.texi +++ b/qemu-deprecated.texi @@ -148,6 +148,11 @@ QEMU 5.0 introduced an alternative syntax to specify the size of the translation block cache, @option{-accel tcg,tb-size=}. The new syntax deprecates the previously available @option{-tb-size} option. +@subsection -show-cursor option (since 5.0) + +Use @option{-display sdl,show-cursor=on} or + @option{-display gtk,show-cursor=on} instead. + @section QEMU Machine Protocol (QMP) commands @subsection change (since 2.5.0) -- 2.18.1
On Thu, 6 Feb 2020 at 11:29, Gerd Hoffmann <kraxel@redhat.com> wrote:
This cover letter is missing a description of what the patchset does... The closest thing the patchset seems to get to documentation is the oneliner in ui.json: +# @show-cursor: Force showing the mouse cursor (default: off). but looking at the ui/cocoa.m implementation that isn't what it actually does -- it just seems to mean "default to shown on startup", because the logic that unconditionally hides the host cursor on mousegrab and unhides it on ungrab remains unchanged. This doesn't on the face of it sound like very useful behaviour, since the option will only have an effect for the short period of time between QEMU startup and the first mouse-grab, but without documentation of what the option is intended to do and in particular how it's intended to interact with grab/ungrab I don't know what your intention for the behaviour was. thanks -- PMM
On Thu, Feb 06, 2020 at 11:52:05AM +0000, Peter Maydell wrote:
On Thu, 6 Feb 2020 at 11:29, Gerd Hoffmann <kraxel@redhat.com> wrote:
This cover letter is missing a description of what the patchset does...
The closest thing the patchset seems to get to documentation is the oneliner in ui.json: +# @show-cursor: Force showing the mouse cursor (default: off).
but looking at the ui/cocoa.m implementation that isn't what it actually does -- it just seems to mean "default to shown on startup", because the logic that unconditionally hides the host cursor on mousegrab and unhides it on ungrab remains unchanged. This doesn't on the face of it sound like very useful behaviour, since the option will only have an effect for the short period of time between QEMU startup and the first mouse-grab, but without documentation of what the option is intended to do and in particular how it's intended to interact with grab/ungrab I don't know what your intention for the behaviour was.
Well, it doesn't change actual behavior for SDL and cocoa. It only adds "-display {sdl,cocoa},show-cursor=on" as replacement for the global "-show-cursor" option. Guess I should reorder the patches (move 5/6 before the individual UI patches) and reword the commit messages. If you think cocoa behavior isn't useful we can revert commit 13aefd303cf996c2d183e94082413885bf1d15bf instead, or drop the cursor_hide check in hideCursor() + unhideCursor(). Your call. It also adds gtk support (based on a patch by jpewhacker@gmail.com). cheers, Gerd
On Thu, 6 Feb 2020 at 13:20, Gerd Hoffmann <kraxel@redhat.com> wrote:
On Thu, Feb 06, 2020 at 11:52:05AM +0000, Peter Maydell wrote:
On Thu, 6 Feb 2020 at 11:29, Gerd Hoffmann <kraxel@redhat.com> wrote: This cover letter is missing a description of what the patchset does...
The closest thing the patchset seems to get to documentation is the oneliner in ui.json: +# @show-cursor: Force showing the mouse cursor (default: off).
but looking at the ui/cocoa.m implementation that isn't what it actually does -- it just seems to mean "default to shown on startup", because the logic that unconditionally hides the host cursor on mousegrab and unhides it on ungrab remains unchanged. This doesn't on the face of it sound like very useful behaviour, since the option will only have an effect for the short period of time between QEMU startup and the first mouse-grab, but without documentation of what the option is intended to do and in particular how it's intended to interact with grab/ungrab I don't know what your intention for the behaviour was.
Well, it doesn't change actual behavior for SDL and cocoa. It only adds "-display {sdl,cocoa},show-cursor=on" as replacement for the global "-show-cursor" option. Guess I should reorder the patches (move 5/6 before the individual UI patches) and reword the commit messages.
If you think cocoa behavior isn't useful we can revert commit 13aefd303cf996c2d183e94082413885bf1d15bf instead, or drop the cursor_hide check in hideCursor() + unhideCursor(). Your call.
I think we should start by documenting what the behaviour we expect of the UI with this option set and unset is. Once we know what we're trying to achieve, we can look at what we need to do to the code to get there from here... Not having the requirements/expectations on the UI frontend clearly documented makes it a lot harder to keep the behaviour of our UIs in sync -- we end up making changes to one UI or another that make sense in isolation but result in random unintended inconsistencies between them. thanks -- PMM
On Thu, Feb 06, 2020 at 02:20:02PM +0100, Gerd Hoffmann wrote:
On Thu, Feb 06, 2020 at 11:52:05AM +0000, Peter Maydell wrote:
On Thu, 6 Feb 2020 at 11:29, Gerd Hoffmann <kraxel@redhat.com> wrote:
This cover letter is missing a description of what the patchset does...
The closest thing the patchset seems to get to documentation is the oneliner in ui.json: +# @show-cursor: Force showing the mouse cursor (default: off).
but looking at the ui/cocoa.m implementation that isn't what it actually does -- it just seems to mean "default to shown on startup", because the logic that unconditionally hides the host cursor on mousegrab and unhides it on ungrab remains unchanged. This doesn't on the face of it sound like very useful behaviour, since the option will only have an effect for the short period of time between QEMU startup and the first mouse-grab, but without documentation of what the option is intended to do and in particular how it's intended to interact with grab/ungrab I don't know what your intention for the behaviour was.
Well, it doesn't change actual behavior for SDL and cocoa. It only adds "-display {sdl,cocoa},show-cursor=on" as replacement for the global "-show-cursor" option. Guess I should reorder the patches (move 5/6 before the individual UI patches) and reword the commit messages.
I suppose this patch set didn't intend to workaround the missing cursor over VNC with NVIDIA vGPUs, right? IIRC the default is that the local cursor is sent over to the destination whereas we need to force the remote cursor to be sent back, do I remember that correctly the setting for which have to advertize vncviewer? Erik
participants (4)
-
Erik Skultety -
Gerd Hoffmann -
Markus Armbruster -
Peter Maydell