diff mbox

[3/3] Move getting XWindow ID from baum driver to graphical backend

Message ID 20161023195433.20102-4-samuel.thibault@ens-lyon.org
State New
Headers show

Commit Message

Samuel Thibault Oct. 23, 2016, 7:54 p.m. UTC
This adds two console functions, qemu_console_set_window_id and
qemu_graphic_console_get_window_id, to let graphical backend record the
window id in the QemuConsole structure, and let the baum driver read it.

We can then move the SDL code from the baum driver to the sdl ui code,
and add SDL2 and Gtk versions of the code.

Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
---
 backends/baum.c      | 25 +++----------------------
 include/ui/console.h |  3 +++
 ui/console.c         | 18 ++++++++++++++++++
 ui/gtk.c             | 25 +++++++++++++++++++++++--
 ui/sdl.c             | 25 +++++++++++++++++++++++++
 ui/sdl2.c            |  7 +++++++
 6 files changed, 79 insertions(+), 24 deletions(-)

Comments

Gerd Hoffmann Oct. 26, 2016, 10:17 a.m. UTC | #1
On So, 2016-10-23 at 21:54 +0200, Samuel Thibault wrote:
> This adds two console functions, qemu_console_set_window_id and
> qemu_graphic_console_get_window_id, to let graphical backend record the
> window id in the QemuConsole structure, and let the baum driver read it.
> 
> We can then move the SDL code from the baum driver to the sdl ui code,
> and add SDL2 and Gtk versions of the code.

Patches 1+2 look good to me, but I don't feel like rushing this one
before the freeze.  Also splitting this up would be nice.

> +int qemu_graphic_console_get_window_id(void)
> +{
> +    int i;
> +    for (i = 0; i < nb_consoles; i++) {
> +        if (consoles[i]->console_type == GRAPHIC_CONSOLE) {
> +            return consoles[i]->window_id;
> +        }
> +    }
> +    return -1;
> +}

No loop needed here.  qemu sorts consoles so the graphic ones come
first.

