Patchwork [v2] gtk: Replace bogus term "VGA" with "Screen"

login
register
mail settings
Submitter Jan Kiszka
Date Feb. 22, 2013, 2:56 p.m.
Message ID <5127871E.9090803@siemens.com>
Download mbox | patch
Permalink /patch/222541/
State New
Headers show

Comments

Jan Kiszka - Feb. 22, 2013, 2:56 p.m.
VGA is a misnomer, only some of our targets have it. Use the more
generic term "Screen" instead.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---

Note that we have "screendump" as well, so this makes more sense than
"VGA". Internal renaming across all QEMU can still be done but is
beyond the scope of this UI fix.

 ui/gtk.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)
Anthony Liguori - Feb. 22, 2013, 3:37 p.m.
Jan Kiszka <jan.kiszka@siemens.com> writes:

> VGA is a misnomer, only some of our targets have it. Use the more
> generic term "Screen" instead.
>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>

Peter and I discussed this in IRC.

I think what I'd prefer to do is add a parameter to
graphics_console_init() for the name of the DisplayState.

This will better support having multiple graphics tabs in the future. I
can spin this patch later today unless you can do it sooner.

Regards,

Anthony Liguori

> ---
>
> Note that we have "screendump" as well, so this makes more sense than
> "VGA". Internal renaming across all QEMU can still be done but is
> beyond the scope of this UI fix.
>
>  ui/gtk.c |    5 +++--
>  1 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/ui/gtk.c b/ui/gtk.c
> index 29156be..bc4ec67 100644
> --- a/ui/gtk.c
> +++ b/ui/gtk.c
> @@ -1044,7 +1044,7 @@ 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");
> +    s->vga_item = gtk_radio_menu_item_new_with_mnemonic(group, _("_Screen"));
>      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");
> @@ -1108,7 +1108,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(_("Screen")));
>  
>      gd_create_menus(s);
>  
> -- 
> 1.7.3.4
Peter Maydell - Feb. 22, 2013, 3:43 p.m.
On 22 February 2013 15:37, Anthony Liguori <aliguori@us.ibm.com> wrote:
> I think what I'd prefer to do is add a parameter to
> graphics_console_init() for the name of the DisplayState.
>
> This will better support having multiple graphics tabs in the future. I
> can spin this patch later today unless you can do it sooner.

I'm not sure this helps, since the thing that calls
graphics_console_init() is the actual graphics device model,
which isn't the thing that has useful labels to distinguish
multiple graphics cards in a single system. (Consider a PC
model with two cirrus cards plugged in: they'll both end
up named "VGA" or "Cirrus" or something unless there's
a lot more infrastructure for the board to be able to
configure the name somehow.)

The right way to deal with this is for the video output to
be wired up from the graphics device to the UI via a QOM
connection of some kind. Then the thing doing the wiring
(the board model) could supply a suitable label when it did
so.

That's a lot of effort to go to for a really minor thing,
though; I'd just call it "Graphics" or "Video" (and have
the UI layer add "(1)" "(2)" etc if there are multiple
displays).

-- PMM
Jan Kiszka - Feb. 22, 2013, 3:43 p.m.
On 2013-02-22 16:37, Anthony Liguori wrote:
> Jan Kiszka <jan.kiszka@siemens.com> writes:
> 
>> VGA is a misnomer, only some of our targets have it. Use the more
>> generic term "Screen" instead.
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> 
> Peter and I discussed this in IRC.
> 
> I think what I'd prefer to do is add a parameter to
> graphics_console_init() for the name of the DisplayState.
> 
> This will better support having multiple graphics tabs in the future.

Yes, I was vaguely thinking of that feature as well. Makes sense.

> I can spin this patch later today unless you can do it sooner.

Fine with me, I'll leave it in your hands.

Jan
Anthony Liguori - Feb. 22, 2013, 4:59 p.m.
Peter Maydell <peter.maydell@linaro.org> writes:

> On 22 February 2013 15:37, Anthony Liguori <aliguori@us.ibm.com> wrote:
>> I think what I'd prefer to do is add a parameter to
>> graphics_console_init() for the name of the DisplayState.
>>
>> This will better support having multiple graphics tabs in the future. I
>> can spin this patch later today unless you can do it sooner.
>
> I'm not sure this helps, since the thing that calls
> graphics_console_init() is the actual graphics device model,
> which isn't the thing that has useful labels to distinguish
> multiple graphics cards in a single system. (Consider a PC
> model with two cirrus cards plugged in: they'll both end
> up named "VGA" or "Cirrus" or something unless there's
> a lot more infrastructure for the board to be able to
> configure the name somehow.)

They way I plan on doing it is simply making a NULL end up with 'video0'
or something like that.

For the video cards we emulate for the PC, I'll give them proper names.

It's pretty handy to have a more specific name IMHO. I'd rather see
'Cirrus VGA' than 'video0'.

Regards,

Anthony Liguori

> The right way to deal with this is for the video output to
> be wired up from the graphics device to the UI via a QOM
> connection of some kind. Then the thing doing the wiring
> (the board model) could supply a suitable label when it did
> so.
>
> That's a lot of effort to go to for a really minor thing,
> though; I'd just call it "Graphics" or "Video" (and have
> the UI layer add "(1)" "(2)" etc if there are multiple
> displays).
>
> -- PMM
Peter Maydell - Feb. 22, 2013, 5:02 p.m.
On 22 February 2013 16:59, Anthony Liguori <aliguori@us.ibm.com> wrote:
> They way I plan on doing it is simply making a NULL end up with 'video0'
> or something like that.

Mmm. (Might be nice to avoid having to touch every device to
add a NULL argument to the function though.)

> For the video cards we emulate for the PC, I'll give them proper names.
>
> It's pretty handy to have a more specific name IMHO. I'd rather see
> 'Cirrus VGA' than 'video0'.

I cope fine with the fact my real world monitors don't have
a digital display in the bezel somewhere that updates itself
to say "Nvidia" or "ATI" :-)

-- PMM
Stefan Weil - Feb. 22, 2013, 9:13 p.m.
Am 22.02.2013 18:02, schrieb Peter Maydell:
> On 22 February 2013 16:59, Anthony Liguori <aliguori@us.ibm.com> wrote:
>> They way I plan on doing it is simply making a NULL end up with 'video0'
>> or something like that.
> Mmm. (Might be nice to avoid having to touch every device to
> add a NULL argument to the function though.)
>
>> For the video cards we emulate for the PC, I'll give them proper names.
>>
>> It's pretty handy to have a more specific name IMHO. I'd rather see
>> 'Cirrus VGA' than 'video0'.
> I cope fine with the fact my real world monitors don't have
> a digital display in the bezel somewhere that updates itself
> to say "Nvidia" or "ATI" :-)
>
> -- PMM

So do I.

We have

    serial console
    printer

which are names from an end user's point of view, not hardware names.
So a logical name might beany of these

    graphical display
    display

(optionally with an index if there are 2 or more of them)
as a replacement for VGA. Of course it's even better when the user
can give any of these screens an individual name.

-- Stefan W.

Patch

diff --git a/ui/gtk.c b/ui/gtk.c
index 29156be..bc4ec67 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -1044,7 +1044,7 @@  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");
+    s->vga_item = gtk_radio_menu_item_new_with_mnemonic(group, _("_Screen"));
     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");
@@ -1108,7 +1108,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(_("Screen")));
 
     gd_create_menus(s);