Re: [Qemu-devel] [PATCH 02/24] sdl: remove -alt-grab and -ctrl-grab support

Re: [Qemu-devel] [PATCH 02/24] sdl: remove -alt-grab and -ctrl-grab support

[Programmingkid]

> With absolute pointer devices such as usb-tablet being widely used
> mouse grabs (for relative pointing devices) should be rarely needed
> these days.  So the benefit of the options to configure the hotkey
> modifiers for grab (and other actions) seems questionable.  Which
> is expecially true for the -ctrl-grab which isn't handled in the
> handle_keyup() code.

So does this mean you are against a patch that would allow the user to chose a key to act as a mouse ungrab key?

Re: [Qemu-devel] [PATCH 02/24] sdl: remove -alt-grab and -ctrl-grab support

[Gerd Hoffmann]

On Fri, Nov 17, 2017 at 11:06:12AM -0500, Programmingkid wrote:
> 
> > With absolute pointer devices such as usb-tablet being widely used
> > mouse grabs (for relative pointing devices) should be rarely needed
> > these days.  So the benefit of the options to configure the hotkey
> > modifiers for grab (and other actions) seems questionable.  Which
> > is expecially true for the -ctrl-grab which isn't handled in the
> > handle_keyup() code.
> 
> So does this mean you are against a patch that would allow the user to
> chose a key to act as a mouse ungrab key?

I don't consider it a top priority, and given there I have lots of other
things to do I most likely wouldn't implement that.  But I wouldn't
object if someone comes up with a sane implementation.

What we have right now is just a big mess though, both in code and the
user interface.  First, these are toplevel command line options.  It
belongs to -display though.  Second, it's two hard-coded names for two
hard-coded variations.  So it's not like you can configure much.  On top
of that the names are not exactly intuitive.  And it works with SDL only.

cheers,
  Gerd

[Qemu-devel] [PATCH 00/24] RfC: rework display initialization

[Gerd Hoffmann]

  Hi,

This series reworks the qemu display initialization.  Changes:

  * Create a QAPI DisplayOptions type for display configuration.
  * Switch all display initialization calls to accept DisplayOptions
    instead of a bunch of bools.
  * Add a registry for displays, and remove alot of #ifdefs from vl.c
  * Build sdl, gtk and curses as modules.