> +    gdk_window = gtk_widget_get_window(s->window);
> +#ifdef GDK_WINDOWING_X11
> +    window_id = GDK_WINDOW_XID(gdk_window);
> +#elif defined(GDK_WINDOWING_WIN32)
> +    window_id = gdk_win32_window_get_impl_hwnd(gdk_window);
> +#endif
> +    for (i = 0; ; i++) {
> +        /* All consoles share the same window */

No.  That is the default setup, but try "View / Detach tab".  Window ID
changing at runtime ...

cheers,
  Gerd
Samuel Thibault Oct. 26, 2016, 11:35 a.m. UTC | #2
Hello,

Gerd Hoffmann, on Wed 26 Oct 2016 12:17:44 +0200, wrote:
> On So, 2016-10-23 at 21:54 +0200, Samuel Thibault wrote:
> > This adds two console functions, qemu_console_set_window_id and
> > qemu_graphic_console_get_window_id, to let graphical backend record the
> > window id in the QemuConsole structure, and let the baum driver read it.
> > 
> > We can then move the SDL code from the baum driver to the sdl ui code,
> > and add SDL2 and Gtk versions of the code.
> 
> Patches 1+2 look good to me, but I don't feel like rushing this one
> before the freeze.

Ok, it'd be nice to have 1+2 in in the next release already indeed, they
are needed fixes. 3 can be applied later, for the time being people can
use sdl1.2.

> Also splitting this up would be nice.

You mean separating the move of the sdl1.2 code from the addition of
sdl2+gtk code? (and perhaps separate sdl2 and gtk?)

> > +int qemu_graphic_console_get_window_id(void)
> > +{
> > +    int i;
> > +    for (i = 0; i < nb_consoles; i++) {
> > +        if (consoles[i]->console_type == GRAPHIC_CONSOLE) {
> > +            return consoles[i]->window_id;
> > +        }
> > +    }
> > +    return -1;
> > +}
> 
> No loop needed here.  qemu sorts consoles so the graphic ones come
> first.

Ok, but are we sure nobody will change that assumption someday?  Or do
we assume that if somebody changes it he will notice the assumption in
qemu_graphic_console_get_window_id?

> > +    gdk_window = gtk_widget_get_window(s->window);
> > +#ifdef GDK_WINDOWING_X11
> > +    window_id = GDK_WINDOW_XID(gdk_window);
> > +#elif defined(GDK_WINDOWING_WIN32)
> > +    window_id = gdk_win32_window_get_impl_hwnd(gdk_window);
> > +#endif
> > +    for (i = 0; ; i++) {
> > +        /* All consoles share the same window */
> 
> No.  That is the default setup, but try "View / Detach tab".  Window ID
> changing at runtime ...

Ah. That'll have to be reworked then indeed.

Samuel
Gerd Hoffmann Oct. 26, 2016, 12:24 p.m. UTC | #3
> > Also splitting this up would be nice.
> 
> You mean separating the move of the sdl1.2 code from the addition of
> sdl2+gtk code? (and perhaps separate sdl2 and gtk?)

One patch adding new console.[ch] interfaces, then one patch per UI,
finally switch over baum to the new interfaces.

Maybe put sdl-ui + baum into one patch as this moves existing code sdl
code from baum.c to sdl*.c for the most part.

> > No loop needed here.  qemu sorts consoles so the graphic ones come
> > first.
> 
> Ok, but are we sure nobody will change that assumption someday?  Or do
> we assume that if somebody changes it he will notice the assumption in
> qemu_graphic_console_get_window_id?

If that assumption ever changes the code here (and in quite a few more
places) must be adapted to it.

cheers,
  Gerd
Samuel Thibault Oct. 30, 2016, 3:24 p.m. UTC | #4
Gerd Hoffmann, on Wed 26 Oct 2016 12:17:44 +0200, wrote:
> > +        /* All consoles share the same window */
> 
> No.  That is the default setup, but try "View / Detach tab".  Window ID
> changing at runtime ...

So we would need to make baum register for notification of Window ID
change.

It could be a mere

typedef void QemuConsoleWindowIDListener(void);
qemu_console_window_id_add_listener(QemuConsoleWindowIDListener listener);
qemu_console_window_id_remove_listener(QemuConsoleWindowIDListener listener);

that adds/removes the listener to a list to be called when
qemu_console_set_window_id is called.

Or we could generalize a bit: 

typedef void QemuConsoleConfigListener(void);
qemu_console_config_add_listener(QemuConsoleConfigListener listener);
qemu_console_config_remove_listener(QemuConsoleConfigListener listener);

Or even more generalized:

struct QemuConsoleListener {
  void (*window_id)(void);
};
typedef struct QemuConsoleListener QemuConsoleListener;
qemu_console_add_listener(QemuConsoleListener *listener);
qemu_console_remove_listener(QemuConsoleListener *listener);

What would be preferrable?

Samuel
Gerd Hoffmann Nov. 1, 2016, 10:03 a.m. UTC | #5
Hi,

> typedef void QemuConsoleConfigListener(void);

I think we should also pass the console which has changed: 

QemuConsoleConfigListener(QemuConsole *con);

> qemu_console_config_add_listener(QemuConsoleConfigListener listener);
> qemu_console_config_remove_listener(QemuConsoleConfigListener listener);

Otherwise this approach looks good to me.

cheers,
  Gerd
diff mbox

Patch

diff --git a/backends/baum.c b/backends/baum.c
index 130eb1b..53d1521 100644
--- a/backends/baum.c
+++ b/backends/baum.c
@@ -27,12 +27,10 @@ 
 #include "sysemu/char.h"
 #include "qemu/timer.h"
 #include "hw/usb.h"
+#include "ui/console.h"
 #include <brlapi.h>
 #include <brlapi_constants.h>
 #include <brlapi_keycodes.h>
-#ifdef CONFIG_SDL
-#include <SDL_syswm.h>
-#endif
 
 #if 0
 #define DPRINTF(fmt, ...) \
@@ -227,11 +225,6 @@  static const uint8_t nabcc_translation[2][256] = {
 /* The guest OS has started discussing with us, finish initializing BrlAPI */
 static int baum_deferred_init(BaumDriverState *baum)
 {
-#if defined(CONFIG_SDL)
-#if SDL_COMPILEDVERSION < SDL_VERSIONNUM(2, 0, 0)
-    SDL_SysWMinfo info;
-#endif
-#endif
     int tty;
 
     if (baum->deferred_init) {
@@ -243,21 +236,9 @@  static int baum_deferred_init(BaumDriverState *baum)
         return 0;
     }
 
-#if defined(CONFIG_SDL)
-#if SDL_COMPILEDVERSION < SDL_VERSIONNUM(2, 0, 0)
-    memset(&info, 0, sizeof(info));
-    SDL_VERSION(&info.version);
-    if (SDL_GetWMInfo(&info)) {
-        tty = info.info.x11.wmwindow;
-    } else {
-#endif
-#endif
+    tty = qemu_graphic_console_get_window_id();
+    if (tty == -1)
         tty = BRLAPI_TTY_DEFAULT;
-#if defined(CONFIG_SDL)
-#if SDL_COMPILEDVERSION < SDL_VERSIONNUM(2, 0, 0)
-    }
-#endif
-#endif
 
     if (brlapi__enterTtyMode(baum->brlapi, tty, NULL) == -1) {
         brlapi_perror("baum: brlapi__enterTtyMode");
diff --git a/include/ui/console.h b/include/ui/console.h
index e2589e2..cf07e41 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -394,6 +394,9 @@  uint32_t qemu_console_get_head(QemuConsole *con);
 QemuUIInfo *qemu_console_get_ui_info(QemuConsole *con);
 int qemu_console_get_width(QemuConsole *con, int fallback);
 int qemu_console_get_height(QemuConsole *con, int fallback);
+/* Return the low-level window id for the first graphical console */
+int qemu_graphic_console_get_window_id(void);
+void qemu_console_set_window_id(int index, int window_id);
 
 void console_select(unsigned int index);
 void qemu_console_resize(QemuConsole *con, int width, int height);
diff --git a/ui/console.c b/ui/console.c
index fa3e658..2d0d57c 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -124,6 +124,7 @@  struct QemuConsole {
     int dcls;
     DisplayChangeListener *gl;
     bool gl_block;
+    int window_id;
 
     /* Graphic console state.  */
     Object *device;
@@ -273,6 +274,23 @@  void graphic_hw_gl_block(QemuConsole *con, bool block)
     }
 }
 
+int qemu_graphic_console_get_window_id(void)
+{
+    int i;
+    for (i = 0; i < nb_consoles; i++) {
+        if (consoles[i]->console_type == GRAPHIC_CONSOLE) {
+            return consoles[i]->window_id;
+        }
+    }
+    return -1;
+}
+
+void qemu_console_set_window_id(int index, int window_id)
+{
+    assert(index < nb_consoles);
+    consoles[index]->window_id = window_id;
+}
+
 void graphic_hw_invalidate(QemuConsole *con)
 {
     if (!con) {
diff --git a/ui/gtk.c b/ui/gtk.c
index 58d20ee..0fe5bdf 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -2155,6 +2155,13 @@  void gtk_display_init(DisplayState *ds, bool full_screen, bool grab_on_hover)
     GtkDisplayState *s = g_malloc0(sizeof(*s));
     char *filename;
     GdkDisplay *window_display;
+    GdkWindow *gdk_window;
+#ifdef GDK_WINDOWING_X11
+    Window window_id;
+#elif defined(GDK_WINDOWING_WIN32)
+    HWND window_id;
+#endif
+    int i;
 
     if (!gtkinit) {
         fprintf(stderr, "gtk initialization failed\n");
@@ -2217,8 +2224,6 @@  void gtk_display_init(DisplayState *ds, bool full_screen, bool grab_on_hover)
     {
         VirtualConsole *cur = gd_vc_find_current(s);
         if (cur) {
-            int i;
-
             for (i = 0; i < s->nb_vcs; i++) {
                 VirtualConsole *vc = &s->vc[i];
                 if (vc && vc->type == GD_VC_VTE && vc != cur) {
@@ -2238,6 +2243,22 @@  void gtk_display_init(DisplayState *ds, bool full_screen, bool grab_on_hover)
     }
 
     gd_set_keycode_type(s);
+
+    gdk_window = gtk_widget_get_window(s->window);
+#ifdef GDK_WINDOWING_X11
+    window_id = GDK_WINDOW_XID(gdk_window);
+#elif defined(GDK_WINDOWING_WIN32)
+    window_id = gdk_win32_window_get_impl_hwnd(gdk_window);
+#endif
+    for (i = 0; ; i++) {
+        /* All consoles share the same window */
+        QemuConsole *con = qemu_console_lookup_by_index(i);
+        if (con) {
+            qemu_console_set_window_id(i, (int) window_id);
+        } else {
+            break;
+        }
+    }
 }
 
 void early_gtk_display_init(int opengl)
diff --git a/ui/sdl.c b/ui/sdl.c
index d8cf5bc..7fa3772 100644
--- a/ui/sdl.c
+++ b/ui/sdl.c
@@ -947,6 +947,7 @@  void sdl_display_init(DisplayState *ds, int full_screen, int no_frame)
     int flags;
     uint8_t data = 0;
     const SDL_VideoInfo *vi;
+    SDL_SysWMinfo info;
     char *filename;
 
 #if defined(__APPLE__)
@@ -1023,5 +1024,29 @@  void sdl_display_init(DisplayState *ds, int full_screen, int no_frame)
     sdl_cursor_hidden = SDL_CreateCursor(&data, &data, 8, 1, 0, 0);
     sdl_cursor_normal = SDL_GetCursor();
 
+    memset(&info, 0, sizeof(info));
+    SDL_VERSION(&info.version);
+    if (SDL_GetWMInfo(&info)) {
+        int i;
+        for (i = 0; ; i++) {
+            /* All consoles share the same window */
+            QemuConsole *con = qemu_console_lookup_by_index(i);
+            if (con) {
+#if defined(SDL_VIDEO_DRIVER_X11)
+                qemu_console_set_window_id(i, info.info.x11.wmwindow);
+#elif defined(SDL_VIDEO_DRIVER_NANOX) || \
+      defined(SDL_VIDEO_DRIVER_WINDIB) || defined(SDL_VIDEO_DRIVER_DDRAW) || \
+      defined(SDL_VIDEO_DRIVER_GAPI) || \
+      defined(SDL_VIDEO_DRIVER_RISCOS)
+                qemu_console_set_window_id(i, (int) info.window);
+#else
+                qemu_console_set_window_id(i, info.data);
+#endif
+            } else {
+                break;
+            }
+        }
+    }
+
     atexit(sdl_cleanup);
 }
diff --git a/ui/sdl2.c b/ui/sdl2.c
index 30d2a3c..b464f16 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -761,6 +761,7 @@  void sdl_display_init(DisplayState *ds, int full_screen, int no_frame)
     uint8_t data = 0;
     char *filename;
     int i;
+    SDL_SysWMinfo info;
 
     if (no_frame) {
         gui_noframe = 1;
@@ -786,6 +787,8 @@  void sdl_display_init(DisplayState *ds, int full_screen, int no_frame)
         exit(1);
     }
     SDL_SetHint(SDL_HINT_GRAB_KEYBOARD, "1");
+    memset(&info, 0, sizeof(info));
+    SDL_VERSION(&info.version);
 
     for (i = 0;; i++) {
         QemuConsole *con = qemu_console_lookup_by_index(i);
@@ -813,6 +816,10 @@  void sdl_display_init(DisplayState *ds, int full_screen, int no_frame)
 #endif
         sdl2_console[i].dcl.con = con;
         register_displaychangelistener(&sdl2_console[i].dcl);
+
+        if (SDL_GetWindowWMInfo(sdl2_console[i].real_window, &info)) {
+            qemu_console_set_window_id(i, info.info.x11.window);
+        }
     }
 
     /* Load a 32x32x4 image. White pixels are transparent. */