[Qemu-devel] SDL2 various fixes

[Qemu-devel] SDL2 various fixes

[Jindrich Makovicka]

Hi,

I am sending a couple of patches I needed to apply to make the
SDL2 GUI work reasonably on Debian Sid, which recently switched
from SDL1.

The main issue is that the QEMU window stops updating when it is
minimized, or after a virtual desktop switch. Then, there are
some regressions due to recently added terminals in multiple
windows, and some issues with fullscreen mode and window scaling.

I am using MATE desktop 1.18 and SDL 2.0.6.

Regards,
--
Jindrich Makovicka

[Qemu-devel] [PATCH 1/7] sdl2: Fix broken display updating after the window is hidden

[Jindrich Makovicka]

With SDL 2.0.6, calling SDL_ShowWindow during SDL_WINDOWEVENT_HIDDEN
blocks all subsequent display updates.
---
 ui/sdl2.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/ui/sdl2.c b/ui/sdl2.c
index 53dd447fd2..7f51933234 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -576,11 +576,6 @@ static void handle_windowevent(SDL_Event *ev)
             SDL_HideWindow(scon->real_window);
         }
         break;
-    case SDL_WINDOWEVENT_HIDDEN:
-        if (!scon->hidden) {
-            SDL_ShowWindow(scon->real_window);
-        }
-        break;
     }
 }
 
-- 
2.15.0.rc0

Re: [Qemu-devel] [PATCH 1/7] sdl2: Fix broken display updating after the window is hidden

[Gerd Hoffmann]

On Mon, 2017-10-23 at 23:07 +0200, Jindrich Makovicka wrote:
> With SDL 2.0.6, calling SDL_ShowWindow during SDL_WINDOWEVENT_HIDDEN
> blocks all subsequent display updates.

Hmm, yes, seems we confuse SDL that way.  Also not letting the user
hide the window isn't very nice (same for the _SHOWN event).  But I
think we should update scon->hidden status on these events, so qemu
keeps track of the state properly and mixing ctrl-alt-<nr> hotkeys and
window buttons for hide/show works correctly.

cheers,
  Gerd

[Qemu-devel] [PATCH 2/7] sdl2: Do not quit the emulator when an auxilliary window is closed

[Jindrich Makovicka]

---
 ui/sdl2.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/ui/sdl2.c b/ui/sdl2.c
index 7f51933234..aa37b39547 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -566,9 +566,13 @@ static void handle_windowevent(SDL_Event *ev)
         update_displaychangelistener(&scon->dcl, 500);
         break;
     case SDL_WINDOWEVENT_CLOSE:
-        if (!no_quit) {
-            no_shutdown = 0;
-            qemu_system_shutdown_request(SHUTDOWN_CAUSE_HOST_UI);
+        if (qemu_console_is_graphic(scon->dcl.con)) {
+            if (!no_quit) {
+                no_shutdown = 0;
+                qemu_system_shutdown_request(SHUTDOWN_CAUSE_HOST_UI);
+            }
+        } else {
+            SDL_HideWindow(scon->real_window);
         }
         break;
     case SDL_WINDOWEVENT_SHOWN:
-- 
2.15.0.rc0

Re: [Qemu-devel] [PATCH 2/7] sdl2: Do not quit the emulator when an auxilliary window is closed

[Gerd Hoffmann]

On Mon, 2017-10-23 at 23:07 +0200, Jindrich Makovicka wrote:
> ---
>  ui/sdl2.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/ui/sdl2.c b/ui/sdl2.c
> index 7f51933234..aa37b39547 100644
> --- a/ui/sdl2.c
> +++ b/ui/sdl2.c
> @@ -566,9 +566,13 @@ static void handle_windowevent(SDL_Event *ev)
>          update_displaychangelistener(&scon->dcl, 500);
>          break;
>      case SDL_WINDOWEVENT_CLOSE:
> -        if (!no_quit) {
> -            no_shutdown = 0;
> -            qemu_system_shutdown_request(SHUTDOWN_CAUSE_HOST_UI);
> +        if (qemu_console_is_graphic(scon->dcl.con)) {
> +            if (!no_quit) {
> +                no_shutdown = 0;
> +                qemu_system_shutdown_request(SHUTDOWN_CAUSE_HOST_UI)
> ;
> +            }
> +        } else {
> +            SDL_HideWindow(scon->real_window);

scon->hidden should be update here.

>          }
>          break;
>      case SDL_WINDOWEVENT_SHOWN:

[Qemu-devel] [PATCH 3/7] sdl2: Do not hide the cursor on auxilliary windows

[Jindrich Makovicka]

---
 ui/sdl2.c | 41 ++++++++++++++++++++++++++++++-----------
 1 file changed, 30 insertions(+), 11 deletions(-)

diff --git a/ui/sdl2.c b/ui/sdl2.c
index aa37b39547..685e4fabec 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -169,10 +169,10 @@ static void sdl_hide_cursor(void)
         return;
     }
 
-    if (qemu_input_is_absolute()) {
-        SDL_ShowCursor(1);
-        SDL_SetCursor(sdl_cursor_hidden);
-    } else {
+    SDL_ShowCursor(SDL_DISABLE);
+    SDL_SetCursor(sdl_cursor_hidden);
+
+    if (!qemu_input_is_absolute()) {
         SDL_SetRelativeMouseMode(SDL_TRUE);
     }
 }
@@ -185,14 +185,16 @@ static void sdl_show_cursor(void)
 
     if (!qemu_input_is_absolute()) {
         SDL_SetRelativeMouseMode(SDL_FALSE);
-        SDL_ShowCursor(1);
-        if (guest_cursor &&
-            (gui_grab || qemu_input_is_absolute() || absolute_enabled)) {
-            SDL_SetCursor(guest_sprite);
-        } else {
-            SDL_SetCursor(sdl_cursor_normal);
-        }
     }
+
+    if (guest_cursor &&
+        (gui_grab || qemu_input_is_absolute() || absolute_enabled)) {
+        SDL_SetCursor(guest_sprite);
+    } else {
+        SDL_SetCursor(sdl_cursor_normal);
+    }
+
+    SDL_ShowCursor(SDL_ENABLE);
 }
 
 static void sdl_grab_start(struct sdl2_console *scon)
@@ -463,6 +465,10 @@ static void handle_mousemotion(SDL_Event *ev)
     int max_x, max_y;
     struct sdl2_console *scon = get_scon_from_window(ev->key.windowID);
 
