| Message ID | 512740E5.4060706@siemens.com |
|---|---|
| State | New |
| Headers | show |
Hi Jan, Jan Kiszka <jan.kiszka@siemens.com> writes: > VGA is a misnomer, only some of our targets have it. Use the more > generic term "display" instead. > > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> Sorry, I responded to your previous note in my head and forgot to also respond on the mailing list. We expose this "misnomer" a lot. For instance, qemu -vga cirrus. OTOH, the term "display" is used to indicate the backend via qemu -display sdl. So calling the VGA tab "Display" after selecting '-display gtk' seems a bit confusing to me. So I'm open to picking better naming but I think we should try to make it consistent throughout QEMU. Regards, Anthony Liguori > --- > > No translation updates yet as I do not know how you like to handle them. > > ui/gtk.c | 45 +++++++++++++++++++++++++-------------------- > 1 files changed, 25 insertions(+), 20 deletions(-) > > diff --git a/ui/gtk.c b/ui/gtk.c > index 29156be..e9faf15 100644 > --- a/ui/gtk.c > +++ b/ui/gtk.c > @@ -93,7 +93,7 @@ typedef struct GtkDisplayState > GtkWidget *zoom_fit_item; > GtkWidget *grab_item; > GtkWidget *grab_on_hover_item; > - GtkWidget *vga_item; > + GtkWidget *display_item; > > int nb_vcs; > VirtualConsole vc[MAX_VCS]; > @@ -133,7 +133,7 @@ static bool gd_grab_on_hover(GtkDisplayState *s) > return gtk_check_menu_item_get_active(GTK_CHECK_MENU_ITEM(s->grab_on_hover_item)); > } > > -static bool gd_on_vga(GtkDisplayState *s) > +static bool gd_on_display(GtkDisplayState *s) > { > return gtk_notebook_get_current_page(GTK_NOTEBOOK(s->notebook)) == 0; > } > @@ -141,13 +141,13 @@ static bool gd_on_vga(GtkDisplayState *s) > static void gd_update_cursor(GtkDisplayState *s, gboolean override) > { > GdkWindow *window; > - bool on_vga; > + bool on_display; > > window = gtk_widget_get_window(GTK_WIDGET(s->drawing_area)); > > - on_vga = gd_on_vga(s); > + on_display = gd_on_display(s); > > - if ((override || on_vga) && > + if ((override || on_display) && > (s->full_screen || kbd_mouse_is_absolute() || gd_is_grab_active(s))) { > gdk_window_set_cursor(window, s->null_cursor); > } else { > @@ -593,7 +593,7 @@ static void gd_menu_switch_vc(GtkMenuItem *item, void *opaque) > { > GtkDisplayState *s = opaque; > > - if (gtk_check_menu_item_get_active(GTK_CHECK_MENU_ITEM(s->vga_item))) { > + if (gtk_check_menu_item_get_active(GTK_CHECK_MENU_ITEM(s->display_item))) { > gtk_notebook_set_current_page(GTK_NOTEBOOK(s->notebook), 0); > } else { > int i; > @@ -627,7 +627,7 @@ static void gd_menu_full_screen(GtkMenuItem *item, void *opaque) > gtk_widget_set_size_request(s->menu_bar, 0, 0); > gtk_widget_set_size_request(s->drawing_area, -1, -1); > gtk_window_fullscreen(GTK_WINDOW(s->window)); > - if (gd_on_vga(s)) { > + if (gd_on_display(s)) { > gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(s->grab_item), TRUE); > } > s->full_screen = TRUE; > @@ -744,7 +744,7 @@ static void gd_change_page(GtkNotebook *nb, gpointer arg1, guint arg2, > { > GtkDisplayState *s = data; > guint last_page; > - gboolean on_vga; > + gboolean on_display; > > if (!gtk_widget_get_realized(s->notebook)) { > return; > @@ -756,9 +756,9 @@ static void gd_change_page(GtkNotebook *nb, gpointer arg1, guint arg2, > gtk_widget_set_size_request(s->vc[last_page - 1].terminal, -1, -1); > } > > - on_vga = arg2 == 0; > + on_display = arg2 == 0; > > - if (!on_vga) { > + if (!on_display) { > gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(s->grab_item), > FALSE); > } else if (s->full_screen) { > @@ -767,7 +767,8 @@ static void gd_change_page(GtkNotebook *nb, gpointer arg1, guint arg2, > } > > if (arg2 == 0) { > - gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(s->vga_item), TRUE); > + gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(s->display_item), > + TRUE); > } else { > VirtualConsole *vc = &s->vc[arg2 - 1]; > VteTerminal *term = VTE_TERMINAL(vc->terminal); > @@ -780,7 +781,7 @@ static void gd_change_page(GtkNotebook *nb, gpointer arg1, guint arg2, > gtk_widget_set_size_request(vc->terminal, width, height); > } > > - gtk_widget_set_sensitive(s->grab_item, on_vga); > + gtk_widget_set_sensitive(s->grab_item, on_display); > > gd_update_cursor(s, TRUE); > } > @@ -964,7 +965,7 @@ static void gd_connect_signals(GtkDisplayState *s) > G_CALLBACK(gd_menu_zoom_fixed), s); > g_signal_connect(s->zoom_fit_item, "activate", > G_CALLBACK(gd_menu_zoom_fit), s); > - g_signal_connect(s->vga_item, "activate", > + g_signal_connect(s->display_item, "activate", > G_CALLBACK(gd_menu_switch_vc), s); > g_signal_connect(s->grab_item, "activate", > G_CALLBACK(gd_menu_grab_input), s); > @@ -1044,12 +1045,15 @@ static void gd_create_menus(GtkDisplayState *s) > separator = gtk_separator_menu_item_new(); > gtk_menu_append(GTK_MENU(s->view_menu), separator); > > - s->vga_item = gtk_radio_menu_item_new_with_mnemonic(group, "_VGA"); > - group = gtk_radio_menu_item_get_group(GTK_RADIO_MENU_ITEM(s->vga_item)); > - gtk_menu_item_set_accel_path(GTK_MENU_ITEM(s->vga_item), > - "<QEMU>/View/VGA"); > - gtk_accel_map_add_entry("<QEMU>/View/VGA", GDK_KEY_1, GDK_CONTROL_MASK | GDK_MOD1_MASK); > - gtk_menu_append(GTK_MENU(s->view_menu), s->vga_item); > + s->display_item = gtk_radio_menu_item_new_with_mnemonic(group, > + _("_Display")); > + group = gtk_radio_menu_item_get_group( > + GTK_RADIO_MENU_ITEM(s->display_item)); > + gtk_menu_item_set_accel_path(GTK_MENU_ITEM(s->display_item), > + "<QEMU>/View/Display"); > + gtk_accel_map_add_entry("<QEMU>/View/Display", GDK_KEY_1, > + GDK_CONTROL_MASK | GDK_MOD1_MASK); > + gtk_menu_append(GTK_MENU(s->view_menu), s->display_item); > > for (i = 0; i < nb_vcs; i++) { > VirtualConsole *vc = &s->vc[i]; > @@ -1108,7 +1112,8 @@ void gtk_display_init(DisplayState *ds) > qemu_add_mouse_mode_change_notifier(&s->mouse_mode_notifier); > qemu_add_vm_change_state_handler(gd_change_runstate, s); > > - gtk_notebook_append_page(GTK_NOTEBOOK(s->notebook), s->drawing_area, gtk_label_new("VGA")); > + gtk_notebook_append_page(GTK_NOTEBOOK(s->notebook), s->drawing_area, > + gtk_label_new("Display")); > > gd_create_menus(s); > > -- > 1.7.3.4 > -- > Siemens AG, Corporate Technology, CT RTC ITP SDP-DE > Corporate Competence Center Embedded Linux
On 2013-02-22 14:21, Anthony Liguori wrote: > Hi Jan, > > Jan Kiszka <jan.kiszka@siemens.com> writes: > >> VGA is a misnomer, only some of our targets have it. Use the more >> generic term "display" instead. >> >> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> > > Sorry, I responded to your previous note in my head and forgot to also > respond on the mailing list. > > We expose this "misnomer" a lot. For instance, qemu -vga cirrus. That's because qemu started for x86 only. No excuse to spread this further in new code. > > OTOH, the term "display" is used to indicate the backend via qemu > -display sdl. > > So calling the VGA tab "Display" after selecting '-display gtk' seems a > bit confusing to me. > > So I'm open to picking better naming but I think we should try to make > it consistent throughout QEMU. We can also call it "screen" or "guest display" - just not VGA, please. Jan
Jan Kiszka <jan.kiszka@siemens.com> writes: > On 2013-02-22 14:21, Anthony Liguori wrote: >> Hi Jan, >> >> Jan Kiszka <jan.kiszka@siemens.com> writes: >> >>> VGA is a misnomer, only some of our targets have it. Use the more >>> generic term "display" instead. >>> >>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> >> >> Sorry, I responded to your previous note in my head and forgot to also >> respond on the mailing list. >> >> We expose this "misnomer" a lot. For instance, qemu -vga cirrus. > > That's because qemu started for x86 only. No excuse to spread this > further in new code. > >> >> OTOH, the term "display" is used to indicate the backend via qemu >> -display sdl. >> >> So calling the VGA tab "Display" after selecting '-display gtk' seems a >> bit confusing to me. >> >> So I'm open to picking better naming but I think we should try to make >> it consistent throughout QEMU. > > We can also call it "screen" or "guest display" - just not VGA, > please. I don't care what we call it, I'm asking that if you want to change it in one place, change it every where else too. Regards, Anthony Liguori > > Jan > > -- > Siemens AG, Corporate Technology, CT RTC ITP SDP-DE > Corporate Competence Center Embedded Linux
On 2013-02-22 15:17, Anthony Liguori wrote: > Jan Kiszka <jan.kiszka@siemens.com> writes: > >> On 2013-02-22 14:21, Anthony Liguori wrote: >>> Hi Jan, >>> >>> Jan Kiszka <jan.kiszka@siemens.com> writes: >>> >>>> VGA is a misnomer, only some of our targets have it. Use the more >>>> generic term "display" instead. >>>> >>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> >>> >>> Sorry, I responded to your previous note in my head and forgot to also >>> respond on the mailing list. >>> >>> We expose this "misnomer" a lot. For instance, qemu -vga cirrus. >> >> That's because qemu started for x86 only. No excuse to spread this >> further in new code. >> >>> >>> OTOH, the term "display" is used to indicate the backend via qemu >>> -display sdl. >>> >>> So calling the VGA tab "Display" after selecting '-display gtk' seems a >>> bit confusing to me. >>> >>> So I'm open to picking better naming but I think we should try to make >>> it consistent throughout QEMU. >> >> We can also call it "screen" or "guest display" - just not VGA, >> please. > > I don't care what we call it, I'm asking that if you want to change it > in one place, change it every where else too. For obvious reasons, -vga has to stay. Also, I can leave the code as is. My primary point is the user-visible misnomer "VGA". That should be fixed before rolling it out. I'll trim down my patch to this. Jan
Jan Kiszka <jan.kiszka@siemens.com> writes: > On 2013-02-22 15:17, Anthony Liguori wrote: >> Jan Kiszka <jan.kiszka@siemens.com> writes: >> >>> On 2013-02-22 14:21, Anthony Liguori wrote: >>>> Hi Jan, >>>> >>>> Jan Kiszka <jan.kiszka@siemens.com> writes: >>>> >>>>> VGA is a misnomer, only some of our targets have it. Use the more >>>>> generic term "display" instead. >>>>> >>>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> >>>> >>>> Sorry, I responded to your previous note in my head and forgot to also >>>> respond on the mailing list. >>>> >>>> We expose this "misnomer" a lot. For instance, qemu -vga cirrus. >>> >>> That's because qemu started for x86 only. No excuse to spread this >>> further in new code. >>> >>>> >>>> OTOH, the term "display" is used to indicate the backend via qemu >>>> -display sdl. >>>> >>>> So calling the VGA tab "Display" after selecting '-display gtk' seems a >>>> bit confusing to me. >>>> >>>> So I'm open to picking better naming but I think we should try to make >>>> it consistent throughout QEMU. >>> >>> We can also call it "screen" or "guest display" - just not VGA, >>> please. >> >> I don't care what we call it, I'm asking that if you want to change it >> in one place, change it every where else too. > > For obvious reasons, -vga has to stay. We can introduce a new option that aliases this the -vga option. [08:50 AM] anthony@titi:~/git/qemu$ grep vga *.c | wc -l 51 This is not in the hw/ directory. All of our internal infrastructure calls it VGA. It's odd to me to call it something different in the UI without changing the internal bits too. Regards, Anthony Liguori > Also, I can leave the code as is. > My primary point is the user-visible misnomer "VGA". That should be > fixed before rolling it out. I'll trim down my patch to this. > > Jan > > -- > Siemens AG, Corporate Technology, CT RTC ITP SDP-DE > Corporate Competence Center Embedded Linux
On 2013-02-22 15:51, Anthony Liguori wrote: > Jan Kiszka <jan.kiszka@siemens.com> writes: > >> On 2013-02-22 15:17, Anthony Liguori wrote: >>> Jan Kiszka <jan.kiszka@siemens.com> writes: >>> >>>> On 2013-02-22 14:21, Anthony Liguori wrote: >>>>> Hi Jan, >>>>> >>>>> Jan Kiszka <jan.kiszka@siemens.com> writes: >>>>> >>>>>> VGA is a misnomer, only some of our targets have it. Use the more >>>>>> generic term "display" instead. >>>>>> >>>>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> >>>>> >>>>> Sorry, I responded to your previous note in my head and forgot to also >>>>> respond on the mailing list. >>>>> >>>>> We expose this "misnomer" a lot. For instance, qemu -vga cirrus. >>>> >>>> That's because qemu started for x86 only. No excuse to spread this >>>> further in new code. >>>> >>>>> >>>>> OTOH, the term "display" is used to indicate the backend via qemu >>>>> -display sdl. >>>>> >>>>> So calling the VGA tab "Display" after selecting '-display gtk' seems a >>>>> bit confusing to me. >>>>> >>>>> So I'm open to picking better naming but I think we should try to make >>>>> it consistent throughout QEMU. >>>> >>>> We can also call it "screen" or "guest display" - just not VGA, >>>> please. >>> >>> I don't care what we call it, I'm asking that if you want to change it >>> in one place, change it every where else too. >> >> For obvious reasons, -vga has to stay. > > We can introduce a new option that aliases this the -vga option. No need, the new way is -device <your-favorite-graphic>. Jan
Il 22/02/2013 15:51, Anthony Liguori ha scritto: >> > For obvious reasons, -vga has to stay. > We can introduce a new option that aliases this the -vga option. > > [08:50 AM] anthony@titi:~/git/qemu$ grep vga *.c | wc -l > 51 > > This is not in the hw/ directory. All of our internal infrastructure > calls it VGA. It's odd to me to call it something different in the UI > without changing the internal bits too. It's all in vl.c, and all about -vga. $ grep -c vga vl.c 46 These are what are called vga_ by "mistake": $ git grep -c vga origin/master -- ui/ origin/master:ui/cocoa.m:2 origin/master:ui/console.c:27 origin/master:ui/curses.c:2 origin/master:ui/gtk.c:18 origin/master:ui/sdl.c:10 origin/master:ui/spice-core.c:1 origin/master:ui/spice-display.c:1 origin/master:ui/vgafont.h:1 origin/master:ui/vnc.c:5 What about: - s/vga_hw_/guest_display_/ (e.g. guest_display_invalidate) - "Guest" or "Display" or "Guest display" for the GTK UI (and similarly in the code, e.g. gd_on_guest_display)? Paolo
Il 22/02/2013 16:26, Paolo Bonzini ha scritto: > Il 22/02/2013 15:51, Anthony Liguori ha scritto: >>>> For obvious reasons, -vga has to stay. >> We can introduce a new option that aliases this the -vga option. >> >> [08:50 AM] anthony@titi:~/git/qemu$ grep vga *.c | wc -l >> 51 >> >> This is not in the hw/ directory. All of our internal infrastructure >> calls it VGA. It's odd to me to call it something different in the UI >> without changing the internal bits too. > > It's all in vl.c, and all about -vga. > > $ grep -c vga vl.c > 46 > > These are what are called vga_ by "mistake": > > $ git grep -c vga origin/master -- ui/ > origin/master:ui/cocoa.m:2 > origin/master:ui/console.c:27 > origin/master:ui/curses.c:2 > origin/master:ui/gtk.c:18 > origin/master:ui/sdl.c:10 > origin/master:ui/spice-core.c:1 > origin/master:ui/spice-display.c:1 > origin/master:ui/vgafont.h:1 > origin/master:ui/vnc.c:5 > > What about: > > - s/vga_hw_/guest_display_/ (e.g. guest_display_invalidate) > > - "Guest" or "Display" or "Guest display" for the GTK UI (and similarly > in the code, e.g. gd_on_guest_display)? Saw Jan's v2, it looks like s/vga_hw_/screen_/ would also have no conflicts. Paolo
diff --git a/ui/gtk.c b/ui/gtk.c index 29156be..e9faf15 100644 --- a/ui/gtk.c +++ b/ui/gtk.c @@ -93,7 +93,7 @@ typedef struct GtkDisplayState GtkWidget *zoom_fit_item; GtkWidget *grab_item; GtkWidget *grab_on_hover_item; - GtkWidget *vga_item; + GtkWidget *display_item; int nb_vcs; VirtualConsole vc[MAX_VCS]; @@ -133,7 +133,7 @@ static bool gd_grab_on_hover(GtkDisplayState *s) return gtk_check_menu_item_get_active(GTK_CHECK_MENU_ITEM(s->grab_on_hover_item)); } -static bool gd_on_vga(GtkDisplayState *s) +static bool gd_on_display(GtkDisplayState *s) { return gtk_notebook_get_current_page(GTK_NOTEBOOK(s->notebook)) == 0; } @@ -141,13 +141,13 @@ static bool gd_on_vga(GtkDisplayState *s) static void gd_update_cursor(GtkDisplayState *s, gboolean override) { GdkWindow *window; - bool on_vga; + bool on_display; window = gtk_widget_get_window(GTK_WIDGET(s->drawing_area)); - on_vga = gd_on_vga(s); + on_display = gd_on_display(s); - if ((override || on_vga) && + if ((override || on_display) && (s->full_screen || kbd_mouse_is_absolute() || gd_is_grab_active(s))) { gdk_window_set_cursor(window, s->null_cursor); } else { @@ -593,7 +593,7 @@ static void gd_menu_switch_vc(GtkMenuItem *item, void *opaque) { GtkDisplayState *s = opaque; - if (gtk_check_menu_item_get_active(GTK_CHECK_MENU_ITEM(s->vga_item))) { + if (gtk_check_menu_item_get_active(GTK_CHECK_MENU_ITEM(s->display_item))) { gtk_notebook_set_current_page(GTK_NOTEBOOK(s->notebook), 0); } else { int i; @@ -627,7 +627,7 @@ static void gd_menu_full_screen(GtkMenuItem *item, void *opaque) gtk_widget_set_size_request(s->menu_bar, 0, 0); gtk_widget_set_size_request(s->drawing_area, -1, -1); gtk_window_fullscreen(GTK_WINDOW(s->window)); - if (gd_on_vga(s)) { + if (gd_on_display(s)) { gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(s->grab_item), TRUE); } s->full_screen = TRUE; @@ -744,7 +744,7 @@ static void gd_change_page(GtkNotebook *nb, gpointer arg1, guint arg2, { GtkDisplayState *s = data; guint last_page; - gboolean on_vga; + gboolean on_display; if (!gtk_widget_get_realized(s->notebook)) { return; @@ -756,9 +756,9 @@ static void gd_change_page(GtkNotebook *nb, gpointer arg1, guint arg2, gtk_widget_set_size_request(s->vc[last_page - 1].terminal, -1, -1); } - on_vga = arg2 == 0; + on_display = arg2 == 0; - if (!on_vga) { + if (!on_display) { gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(s->grab_item), FALSE); } else if (s->full_screen) { @@ -767,7 +767,8 @@ static void gd_change_page(GtkNotebook *nb, gpointer arg1, guint arg2, } if (arg2 == 0) { - gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(s->vga_item), TRUE); + gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(s->display_item), + TRUE); } else { VirtualConsole *vc = &s->vc[arg2 - 1]; VteTerminal *term = VTE_TERMINAL(vc->terminal); @@ -780,7 +781,7 @@ static void gd_change_page(GtkNotebook *nb, gpointer arg1, guint arg2, gtk_widget_set_size_request(vc->terminal, width, height); } - gtk_widget_set_sensitive(s->grab_item, on_vga); + gtk_widget_set_sensitive(s->grab_item, on_display); gd_update_cursor(s, TRUE); } @@ -964,7 +965,7 @@ static void gd_connect_signals(GtkDisplayState *s) G_CALLBACK(gd_menu_zoom_fixed), s); g_signal_connect(s->zoom_fit_item, "activate", G_CALLBACK(gd_menu_zoom_fit), s); - g_signal_connect(s->vga_item, "activate", + g_signal_connect(s->display_item, "activate", G_CALLBACK(gd_menu_switch_vc), s); g_signal_connect(s->grab_item, "activate", G_CALLBACK(gd_menu_grab_input), s); @@ -1044,12 +1045,15 @@ static void gd_create_menus(GtkDisplayState *s) separator = gtk_separator_menu_item_new(); gtk_menu_append(GTK_MENU(s->view_menu), separator); - s->vga_item = gtk_radio_menu_item_new_with_mnemonic(group, "_VGA"); - group = gtk_radio_menu_item_get_group(GTK_RADIO_MENU_ITEM(s->vga_item)); - gtk_menu_item_set_accel_path(GTK_MENU_ITEM(s->vga_item), - "<QEMU>/View/VGA"); - gtk_accel_map_add_entry("<QEMU>/View/VGA", GDK_KEY_1, GDK_CONTROL_MASK | GDK_MOD1_MASK); - gtk_menu_append(GTK_MENU(s->view_menu), s->vga_item); + s->display_item = gtk_radio_menu_item_new_with_mnemonic(group, + _("_Display")); + group = gtk_radio_menu_item_get_group( + GTK_RADIO_MENU_ITEM(s->display_item)); + gtk_menu_item_set_accel_path(GTK_MENU_ITEM(s->display_item), + "<QEMU>/View/Display"); + gtk_accel_map_add_entry("<QEMU>/View/Display", GDK_KEY_1, + GDK_CONTROL_MASK | GDK_MOD1_MASK); + gtk_menu_append(GTK_MENU(s->view_menu), s->display_item); for (i = 0; i < nb_vcs; i++) { VirtualConsole *vc = &s->vc[i]; @@ -1108,7 +1112,8 @@ void gtk_display_init(DisplayState *ds) qemu_add_mouse_mode_change_notifier(&s->mouse_mode_notifier); qemu_add_vm_change_state_handler(gd_change_runstate, s); - gtk_notebook_append_page(GTK_NOTEBOOK(s->notebook), s->drawing_area, gtk_label_new("VGA")); + gtk_notebook_append_page(GTK_NOTEBOOK(s->notebook), s->drawing_area, + gtk_label_new("Display")); gd_create_menus(s);
VGA is a misnomer, only some of our targets have it. Use the more generic term "display" instead. Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> --- No translation updates yet as I do not know how you like to handle them. ui/gtk.c | 45 +++++++++++++++++++++++++-------------------- 1 files changed, 25 insertions(+), 20 deletions(-)