diff mbox

[6/8] gtk: add support for screen scaling and full screen (v2)

Message ID 1330299995-8688-7-git-send-email-aliguori@us.ibm.com
State New
Headers show

Commit Message

Anthony Liguori Feb. 26, 2012, 11:46 p.m. UTC
Basic menu items to enter full screen mode and zoom in/out.  Unlike SDL, we
don't allow arbitrary scaling based on window resizing.  The current behavior
with SDL causes a lot of problems for me.

Sometimes I accidentally resize the window a tiny bit while trying to move it
(Ubuntu's 1-pixel window decorations don't help here).  After that, scaling is
now active and if the screen changes size again, badness ensues since the
aspect ratio is skewed.

Allowing zooming by 25% in and out should cover most use cases.  We can add a
more flexible scaling later but for now, I think this is a more friendly
behavior.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
v1 -> v2
 - fix scaling (Paolo)
 - use ctrl-alt-+ instead of ctrl-alt-= for zoom
---
 ui/gtk.c |   92 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
 1 files changed, 88 insertions(+), 4 deletions(-)

Comments

Stefan Weil Feb. 27, 2012, 8:10 p.m. UTC | #1
Am 27.02.2012 00:46, schrieb Anthony Liguori:
> Basic menu items to enter full screen mode and zoom in/out. Unlike SDL, we
> don't allow arbitrary scaling based on window resizing. The current 
> behavior
> with SDL causes a lot of problems for me.
>
> Sometimes I accidentally resize the window a tiny bit while trying to 
> move it
> (Ubuntu's 1-pixel window decorations don't help here). After that, 
> scaling is
> now active and if the screen changes size again, badness ensues since the
> aspect ratio is skewed.
>
> Allowing zooming by 25% in and out should cover most use cases. We can 
> add a
> more flexible scaling later but for now, I think this is a more friendly
> behavior.
>
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> ---
> v1 -> v2
> - fix scaling (Paolo)
> - use ctrl-alt-+ instead of ctrl-alt-= for zoom
> ---
> ui/gtk.c | 92 
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
> 1 files changed, 88 insertions(+), 4 deletions(-)
>
> diff --git a/ui/gtk.c b/ui/gtk.c
> index 0dac807..578cb94 100644
> --- a/ui/gtk.c
> +++ b/ui/gtk.c

[...]

