Skip to contentSkip to conversation

SDL2 various fixes

Qemu-devel In review Jindrich Makovicka 8 participants 63 messages Oct 23, 2017 – Nov 14, 2017

lore
Share & tools
lore

Jindrich Makovicka

author
#0 lore raw

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

author
#1 lore raw

PATCH 1/7 sdl2: Fix broken display updating after the window is hidden

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(-)
ui/sdl2.c +0 -5
index 53dd447fd2..7f51933234 100644
@@ -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;
     }
 }
Signature · 1 line
2.15.0.rc0
#2 lore raw

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

Quoted 2 lines · Jindrich Makovicka

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

author
#3 lore raw

PATCH 2/7 sdl2: Do not quit the emulator when an auxilliary window is closed

In reply to Jindrich Makovicka
---
 ui/sdl2.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)
ui/sdl2.c +7 -3
index 7f51933234..aa37b39547 100644
@@ -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:
Signature · 1 line
2.15.0.rc0
#4 lore raw

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

Quoted 15 lines · Jindrich Makovicka

---
 ui/sdl2.c | 10 +++++++---

 1 file changed, 7 insertions(+), 3 deletions(-)
ui/sdl2.c +0 -0
index 7f51933234..aa37b39547 100644
@@ -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);

Quoted 8 lines

+        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.

Quoted 3 lines

         }
         break;
     case SDL_WINDOWEVENT_SHOWN:

author
#5 lore raw

PATCH 3/7 sdl2: Do not hide the cursor on auxilliary windows

In reply to Jindrich Makovicka
---
 ui/sdl2.c | 41 ++++++++++++++++++++++++++++++-----------
 1 file changed, 30 insertions(+), 11 deletions(-)
ui/sdl2.c +30 -11
index aa37b39547..685e4fabec 100644
@@ -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();
Signature · 1 line
2.15.0.rc0
Reviewed-by on v1 3/7
#6 lore raw

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

Hi Jindrich,

This looks like 2 different patches, can you split?

Quoted 35 lines · Jindrich Makovicka
---
 ui/sdl2.c | 41 ++++++++++++++++++++++++++++++-----------
 1 file changed, 30 insertions(+), 11 deletions(-)
ui/sdl2.c +4 -11
index aa37b39547..685e4fabec 100644
@@ -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);
-        }
     }

ui/sdl2.c static void sdl_show_cursor(void) · line 198

Quoted 10 lines
ui/sdl2.c +9 -0
+
+    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é <[email protected]>

---

second patch:

Quoted 38 lines
ui/sdl2.c +12 -0
 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)

ui/sdl2.c static void sdl_mouse_warp(DisplayChangeListener *dcl, · line 675

Quoted 10 lines
ui/sdl2.c +5 -0
 {
     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.

author
#7 lore raw

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

Quoted 3 lines · On Mon, 23 Oct 2017 23:38:16 -0300 Philippe Mathieu-Daudé <[email protected]>…

Hi Jindrich,

This looks like 2 different patches, can you split?

Hi,

here is the split version.

Quoted 109 lines · Jindrich Makovicka
Quoted 37 lines
---
 ui/sdl2.c | 41 ++++++++++++++++++++++++++++++-----------
 1 file changed, 30 insertions(+), 11 deletions(-)
ui/sdl2.c +5 -11
index aa37b39547..685e4fabec 100644
@@ -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);
-        }
     }
+

ui/sdl2.c static void sdl_show_cursor(void) · line 200

Quoted 10 lines
ui/sdl2.c +8 -0
+    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é <[email protected]>

---

second patch:

Quoted 41 lines

static void sdl_grab_start(struct sdl2_console *scon)

ui/sdl2.c +8 -0
@@ -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;

ui/sdl2.c +4 -0
+    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); +

ui/sdl2.c

