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 <address@hidden> 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 <address@hidden>
> > ---
> > 
> >  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