+    if (!qemu_console_is_graphic(scon->dcl.con)) {
+        return;
+    }
+    
     if (qemu_input_is_absolute() || absolute_enabled) {
         int scr_w, scr_h;
         SDL_GetWindowSize(scon->real_window, &scr_w, &scr_h);
@@ -490,6 +496,10 @@ static void handle_mousebutton(SDL_Event *ev)
     SDL_MouseButtonEvent *bev;
     struct sdl2_console *scon = get_scon_from_window(ev->key.windowID);
 
+    if (!qemu_console_is_graphic(scon->dcl.con)) {
+        return;
+    }
+    
     bev = &ev->button;
     if (!gui_grab && !qemu_input_is_absolute()) {
         if (ev->type == SDL_MOUSEBUTTONUP && bev->button == SDL_BUTTON_LEFT) {
@@ -512,6 +522,10 @@ static void handle_mousewheel(SDL_Event *ev)
     SDL_MouseWheelEvent *wev = &ev->wheel;
     InputButton btn;
 
+    if (!qemu_console_is_graphic(scon->dcl.con)) {
+        return;
+    }
+    
     if (wev->y > 0) {
         btn = INPUT_BUTTON_WHEEL_UP;
     } else if (wev->y < 0) {
@@ -651,6 +665,11 @@ static void sdl_mouse_warp(DisplayChangeListener *dcl,
                            int x, int y, int on)
 {
     struct sdl2_console *scon = container_of(dcl, struct sdl2_console, dcl);
+
+    if (!qemu_console_is_graphic(scon->dcl.con)) {
+        return;
+    }
+    
     if (on) {
         if (!guest_cursor) {
             sdl_show_cursor();
-- 
2.15.0.rc0

Re: [Qemu-devel] [PATCH 3/7] sdl2: Do not hide the cursor on auxilliary windows

[Philippe Mathieu-Daudé]

Hi Jindrich,

This looks like 2 different patches, can you split?

On 10/23/2017 06:07 PM, Jindrich Makovicka wrote:
> ---
>  ui/sdl2.c | 41 ++++++++++++++++++++++++++++++-----------
>  1 file changed, 30 insertions(+), 11 deletions(-)
> 
> diff --git a/ui/sdl2.c b/ui/sdl2.c
> index aa37b39547..685e4fabec 100644
> --- a/ui/sdl2.c
> +++ b/ui/sdl2.c
> @@ -169,10 +169,10 @@ static void sdl_hide_cursor(void)
>          return;
>      }
>  
> -    if (qemu_input_is_absolute()) {
> -        SDL_ShowCursor(1);
> -        SDL_SetCursor(sdl_cursor_hidden);
> -    } else {
> +    SDL_ShowCursor(SDL_DISABLE);
> +    SDL_SetCursor(sdl_cursor_hidden);
> +
> +    if (!qemu_input_is_absolute()) {
>          SDL_SetRelativeMouseMode(SDL_TRUE);
>      }
>  }
> @@ -185,14 +185,16 @@ static void sdl_show_cursor(void)
>  
>      if (!qemu_input_is_absolute()) {
>          SDL_SetRelativeMouseMode(SDL_FALSE);
> -        SDL_ShowCursor(1);
> -        if (guest_cursor &&
> -            (gui_grab || qemu_input_is_absolute() || absolute_enabled)) {
> -            SDL_SetCursor(guest_sprite);
> -        } else {
> -            SDL_SetCursor(sdl_cursor_normal);
> -        }
>      }
> +
> +    if (guest_cursor &&
> +        (gui_grab || qemu_input_is_absolute() || absolute_enabled)) {
> +        SDL_SetCursor(guest_sprite);
> +    } else {
> +        SDL_SetCursor(sdl_cursor_normal);
> +    }
> +
> +    SDL_ShowCursor(SDL_ENABLE);
>  }

first patch:
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

---

second patch:

>  
>  static void sdl_grab_start(struct sdl2_console *scon)
> @@ -463,6 +465,10 @@ static void handle_mousemotion(SDL_Event *ev)
>      int max_x, max_y;
>      struct sdl2_console *scon = get_scon_from_window(ev->key.windowID);
>  
> +    if (!qemu_console_is_graphic(scon->dcl.con)) {
> +        return;
> +    }
> +    
>      if (qemu_input_is_absolute() || absolute_enabled) {
>          int scr_w, scr_h;
>          SDL_GetWindowSize(scon->real_window, &scr_w, &scr_h);
> @@ -490,6 +496,10 @@ static void handle_mousebutton(SDL_Event *ev)
>      SDL_MouseButtonEvent *bev;
>      struct sdl2_console *scon = get_scon_from_window(ev->key.windowID);
>  
> +    if (!qemu_console_is_graphic(scon->dcl.con)) {
> +        return;
> +    }
> +    
>      bev = &ev->button;
>      if (!gui_grab && !qemu_input_is_absolute()) {
>          if (ev->type == SDL_MOUSEBUTTONUP && bev->button == SDL_BUTTON_LEFT) {
> @@ -512,6 +522,10 @@ static void handle_mousewheel(SDL_Event *ev)
>      SDL_MouseWheelEvent *wev = &ev->wheel;
>      InputButton btn;
>  
> +    if (!qemu_console_is_graphic(scon->dcl.con)) {
> +        return;
> +    }
> +    
>      if (wev->y > 0) {
>          btn = INPUT_BUTTON_WHEEL_UP;
>      } else if (wev->y < 0) {
> @@ -651,6 +665,11 @@ static void sdl_mouse_warp(DisplayChangeListener *dcl,
>                             int x, int y, int on)
>  {
>      struct sdl2_console *scon = container_of(dcl, struct sdl2_console, dcl);
> +
> +    if (!qemu_console_is_graphic(scon->dcl.con)) {
> +        return;
> +    }
> +    
>      if (on) {
>          if (!guest_cursor) {
>              sdl_show_cursor();
> 

Regards,

Phil.

Re: [Qemu-devel] [PATCH 3/7] sdl2: Do not hide the cursor on auxilliary windows

[Jindřich Makovička]

[-- Attachment #1: Type: text/plain, Size: 3636 bytes --]

On Mon, 23 Oct 2017 23:38:16 -0300
Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:

> Hi Jindrich,
> 
> This looks like 2 different patches, can you split?

Hi,

here is the split version.

> 
> On 10/23/2017 06:07 PM, Jindrich Makovicka wrote:
> > ---
> >  ui/sdl2.c | 41 ++++++++++++++++++++++++++++++-----------
> >  1 file changed, 30 insertions(+), 11 deletions(-)
> > 
> > diff --git a/ui/sdl2.c b/ui/sdl2.c
> > index aa37b39547..685e4fabec 100644
> > --- a/ui/sdl2.c
> > +++ b/ui/sdl2.c
> > @@ -169,10 +169,10 @@ static void sdl_hide_cursor(void)
> >          return;
> >      }
> >  
> > -    if (qemu_input_is_absolute()) {
> > -        SDL_ShowCursor(1);
> > -        SDL_SetCursor(sdl_cursor_hidden);
> > -    } else {
> > +    SDL_ShowCursor(SDL_DISABLE);
> > +    SDL_SetCursor(sdl_cursor_hidden);
> > +
> > +    if (!qemu_input_is_absolute()) {
> >          SDL_SetRelativeMouseMode(SDL_TRUE);
> >      }
> >  }
> > @@ -185,14 +185,16 @@ static void sdl_show_cursor(void)
> >  
> >      if (!qemu_input_is_absolute()) {
> >          SDL_SetRelativeMouseMode(SDL_FALSE);
> > -        SDL_ShowCursor(1);
> > -        if (guest_cursor &&
> > -            (gui_grab || qemu_input_is_absolute() ||
> > absolute_enabled)) {
> > -            SDL_SetCursor(guest_sprite);
> > -        } else {
> > -            SDL_SetCursor(sdl_cursor_normal);
> > -        }
> >      }
> > +
> > +    if (guest_cursor &&
> > +        (gui_grab || qemu_input_is_absolute() ||
> > absolute_enabled)) {
> > +        SDL_SetCursor(guest_sprite);
> > +    } else {
> > +        SDL_SetCursor(sdl_cursor_normal);
> > +    }
> > +
> > +    SDL_ShowCursor(SDL_ENABLE);
> >  }  
> 
> first patch:
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> 
> ---
> 
> second patch:
> 
> >  
> >  static void sdl_grab_start(struct sdl2_console *scon)
> > @@ -463,6 +465,10 @@ static void handle_mousemotion(SDL_Event *ev)
> >      int max_x, max_y;
> >      struct sdl2_console *scon =
> > get_scon_from_window(ev->key.windowID); 
> > +    if (!qemu_console_is_graphic(scon->dcl.con)) {
> > +        return;
> > +    }
> > +    
> >      if (qemu_input_is_absolute() || absolute_enabled) {
> >          int scr_w, scr_h;
> >          SDL_GetWindowSize(scon->real_window, &scr_w, &scr_h);
> > @@ -490,6 +496,10 @@ static void handle_mousebutton(SDL_Event *ev)
> >      SDL_MouseButtonEvent *bev;
> >      struct sdl2_console *scon =
> > get_scon_from_window(ev->key.windowID); 
> > +    if (!qemu_console_is_graphic(scon->dcl.con)) {
> > +        return;
> > +    }
> > +    
> >      bev = &ev->button;
> >      if (!gui_grab && !qemu_input_is_absolute()) {
> >          if (ev->type == SDL_MOUSEBUTTONUP && bev->button ==
> > SDL_BUTTON_LEFT) { @@ -512,6 +522,10 @@ static void
> > handle_mousewheel(SDL_Event *ev) SDL_MouseWheelEvent *wev =
> > &ev->wheel; InputButton btn;
> >  
> > +    if (!qemu_console_is_graphic(scon->dcl.con)) {
> > +        return;
> > +    }
> > +    
> >      if (wev->y > 0) {
> >          btn = INPUT_BUTTON_WHEEL_UP;
> >      } else if (wev->y < 0) {
> > @@ -651,6 +665,11 @@ static void
> > sdl_mouse_warp(DisplayChangeListener *dcl, int x, int y, int on)
> >  {
> >      struct sdl2_console *scon = container_of(dcl, struct
> > sdl2_console, dcl); +
> > +    if (!qemu_console_is_graphic(scon->dcl.con)) {
> > +        return;
> > +    }
> > +    
> >      if (on) {
> >          if (!guest_cursor) {
> >              sdl_show_cursor();
> >   
> 
> Regards,
> 
> Phil.



-- 
Jindrich Makovicka

[-- Attachment #2: 0003-sdl2-Use-the-same-pointer-show-hide-logic-for-absolu.patch --]
[-- Type: text/x-patch, Size: 1587 bytes --]

>From 746858b932706f49f320c34a4e1259fd005625a3 Mon Sep 17 00:00:00 2001
From: Jindrich Makovicka <makovick@gmail.com>
Date: Tue, 24 Oct 2017 19:30:58 +0200
Subject: [PATCH 3/8] sdl2: Use the same pointer show/hide logic for absolute
 and relative mode

Also use a proper enum parameter for SDL_ShowCursor
---
 ui/sdl2.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/ui/sdl2.c b/ui/sdl2.c
index aa37b39547..2d0470189f 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -169,10 +169,10 @@ static void sdl_hide_cursor(void)
         return;
     }
 
-    if (qemu_input_is_absolute()) {
-        SDL_ShowCursor(1);
-        SDL_SetCursor(sdl_cursor_hidden);
-    } else {
+    SDL_ShowCursor(SDL_DISABLE);
+    SDL_SetCursor(sdl_cursor_hidden);
+
+    if (!qemu_input_is_absolute()) {
         SDL_SetRelativeMouseMode(SDL_TRUE);
     }
 }
@@ -185,14 +185,16 @@ static void sdl_show_cursor(void)
 
     if (!qemu_input_is_absolute()) {
         SDL_SetRelativeMouseMode(SDL_FALSE);
-        SDL_ShowCursor(1);
-        if (guest_cursor &&
-            (gui_grab || qemu_input_is_absolute() || absolute_enabled)) {
-            SDL_SetCursor(guest_sprite);
-        } else {
-            SDL_SetCursor(sdl_cursor_normal);
-        }
     }
+
+    if (guest_cursor &&
+        (gui_grab || qemu_input_is_absolute() || absolute_enabled)) {
+        SDL_SetCursor(guest_sprite);
+    } else {
+        SDL_SetCursor(sdl_cursor_normal);
+    }
+
+    SDL_ShowCursor(SDL_ENABLE);
 }
 
 static void sdl_grab_start(struct sdl2_console *scon)
-- 
2.15.0.rc2


[-- Attachment #3: 0004-sdl2-Do-not-hide-the-cursor-on-auxilliary-windows.patch --]
[-- Type: text/x-patch, Size: 1901 bytes --]

>From 30174a668c17d230f8e55909f1406c9897baf395 Mon Sep 17 00:00:00 2001
From: Jindrich Makovicka <makovick@gmail.com>
Date: Sat, 14 Oct 2017 16:27:23 +0200
Subject: [PATCH 4/8] sdl2: Do not hide the cursor on auxilliary windows

---
 ui/sdl2.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/ui/sdl2.c b/ui/sdl2.c
index 2d0470189f..685e4fabec 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -465,6 +465,10 @@ static void handle_mousemotion(SDL_Event *ev)
     int max_x, max_y;
     struct sdl2_console *scon = get_scon_from_window(ev->key.windowID);
 
+    if (!qemu_console_is_graphic(scon->dcl.con)) {
+        return;
+    }
+    
     if (qemu_input_is_absolute() || absolute_enabled) {
         int scr_w, scr_h;
         SDL_GetWindowSize(scon->real_window, &scr_w, &scr_h);
@@ -492,6 +496,10 @@ static void handle_mousebutton(SDL_Event *ev)
     SDL_MouseButtonEvent *bev;
     struct sdl2_console *scon = get_scon_from_window(ev->key.windowID);
 
+    if (!qemu_console_is_graphic(scon->dcl.con)) {
+        return;
+    }
+    
     bev = &ev->button;
     if (!gui_grab && !qemu_input_is_absolute()) {
         if (ev->type == SDL_MOUSEBUTTONUP && bev->button == SDL_BUTTON_LEFT) {
@@ -514,6 +522,10 @@ static void handle_mousewheel(SDL_Event *ev)
     SDL_MouseWheelEvent *wev = &ev->wheel;
     InputButton btn;
 
+    if (!qemu_console_is_graphic(scon->dcl.con)) {
+        return;
+    }
+    
     if (wev->y > 0) {
         btn = INPUT_BUTTON_WHEEL_UP;
     } else if (wev->y < 0) {
@@ -653,6 +665,11 @@ static void sdl_mouse_warp(DisplayChangeListener *dcl,
                            int x, int y, int on)
 {
     struct sdl2_console *scon = container_of(dcl, struct sdl2_console, dcl);
+
+    if (!qemu_console_is_graphic(scon->dcl.con)) {
+        return;
+    }
+    
     if (on) {
         if (!guest_cursor) {
             sdl_show_cursor();
-- 
2.15.0.rc2

[Qemu-devel] [PATCH 4/7] sdl2: Only accept the hotkeys on the main window

[Jindrich Makovicka]

---
 ui/sdl2.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ui/sdl2.c b/ui/sdl2.c
index 685e4fabec..fa54353430 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -349,7 +349,7 @@ static void handle_keydown(SDL_Event *ev)
     }
     gui_key_modifier_pressed = mod_state;
 
-    if (gui_key_modifier_pressed) {
+    if (gui_key_modifier_pressed && !ev->key.repeat && qemu_console_is_graphic(scon->dcl.con)) {
         switch (ev->key.keysym.scancode) {
         case SDL_SCANCODE_2:
         case SDL_SCANCODE_3:
-- 
2.15.0.rc0

Re: [Qemu-devel] [PATCH 4/7] sdl2: Only accept the hotkeys on the main window

[Gerd Hoffmann]

Why?

> ---
>  ui/sdl2.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/ui/sdl2.c b/ui/sdl2.c
> index 685e4fabec..fa54353430 100644
> --- a/ui/sdl2.c
> +++ b/ui/sdl2.c
> @@ -349,7 +349,7 @@ static void handle_keydown(SDL_Event *ev)
>      }
>      gui_key_modifier_pressed = mod_state;
>  
> -    if (gui_key_modifier_pressed) {
> +    if (gui_key_modifier_pressed && !ev->key.repeat &&
> qemu_console_is_graphic(scon->dcl.con)) {
>          switch (ev->key.keysym.scancode) {
>          case SDL_SCANCODE_2:
>          case SDL_SCANCODE_3:

Re: [Qemu-devel] [PATCH 4/7] sdl2: Only accept the hotkeys on the main window

[Jindřich Makovička]

This fixes the following case:

1) Ctrl-Alt-2 for console is pressed
2) console pops up and gets focus
3) console receives the Ctrl-Alt-2 keypress event
4) console closes
5) focus returns to the main window
6) main window gets the keypress event
7) goto 2 until the key is released, with 50% chance that the console
window is shown at the end


On Wed, Nov 1, 2017 at 11:40 AM Gerd Hoffmann <kraxel@redhat.com> wrote:

> Why?
>
> > ---
> >  ui/sdl2.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/ui/sdl2.c b/ui/sdl2.c
> > index 685e4fabec..fa54353430 100644
> > --- a/ui/sdl2.c
> > +++ b/ui/sdl2.c
> > @@ -349,7 +349,7 @@ static void handle_keydown(SDL_Event *ev)
> >      }
> >      gui_key_modifier_pressed = mod_state;
> >
> > -    if (gui_key_modifier_pressed) {
> > +    if (gui_key_modifier_pressed && !ev->key.repeat &&
> > qemu_console_is_graphic(scon->dcl.con)) {
> >          switch (ev->key.keysym.scancode) {
> >          case SDL_SCANCODE_2:
> >          case SDL_SCANCODE_3:
>
-- 
Jindřich Makovička

Re: [Qemu-devel] [PATCH 4/7] sdl2: Only accept the hotkeys on the main window

[Gerd Hoffmann]

On Thu, 2017-11-02 at 17:12 +0000, Jindřich Makovička wrote:
> This fixes the following case:
> 
> 1) Ctrl-Alt-2 for console is pressed
> 2) console pops up and gets focus
> 3) console receives the Ctrl-Alt-2 keypress event

Hmm, doesn't reproduce here.
Also: why should the same event be delivered twice?

Ignoring autorepeat events (as the patch does, but which the commit
message doesn't mention) makes sense.  But I still fail to see why we
should limit hotkeys to the main window.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH 4/7] sdl2: Only accept the hotkeys on the main window

[Peter Maydell]

On 3 November 2017 at 09:26, Gerd Hoffmann <kraxel@redhat.com> wrote:
> Ignoring autorepeat events (as the patch does, but which the commit
> message doesn't mention) makes sense.  But I still fail to see why we
> should limit hotkeys to the main window.

Is there documentation about how the input subsystem wants to
handle autorepeat? (ie how UI frontends should present repeating
keys to it, and how keyboard backends should expect to see
repeating keys delivered from it). There's been discussion on
this in another thread about xenfb and vnc...

Also I think it would be nice to have written up what the
"expected" set of hotkeys for UI frontends are, so we can
be consistent. I think SDL and gtk are different about what
their ungrab keycombo is, for instance. (I've been assuming
gtk is the "this is the right behaviour" exemplar to follow.)

thanks
-- PMM

Re: [Qemu-devel] [PATCH 4/7] sdl2: Only accept the hotkeys on the main window

[Gerd Hoffmann]

On Fri, 2017-11-03 at 09:37 +0000, Peter Maydell wrote:
> On 3 November 2017 at 09:26, Gerd Hoffmann <kraxel@redhat.com> wrote:
> > Ignoring autorepeat events (as the patch does, but which the commit
> > message doesn't mention) makes sense.  But I still fail to see why
> > we
> > should limit hotkeys to the main window.
> 
> Is there documentation about how the input subsystem wants to
> handle autorepeat? (ie how UI frontends should present repeating
> keys to it, and how keyboard backends should expect to see
> repeating keys delivered from it). There's been discussion on
> this in another thread about xenfb and vnc...

Well, that issue didn't got much attention, probably due to the fact
that the linux kernel does autorepeat in software anyway so at least
for linux guests it doesn't matter at all what qemu is doing here.

Currently the input layer doesn't special-case repeat events in any
way, so they are just passed through to the device in case the ui
submits them.

>From a design point of view I think it makes sense to have UIs send
only keydown and keyup events and not any repeat events.  Then leave it
to the device emulation (i.e. ps/2 kbs) to actually generate repeat
events.  That is the only way the repeat rate programmed by the guest
will work as expected.

> Also I think it would be nice to have written up what the
> "expected" set of hotkeys for UI frontends are, so we can
> be consistent. I think SDL and gtk are different about what
> their ungrab keycombo is, for instance. (I've been assuming
> gtk is the "this is the right behaviour" exemplar to follow.)

That is another mess I'll plan to look at, because I want rework the
display command line switches parsing.  So, the current state is:

gtk ungrab is ctrl-alt-g.
sdl ungrab is ctrl-alt (by default, but see below).
cocoa ungrab is ctl-alt too.
For reference: virt-viewer (vnc/spice client) likewise uses ctrl-alt.

So it seems gtk is the odd one here.  Possibly due to how hotkeys are
implemented in gtk, could be a GtkAccelGroup simply doesn't support
modifier-only hotkeys.

Also:  We have a -alt-grab switch which makes sdl use the ctrl-alt-
shift modifier combo (for both ungrab and the other hotkeys).  We have
a -ctrl-grab switch which makes sdl use the right ctrl key (again for
both ungrab and hotkeys).

While being at it:  We have a -no-frame switch which makes sdl disable
the window frame, but it works on sdl1 only and nobody complained so
far ...

I'm tempted to just drop support for -alt-grab, -ctrl-grab and -no-
frame switches.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH 4/7] sdl2: Only accept the hotkeys on the main window

[Peter Maydell]

On 3 November 2017 at 10:35, Gerd Hoffmann <kraxel@redhat.com> wrote:
> On Fri, 2017-11-03 at 09:37 +0000, Peter Maydell wrote:
>> Also I think it would be nice to have written up what the
>> "expected" set of hotkeys for UI frontends are, so we can
>> be consistent. I think SDL and gtk are different about what
>> their ungrab keycombo is, for instance. (I've been assuming
>> gtk is the "this is the right behaviour" exemplar to follow.)
>
> That is another mess I'll plan to look at, because I want rework the
> display command line switches parsing.  So, the current state is:
>
> gtk ungrab is ctrl-alt-g.
> sdl ungrab is ctrl-alt (by default, but see below).
> cocoa ungrab is ctl-alt too.
> For reference: virt-viewer (vnc/spice client) likewise uses ctrl-alt.
>
> So it seems gtk is the odd one here.  Possibly due to how hotkeys are
> implemented in gtk, could be a GtkAccelGroup simply doesn't support
> modifier-only hotkeys.

There's a patch in-flight to switch cocoa to ctrl-alt-g.
(If you think that's a bad idea it would be useful to reply
to that patch.)

Using plain ctrl-alt for ungrab has the disadvantage that you
then can't send the guest any ctrl-alt-anything combos at all,
I think.

thanks
-- PMM

Re: [Qemu-devel] [PATCH 4/7] sdl2: Only accept the hotkeys on the main window

[Daniel P. Berrange]

On Fri, Nov 03, 2017 at 10:41:35AM +0000, Peter Maydell wrote:
> On 3 November 2017 at 10:35, Gerd Hoffmann <kraxel@redhat.com> wrote:
> > On Fri, 2017-11-03 at 09:37 +0000, Peter Maydell wrote:
> >> Also I think it would be nice to have written up what the
> >> "expected" set of hotkeys for UI frontends are, so we can
> >> be consistent. I think SDL and gtk are different about what
> >> their ungrab keycombo is, for instance. (I've been assuming
> >> gtk is the "this is the right behaviour" exemplar to follow.)
> >
> > That is another mess I'll plan to look at, because I want rework the
> > display command line switches parsing.  So, the current state is:
> >
> > gtk ungrab is ctrl-alt-g.
> > sdl ungrab is ctrl-alt (by default, but see below).
> > cocoa ungrab is ctl-alt too.
> > For reference: virt-viewer (vnc/spice client) likewise uses ctrl-alt.
> >
> > So it seems gtk is the odd one here.  Possibly due to how hotkeys are
> > implemented in gtk, could be a GtkAccelGroup simply doesn't support
> > modifier-only hotkeys.
> 
> There's a patch in-flight to switch cocoa to ctrl-alt-g.
> (If you think that's a bad idea it would be useful to reply
> to that patch.)
> 
> Using plain ctrl-alt for ungrab has the disadvantage that you
> then can't send the guest any ctrl-alt-anything combos at all,
> I think.

You can deal with that by only processing the  ctrl-alt pair on key
release, and require that the immediately preceeding key press events
were also ctrl-alt.

eg require the specific sequence

  press(ctrl), press(alt), release(alt), release(ctrl)

and thus you can still do 

  press(ctrl), press(alt), press(a), release(a) release(alt), release(ctrl)

and have it go to the guest as normal

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 4/7] sdl2: Only accept the hotkeys on the main window

[Gerd Hoffmann]

  Hi,

> There's a patch in-flight to switch cocoa to ctrl-alt-g.
> (If you think that's a bad idea it would be useful to reply
> to that patch.)

I personally don't mind much.  I rarely use it.  With a tablet added to
the guest (which I expect most people do these days) mouse grabs don't
happen anyway.

But when switching cocoa it makes sense to switch sdl too for
consistency.

> Using plain ctrl-alt for ungrab has the disadvantage that you
> then can't send the guest any ctrl-alt-anything combos at all,
> I think.

I think it works, because SDL does the ungrab on ctrl-alt key
*release*.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH 4/7] sdl2: Only accept the hotkeys on the main window

[Peter Maydell]

On 3 November 2017 at 11:01, Gerd Hoffmann <kraxel@redhat.com> wrote:
>   Hi,
>
>> There's a patch in-flight to switch cocoa to ctrl-alt-g.
>> (If you think that's a bad idea it would be useful to reply
>> to that patch.)
>
> I personally don't mind much.  I rarely use it.  With a tablet added to
> the guest (which I expect most people do these days) mouse grabs don't
> happen anyway.

I don't think we have the same grab semantics exactly on all
UI frontends either, so I dunno that that "doesn't grab on tablet"
is true either.

> But when switching cocoa it makes sense to switch sdl too for
> consistency.
>
>> Using plain ctrl-alt for ungrab has the disadvantage that you
>> then can't send the guest any ctrl-alt-anything combos at all,
>> I think.
>
> I think it works, because SDL does the ungrab on ctrl-alt key
> *release*.

This is exactly the kind of fine detail it would be helpful to
have written down so we can be consistent :-)

thanks
-- PMM

Re: [Qemu-devel] [PATCH 4/7] sdl2: Only accept the hotkeys on the main window

[Gerd Hoffmann]

  Hi,

> I don't think we have the same grab semantics exactly on all
> UI frontends either, so I dunno that that "doesn't grab on tablet"
> is true either.

Well, there are pointer grabs and keyboard grabs.

Pointer grabs happen in relative mouse mode only, and I'm pretty sure
we are consistent here.

Keyboard grabs typically happen if the mouse is within the qemu window,
so keys usually captured by the window manager go to the guest instead.
  Allows to alt-tab cycle through the guest windows instead of the host
windows.  gtk has a menu option for that to enable/disable, for sdl it
is always on.  Dunno about cocoa.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH 4/7] sdl2: Only accept the hotkeys on the main window

[Peter Maydell]

On 3 November 2017 at 12:07, Gerd Hoffmann <kraxel@redhat.com> wrote:
>   Hi,
>
>> I don't think we have the same grab semantics exactly on all
>> UI frontends either, so I dunno that that "doesn't grab on tablet"
>> is true either.
>
> Well, there are pointer grabs and keyboard grabs.
>
> Pointer grabs happen in relative mouse mode only, and I'm pretty sure
> we are consistent here.

Looking at the cocoa code, we grab the mouse even for absolute.
Commit f61c387ea627079b3 says we did that to align with SDL.

There's also the question of how grabs interact with fullscreen.

thanks
-- PMM

Re: [Qemu-devel] [PATCH 4/7] sdl2: Only accept the hotkeys on the main window

[Gerd Hoffmann]

On Fri, 2017-11-03 at 12:10 +0000, Peter Maydell wrote:
> On 3 November 2017 at 12:07, Gerd Hoffmann <kraxel@redhat.com> wrote:
> >   Hi,
> > 
> > > I don't think we have the same grab semantics exactly on all
> > > UI frontends either, so I dunno that that "doesn't grab on
> > > tablet"
> > > is true either.
> > 
> > Well, there are pointer grabs and keyboard grabs.
> > 
> > Pointer grabs happen in relative mouse mode only, and I'm pretty
> > sure
> > we are consistent here.
> 
> Looking at the cocoa code, we grab the mouse even for absolute.
> Commit f61c387ea627079b3 says we did that to align with SDL.

That seems to be a *keyboard* grab ...

cheers,
  Gerd

Re: [Qemu-devel] [PATCH 4/7] sdl2: Only accept the hotkeys on the main window

[Cole Robinson]

On 11/02/2017 01:12 PM, Jindřich Makovička wrote:
> This fixes the following case:
> 
> 1) Ctrl-Alt-2 for console is pressed
> 2) console pops up and gets focus
> 3) console receives the Ctrl-Alt-2 keypress event
> 4) console closes
> 5) focus returns to the main window
> 6) main window gets the keypress event
> 7) goto 2 until the key is released, with 50% chance that the console
> window is shown at the end

Sounds similar to what I reported in my comment here a while back:
https://bugzilla.libsdl.org/show_bug.cgi?id=3287

- Cole

Re: [Qemu-devel] [PATCH 4/7] sdl2: Only accept the hotkeys on the main window

[Gerd Hoffmann]

On Sun, Nov 05, 2017 at 09:10:45AM -0500, Cole Robinson wrote:
> On 11/02/2017 01:12 PM, Jindřich Makovička wrote:
> > This fixes the following case:
> > 
> > 1) Ctrl-Alt-2 for console is pressed
> > 2) console pops up and gets focus
> > 3) console receives the Ctrl-Alt-2 keypress event
> > 4) console closes
> > 5) focus returns to the main window
> > 6) main window gets the keypress event
> > 7) goto 2 until the key is released, with 50% chance that the console
> > window is shown at the end
> 
> Sounds similar to what I reported in my comment here a while back:
> https://bugzilla.libsdl.org/show_bug.cgi?id=3287

Hmm, so it seems to be a SDL bug.  I'm running SDL 2.0.3 here, which
explains why I can't reproduce it.  According to the bug it seem to be
*repeat* keydown events, so is simply ignoring repeat events enough to
avoid this issue?

Also: Jindřich, do you plan to send a new of the patch series with the
issues discussed fixed?

cheers,
  Gerd

Re: [Qemu-devel] [PATCH 4/7] sdl2: Only accept the hotkeys on the main window

[Jindřich Makovička]

Hi Gerd,

I will try to respin the patch set during the weekend.

Regards, Jindrich

On Wed, Nov 8, 2017 at 11:10 AM Gerd Hoffmann <kraxel@redhat.com> wrote:

> On Sun, Nov 05, 2017 at 09:10:45AM -0500, Cole Robinson wrote:
> > On 11/02/2017 01:12 PM, Jindřich Makovička wrote:
> > > This fixes the following case:
> > >
> > > 1) Ctrl-Alt-2 for console is pressed
> > > 2) console pops up and gets focus
> > > 3) console receives the Ctrl-Alt-2 keypress event
> > > 4) console closes
> > > 5) focus returns to the main window
> > > 6) main window gets the keypress event
> > > 7) goto 2 until the key is released, with 50% chance that the console
> > > window is shown at the end
> >
> > Sounds similar to what I reported in my comment here a while back:
> > https://bugzilla.libsdl.org/show_bug.cgi?id=3287
>
> Hmm, so it seems to be a SDL bug.  I'm running SDL 2.0.3 here, which
> explains why I can't reproduce it.  According to the bug it seem to be
> *repeat* keydown events, so is simply ignoring repeat events enough to
> avoid this issue?
>
> Also: Jindřich, do you plan to send a new of the patch series with the
> issues discussed fixed?
>
> cheers,
>   Gerd
>
> --
Jindřich Makovička

