[Qemu-devel] [PATCH] gtk: implement -full-screen and -no-frame

[Qemu-devel] [PATCH] gtk: implement -full-screen and -no-frame

[Peter Wu]

Aiming for GTK as replacement for SDL, features like -full-screen and -no-frame
should also be implemented.

Bringing the window into full-screen mode is done by faking activating the full
screen menu item with a NULL menu item (which currently is not used by
gd_menu_full_screen). This is done after showing the windows to make the cursor
and menu hidden.

Signed-off-by: Peter Wu <lekensteyn@gmail.com>
---
 include/ui/console.h |  2 +-
 ui/gtk.c             | 10 +++++++++-
 vl.c                 |  2 +-
 3 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/include/ui/console.h b/include/ui/console.h
index 4307b5f..7174ba9 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -339,6 +339,6 @@ int index_from_keycode(int code);
 
 /* gtk.c */
 void early_gtk_display_init(void);
-void gtk_display_init(DisplayState *ds);
+void gtk_display_init(DisplayState *ds, int full_screen, int no_frame);
 
 #endif
diff --git a/ui/gtk.c b/ui/gtk.c
index 3bc2842..3df2611 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -1433,7 +1433,7 @@ static const DisplayChangeListenerOps dcl_ops = {
     .dpy_cursor_define = gd_cursor_define,
 };
 
