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 lekenst...@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 lekenst...@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] [PATCH] gtk: implement -full-screen and -no-frame

[Anthony Liguori]

On Sun, Jun 9, 2013 at 5:30 AM, Peter Wu lekenst...@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 lekenst...@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 lekenst...@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 lekenst...@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