[Qemu-devel] [PATCH 5/7] sdl2 uses surface relative coordinates

[Jindrich Makovicka]

---
 ui/sdl2.c | 28 ++--------------------------
 1 file changed, 2 insertions(+), 26 deletions(-)

diff --git a/ui/sdl2.c b/ui/sdl2.c
index fa54353430..092eab37dc 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -276,32 +276,8 @@ static void sdl_send_mouse_event(struct sdl2_console *scon, int dx, int dy,
     }
 
     if (qemu_input_is_absolute()) {
-        int scr_w, scr_h;
-        int max_w = 0, max_h = 0;
-        int off_x = 0, off_y = 0;
-        int cur_off_x = 0, cur_off_y = 0;
-        int i;
-
-        for (i = 0; i < sdl2_num_outputs; i++) {
-            struct sdl2_console *thiscon = &sdl2_console[i];
-            if (thiscon->real_window && thiscon->surface) {
-                SDL_GetWindowSize(thiscon->real_window, &scr_w, &scr_h);
-                cur_off_x = thiscon->x;
-                cur_off_y = thiscon->y;
-                if (scr_w + cur_off_x > max_w) {
-                    max_w = scr_w + cur_off_x;
-                }
-                if (scr_h + cur_off_y > max_h) {
-                    max_h = scr_h + cur_off_y;
-                }
-                if (i == scon->idx) {
-                    off_x = cur_off_x;
-                    off_y = cur_off_y;
-                }
-            }
-        }
-        qemu_input_queue_abs(scon->dcl.con, INPUT_AXIS_X, off_x + x, 0, max_w);
-        qemu_input_queue_abs(scon->dcl.con, INPUT_AXIS_Y, off_y + y, 0, max_h);
+        qemu_input_queue_abs(scon->dcl.con, INPUT_AXIS_X, x, 0, surface_width(scon->surface));
+        qemu_input_queue_abs(scon->dcl.con, INPUT_AXIS_Y, y, 0, surface_height(scon->surface));
     } else {
         if (guest_cursor) {
             x -= guest_x;
-- 
2.15.0.rc0

Re: [Qemu-devel] [PATCH 5/7] sdl2 uses surface relative coordinates

[Gerd Hoffmann]

Why?

Also I think this breaks the pointer in case window size doesn't match
surface size.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH 5/7] sdl2 uses surface relative coordinates

[Jindřich Makovička]

Hi,

this patch fixes the exact case you mentioned. Currently, with surface size
smaller than the window, the pointer is confined to the top left corner
when using the absolute mode (-usbdevice tablet).

On Wed, Nov 1, 2017 at 11:43 AM Gerd Hoffmann <kraxel@redhat.com> wrote:

> Why?
>
> Also I think this breaks the pointer in case window size doesn't match
> surface size.
>
> cheers,
>   Gerd
>
> --
Jindřich Makovička

Re: [Qemu-devel] [PATCH 5/7] sdl2 uses surface relative coordinates

[Gerd Hoffmann]

On Thu, 2017-11-02 at 17:07 +0000, Jindřich Makovička wrote:
> Hi,
> 
> this patch fixes the exact case you mentioned. Currently, with
> surface size
> smaller than the window, the pointer is confined to the top left
> corner
> when using the absolute mode (-usbdevice tablet).

Tracked down.  Seems to be broken since commit
46522a82236ea0cf9011b89896d2d8f8ddaf2443.  SDL doesn't only scale
window output for us, but also pointer input, so qemu doesn't need to
do this by itself any more.

Patch is correct then.

cheers,
  Gerd

[Qemu-devel] [PATCH 6/7] sdl2: Fix dead keyboard after fullsceen

[Jindrich Makovicka]

---
 ui/sdl2.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/ui/sdl2.c b/ui/sdl2.c
index 092eab37dc..3823f0a834 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -418,6 +418,7 @@ static void handle_keyup(SDL_Event *ev)
             sdl2_reset_keys(scon);
             return;
         }