-void gtk_display_init(DisplayState *ds)
+void gtk_display_init(DisplayState *ds, int full_screen, int no_frame)
 {
     GtkDisplayState *s = g_malloc0(sizeof(*s));
     char *filename;
@@ -1457,6 +1457,10 @@ void gtk_display_init(DisplayState *ds)
     s->scale_y = 1.0;
     s->free_scale = FALSE;
 
+    if (no_frame) {
+        gtk_window_set_decorated(GTK_WINDOW(s->window), FALSE);
+    }
+
     setlocale(LC_ALL, "");
     bindtextdomain("qemu", CONFIG_QEMU_LOCALEDIR);
     textdomain("qemu");
@@ -1509,6 +1513,10 @@ void gtk_display_init(DisplayState *ds)
 
     gtk_widget_show_all(s->window);
 
+    if (full_screen) {
+        gd_menu_full_screen(NULL, s);
+    }
+
     register_displaychangelistener(&s->dcl);
 
     global_state = s;
diff --git a/vl.c b/vl.c
index 47ab45d..5a00710 100644
--- a/vl.c
+++ b/vl.c
@@ -4347,7 +4347,7 @@ int main(int argc, char **argv, char **envp)
 #endif
 #if defined(CONFIG_GTK)
     case DT_GTK:
-        gtk_display_init(ds);
+        gtk_display_init(ds, full_screen, no_frame);
         break;
 #endif
     default:
-- 
1.8.3

Re: [Qemu-devel] [PATCH] gtk: implement -full-screen and -no-frame

[Anthony Liguori]

On Sun, Jun 9, 2013 at 5:30 AM, Peter Wu <lekensteyn@gmail.com> wrote:
> Aiming for GTK as replacement for SDL, features like -full-screen and -no-frame
> should also be implemented.
>
> Bringing the window into full-screen mode is done by faking activating the full
> screen menu item with a NULL menu item (which currently is not used by
> gd_menu_full_screen). This is done after showing the windows to make the cursor
> and menu hidden.
>
> Signed-off-by: Peter Wu <lekensteyn@gmail.com>
> ---
>  include/ui/console.h |  2 +-
>  ui/gtk.c             | 10 +++++++++-
>  vl.c                 |  2 +-
>  3 files changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/include/ui/console.h b/include/ui/console.h
> index 4307b5f..7174ba9 100644
> --- a/include/ui/console.h
> +++ b/include/ui/console.h
> @@ -339,6 +339,6 @@ int index_from_keycode(int code);
>
>  /* gtk.c */
>  void early_gtk_display_init(void);
> -void gtk_display_init(DisplayState *ds);
> +void gtk_display_init(DisplayState *ds, int full_screen, int no_frame);
>
>  #endif
> diff --git a/ui/gtk.c b/ui/gtk.c
> index 3bc2842..3df2611 100644
> --- a/ui/gtk.c
> +++ b/ui/gtk.c
> @@ -1433,7 +1433,7 @@ static const DisplayChangeListenerOps dcl_ops = {
>      .dpy_cursor_define = gd_cursor_define,
>  };
>
> -void gtk_display_init(DisplayState *ds)
> +void gtk_display_init(DisplayState *ds, int full_screen, int no_frame)
>  {
>      GtkDisplayState *s = g_malloc0(sizeof(*s));
>      char *filename;
> @@ -1457,6 +1457,10 @@ void gtk_display_init(DisplayState *ds)
>      s->scale_y = 1.0;
>      s->free_scale = FALSE;
>
> +    if (no_frame) {
> +        gtk_window_set_decorated(GTK_WINDOW(s->window), FALSE);
> +    }
> +

Is this really desirable?  Why do people use -no-frame?  I don't think
we should carry over features from SDL just because they are there.

>      setlocale(LC_ALL, "");
>      bindtextdomain("qemu", CONFIG_QEMU_LOCALEDIR);
>      textdomain("qemu");
> @@ -1509,6 +1513,10 @@ void gtk_display_init(DisplayState *ds)
>
>      gtk_widget_show_all(s->window);
>
> +    if (full_screen) {
> +        gd_menu_full_screen(NULL, s);
> +    }
> +

You should activate the menu item by using gtk_check_menu_item_set_active().

Otherwise the menu with be out of sync with the UI state.

Regards,

Anthony Liguori

>      register_displaychangelistener(&s->dcl);
>
>      global_state = s;
> diff --git a/vl.c b/vl.c
> index 47ab45d..5a00710 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -4347,7 +4347,7 @@ int main(int argc, char **argv, char **envp)
>  #endif
>  #if defined(CONFIG_GTK)
>      case DT_GTK:
> -        gtk_display_init(ds);
> +        gtk_display_init(ds, full_screen, no_frame);
>          break;
>  #endif
>      default:
> --
> 1.8.3
>
>
>

Re: [Qemu-devel] [PATCH] gtk: implement -full-screen and -no-frame

[Peter Wu]

On Sunday 09 June 2013 13:33:14 Anthony Liguori wrote:
> On Sun, Jun 9, 2013 at 5:30 AM, Peter Wu <lekensteyn@gmail.com> wrote:
> > Aiming for GTK as replacement for SDL, features like -full-screen and
> > -no-frame should also be implemented.
> > 
> > Bringing the window into full-screen mode is done by faking activating the
> > full screen menu item with a NULL menu item (which currently is not used
> > by gd_menu_full_screen). This is done after showing the windows to make
> > the cursor and menu hidden.
> > 
> > Signed-off-by: Peter Wu <lekensteyn@gmail.com>
> > ---
> > 
> >  include/ui/console.h |  2 +-
> >  ui/gtk.c             | 10 +++++++++-
> >  vl.c                 |  2 +-
> >  3 files changed, 11 insertions(+), 3 deletions(-)
> > 
> > diff --git a/include/ui/console.h b/include/ui/console.h
> > index 4307b5f..7174ba9 100644
> > --- a/include/ui/console.h
> > +++ b/include/ui/console.h
> > @@ -339,6 +339,6 @@ int index_from_keycode(int code);
> > 
> >  /* gtk.c */
> >  void early_gtk_display_init(void);
> > 
> > -void gtk_display_init(DisplayState *ds);
> > +void gtk_display_init(DisplayState *ds, int full_screen, int no_frame);
> > 
> >  #endif
> > 
> > diff --git a/ui/gtk.c b/ui/gtk.c
> > index 3bc2842..3df2611 100644
> > --- a/ui/gtk.c
> > +++ b/ui/gtk.c
> > @@ -1433,7 +1433,7 @@ static const DisplayChangeListenerOps dcl_ops = {
> > 
> >      .dpy_cursor_define = gd_cursor_define,
> >  
> >  };
> > 
> > -void gtk_display_init(DisplayState *ds)
> > +void gtk_display_init(DisplayState *ds, int full_screen, int no_frame)
> > 
> >  {
> >  
> >      GtkDisplayState *s = g_malloc0(sizeof(*s));
> >      char *filename;
> > 
> > @@ -1457,6 +1457,10 @@ void gtk_display_init(DisplayState *ds)
> > 
> >      s->scale_y = 1.0;
> >      s->free_scale = FALSE;
> > 
> > +    if (no_frame) {
> > +        gtk_window_set_decorated(GTK_WINDOW(s->window), FALSE);
> > +    }
> > +
> 
> Is this really desirable?  Why do people use -no-frame?  I don't think
> we should carry over features from SDL just because they are there.

I started using it because -full-screen didn't work well in SDL. In SDL, full-
screen mode changes de video mode too. At some point, I even managed to crash 
QEMU in full screen mode with certain Xorg settings.

I just added it to GTK because it was possible to do so. In GTK, I -full-
screen hides the menu bar (although there is still some pixels visible on the 
top!) and the border without changing modes (which is a plus!). Due to this, 
there is not really a need for -no-frame for me.

Others are likely using -no-frame in a space-constrained environment where 
they need to show multiple windows.

> >      setlocale(LC_ALL, "");
> >      bindtextdomain("qemu", CONFIG_QEMU_LOCALEDIR);
> >      textdomain("qemu");
> > 
> > @@ -1509,6 +1513,10 @@ void gtk_display_init(DisplayState *ds)
> > 
> >      gtk_widget_show_all(s->window);
> > 
> > +    if (full_screen) {
> > +        gd_menu_full_screen(NULL, s);
> > +    }
> > +
> 
> You should activate the menu item by using gtk_check_menu_item_set_active().
> 
> Otherwise the menu with be out of sync with the UI state.

Alright, I forgot to check that. I will this fix in a next patch where I will 
leave out -no-frame as well.

Thanks for review!

Regards,
Peter

> Regards,
> 
> Anthony Liguori
> 
> >      register_displaychangelistener(&s->dcl);
> >      
> >      global_state = s;
> > 
> > diff --git a/vl.c b/vl.c
> > index 47ab45d..5a00710 100644
> > --- a/vl.c
> > +++ b/vl.c
> > @@ -4347,7 +4347,7 @@ int main(int argc, char **argv, char **envp)
> > 
> >  #endif
> >  #if defined(CONFIG_GTK)
> >  
> >      case DT_GTK:
> > -        gtk_display_init(ds);
> > +        gtk_display_init(ds, full_screen, no_frame);
> > 
> >          break;
> >  
> >  #endif
> >  
> >      default:
> > --
> > 1.8.3

Re: [Qemu-devel] [PATCH] gtk: implement -full-screen and -no-frame

[Kevin Wolf]

Am 09.06.2013 um 12:30 hat Peter Wu geschrieben:
> Aiming for GTK as replacement for SDL, features like -full-screen and -no-frame
> should also be implemented.
> 
> Bringing the window into full-screen mode is done by faking activating the full
> screen menu item with a NULL menu item (which currently is not used by
> gd_menu_full_screen). This is done after showing the windows to make the cursor
> and menu hidden.
> 
> Signed-off-by: Peter Wu <lekensteyn@gmail.com>
> ---
>  include/ui/console.h |  2 +-
>  ui/gtk.c             | 10 +++++++++-
>  vl.c                 |  2 +-
>  3 files changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/include/ui/console.h b/include/ui/console.h
> index 4307b5f..7174ba9 100644
> --- a/include/ui/console.h
> +++ b/include/ui/console.h
> @@ -339,6 +339,6 @@ int index_from_keycode(int code);
>  
>  /* gtk.c */
>  void early_gtk_display_init(void);
> -void gtk_display_init(DisplayState *ds);
> +void gtk_display_init(DisplayState *ds, int full_screen, int no_frame);

Should the new arguments be bool?

Kevin

Re: [Qemu-devel] [PATCH] gtk: implement -full-screen and -no-frame

[Peter Wu]

On Monday 10 June 2013 14:33:28 Kevin Wolf wrote:
> Am 09.06.2013 um 12:30 hat Peter Wu geschrieben:
> > Aiming for GTK as replacement for SDL, features like -full-screen and
> > -no-frame should also be implemented.
> >
> > 
> >
> > Bringing the window into full-screen mode is done by faking activating the
> > full screen menu item with a NULL menu item (which currently is not used
> > by gd_menu_full_screen). This is done after showing the windows to make
> > the cursor and menu hidden.
> >
> > 
> >
> > Signed-off-by: Peter Wu <lekensteyn@gmail.com>
> > ---
> >
> >  include/ui/console.h |  2 +-
> >  ui/gtk.c             | 10 +++++++++-
> >  vl.c                 |  2 +-
> >  3 files changed, 11 insertions(+), 3 deletions(-)
> > 
> >
> > diff --git a/include/ui/console.h b/include/ui/console.h
> > index 4307b5f..7174ba9 100644
> > --- a/include/ui/console.h
> > +++ b/include/ui/console.h
> > @@ -339,6 +339,6 @@ int index_from_keycode(int code);
> >
> >  
> >  /* gtk.c */
> >  void early_gtk_display_init(void);
> >
> > -void gtk_display_init(DisplayState *ds);
> > +void gtk_display_init(DisplayState *ds, int full_screen, int no_frame);
> 
> Should the new arguments be bool?

Probably yes, but for consistency with the existing types I kept it as int. A 
future patch could change all uses of "int" to "bool" where 1 or 0 are used, 
do you prefer to use bool here anyway?

Regards,
Peter

Re: [Qemu-devel] [PATCH] gtk: implement -full-screen and -no-frame

[Kevin Wolf]

Am 10.06.2013 um 14:43 hat Peter Wu geschrieben:
> On Monday 10 June 2013 14:33:28 Kevin Wolf wrote:
> > Am 09.06.2013 um 12:30 hat Peter Wu geschrieben:
> > > --- a/include/ui/console.h
> > > +++ b/include/ui/console.h
> > > @@ -339,6 +339,6 @@ int index_from_keycode(int code);
> > >
> > >  
> > >  /* gtk.c */
> > >  void early_gtk_display_init(void);
> > >
> > > -void gtk_display_init(DisplayState *ds);
> > > +void gtk_display_init(DisplayState *ds, int full_screen, int no_frame);
> > 
> > Should the new arguments be bool?
> 
> Probably yes, but for consistency with the existing types I kept it as int. A 
> future patch could change all uses of "int" to "bool" where 1 or 0 are used, 
> do you prefer to use bool here anyway?

Yes, if you have to respin anyway, I would prefer if you changed it to
bool. I mean it's not a big deal, but it moves us one step closer to
consistent use of bool for boolean values, so it can't be bad.

I don't think we'll ever see one big conversion patch, but as the code
is touched over time, ints abused as bools will slowly disappear if we
don't introduce new instances.

Kevin

Re: [Qemu-devel] [Qemu-trivial] [PATCH] gtk: implement -full-screen and -no-frame

[Michael Tokarev]

09.06.2013 14:30, Peter Wu wrote:
> Aiming for GTK as replacement for SDL, features like -full-screen and -no-frame
> should also be implemented.

I'm not sure both options really should be implemented.  I think -no-frame makes
little sense actually.  But -full-screen is very useful, and hiding the menu bar --
actually in both full-screen mode and windowed mode -- makes very good sense, it
is something the gtk interface miss alot compared with sdl.

Thanks,

/mjt