>
> + s->full_screen_item = gtk_check_menu_item_new_with_mnemonic("_Full 
> Screen");

I suggest using the GTK standard widget GTK_STOCK_FULLSCREEN here.
That's not a check menu item, so some more changes will be needed.

Full screen mode does not need a check menu item, because you only
see the menu item when it is not in full screen mode.

> + gtk_menu_item_set_accel_path(GTK_MENU_ITEM(s->full_screen_item),
> + "<QEMU>/View/Full Screen");
> + gtk_accel_map_add_entry("<QEMU>/View/Full Screen", GDK_KEY_f, 
> GDK_CONTROL_MASK | GDK_MOD1_MASK);
> + gtk_menu_append(GTK_MENU(s->view_menu), s->full_screen_item);
> +
> + separator = gtk_separator_menu_item_new();
> + gtk_menu_append(GTK_MENU(s->view_menu), separator);
> +
> + s->zoom_in_item = 
> gtk_image_menu_item_new_from_stock(GTK_STOCK_ZOOM_IN, NULL);
> + gtk_menu_item_set_accel_path(GTK_MENU_ITEM(s->zoom_in_item),
> + "<QEMU>/View/Zoom In");
> + gtk_accel_map_add_entry("<QEMU>/View/Zoom In", GDK_KEY_plus, 
> GDK_CONTROL_MASK | GDK_MOD1_MASK);
> + gtk_menu_append(GTK_MENU(s->view_menu), s->zoom_in_item);
> +
> + s->zoom_out_item = 
> gtk_image_menu_item_new_from_stock(GTK_STOCK_ZOOM_OUT, NULL);
> + gtk_menu_item_set_accel_path(GTK_MENU_ITEM(s->zoom_out_item),
> + "<QEMU>/View/Zoom Out");
> + gtk_accel_map_add_entry("<QEMU>/View/Zoom Out", GDK_KEY_minus, 
> GDK_CONTROL_MASK | GDK_MOD1_MASK);
> + gtk_menu_append(GTK_MENU(s->view_menu), s->zoom_out_item);
> +
> separator = gtk_separator_menu_item_new();
> gtk_menu_append(GTK_MENU(s->view_menu), separator);
>

As was already said, GTK_STOCK_ZOOM_100 with accelerator GDK_KEY_0
would complete the zooming menu entries.

In your previous patch, you scaled s->scale_x *= 1.25 (which was correct)
and s->scale_x *= 0.75 (which should have been s->scale_x *= 0.8).

Why do you now use s->scale_x += .25, s->scale_x -= .25?
Linear scaling is uncommon and not what I'd expect when I read
25 % zooming in / out. I know that SDL also uses linear scaling,
but GTK wants to be better than SDL.

Regards,

Stefan Weil
Anthony Liguori Feb. 27, 2012, 10:01 p.m. UTC | #2
On 02/27/2012 02:10 PM, Stefan Weil wrote:
> Am 27.02.2012 00:46, schrieb Anthony Liguori:
>> Basic menu items to enter full screen mode and zoom in/out. Unlike SDL, we
>> don't allow arbitrary scaling based on window resizing. The current behavior
>> with SDL causes a lot of problems for me.
>>
>> Sometimes I accidentally resize the window a tiny bit while trying to move it
>> (Ubuntu's 1-pixel window decorations don't help here). After that, scaling is
>> now active and if the screen changes size again, badness ensues since the
>> aspect ratio is skewed.
>>
>> Allowing zooming by 25% in and out should cover most use cases. We can add a
>> more flexible scaling later but for now, I think this is a more friendly
>> behavior.
>>
>> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
>> ---
>> v1 -> v2
>> - fix scaling (Paolo)
>> - use ctrl-alt-+ instead of ctrl-alt-= for zoom
>> ---
>> ui/gtk.c | 92 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
>> 1 files changed, 88 insertions(+), 4 deletions(-)
>>
>> diff --git a/ui/gtk.c b/ui/gtk.c
>> index 0dac807..578cb94 100644
>> --- a/ui/gtk.c
>> +++ b/ui/gtk.c
>
> [...]
>
>>
>> + s->full_screen_item = gtk_check_menu_item_new_with_mnemonic("_Full Screen");
>
> I suggest using the GTK standard widget GTK_STOCK_FULLSCREEN here.
> That's not a check menu item, so some more changes will be needed.
>
> Full screen mode does not need a check menu item, because you only
> see the menu item when it is not in full screen mode.

Yeah, I've thought about this myself too.  In the very least, we could use the 
stock item's label.  I've been on the fence about check vs. image.

Regarding use of a check menu item.  I've thought about this too.  In the past, 
it was common to have an "Enter Fullscreen" and a "Leave Fullscreen" option 
although now it seems to have a single check item for Fullscreen.  Both evince 
and firefox do this for instance.

It definitely makes sense programatically (it's a toggle after all).  If we did 
something like have an autohiding menubar when in full screen mode, it would 
also make more sense from a UI perspective.

>
>> + gtk_menu_item_set_accel_path(GTK_MENU_ITEM(s->full_screen_item),
>> + "<QEMU>/View/Full Screen");
>> + gtk_accel_map_add_entry("<QEMU>/View/Full Screen", GDK_KEY_f,
>> GDK_CONTROL_MASK | GDK_MOD1_MASK);
>> + gtk_menu_append(GTK_MENU(s->view_menu), s->full_screen_item);
>> +
>> + separator = gtk_separator_menu_item_new();
>> + gtk_menu_append(GTK_MENU(s->view_menu), separator);
>> +
>> + s->zoom_in_item = gtk_image_menu_item_new_from_stock(GTK_STOCK_ZOOM_IN, NULL);
>> + gtk_menu_item_set_accel_path(GTK_MENU_ITEM(s->zoom_in_item),
>> + "<QEMU>/View/Zoom In");
>> + gtk_accel_map_add_entry("<QEMU>/View/Zoom In", GDK_KEY_plus,
>> GDK_CONTROL_MASK | GDK_MOD1_MASK);
>> + gtk_menu_append(GTK_MENU(s->view_menu), s->zoom_in_item);
>> +
>> + s->zoom_out_item = gtk_image_menu_item_new_from_stock(GTK_STOCK_ZOOM_OUT,
>> NULL);
>> + gtk_menu_item_set_accel_path(GTK_MENU_ITEM(s->zoom_out_item),
>> + "<QEMU>/View/Zoom Out");
>> + gtk_accel_map_add_entry("<QEMU>/View/Zoom Out", GDK_KEY_minus,
>> GDK_CONTROL_MASK | GDK_MOD1_MASK);
>> + gtk_menu_append(GTK_MENU(s->view_menu), s->zoom_out_item);
>> +
>> separator = gtk_separator_menu_item_new();
>> gtk_menu_append(GTK_MENU(s->view_menu), separator);
>>
>
> As was already said, GTK_STOCK_ZOOM_100 with accelerator GDK_KEY_0
> would complete the zooming menu entries.

Yeah, should be easy enough to add.

> In your previous patch, you scaled s->scale_x *= 1.25 (which was correct)
> and s->scale_x *= 0.75 (which should have been s->scale_x *= 0.8).
>
> Why do you now use s->scale_x += .25, s->scale_x -= .25?
> Linear scaling is uncommon and not what I'd expect when I read
> 25 % zooming in / out. I know that SDL also uses linear scaling,
> but GTK wants to be better than SDL.

I was suspicious that floating point math was causing undesirable rounding such 
that I was a pixel off when you zoomed out and zoomed back to 100%.  Stretching 
by 1 pixel tends to create a very noticeable artifact with text.

I think we could do exponential scaling but store the exponent instead of the 
straight factor.  That would avoid any accumulated rounding error.

Regards,

Anthony Liguori

> Regards,
>
> Stefan Weil
>
>
Kevin Wolf Feb. 28, 2012, 2:18 p.m. UTC | #3
Am 27.02.2012 21:10, schrieb Stefan Weil:
> Am 27.02.2012 00:46, schrieb Anthony Liguori:
>> Basic menu items to enter full screen mode and zoom in/out. Unlike SDL, we
>> don't allow arbitrary scaling based on window resizing. The current 
>> behavior
>> with SDL causes a lot of problems for me.
>>
>> Sometimes I accidentally resize the window a tiny bit while trying to 
>> move it
>> (Ubuntu's 1-pixel window decorations don't help here). After that, 
>> scaling is
>> now active and if the screen changes size again, badness ensues since the
>> aspect ratio is skewed.
>>
>> Allowing zooming by 25% in and out should cover most use cases. We can 
>> add a
>> more flexible scaling later but for now, I think this is a more friendly
>> behavior.
>>
>> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
>> ---
>> v1 -> v2
>> - fix scaling (Paolo)
>> - use ctrl-alt-+ instead of ctrl-alt-= for zoom
>> ---
>> ui/gtk.c | 92 
>> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
>> 1 files changed, 88 insertions(+), 4 deletions(-)
>>
>> diff --git a/ui/gtk.c b/ui/gtk.c
>> index 0dac807..578cb94 100644
>> --- a/ui/gtk.c
>> +++ b/ui/gtk.c
> 
> [...]
> 
>>
>> + s->full_screen_item = gtk_check_menu_item_new_with_mnemonic("_Full 
>> Screen");
> 
> I suggest using the GTK standard widget GTK_STOCK_FULLSCREEN here.
> That's not a check menu item, so some more changes will be needed.
> 
> Full screen mode does not need a check menu item, because you only
> see the menu item when it is not in full screen mode.

Tried Alt-V in full screen mode? ;-)

(Yes, I'd consider it a bug)

Kevin
diff mbox

Patch

diff --git a/ui/gtk.c b/ui/gtk.c
index 0dac807..578cb94 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -79,6 +79,9 @@  typedef struct GtkDisplayState
 
     GtkWidget *view_menu_item;
     GtkWidget *view_menu;
+    GtkWidget *full_screen_item;
+    GtkWidget *zoom_in_item;
+    GtkWidget *zoom_out_item;
     GtkWidget *grab_item;
     GtkWidget *vga_item;
 
@@ -99,6 +102,7 @@  typedef struct GtkDisplayState
 
     double scale_x;
     double scale_y;
+    gboolean full_screen;
 
     GdkCursor *null_cursor;
     Notifier mouse_mode_notifier;
@@ -123,7 +127,7 @@  static void gd_update_cursor(GtkDisplayState *s, gboolean override)
     on_vga = (gtk_notebook_get_current_page(GTK_NOTEBOOK(s->notebook)) == 0);
 
     if ((override || on_vga) &&
-        (kbd_mouse_is_absolute() || gd_is_grab_active(s))) {
+        (s->full_screen || kbd_mouse_is_absolute() || gd_is_grab_active(s))) {
 	gdk_window_set_cursor(window, s->null_cursor);
     } else {
 	gdk_window_set_cursor(window, NULL);
@@ -215,9 +219,11 @@  static void gd_resize(DisplayState *ds)
                                                      ds->surface->height,
                                                      ds->surface->linesize);
 
-    gtk_widget_set_size_request(s->drawing_area,
-                                ds->surface->width * s->scale_x,
-                                ds->surface->height * s->scale_y);
+    if (!s->full_screen) {
+        gtk_widget_set_size_request(s->drawing_area,
+                                    ds->surface->width * s->scale_x,
+                                    ds->surface->height * s->scale_y);
+    }
 }
 
 /** QEMU Events **/
@@ -474,6 +480,54 @@  static void gd_menu_show_tabs(GtkMenuItem *item, void *opaque)
     }
 }
 