For now I'm looking for comments especially on the DisplayOptions QAPI
type.  Is there a more elegant way than the empty DisplayNoOpts struct
(see patch #5)?

cheers,
  Gerd

Gerd Hoffmann (24):
  sdl: remove -no-frame support
  sdl: remove -alt-grab and -ctrl-grab support
  sdl: use ctrl-alt-g as grab hotkey
  vl: rename DisplayType to LegacyDisplayType
  gtk: add and use DisplayOptions + DisplayGTK
  sdl: use DisplayOptions
  vl: drop no_quit variable
  egl-headless: use DisplayOptions
  curses: use DisplayOptions
  cocoa: use DisplayOptions
  vl: drop full_screen variable
  vl: drop display_type variable
  vl: drop request_opengl variable
  console: add qemu display registry, add gtk
  sdl: hook up to display registry
  cocoa: hook up to display registry
  curses: hook up to display registry
  egl-headless: hook up to display registry
  console: add and use qemu_display_find_default
  console: add ui module loading support
  sdl: build as module
  gtk: build as module
  curses: build as module
  build: opengl should not need X11

 configure             |  17 +++--
 Makefile.objs         |   1 +
 include/qemu/module.h |   1 +
 include/ui/console.h  |  77 +++------------------
 ui/console.c          |  59 ++++++++++++++++
 ui/curses.c           |  14 +++-
 ui/egl-headless.c     |  20 +++++-
 ui/gtk.c              |  45 ++++++++----
 ui/sdl.c              |  89 ++++++++++--------------
 ui/sdl2.c             |  92 ++++++++++++-------------
 vl.c                  | 186 +++++++++++++-------------------------------------
 qapi/ui.json          |  49 +++++++++++++
 ui/Makefile.objs      |  26 ++++---
 ui/cocoa.m            |  16 ++++-
 14 files changed, 348 insertions(+), 344 deletions(-)

-- 
2.9.3

[Qemu-devel] [PATCH 02/24] sdl: remove -alt-grab and -ctrl-grab support

[Gerd Hoffmann]

With absolute pointer devices such as usb-tablet being widely used
mouse grabs (for relative pointing devices) should be rarely needed
these days.  So the benefit of the options to configure the hotkey
modifiers for grab (and other actions) seems questionable.  Which
is expecially true for the -ctrl-grab which isn't handled in the
handle_keyup() code.

Drop the code, print a notice when the option is still used.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 ui/sdl.c  | 22 +++-------------------
 ui/sdl2.c | 23 +++--------------------
 vl.c      | 26 ++++----------------------
 3 files changed, 10 insertions(+), 61 deletions(-)

diff --git a/ui/sdl.c b/ui/sdl.c
index f3adbb7a64..2c85482b5e 100644
--- a/ui/sdl.c
+++ b/ui/sdl.c
@@ -364,12 +364,7 @@ static void sdl_update_caption(void)
     if (!runstate_is_running())
         status = " [Stopped]";
     else if (gui_grab) {
-        if (alt_grab)
-            status = " - Press Ctrl-Alt-Shift to exit mouse grab";
-        else if (ctrl_grab)
-            status = " - Press Right-Ctrl to exit mouse grab";
-        else
-            status = " - Press Ctrl-Alt to exit mouse grab";
+        status = " - Press Ctrl-Alt to exit mouse grab";
     }
 
     if (qemu_name) {
@@ -556,14 +551,7 @@ static void handle_keydown(SDL_Event *ev)
     int mod_state;
     int keycode;
 
-    if (alt_grab) {
-        mod_state = (SDL_GetModState() & (gui_grab_code | KMOD_LSHIFT)) ==
-                    (gui_grab_code | KMOD_LSHIFT);
-    } else if (ctrl_grab) {
-        mod_state = (SDL_GetModState() & KMOD_RCTRL) == KMOD_RCTRL;
-    } else {
-        mod_state = (SDL_GetModState() & gui_grab_code) == gui_grab_code;
-    }
+    mod_state = (SDL_GetModState() & gui_grab_code) == gui_grab_code;
     gui_key_modifier_pressed = mod_state;
 
     if (gui_key_modifier_pressed) {
@@ -701,11 +689,7 @@ static void handle_keyup(SDL_Event *ev)
 {
     int mod_state;
 
-    if (!alt_grab) {
-        mod_state = (ev->key.keysym.mod & gui_grab_code);
-    } else {
-        mod_state = (ev->key.keysym.mod & (gui_grab_code | KMOD_LSHIFT));
-    }
+    mod_state = (ev->key.keysym.mod & gui_grab_code);
     if (!mod_state && gui_key_modifier_pressed) {
         gui_key_modifier_pressed = 0;
         if (gui_keysym == 0) {
diff --git a/ui/sdl2.c b/ui/sdl2.c
index 3802a9b477..e15566877b 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -139,13 +139,7 @@ static void sdl_update_caption(struct sdl2_console *scon)
     if (!runstate_is_running()) {
         status = " [Stopped]";
     } else if (gui_grab) {
-        if (alt_grab) {
-            status = " - Press Ctrl-Alt-Shift to exit grab";
-        } else if (ctrl_grab) {
-            status = " - Press Right-Ctrl to exit grab";
-        } else {
-            status = " - Press Ctrl-Alt to exit grab";
-        }
+        status = " - Press Ctrl-Alt to exit grab";
     }
 
     if (qemu_name) {
@@ -336,14 +330,7 @@ static void handle_keydown(SDL_Event *ev)
     int mod_state, win;
     struct sdl2_console *scon = get_scon_from_window(ev->key.windowID);
 
-    if (alt_grab) {
-        mod_state = (SDL_GetModState() & (gui_grab_code | KMOD_LSHIFT)) ==
-            (gui_grab_code | KMOD_LSHIFT);
-    } else if (ctrl_grab) {
-        mod_state = (SDL_GetModState() & KMOD_RCTRL) == KMOD_RCTRL;
-    } else {
-        mod_state = (SDL_GetModState() & gui_grab_code) == gui_grab_code;
-    }
+    mod_state = (SDL_GetModState() & gui_grab_code) == gui_grab_code;
     gui_key_modifier_pressed = mod_state;
 
     if (gui_key_modifier_pressed) {
@@ -420,11 +407,7 @@ static void handle_keyup(SDL_Event *ev)
     int mod_state;
     struct sdl2_console *scon = get_scon_from_window(ev->key.windowID);
 
-    if (!alt_grab) {
-        mod_state = (ev->key.keysym.mod & gui_grab_code);
-    } else {
-        mod_state = (ev->key.keysym.mod & (gui_grab_code | KMOD_LSHIFT));
-    }
+    mod_state = (ev->key.keysym.mod & gui_grab_code);
     if (!mod_state && gui_key_modifier_pressed) {
         gui_key_modifier_pressed = 0;
         if (gui_keysym == 0) {
diff --git a/vl.c b/vl.c
index 4d8553b433..7dab7523a6 100644
--- a/vl.c
+++ b/vl.c
@@ -175,8 +175,6 @@ QEMUOptionRom option_rom[MAX_OPTION_ROMS];
 int nb_option_roms;
 int old_param = 0;
 const char *qemu_name;
-int alt_grab = 0;
-int ctrl_grab = 0;
 unsigned int nb_prom_envs = 0;
 const char *prom_envs[MAX_PROM_ENVS];
 int boot_menu;
@@ -2167,22 +2165,10 @@ static DisplayType select_display(const char *p)
                 warn_report("frame sdl option is unsupported, ignoring");
             } else if (strstart(opts, ",alt_grab=", &nextopt)) {
                 opts = nextopt;
-                if (strstart(opts, "on", &nextopt)) {
-                    alt_grab = 1;
-                } else if (strstart(opts, "off", &nextopt)) {
-                    alt_grab = 0;
-                } else {
-                    goto invalid_sdl_args;
-                }
+                warn_report("ctrl_grab sdl option is unsupported, ignoring");
             } else if (strstart(opts, ",ctrl_grab=", &nextopt)) {
                 opts = nextopt;
-                if (strstart(opts, "on", &nextopt)) {
-                    ctrl_grab = 1;
-                } else if (strstart(opts, "off", &nextopt)) {
-                    ctrl_grab = 0;
-                } else {
-                    goto invalid_sdl_args;
-                }
+                warn_report("alt_grab sdl option is unsupported, ignoring");
             } else if (strstart(opts, ",window_close=", &nextopt)) {
                 opts = nextopt;
                 if (strstart(opts, "on", &nextopt)) {
@@ -3780,10 +3766,10 @@ int main(int argc, char **argv, char **envp)
                 warn_report("-no-frame switch is unsupported, ignoring");
                 break;
             case QEMU_OPTION_alt_grab:
-                alt_grab = 1;
+                warn_report("-alt-grab switch is unsupported, ignoring");
                 break;
             case QEMU_OPTION_ctrl_grab:
-                ctrl_grab = 1;
+                warn_report("-ctrl-grab switch is unsupported, ignoring");
                 break;
             case QEMU_OPTION_no_quit:
                 no_quit = 1;
@@ -4466,10 +4452,6 @@ int main(int argc, char **argv, char **envp)
 #endif
     }
 
-    if ((alt_grab || ctrl_grab) && display_type != DT_SDL) {
-        error_report("-no-frame, -alt-grab and -ctrl-grab are only valid "
-                     "for SDL, ignoring option");
-    }
     if (no_quit && (display_type != DT_GTK && display_type != DT_SDL)) {
         error_report("-no-quit is only valid for GTK and SDL, "
                      "ignoring option");
-- 
2.9.3

Re: [Qemu-devel] [PATCH 02/24] sdl: remove -alt-grab and -ctrl-grab support

[Daniel P. Berrange]

On Fri, Nov 17, 2017 at 11:30:24AM +0100, Gerd Hoffmann wrote:
> With absolute pointer devices such as usb-tablet being widely used
> mouse grabs (for relative pointing devices) should be rarely needed
> these days.  So the benefit of the options to configure the hotkey
> modifiers for grab (and other actions) seems questionable.  Which
> is expecially true for the -ctrl-grab which isn't handled in the
> handle_keyup() code.
> 
> Drop the code, print a notice when the option is still used.

These need to go through the proper deprecation process too.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH 02/24] sdl: remove -alt-grab and -ctrl-grab support

[Gerd Hoffmann]

On Fri, Nov 17, 2017 at 02:22:22PM +0000, Daniel P. Berrange wrote:
> On Fri, Nov 17, 2017 at 11:30:24AM +0100, Gerd Hoffmann wrote:
> > With absolute pointer devices such as usb-tablet being widely used
> > mouse grabs (for relative pointing devices) should be rarely needed
> > these days.  So the benefit of the options to configure the hotkey
> > modifiers for grab (and other actions) seems questionable.  Which
> > is expecially true for the -ctrl-grab which isn't handled in the
> > handle_keyup() code.
> > 
> > Drop the code, print a notice when the option is still used.
> 
> These need to go through the proper deprecation process too.

Ok, will do.

cheers,
  Gerd