Quoted 7 lines
ui/sdl2.c +4 -0
+    if (!qemu_console_is_graphic(scon->dcl.con)) {
+        return;
+    }
+    
     if (on) {
         if (!guest_cursor) {
             sdl_show_cursor();

Regards,

Phil.

Signature · 1 line
Jindrich Makovicka
Quoted 1 line

From 746858b932706f49f320c34a4e1259fd005625a3 Mon Sep 17 00:00:00 2001

From: Jindrich Makovicka <[email protected]>
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(-)
ui/sdl2.c +13 -11
index aa37b39547..2d0470189f 100644
@@ -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)
Signature · 1 line
2.15.0.rc2
Quoted 1 line

From 30174a668c17d230f8e55909f1406c9897baf395 Mon Sep 17 00:00:00 2001

From: Jindrich Makovicka <[email protected]>
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(+)
ui/sdl2.c +17 -0
index 2d0470189f..685e4fabec 100644
@@ -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();
Signature · 1 line
2.15.0.rc2
author
#8 lore raw

PATCH 4/7 sdl2: Only accept the hotkeys on the main window

In reply to Jindrich Makovicka
---
 ui/sdl2.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
ui/sdl2.c +1 -1
index 685e4fabec..fa54353430 100644
@@ -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:
Signature · 1 line
2.15.0.rc0
#9 lore raw

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

Why?

Quoted 18 lines

---
 ui/sdl2.c | 2 +-

 1 file changed, 1 insertion(+), 1 deletion(-)
ui/sdl2.c +0 -0
index 685e4fabec..fa54353430 100644
@@ -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:

author
#10 lore raw

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

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

Quoted 21 lines · Gerd Hoffmann

Why?

Quoted 18 lines
---
 ui/sdl2.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
ui/sdl2.c +1 -1
index 685e4fabec..fa54353430 100644
@@ -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:

Signature · 1 line
Jindřich Makovička
#11 lore raw

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

Quoted 5 lines · 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

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

#12 lore raw

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

Quoted 3 lines · Gerd Hoffmann

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

#13 lore raw

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

Quoted 11 lines · Peter Maydell
Quoted 4 lines · Gerd Hoffmann

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.

Quoted 1 line

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.

Quoted 5 lines

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

#14 lore raw

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

Quoted 10 lines · Gerd Hoffmann
Quoted 5 lines · Peter Maydell

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:

Quoted 8 lines

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

#15 lore raw

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

Quoted 20 lines · Peter Maydell
Quoted 10 lines · Gerd Hoffmann
Quoted 5 lines · Peter Maydell

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:

Quoted 8 lines

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.

Quoted 7 lines

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

Signature · 3 lines
|: 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 :|
#16 lore raw

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

In reply to Peter Maydell

Hi,

Quoted 3 lines

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.

Quoted 3 lines

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

#17 lore raw

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

Quoted 9 lines · Gerd Hoffmann

Hi,

Quoted 3 lines

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.

Quoted 9 lines

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

Quoted 3 lines

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

#18 lore raw

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

Hi,

Quoted 3 lines

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

#19 lore raw

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

Quoted 10 lines · Gerd Hoffmann

Hi,

Quoted 3 lines

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

#20 lore raw

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

Quoted 8 lines · Peter Maydell
Quoted 7 lines · Gerd Hoffmann

  Hi,

Quoted 4 lines

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.

Quoted 8 lines
Quoted 5 lines

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

#21 lore raw

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

In reply to Jindřich Makovička
Quoted 10 lines · 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

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

- Cole

#22 lore raw

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

Quoted 12 lines · Cole Robinson
Quoted 10 lines · 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

Quoted 2 lines

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

author
#23 lore raw

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

Hi Gerd,

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

Regards, Jindrich

Quoted 21 lines · Gerd Hoffmann
Quoted 12 lines · Cole Robinson
Quoted 10 lines · 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

Quoted 2 lines

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?

Quoted 7 lines

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

author
#24 lore raw

PATCH 5/7 sdl2 uses surface relative coordinates

In reply to Jindrich Makovicka
---
 ui/sdl2.c | 28 ++--------------------------
 1 file changed, 2 insertions(+), 26 deletions(-)
ui/sdl2.c +2 -26
index fa54353430..092eab37dc 100644
@@ -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;
Signature · 1 line
2.15.0.rc0
#25 lore raw

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

Why?

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

cheers,
Gerd

author
#26 lore raw

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

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).

Quoted 9 lines · Gerd Hoffmann

Why?

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

cheers,
Gerd

--

Jindřich Makovička

#27 lore raw

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

Quoted 7 lines · 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).

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

author
#28 lore raw