+static void gd_menu_full_screen(GtkMenuItem *item, void *opaque)
+{
+    GtkDisplayState *s = opaque;
+
+    if (gtk_check_menu_item_get_active(GTK_CHECK_MENU_ITEM(s->full_screen_item))) {
+        gtk_notebook_set_show_tabs(GTK_NOTEBOOK(s->notebook), FALSE);
+        gtk_widget_set_size_request(s->menu_bar, 0, 0);
+        gtk_widget_set_size_request(s->drawing_area, -1, -1);
+        gtk_window_set_resizable(GTK_WINDOW(s->window), TRUE);
+        gtk_window_fullscreen(GTK_WINDOW(s->window));
+        gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(s->grab_item), TRUE);
+        s->full_screen = TRUE;
+    } else {
+        gtk_window_unfullscreen(GTK_WINDOW(s->window));
+        gd_menu_show_tabs(GTK_MENU_ITEM(s->show_tabs_item), s);
+        gtk_widget_set_size_request(s->menu_bar, -1, -1);
+        gtk_widget_set_size_request(s->drawing_area, s->ds->surface->width, s->ds->surface->height);
+        gtk_window_set_resizable(GTK_WINDOW(s->window), FALSE);
+        gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(s->grab_item), FALSE);
+        s->full_screen = FALSE;
+    }
+
+    gd_update_cursor(s, FALSE);
+}
+
+static void gd_menu_zoom_in(GtkMenuItem *item, void *opaque)
+{
+    GtkDisplayState *s = opaque;
+
+    s->scale_x += .25;
+    s->scale_y += .25;
+
+    gd_resize(s->ds);
+}
+
+static void gd_menu_zoom_out(GtkMenuItem *item, void *opaque)
+{
+    GtkDisplayState *s = opaque;
+
+    s->scale_x -= .25;
+    s->scale_y -= .25;
+
+    s->scale_x = MAX(s->scale_x, .25);
+    s->scale_y = MAX(s->scale_y, .25);
+
+    gd_resize(s->ds);
+}
+
 static void gd_menu_grab_input(GtkMenuItem *item, void *opaque)
 {
     GtkDisplayState *s = opaque;
@@ -523,6 +577,9 @@  static void gd_change_page(GtkNotebook *nb, gpointer arg1, guint arg2,
     if (!on_vga) {
         gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(s->grab_item),
                                        FALSE);
+    } else if (s->full_screen) {
+        gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(s->grab_item),
+                                       TRUE);
     }
 
     if (arg2 == 0) {
@@ -690,6 +747,12 @@  static void gd_connect_signals(GtkDisplayState *s)
 
     g_signal_connect(s->quit_item, "activate",
                      G_CALLBACK(gd_menu_quit), s);
+    g_signal_connect(s->full_screen_item, "activate",
+                     G_CALLBACK(gd_menu_full_screen), s);
+    g_signal_connect(s->zoom_in_item, "activate",
+                     G_CALLBACK(gd_menu_zoom_in), s);
+    g_signal_connect(s->zoom_out_item, "activate",
+                     G_CALLBACK(gd_menu_zoom_out), s);
     g_signal_connect(s->vga_item, "activate",
                      G_CALLBACK(gd_menu_switch_vc), s);
     g_signal_connect(s->grab_item, "activate",
@@ -721,6 +784,27 @@  static void gd_create_menus(GtkDisplayState *s)
     gtk_menu_set_accel_group(GTK_MENU(s->view_menu), accel_group);
     s->view_menu_item = gtk_menu_item_new_with_mnemonic("_View");
 
+    s->full_screen_item = gtk_check_menu_item_new_with_mnemonic("_Full Screen");
+    gtk_menu_item_set_accel_path(GTK_MENU_ITEM(s->full_screen_item),
+                                 "<QEMU>/View/Full Screen");
+    gtk_accel_map_add_entry("<QEMU>/View/Full Screen", GDK_KEY_f, GDK_CONTROL_MASK | GDK_MOD1_MASK);
+    gtk_menu_append(GTK_MENU(s->view_menu), s->full_screen_item);
+
+    separator = gtk_separator_menu_item_new();
+    gtk_menu_append(GTK_MENU(s->view_menu), separator);
+
+    s->zoom_in_item = gtk_image_menu_item_new_from_stock(GTK_STOCK_ZOOM_IN, NULL);
+    gtk_menu_item_set_accel_path(GTK_MENU_ITEM(s->zoom_in_item),
+                                 "<QEMU>/View/Zoom In");
+    gtk_accel_map_add_entry("<QEMU>/View/Zoom In", GDK_KEY_plus, GDK_CONTROL_MASK | GDK_MOD1_MASK);
+    gtk_menu_append(GTK_MENU(s->view_menu), s->zoom_in_item);
+
+    s->zoom_out_item = gtk_image_menu_item_new_from_stock(GTK_STOCK_ZOOM_OUT, NULL);
+    gtk_menu_item_set_accel_path(GTK_MENU_ITEM(s->zoom_out_item),
+                                 "<QEMU>/View/Zoom Out");
+    gtk_accel_map_add_entry("<QEMU>/View/Zoom Out", GDK_KEY_minus, GDK_CONTROL_MASK | GDK_MOD1_MASK);
+    gtk_menu_append(GTK_MENU(s->view_menu), s->zoom_out_item);
+
     separator = gtk_separator_menu_item_new();
     gtk_menu_append(GTK_MENU(s->view_menu), separator);