+        sdl2_reset_keys(scon);
         gui_keysym = 0;
     }
     if (!gui_keysym) {
-- 
2.15.0.rc0

[Qemu-devel] [PATCH 7/7] sdl2: Do not leave grab when fullscreen

[Jindrich Makovicka]

---
 ui/sdl2.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/ui/sdl2.c b/ui/sdl2.c
index 3823f0a834..811c21da27 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -451,8 +451,9 @@ static void handle_mousemotion(SDL_Event *ev)
         SDL_GetWindowSize(scon->real_window, &scr_w, &scr_h);
         max_x = scr_w - 1;
         max_y = scr_h - 1;
-        if (gui_grab && (ev->motion.x == 0 || ev->motion.y == 0 ||
-                         ev->motion.x == max_x || ev->motion.y == max_y)) {
+        if (gui_grab && !gui_fullscreen
+            && (ev->motion.x == 0 || ev->motion.y == 0 ||
+                ev->motion.x == max_x || ev->motion.y == max_y)) {
             sdl_grab_end(scon);
         }
         if (!gui_grab &&
-- 
2.15.0.rc0

Re: [Qemu-devel] SDL2 various fixes

[Philippe Mathieu-Daudé]

Cc'ing Gerd, the maintainer of this area, please include him if you respin.

On 10/23/2017 06:07 PM, Jindrich Makovicka wrote:
> I am sending a couple of patches I needed to apply to make the
> SDL2 GUI work reasonably on Debian Sid, which recently switched
> from SDL1.
> 
> The main issue is that the QEMU window stops updating when it is
> minimized, or after a virtual desktop switch. Then, there are
> some regressions due to recently added terminals in multiple
> windows, and some issues with fullscreen mode and window scaling.
> 
> I am using MATE desktop 1.18 and SDL 2.0.6.

[Qemu-devel] [PATCH] SDL2 various fixes

[Jindrich Makovicka]

Hi,

here is a revised patchset:

- sdl2: Do not hide the cursor on auxilliary windows 
    split into two

- sdl2: Only accept the hotkeys on the main window 
    reworked, also with a more descriptive commit message

- sdl2: Do not quit the emulator when an auxilliary window is closed
    added missing scon->hidden update

- sdl2: Fix broken display updating after the window is hidden
    added scon->hidden update instead + the SHOWN part changed
    the same way

Regards, Jindrich

[Qemu-devel] [PATCH 1/8] sdl2: Fix broken display updating after the window is hidden

[Jindrich Makovicka]

With SDL 2.0.6, calling SDL_ShowWindow during SDL_WINDOWEVENT_HIDDEN
blocks all subsequent display updates.

Instead of trying to override the change, just update the scon->hidden
flag.
---
 ui/sdl2.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/ui/sdl2.c b/ui/sdl2.c
index 53dd447fd2..774904cbf2 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -572,14 +572,10 @@ static void handle_windowevent(SDL_Event *ev)
         }
         break;
     case SDL_WINDOWEVENT_SHOWN:
-        if (scon->hidden) {
-            SDL_HideWindow(scon->real_window);
-        }
+        scon->hidden = false;
         break;
     case SDL_WINDOWEVENT_HIDDEN:
-        if (!scon->hidden) {
-            SDL_ShowWindow(scon->real_window);
-        }
+        scon->hidden = true;
         break;
     }
 }