PATCH 6/7 sdl2: Fix dead keyboard after fullsceen

In reply to Jindrich Makovicka
---
 ui/sdl2.c | 1 +
 1 file changed, 1 insertion(+)
ui/sdl2.c +1 -0
index 092eab37dc..3823f0a834 100644
@@ -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) {
Signature · 1 line
2.15.0.rc0
author
#29 lore raw

PATCH 7/7 sdl2: Do not leave grab when fullscreen

In reply to Jindrich Makovicka
---
 ui/sdl2.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
ui/sdl2.c +3 -2
index 3823f0a834..811c21da27 100644
@@ -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 &&
Signature · 1 line
2.15.0.rc0
#30 lore raw
In reply to Jindrich Makovicka

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

Quoted 10 lines · Jindrich Makovicka

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.

author
#31 lore raw

PATCH 1/1 SDL2 various fixes

In reply to 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

author
#32 lore raw

PATCH 1/8 sdl2: Fix broken display updating after the window is hidden

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(-)
ui/sdl2.c +2 -6
index 53dd447fd2..774904cbf2 100644
@@ -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;
     }
 }
Signature · 1 line
2.15.0
author
#33 lore raw

PATCH 2/8 sdl2: Do not quit the emulator when an auxilliary window is closed

In reply to Jindrich Makovicka
---
 ui/sdl2.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)
ui/sdl2.c +8 -3
index 774904cbf2..dfbd0de791 100644
@@ -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:
Signature · 1 line
2.15.0
author
#34 lore raw

PATCH 3/8 sdl2: Use the same pointer show/hide logic for absolute and relative mode

In reply to Jindrich Makovicka

Also use a proper enum parameter for SDL_ShowCursor

---
 ui/sdl2.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)
ui/sdl2.c +13 -11
index dfbd0de791..9cf4b1772b 100644
@@ -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)
Signature · 1 line
2.15.0
author
#35 lore raw

PATCH 4/8 sdl2: Do not hide the cursor on auxilliary windows

In reply to Jindrich Makovicka
---
 ui/sdl2.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)
ui/sdl2.c +17 -0
index 9cf4b1772b..677c22282d 100644
@@ -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();
Signature · 1 line
2.15.0
author
#36 lore raw

PATCH 5/8 sdl2 uses surface relative coordinates

In reply to 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(-)
ui/sdl2.c +2 -26
index 677c22282d..51721d764e 100644
@@ -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;
Signature · 1 line
2.15.0
author
#37 lore raw

PATCH 6/8 sdl2: Fix dead keyboard after fullsceen

In reply to Jindrich Makovicka
---
 ui/sdl2.c | 1 +
 1 file changed, 1 insertion(+)
ui/sdl2.c +1 -0
index 51721d764e..cdc599be48 100644
@@ -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) {
Signature · 1 line
2.15.0
author
#38 lore raw

PATCH 7/8 sdl2: Do not leave grab when fullscreen

In reply to 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(-)
ui/sdl2.c +3 -2
index cdc599be48..6feb637739 100644
@@ -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 &&
Signature · 1 line
2.15.0
author
#39 lore raw

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

In reply to 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(-)
include/ui/sdl2.h +1 -0
index b29cf803c9..51084e6320 100644
@@ -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;
ui/sdl2.c +16 -9
index 6feb637739..2d48686b83 100644
@@ -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) {
Signature · 1 line
2.15.0
#40 lore raw
In reply to Jindrich Makovicka

Hi Jindrich,

Quoted 3 lines · Jindrich Makovicka

Hi,

here is a revised patchset:

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

Quoted 7 lines

- 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

Quoted 6 lines

- 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.

author
#41 lore raw

PATCH v3 1/1 SDL2 various fixes

Hi,

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

Regards, Jindrich

author
#42 lore raw

PATCH 1/8 sdl2: Fix broken display updating after the window is hidden

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 <[email protected]>
---
 ui/sdl2.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)
ui/sdl2.c +2 -6
index 53dd447fd2..774904cbf2 100644
@@ -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;
     }
 }
Signature · 1 line
2.15.0
#43 lore raw

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

Quoted 5 lines · 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.

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

#44 lore raw

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

Quoted 11 lines · Gerd Hoffmann
Quoted 5 lines · 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.

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.

Quoted 6 lines

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

author
#45 lore raw

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

Quoted 16 lines · Gerd Hoffmann
Quoted 15 lines · Gerd Hoffmann
Quoted 5 lines · 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.

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.

Quoted 8 lines
Quoted 2 lines

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,

Signature · 1 line
Jindřich Makovička
#46 lore raw

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

Hi,

Quoted 11 lines
Quoted 8 lines
Quoted 2 lines

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

#47 lore raw

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

In reply to Gerd Hoffmann
Quoted 15 lines · Gerd Hoffmann
Quoted 5 lines · 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.

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.

Quoted 7 lines

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

#48 lore raw

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

Quoted 7 lines
Quoted 2 lines

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

#49 lore raw

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

In reply to Max Reitz
Quoted 22 lines · Max Reitz
Quoted 21 lines · Gerd Hoffmann
Quoted 5 lines · 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.

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

Quoted 6 lines
Quoted 1 line

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

#50 lore raw

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

Quoted 9 lines
Quoted 4 lines

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

author
#51 lore raw

PATCH 2/8 sdl2: Do not quit the emulator when an auxilliary window is closed

In reply to Jindrich Makovicka
Signed-off-by Jindrich Makovicka <[email protected]>
---
 ui/sdl2.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)
ui/sdl2.c +8 -3
index 774904cbf2..dfbd0de791 100644
@@ -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:
Signature · 1 line
2.15.0
author
#52 lore raw

PATCH 3/8 sdl2: Use the same pointer show/hide logic for absolute and relative mode

In reply to Jindrich Makovicka

Also use a proper enum parameter for SDL_ShowCursor

Signed-off-by Jindrich Makovicka <[email protected]>
---
 ui/sdl2.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)
ui/sdl2.c +13 -11
index dfbd0de791..9cf4b1772b 100644
@@ -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)
Signature · 1 line
2.15.0
author
#53 lore raw

PATCH 4/8 sdl2: Do not hide the cursor on auxilliary windows

In reply to Jindrich Makovicka
Signed-off-by Jindrich Makovicka <[email protected]>
---
 ui/sdl2.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)
ui/sdl2.c +17 -0
index 9cf4b1772b..677c22282d 100644
@@ -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();
Signature · 1 line
2.15.0
#54 lore raw

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

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

author
#55 lore raw

PATCH 5/8 sdl2 uses surface relative coordinates

In reply to Jindrich Makovicka

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

Signed-off-by Jindrich Makovicka <[email protected]>
---
 ui/sdl2.c | 28 ++--------------------------
 1 file changed, 2 insertions(+), 26 deletions(-)
ui/sdl2.c +2 -26
index 677c22282d..51721d764e 100644
@@ -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;
Signature · 1 line
2.15.0
#56 lore raw

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

Quoted 2 lines · Jindrich Makovicka

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

author
#58 lore raw

PATCH 7/8 sdl2: Do not leave grab when fullscreen

In reply to Jindrich Makovicka

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

Signed-off-by Jindrich Makovicka <[email protected]>
---
 ui/sdl2.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
ui/sdl2.c +3 -2
index cdc599be48..6feb637739 100644
@@ -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 &&
Signature · 1 line
2.15.0
author
#59 lore raw

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

In reply to 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 <[email protected]>
---
 include/ui/sdl2.h |  1 +
 ui/sdl2.c         | 25 ++++++++++++++++---------
 2 files changed, 17 insertions(+), 9 deletions(-)
include/ui/sdl2.h +1 -0
index b29cf803c9..51084e6320 100644
@@ -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;
ui/sdl2.c +16 -9
index 6feb637739..2d48686b83 100644
@@ -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) {
Signature · 1 line
2.15.0
#60 lore raw

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

Hi,

Quoted 3 lines
         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?

Quoted 1 line

From the code alone it isn't obvious that this is a workaround

for a SDL bug.

Quoted 2 lines
+        scon->ignore_hotkeys = get_mod_state();
         break;

cheers,
Gerd

#61 lore raw
In reply to Jindrich Makovicka
Quoted 3 lines · Jindrich Makovicka

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

#62 lore raw
In reply to Jindrich Makovicka
Quoted 3 lines · Jindrich Makovicka

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

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