-- 
2.15.0

[Qemu-devel] [PATCH 2/8] sdl2: Do not quit the emulator when an auxilliary window is closed

[Jindrich Makovicka]

---
 ui/sdl2.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/ui/sdl2.c b/ui/sdl2.c
index 774904cbf2..dfbd0de791 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -566,9 +566,14 @@ static void handle_windowevent(SDL_Event *ev)
         update_displaychangelistener(&scon->dcl, 500);
         break;
     case SDL_WINDOWEVENT_CLOSE:
-        if (!no_quit) {
-            no_shutdown = 0;
-            qemu_system_shutdown_request(SHUTDOWN_CAUSE_HOST_UI);
+        if (qemu_console_is_graphic(scon->dcl.con)) {
+            if (!no_quit) {
+                no_shutdown = 0;
+                qemu_system_shutdown_request(SHUTDOWN_CAUSE_HOST_UI);
+            }
+        } else {
+            SDL_HideWindow(scon->real_window);
+            scon->hidden = true;
         }
         break;
     case SDL_WINDOWEVENT_SHOWN:
-- 
2.15.0

[Qemu-devel] [PATCH 3/8] sdl2: Use the same pointer show/hide logic for absolute and relative mode

[Jindrich Makovicka]

Also use a proper enum parameter for SDL_ShowCursor
---
 ui/sdl2.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/ui/sdl2.c b/ui/sdl2.c
index dfbd0de791..9cf4b1772b 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -169,10 +169,10 @@ static void sdl_hide_cursor(void)
         return;
     }
 
-    if (qemu_input_is_absolute()) {
-        SDL_ShowCursor(1);
-        SDL_SetCursor(sdl_cursor_hidden);
-    } else {
+    SDL_ShowCursor(SDL_DISABLE);
+    SDL_SetCursor(sdl_cursor_hidden);
+
+    if (!qemu_input_is_absolute()) {
         SDL_SetRelativeMouseMode(SDL_TRUE);
     }
 }
@@ -185,14 +185,16 @@ static void sdl_show_cursor(void)
 
     if (!qemu_input_is_absolute()) {
         SDL_SetRelativeMouseMode(SDL_FALSE);
-        SDL_ShowCursor(1);
-        if (guest_cursor &&
-            (gui_grab || qemu_input_is_absolute() || absolute_enabled)) {
-            SDL_SetCursor(guest_sprite);
-        } else {
-            SDL_SetCursor(sdl_cursor_normal);
-        }
     }
+
+    if (guest_cursor &&
+        (gui_grab || qemu_input_is_absolute() || absolute_enabled)) {
+        SDL_SetCursor(guest_sprite);
+    } else {
+        SDL_SetCursor(sdl_cursor_normal);
+    }
+
+    SDL_ShowCursor(SDL_ENABLE);
 }
 
 static void sdl_grab_start(struct sdl2_console *scon)
-- 
2.15.0

[Qemu-devel] [PATCH 4/8] sdl2: Do not hide the cursor on auxilliary windows

[Jindrich Makovicka]

---
 ui/sdl2.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/ui/sdl2.c b/ui/sdl2.c
index 9cf4b1772b..677c22282d 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -465,6 +465,10 @@ static void handle_mousemotion(SDL_Event *ev)
     int max_x, max_y;
     struct sdl2_console *scon = get_scon_from_window(ev->key.windowID);
 
+    if (!qemu_console_is_graphic(scon->dcl.con)) {
+        return;
+    }
+    
     if (qemu_input_is_absolute() || absolute_enabled) {
         int scr_w, scr_h;
         SDL_GetWindowSize(scon->real_window, &scr_w, &scr_h);
@@ -492,6 +496,10 @@ static void handle_mousebutton(SDL_Event *ev)
     SDL_MouseButtonEvent *bev;
     struct sdl2_console *scon = get_scon_from_window(ev->key.windowID);
 
+    if (!qemu_console_is_graphic(scon->dcl.con)) {
+        return;
+    }
+    
     bev = &ev->button;
     if (!gui_grab && !qemu_input_is_absolute()) {
         if (ev->type == SDL_MOUSEBUTTONUP && bev->button == SDL_BUTTON_LEFT) {
@@ -514,6 +522,10 @@ static void handle_mousewheel(SDL_Event *ev)
     SDL_MouseWheelEvent *wev = &ev->wheel;
     InputButton btn;
 
+    if (!qemu_console_is_graphic(scon->dcl.con)) {
+        return;
+    }
+    
     if (wev->y > 0) {
         btn = INPUT_BUTTON_WHEEL_UP;
     } else if (wev->y < 0) {
@@ -655,6 +667,11 @@ static void sdl_mouse_warp(DisplayChangeListener *dcl,
                            int x, int y, int on)
 {
     struct sdl2_console *scon = container_of(dcl, struct sdl2_console, dcl);
+
+    if (!qemu_console_is_graphic(scon->dcl.con)) {
+        return;
+    }
+    
     if (on) {
         if (!guest_cursor) {
             sdl_show_cursor();
-- 
2.15.0

[Qemu-devel] [PATCH 5/8] sdl2 uses surface relative coordinates

[Jindrich Makovicka]

This patch fixes mouse positioning with -device usb-tablet and fullscreen
or resized window.
---
 ui/sdl2.c | 28 ++--------------------------
 1 file changed, 2 insertions(+), 26 deletions(-)

diff --git a/ui/sdl2.c b/ui/sdl2.c
index 677c22282d..51721d764e 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -276,32 +276,8 @@ static void sdl_send_mouse_event(struct sdl2_console *scon, int dx, int dy,
     }
 
     if (qemu_input_is_absolute()) {
-        int scr_w, scr_h;
-        int max_w = 0, max_h = 0;
-        int off_x = 0, off_y = 0;
-        int cur_off_x = 0, cur_off_y = 0;
-        int i;
-
-        for (i = 0; i < sdl2_num_outputs; i++) {
-            struct sdl2_console *thiscon = &sdl2_console[i];
-            if (thiscon->real_window && thiscon->surface) {
-                SDL_GetWindowSize(thiscon->real_window, &scr_w, &scr_h);
-                cur_off_x = thiscon->x;
-                cur_off_y = thiscon->y;
-                if (scr_w + cur_off_x > max_w) {
-                    max_w = scr_w + cur_off_x;
-                }
-                if (scr_h + cur_off_y > max_h) {
-                    max_h = scr_h + cur_off_y;
-                }
-                if (i == scon->idx) {
-                    off_x = cur_off_x;
-                    off_y = cur_off_y;
-                }
-            }
-        }
-        qemu_input_queue_abs(scon->dcl.con, INPUT_AXIS_X, off_x + x, 0, max_w);
-        qemu_input_queue_abs(scon->dcl.con, INPUT_AXIS_Y, off_y + y, 0, max_h);
+        qemu_input_queue_abs(scon->dcl.con, INPUT_AXIS_X, x, 0, surface_width(scon->surface));
+        qemu_input_queue_abs(scon->dcl.con, INPUT_AXIS_Y, y, 0, surface_height(scon->surface));
     } else {
         if (guest_cursor) {
             x -= guest_x;
-- 
2.15.0

[Qemu-devel] [PATCH 6/8] sdl2: Fix dead keyboard after fullsceen

[Jindrich Makovicka]

---
 ui/sdl2.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/ui/sdl2.c b/ui/sdl2.c
index 51721d764e..cdc599be48 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -418,6 +418,7 @@ static void handle_keyup(SDL_Event *ev)
             sdl2_reset_keys(scon);
             return;
         }
+        sdl2_reset_keys(scon);
         gui_keysym = 0;
     }
     if (!gui_keysym) {
-- 
2.15.0

[Qemu-devel] [PATCH 7/8] sdl2: Do not leave grab when fullscreen

[Jindrich Makovicka]

Prevents displaying of a doubled mouse pointer when moving the pointer
to the screen edges when fullscreen.
---
 ui/sdl2.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/ui/sdl2.c b/ui/sdl2.c
index cdc599be48..6feb637739 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -451,8 +451,9 @@ static void handle_mousemotion(SDL_Event *ev)
         SDL_GetWindowSize(scon->real_window, &scr_w, &scr_h);
         max_x = scr_w - 1;
         max_y = scr_h - 1;
-        if (gui_grab && (ev->motion.x == 0 || ev->motion.y == 0 ||
-                         ev->motion.x == max_x || ev->motion.y == max_y)) {
+        if (gui_grab && !gui_fullscreen
+            && (ev->motion.x == 0 || ev->motion.y == 0 ||
+                ev->motion.x == max_x || ev->motion.y == max_y)) {
             sdl_grab_end(scon);
         }
         if (!gui_grab &&
-- 
2.15.0

[Qemu-devel] [PATCH 8/8] sdl2: Ignore UI hotkeys after a focus change when GUI modifier is held

[Jindrich Makovicka]

When SDL2 windows change focus while a key is held, the window that
receives the focus also receives a new KeyDown event, without an
autorepeat flag. This means that if a WM places the qemu console
over the main window after Ctrl-Alt-2, the console closes immediately
after opening. Then, the main window receives the KeyDown event again
and the whole process repeats.

This patch makes the SDL2 UI ignore the KeyDown events on a window that
just received the focus, if the GUI modifier was held. The ignore flag
is reset on a first KeyUp event. This effectively works around the issue
above.
---
 include/ui/sdl2.h |  1 +
 ui/sdl2.c         | 25 ++++++++++++++++---------
 2 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/include/ui/sdl2.h b/include/ui/sdl2.h
index b29cf803c9..51084e6320 100644
--- a/include/ui/sdl2.h
+++ b/include/ui/sdl2.h
@@ -24,6 +24,7 @@ struct sdl2_console {
     int opengl;
     int updates;
     int idle_counter;
+    int ignore_hotkeys;
     SDL_GLContext winctx;
 #ifdef CONFIG_OPENGL
     QemuGLShader *gls;
diff --git a/ui/sdl2.c b/ui/sdl2.c
index 6feb637739..2d48686b83 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -310,22 +310,26 @@ static void toggle_full_screen(struct sdl2_console *scon)
     sdl2_redraw(scon);
 }
 
-static void handle_keydown(SDL_Event *ev)
+static int get_mod_state(void)
 {
-    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)) ==
+        return (SDL_GetModState() & (gui_grab_code | KMOD_LSHIFT)) ==
             (gui_grab_code | KMOD_LSHIFT);
     } else if (ctrl_grab) {
-        mod_state = (SDL_GetModState() & KMOD_RCTRL) == KMOD_RCTRL;
+        return (SDL_GetModState() & KMOD_RCTRL) == KMOD_RCTRL;
     } else {
-        mod_state = (SDL_GetModState() & gui_grab_code) == gui_grab_code;
+        return (SDL_GetModState() & gui_grab_code) == gui_grab_code;
     }
-    gui_key_modifier_pressed = mod_state;
+}
+
+static void handle_keydown(SDL_Event *ev)
+{
+    int win;
+    struct sdl2_console *scon = get_scon_from_window(ev->key.windowID);
 
-    if (gui_key_modifier_pressed) {
+    gui_key_modifier_pressed = get_mod_state();
+
+    if (!scon->ignore_hotkeys && gui_key_modifier_pressed && !ev->key.repeat) {
         switch (ev->key.keysym.scancode) {
         case SDL_SCANCODE_2:
         case SDL_SCANCODE_3:
@@ -399,6 +403,8 @@ static void handle_keyup(SDL_Event *ev)
     int mod_state;
     struct sdl2_console *scon = get_scon_from_window(ev->key.windowID);
 
+    scon->ignore_hotkeys = false;
+
     if (!alt_grab) {
         mod_state = (ev->key.keysym.mod & gui_grab_code);
     } else {
@@ -545,6 +551,7 @@ static void handle_windowevent(SDL_Event *ev)
         if (!gui_grab && (qemu_input_is_absolute() || absolute_enabled)) {
             absolute_mouse_grab(scon);
         }
+        scon->ignore_hotkeys = get_mod_state();
         break;
     case SDL_WINDOWEVENT_FOCUS_LOST:
         if (gui_grab && !gui_fullscreen) {
-- 
2.15.0

Re: [Qemu-devel] [PATCH] SDL2 various fixes

[Philippe Mathieu-Daudé]

Hi Jindrich,

On 11/12/2017 07:42 AM, Jindrich Makovicka wrote:
> Hi,
> 
> here is a revised patchset:

Please increment the patchset version when you respin (this is v2).

> 
> - sdl2: Do not hide the cursor on auxilliary windows 
>     split into two
> 
> - sdl2: Only accept the hotkeys on the main window 
>     reworked, also with a more descriptive commit message
> 
> - sdl2: Do not quit the emulator when an auxilliary window is closed
>     added missing scon->hidden update
> 
> - sdl2: Fix broken display updating after the window is hidden
>     added scon->hidden update instead + the SHOWN part changed
>     the same way

You missed to add your Signed-off-by tag in the whole series.

See:
https://wiki.qemu.org/Contribute/SubmitAPatch#Patch_emails_must_include_a_Signed-off-by:_line

This can be added automatically using the "-s" option of the "git
format-patch" command, also the resend version using the "-v" option.

(https://wiki.qemu.org/Contribute/SubmitAPatch#Use_git_format-patch)

Regards,

Phil.

[Qemu-devel] [PATCH v3] SDL2 various fixes

[Jindrich Makovicka]

Hi,

here is an identical patchset with Signed-off-by.

Regards, Jindrich

[Qemu-devel] [PATCH 1/8] sdl2: Fix broken display updating after the window is hidden

[Jindrich Makovicka]

With SDL 2.0.6, calling SDL_ShowWindow during SDL_WINDOWEVENT_HIDDEN
blocks all subsequent display updates.

Instead of trying to override the change, just update the scon->hidden
flag.

Signed-off-by: Jindrich Makovicka <makovick@gmail.com>
---
 ui/sdl2.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/ui/sdl2.c b/ui/sdl2.c
index 53dd447fd2..774904cbf2 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -572,14 +572,10 @@ static void handle_windowevent(SDL_Event *ev)
         }
         break;
     case SDL_WINDOWEVENT_SHOWN:
-        if (scon->hidden) {
-            SDL_HideWindow(scon->real_window);
-        }
+        scon->hidden = false;
         break;
     case SDL_WINDOWEVENT_HIDDEN:
-        if (!scon->hidden) {
-            SDL_ShowWindow(scon->real_window);
-        }
+        scon->hidden = true;
         break;
     }
 }
-- 
2.15.0

Re: [Qemu-devel] [PATCH 1/8] sdl2: Fix broken display updating after the window is hidden

[Gerd Hoffmann]

On Sun, Nov 12, 2017 at 08:30:25PM +0100, Jindrich Makovicka wrote:
> With SDL 2.0.6, calling SDL_ShowWindow during SDL_WINDOWEVENT_HIDDEN
> blocks all subsequent display updates.
> 
> Instead of trying to override the change, just update the scon->hidden
> flag.

Has for me the side effect that sometimes I have to press ctrl-alt-2
twice.  Showing window first time works.  Hiding the window works too.
Showing it the second time needs the double keypress.

Added fprintfs to figure why [1].  Reason for that seems to be a suspious
SDL_WINDOWEVENT_SHOWN event from SDL, so qemu thinks the window is
visible even though it actually is hidden.

Have a slightly older SDL version (2.0.3).  Do you see that effect with
2.0.6 too?

cheers,
  Gerd

https://www.kraxel.org/cgit/qemu/commit/?h=testing/sdl&id=8235bfb66e7c82870ff1b1f47bdaaf4f522c5444

Re: [Qemu-devel] [PATCH 1/8] sdl2: Fix broken display updating after the window is hidden

[Gerd Hoffmann]

On Mon, Nov 13, 2017 at 01:39:14PM +0100, Gerd Hoffmann wrote:
> On Sun, Nov 12, 2017 at 08:30:25PM +0100, Jindrich Makovicka wrote:
> > With SDL 2.0.6, calling SDL_ShowWindow during SDL_WINDOWEVENT_HIDDEN
> > blocks all subsequent display updates.
> > 
> > Instead of trying to override the change, just update the scon->hidden
> > flag.
> 
> Has for me the side effect that sometimes I have to press ctrl-alt-2
> twice.  Showing window first time works.  Hiding the window works too.
> Showing it the second time needs the double keypress.
> 
> Added fprintfs to figure why [1].  Reason for that seems to be a suspious
> SDL_WINDOWEVENT_SHOWN event from SDL, so qemu thinks the window is
> visible even though it actually is hidden.
> 
> Have a slightly older SDL version (2.0.3).  Do you see that effect with
> 2.0.6 too?

Oh, and I've just seen the current code has been added as attempt to
workaround that bug, see commit d3f3a0f453ea590be529079ae214c200bb5ecc1a.

Hmm.  Seems we are trading one issue for another here, due to bugs in
SDL :(

cheers,
  Gerd

Re: [Qemu-devel] [PATCH 1/8] sdl2: Fix broken display updating after the window is hidden

[Jindřich Makovička]

On Mon, Nov 13, 2017 at 1:59 PM, Gerd Hoffmann <kraxel@redhat.com> wrote:
> On Mon, Nov 13, 2017 at 01:39:14PM +0100, Gerd Hoffmann wrote:
>> On Sun, Nov 12, 2017 at 08:30:25PM +0100, Jindrich Makovicka wrote:
>> > With SDL 2.0.6, calling SDL_ShowWindow during SDL_WINDOWEVENT_HIDDEN
>> > blocks all subsequent display updates.
>> >
>> > Instead of trying to override the change, just update the scon->hidden
>> > flag.
>>
>> Has for me the side effect that sometimes I have to press ctrl-alt-2
>> twice.  Showing window first time works.  Hiding the window works too.
>> Showing it the second time needs the double keypress.
>>
>> Added fprintfs to figure why [1].  Reason for that seems to be a suspious
>> SDL_WINDOWEVENT_SHOWN event from SDL, so qemu thinks the window is
>> visible even though it actually is hidden.
>>
>> Have a slightly older SDL version (2.0.3).  Do you see that effect with
>> 2.0.6 too?
>
> Oh, and I've just seen the current code has been added as attempt to
> workaround that bug, see commit d3f3a0f453ea590be529079ae214c200bb5ecc1a.
>
> Hmm.  Seems we are trading one issue for another here, due to bugs in
> SDL :(

Does this happen for you with the whole series applied, or just the
1st patch? Especially

[PATCH 8/8] sdl2: Ignore UI hotkeys after a focus change when GUI
modifier is held

can have an impact on the behavior you described.

Regards,
-- 
Jindřich Makovička

Re: [Qemu-devel] [PATCH 1/8] sdl2: Fix broken display updating after the window is hidden

[Gerd Hoffmann]

  Hi,

> >> Have a slightly older SDL version (2.0.3).  Do you see that effect with
> >> 2.0.6 too?
> >
> > Oh, and I've just seen the current code has been added as attempt to
> > workaround that bug, see commit d3f3a0f453ea590be529079ae214c200bb5ecc1a.
> >
> > Hmm.  Seems we are trading one issue for another here, due to bugs in
> > SDL :(
> 
> Does this happen for you with the whole series applied, or just the
> 1st patch? Especially

Same behavior here with only first patch and whole series applied.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH 1/8] sdl2: Fix broken display updating after the window is hidden

[Max Reitz]

[-- Attachment #1: Type: text/plain, Size: 1178 bytes --]

On 2017-11-13 13:39, Gerd Hoffmann wrote:
> On Sun, Nov 12, 2017 at 08:30:25PM +0100, Jindrich Makovicka wrote:
>> With SDL 2.0.6, calling SDL_ShowWindow during SDL_WINDOWEVENT_HIDDEN
>> blocks all subsequent display updates.
>>
>> Instead of trying to override the change, just update the scon->hidden
>> flag.
> 
> Has for me the side effect that sometimes I have to press ctrl-alt-2
> twice.  Showing window first time works.  Hiding the window works too.
> Showing it the second time needs the double keypress.
> 
> Added fprintfs to figure why [1].  Reason for that seems to be a suspious
> SDL_WINDOWEVENT_SHOWN event from SDL, so qemu thinks the window is
> visible even though it actually is hidden.
> 
> Have a slightly older SDL version (2.0.3).  Do you see that effect with
> 2.0.6 too?
> 
> cheers,
>   Gerd
> 
> https://www.kraxel.org/cgit/qemu/commit/?h=testing/sdl&id=8235bfb66e7c82870ff1b1f47bdaaf4f522c5444

The only thing I can say is that I submitted
https://bugzilla.libsdl.org/show_bug.cgi?id=2818 as a result.  Nobody
has replied to that in three years, but when testing it on SDL 2.0.7,
the issue appears to be fixed.

Max


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 512 bytes --]

Re: [Qemu-devel] [PATCH 1/8] sdl2: Fix broken display updating after the window is hidden

[Gerd Hoffmann]

> > Have a slightly older SDL version (2.0.3).  Do you see that effect with
> > 2.0.6 too?
> 
> The only thing I can say is that I submitted
> https://bugzilla.libsdl.org/show_bug.cgi?id=2818 as a result.  Nobody
> has replied to that in three years, but when testing it on SDL 2.0.7,
> the issue appears to be fixed.

Ok, good.  I think I'm fine with merging the patch as-is then.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH 1/8] sdl2: Fix broken display updating after the window is hidden

[Max Reitz]

[-- Attachment #1: Type: text/plain, Size: 1411 bytes --]

On 2017-11-13 17:35, Max Reitz wrote:
> On 2017-11-13 13:39, Gerd Hoffmann wrote:
>> On Sun, Nov 12, 2017 at 08:30:25PM +0100, Jindrich Makovicka wrote:
>>> With SDL 2.0.6, calling SDL_ShowWindow during SDL_WINDOWEVENT_HIDDEN
>>> blocks all subsequent display updates.
>>>
>>> Instead of trying to override the change, just update the scon->hidden
>>> flag.
>>
>> Has for me the side effect that sometimes I have to press ctrl-alt-2
>> twice.  Showing window first time works.  Hiding the window works too.
>> Showing it the second time needs the double keypress.
>>
>> Added fprintfs to figure why [1].  Reason for that seems to be a suspious
>> SDL_WINDOWEVENT_SHOWN event from SDL, so qemu thinks the window is
>> visible even though it actually is hidden.
>>
>> Have a slightly older SDL version (2.0.3).  Do you see that effect with
>> 2.0.6 too?
>>
>> cheers,
>>   Gerd
>>
>> https://www.kraxel.org/cgit/qemu/commit/?h=testing/sdl&id=8235bfb66e7c82870ff1b1f47bdaaf4f522c5444
> 
> The only thing I can say is that I submitted
> https://bugzilla.libsdl.org/show_bug.cgi?id=2818 as a result.  Nobody
> has replied to that in three years, but when testing it on SDL 2.0.7,
> the issue appears to be fixed.

Having done a bisect, SDL2 revision 11151:7cfe088fafba seems to have
fixed the issue (for 2.0.6).  See also:

https://bugzilla.libsdl.org/show_bug.cgi?id=3410

Max


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 512 bytes --]

Re: [Qemu-devel] [PATCH 1/8] sdl2: Fix broken display updating after the window is hidden

[Gerd Hoffmann]

> > The only thing I can say is that I submitted
> > https://bugzilla.libsdl.org/show_bug.cgi?id=2818 as a result.  Nobody
> > has replied to that in three years, but when testing it on SDL 2.0.7,
> > the issue appears to be fixed.
> 
> Having done a bisect, SDL2 revision 11151:7cfe088fafba seems to have
> fixed the issue (for 2.0.6).  See also:
> 
> https://bugzilla.libsdl.org/show_bug.cgi?id=3410

Thanks for the effort.  Adding a note to the commit message.

cheers,
  Gerd

[Qemu-devel] [PATCH 2/8] sdl2: Do not quit the emulator when an auxilliary window is closed

[Jindrich Makovicka]

Signed-off-by: Jindrich Makovicka <makovick@gmail.com>
---
 ui/sdl2.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/ui/sdl2.c b/ui/sdl2.c
index 774904cbf2..dfbd0de791 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -566,9 +566,14 @@ static void handle_windowevent(SDL_Event *ev)
         update_displaychangelistener(&scon->dcl, 500);
         break;
     case SDL_WINDOWEVENT_CLOSE:
-        if (!no_quit) {
-            no_shutdown = 0;
-            qemu_system_shutdown_request(SHUTDOWN_CAUSE_HOST_UI);
+        if (qemu_console_is_graphic(scon->dcl.con)) {
+            if (!no_quit) {
+                no_shutdown = 0;
+                qemu_system_shutdown_request(SHUTDOWN_CAUSE_HOST_UI);
+            }
+        } else {
+            SDL_HideWindow(scon->real_window);
+            scon->hidden = true;
         }
         break;
     case SDL_WINDOWEVENT_SHOWN:
-- 
2.15.0

[Qemu-devel] [PATCH 3/8] sdl2: Use the same pointer show/hide logic for absolute and relative mode

[Jindrich Makovicka]

Also use a proper enum parameter for SDL_ShowCursor

Signed-off-by: Jindrich Makovicka <makovick@gmail.com>
---
 ui/sdl2.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/ui/sdl2.c b/ui/sdl2.c
index dfbd0de791..9cf4b1772b 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -169,10 +169,10 @@ static void sdl_hide_cursor(void)
         return;
     }
 
-    if (qemu_input_is_absolute()) {
-        SDL_ShowCursor(1);
-        SDL_SetCursor(sdl_cursor_hidden);
-    } else {
+    SDL_ShowCursor(SDL_DISABLE);
+    SDL_SetCursor(sdl_cursor_hidden);
+
+    if (!qemu_input_is_absolute()) {
         SDL_SetRelativeMouseMode(SDL_TRUE);
     }
 }
@@ -185,14 +185,16 @@ static void sdl_show_cursor(void)
 
     if (!qemu_input_is_absolute()) {
         SDL_SetRelativeMouseMode(SDL_FALSE);
-        SDL_ShowCursor(1);
-        if (guest_cursor &&
-            (gui_grab || qemu_input_is_absolute() || absolute_enabled)) {
-            SDL_SetCursor(guest_sprite);
-        } else {
-            SDL_SetCursor(sdl_cursor_normal);
-        }
     }
+
+    if (guest_cursor &&
+        (gui_grab || qemu_input_is_absolute() || absolute_enabled)) {
+        SDL_SetCursor(guest_sprite);
+    } else {
+        SDL_SetCursor(sdl_cursor_normal);
+    }
+
+    SDL_ShowCursor(SDL_ENABLE);
 }
 
 static void sdl_grab_start(struct sdl2_console *scon)
-- 
2.15.0

[Qemu-devel] [PATCH 4/8] sdl2: Do not hide the cursor on auxilliary windows

[Jindrich Makovicka]

Signed-off-by: Jindrich Makovicka <makovick@gmail.com>
---
 ui/sdl2.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/ui/sdl2.c b/ui/sdl2.c
index 9cf4b1772b..677c22282d 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -465,6 +465,10 @@ static void handle_mousemotion(SDL_Event *ev)
     int max_x, max_y;
     struct sdl2_console *scon = get_scon_from_window(ev->key.windowID);
 
+    if (!qemu_console_is_graphic(scon->dcl.con)) {
+        return;
+    }
+    
     if (qemu_input_is_absolute() || absolute_enabled) {
         int scr_w, scr_h;
         SDL_GetWindowSize(scon->real_window, &scr_w, &scr_h);
@@ -492,6 +496,10 @@ static void handle_mousebutton(SDL_Event *ev)
     SDL_MouseButtonEvent *bev;
     struct sdl2_console *scon = get_scon_from_window(ev->key.windowID);
 
+    if (!qemu_console_is_graphic(scon->dcl.con)) {
+        return;
+    }
+    
     bev = &ev->button;
     if (!gui_grab && !qemu_input_is_absolute()) {
         if (ev->type == SDL_MOUSEBUTTONUP && bev->button == SDL_BUTTON_LEFT) {
@@ -514,6 +522,10 @@ static void handle_mousewheel(SDL_Event *ev)
     SDL_MouseWheelEvent *wev = &ev->wheel;
     InputButton btn;
 
+    if (!qemu_console_is_graphic(scon->dcl.con)) {
+        return;
+    }
+    
     if (wev->y > 0) {
         btn = INPUT_BUTTON_WHEEL_UP;
     } else if (wev->y < 0) {
@@ -655,6 +667,11 @@ static void sdl_mouse_warp(DisplayChangeListener *dcl,
                            int x, int y, int on)
 {
     struct sdl2_console *scon = container_of(dcl, struct sdl2_console, dcl);
+
+    if (!qemu_console_is_graphic(scon->dcl.con)) {
+        return;
+    }
+    
     if (on) {
         if (!guest_cursor) {
             sdl_show_cursor();
-- 
2.15.0

Re: [Qemu-devel] [PATCH 4/8] sdl2: Do not hide the cursor on auxilliary windows

[Gerd Hoffmann]

Fails scripts/checkpatch.pl (trailing whitespace on several lines).

[Qemu-devel] [PATCH 5/8] sdl2 uses surface relative coordinates

[Jindrich Makovicka]

This patch fixes mouse positioning with -device usb-tablet and fullscreen
or resized window.

Signed-off-by: Jindrich Makovicka <makovick@gmail.com>
---
 ui/sdl2.c | 28 ++--------------------------
 1 file changed, 2 insertions(+), 26 deletions(-)

diff --git a/ui/sdl2.c b/ui/sdl2.c
index 677c22282d..51721d764e 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -276,32 +276,8 @@ static void sdl_send_mouse_event(struct sdl2_console *scon, int dx, int dy,
     }
 
     if (qemu_input_is_absolute()) {
-        int scr_w, scr_h;
-        int max_w = 0, max_h = 0;
-        int off_x = 0, off_y = 0;
-        int cur_off_x = 0, cur_off_y = 0;
-        int i;
-
-        for (i = 0; i < sdl2_num_outputs; i++) {
-            struct sdl2_console *thiscon = &sdl2_console[i];
-            if (thiscon->real_window && thiscon->surface) {
-                SDL_GetWindowSize(thiscon->real_window, &scr_w, &scr_h);
-                cur_off_x = thiscon->x;
-                cur_off_y = thiscon->y;
-                if (scr_w + cur_off_x > max_w) {
-                    max_w = scr_w + cur_off_x;
-                }
-                if (scr_h + cur_off_y > max_h) {
-                    max_h = scr_h + cur_off_y;
-                }
-                if (i == scon->idx) {
-                    off_x = cur_off_x;
-                    off_y = cur_off_y;
-                }
-            }
-        }
-        qemu_input_queue_abs(scon->dcl.con, INPUT_AXIS_X, off_x + x, 0, max_w);
-        qemu_input_queue_abs(scon->dcl.con, INPUT_AXIS_Y, off_y + y, 0, max_h);
+        qemu_input_queue_abs(scon->dcl.con, INPUT_AXIS_X, x, 0, surface_width(scon->surface));
+        qemu_input_queue_abs(scon->dcl.con, INPUT_AXIS_Y, y, 0, surface_height(scon->surface));
     } else {
         if (guest_cursor) {
             x -= guest_x;
-- 
2.15.0

Re: [Qemu-devel] [PATCH 5/8] sdl2 uses surface relative coordinates

[Gerd Hoffmann]

On Sun, Nov 12, 2017 at 08:30:29PM +0100, Jindrich Makovicka wrote:
> This patch fixes mouse positioning with -device usb-tablet and fullscreen
> or resized window.

Fails checkpatch too (long lines).

Also: can you add a "Fixes: 46522a82236ea0cf9011b89896d2d8f8ddaf2443"
line (that is the commit which added the bug).

thanks,
  Gerd

[Qemu-devel] [PATCH 6/8] sdl2: Fix dead keyboard after fullsceen

[Jindrich Makovicka]

Signed-off-by: Jindrich Makovicka <makovick@gmail.com>
---
 ui/sdl2.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/ui/sdl2.c b/ui/sdl2.c
index 51721d764e..cdc599be48 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -418,6 +418,7 @@ static void handle_keyup(SDL_Event *ev)
             sdl2_reset_keys(scon);
             return;
         }
+        sdl2_reset_keys(scon);
         gui_keysym = 0;
     }
     if (!gui_keysym) {
-- 
2.15.0

[Qemu-devel] [PATCH 7/8] sdl2: Do not leave grab when fullscreen

[Jindrich Makovicka]

Prevents displaying of a doubled mouse pointer when moving the pointer
to the screen edges when fullscreen.

Signed-off-by: Jindrich Makovicka <makovick@gmail.com>
---
 ui/sdl2.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/ui/sdl2.c b/ui/sdl2.c
index cdc599be48..6feb637739 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -451,8 +451,9 @@ static void handle_mousemotion(SDL_Event *ev)
         SDL_GetWindowSize(scon->real_window, &scr_w, &scr_h);
         max_x = scr_w - 1;
         max_y = scr_h - 1;
-        if (gui_grab && (ev->motion.x == 0 || ev->motion.y == 0 ||
-                         ev->motion.x == max_x || ev->motion.y == max_y)) {
+        if (gui_grab && !gui_fullscreen
+            && (ev->motion.x == 0 || ev->motion.y == 0 ||
+                ev->motion.x == max_x || ev->motion.y == max_y)) {
             sdl_grab_end(scon);
         }
         if (!gui_grab &&
-- 
2.15.0

[Qemu-devel] [PATCH 8/8] sdl2: Ignore UI hotkeys after a focus change when GUI modifier is held

[Jindrich Makovicka]

When SDL2 windows change focus while a key is held, the window that
receives the focus also receives a new KeyDown event, without an
autorepeat flag. This means that if a WM places the qemu console
over the main window after Ctrl-Alt-2, the console closes immediately
after opening. Then, the main window receives the KeyDown event again
and the whole process repeats.

This patch makes the SDL2 UI ignore the KeyDown events on a window that
just received the focus, if the GUI modifier was held. The ignore flag
is reset on a first KeyUp event. This effectively works around the issue
above.

Signed-off-by: Jindrich Makovicka <makovick@gmail.com>
---
 include/ui/sdl2.h |  1 +
 ui/sdl2.c         | 25 ++++++++++++++++---------
 2 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/include/ui/sdl2.h b/include/ui/sdl2.h
index b29cf803c9..51084e6320 100644
--- a/include/ui/sdl2.h
+++ b/include/ui/sdl2.h
@@ -24,6 +24,7 @@ struct sdl2_console {
     int opengl;
     int updates;
     int idle_counter;
+    int ignore_hotkeys;
     SDL_GLContext winctx;
 #ifdef CONFIG_OPENGL
     QemuGLShader *gls;
diff --git a/ui/sdl2.c b/ui/sdl2.c
index 6feb637739..2d48686b83 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -310,22 +310,26 @@ static void toggle_full_screen(struct sdl2_console *scon)
     sdl2_redraw(scon);
 }
 
-static void handle_keydown(SDL_Event *ev)
+static int get_mod_state(void)
 {
-    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)) ==
+        return (SDL_GetModState() & (gui_grab_code | KMOD_LSHIFT)) ==
             (gui_grab_code | KMOD_LSHIFT);
     } else if (ctrl_grab) {
-        mod_state = (SDL_GetModState() & KMOD_RCTRL) == KMOD_RCTRL;
+        return (SDL_GetModState() & KMOD_RCTRL) == KMOD_RCTRL;
     } else {
-        mod_state = (SDL_GetModState() & gui_grab_code) == gui_grab_code;
+        return (SDL_GetModState() & gui_grab_code) == gui_grab_code;
     }
-    gui_key_modifier_pressed = mod_state;
+}
+
+static void handle_keydown(SDL_Event *ev)
+{
+    int win;
+    struct sdl2_console *scon = get_scon_from_window(ev->key.windowID);
 
-    if (gui_key_modifier_pressed) {
+    gui_key_modifier_pressed = get_mod_state();
+
+    if (!scon->ignore_hotkeys && gui_key_modifier_pressed && !ev->key.repeat) {
         switch (ev->key.keysym.scancode) {
         case SDL_SCANCODE_2:
         case SDL_SCANCODE_3:
@@ -399,6 +403,8 @@ static void handle_keyup(SDL_Event *ev)
     int mod_state;
     struct sdl2_console *scon = get_scon_from_window(ev->key.windowID);
 
+    scon->ignore_hotkeys = false;
+
     if (!alt_grab) {
         mod_state = (ev->key.keysym.mod & gui_grab_code);
     } else {
@@ -545,6 +551,7 @@ static void handle_windowevent(SDL_Event *ev)
         if (!gui_grab && (qemu_input_is_absolute() || absolute_enabled)) {
             absolute_mouse_grab(scon);
         }
+        scon->ignore_hotkeys = get_mod_state();
         break;
     case SDL_WINDOWEVENT_FOCUS_LOST:
         if (gui_grab && !gui_fullscreen) {
-- 
2.15.0

Re: [Qemu-devel] [PATCH 8/8] sdl2: Ignore UI hotkeys after a focus change when GUI modifier is held

[Gerd Hoffmann]

 Hi,

>          if (!gui_grab && (qemu_input_is_absolute() || absolute_enabled)) {
>              absolute_mouse_grab(scon);
>          }

Can you please add a comment here describing why this is done?
>From the code alone it isn't obvious that this is a workaround
for a SDL bug.

> +        scon->ignore_hotkeys = get_mod_state();
>          break;

cheers,
  Gerd

Re: [Qemu-devel] [PATCH v3] SDL2 various fixes

[Gerd Hoffmann]

On Sun, Nov 12, 2017 at 08:30:24PM +0100, Jindrich Makovicka wrote:
> Hi,
> 
> here is an identical patchset with Signed-off-by.

Please post a new version without reply-to.

Queued up four patches:
  sdl2: Do not leave grab when fullscreen
  sdl2: Fix dead keyboard after fullsceen
  sdl2: Use the same pointer show/hide logic for absolute and relative mode
  sdl2: Do not quit the emulator when an auxilliary window is closed

See: https://www.kraxel.org/cgit/qemu/log/?h=queue/ui

The other ones need some clarification / testing / fixups, see
individual replies.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH v3] SDL2 various fixes

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 523 bytes --]

On 11/12/2017 01:30 PM, Jindrich Makovicka wrote:
> Hi,
> 
> here is an identical patchset with Signed-off-by.

No need to resend v4 just for this comment, but in general, we like new
revisions of a patchset to be a new standalone top-level thread, rather
than buried in-reply-to an earlier revision.

More patch submission hints at:
https://wiki.qemu.org/Contribute/SubmitAPatch

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]