[virt-tools-list] [PATCH virt-viewer 4/4 v2] virt-viewer-window: Change zoom of the display only when it is possible

[Fabiano Fidêncio]

On Mon, Mar 30, 2015 at 10:17 PM, Pavel Grunt <pgrunt at redhat.com> wrote:
> Do not allow to zoom out if it is not possible due to the width of
> the top menu. It avoids emitting size allocation events that will
> change the display resolution of the spice guest.
>
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1206460
> ---
> v2:
>  - treatment of zoom level moved to _window_set_zoom_level() in order to make it work in all cases (ie. with the command line option '-z')
>  - _get_minimal_zoom_level() instead of _can_zoom_out()
>  - more debug logs
> ---
>  src/virt-viewer-window.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 68 insertions(+), 2 deletions(-)
>
> diff --git a/src/virt-viewer-window.c b/src/virt-viewer-window.c
> index 799adce..48bb543 100644
> --- a/src/virt-viewer-window.c
> +++ b/src/virt-viewer-window.c
> @@ -34,9 +34,11 @@
>  #include <locale.h>
>  #include <glib/gprintf.h>
>  #include <glib/gi18n.h>
> +#include <math.h>
>
>  #include "virt-gtk-compat.h"
>  #include "virt-viewer-window.h"
> +#include "virt-viewer-display.h"
>  #include "virt-viewer-session.h"
>  #include "virt-viewer-app.h"
>  #include "virt-viewer-util.h"
> @@ -67,6 +69,8 @@ static void virt_viewer_window_disable_modifiers(VirtViewerWindow *self);
>  static void virt_viewer_window_resize(VirtViewerWindow *self, gboolean keep_win_size);
>  static void virt_viewer_window_toolbar_setup(VirtViewerWindow *self);
>  static GtkMenu* virt_viewer_window_get_keycombo_menu(VirtViewerWindow *self);
> +static void virt_viewer_window_get_minimal_dimensions(VirtViewerWindow *self, guint *width, guint *height);
> +static gint virt_viewer_window_get_minimal_zoom_level(VirtViewerWindow *self);
>
>  G_DEFINE_TYPE (VirtViewerWindow, virt_viewer_window, G_TYPE_OBJECT)
>
> @@ -1306,7 +1310,7 @@ virt_viewer_window_set_display(VirtViewerWindow *self, VirtViewerDisplay *displa
>      if (display != NULL) {
>          priv->display = g_object_ref(display);
>
> -        virt_viewer_display_set_zoom_level(VIRT_VIEWER_DISPLAY(priv->display), priv->zoomlevel);
> +        virt_viewer_window_set_zoom_level(self, priv->zoomlevel);
>          virt_viewer_display_set_monitor(VIRT_VIEWER_DISPLAY(priv->display), priv->fullscreen_monitor);
>          virt_viewer_display_set_fullscreen(VIRT_VIEWER_DISPLAY(priv->display), priv->fullscreen);
>
> @@ -1410,8 +1414,20 @@ virt_viewer_window_set_zoom_level(VirtViewerWindow *self, gint zoom_level)
>          zoom_level = MIN_ZOOM_LEVEL;
>      if (zoom_level > MAX_ZOOM_LEVEL)
>          zoom_level = MAX_ZOOM_LEVEL;
> -    priv->zoomlevel = zoom_level;
>
> +    if (priv->display != NULL) {
> +        guint min_zoom = virt_viewer_window_get_minimal_zoom_level(self);
> +        if (min_zoom > zoom_level) {
> +            g_debug("Cannot set zoom level %d, using %d", zoom_level, min_zoom);
> +            zoom_level = min_zoom;
> +        }
> +    }

Hmm. Right below you're checking again if display is NULL and
returning in case it is ...

> +
> +    if (priv->zoomlevel == zoom_level) {
> +        g_debug("Maximum/Minimum zoom level reached");

Actually, this is reached when the zoom is not changed ...

> +    }
> +

You can simply return here, right?

> +    priv->zoomlevel = zoom_level;
>      if (!priv->display)
>          return;


In this whole function I'd go for something like:

if (!priv->display)
    return;

if (zoom_level < MIN...)
if (zoom_level > MAX...)

min_zoom = virt_viewer_window_get_minimal_zoom_level(self);
if (min_zoom > zoom_level) {
    g_debug("Cannot set zoom level %d, using %d", zoom_level, min_zoom);
    zoom_level = min_zoom;
}

if (priv->zoomlevel == zoom_level) {
    g_debug("Zoom level not changed, using: %d", priv->zoomlevel)
    return;
}

>
> @@ -1467,6 +1483,56 @@ virt_viewer_window_set_kiosk(VirtViewerWindow *self, gboolean enabled)
>          g_debug("disabling kiosk not implemented yet");
>  }
>
> +static void
> +virt_viewer_window_get_minimal_dimensions(VirtViewerWindow *self,
> +                                          guint *width,
> +                                          guint *height)
> +{
> +    GtkRequisition req;
> +    GtkWidget *top_menu;
> +
> +    top_menu = GTK_WIDGET(gtk_builder_get_object(virt_viewer_window_get_builder(self), "top-menu"));
> +#if !GTK_CHECK_VERSION(3, 0, 0)
> +    gtk_widget_get_child_requisition(top_menu, &req);
> +#else
> +    gtk_widget_get_preferred_size(top_menu, &req, NULL);
> +#endif
> +    /* minimal dimensions of the window are the maximum of dimensions of the top-menu
> +     * and minimal dimension of the display
> +     */
> +    *height = MAX(MIN_DISPLAY_HEIGHT, req.height);
> +    *width = MAX(MIN_DISPLAY_WIDTH, req.width);
> +}
> +
> +/**
> + * virt_viewer_window_get_minimal_zoom_level:
> + * @self: a #VirtViewerWindow
> + *
> + * Calculates the zoom level with respect to the desktop dimensions
> + *
> + * Returns: minimal possible zoom level (multiple of ZOOM_STEP)
> + */
> +static gint
> +virt_viewer_window_get_minimal_zoom_level(VirtViewerWindow *self)
> +{
> +    guint min_width, min_height,
> +          width, height; /* desktop dimensions */
> +    gint zoom;
> +    double width_zoom, height_zoom; /* zoom percentage */
> +
> +    g_return_val_if_fail(VIRT_VIEWER_IS_WINDOW(self) &&
> +                         self->priv->display != NULL, MIN_ZOOM_LEVEL);
> +
> +    virt_viewer_window_get_minimal_dimensions(self, &min_width, &min_height);
> +    virt_viewer_display_get_desktop_size(virt_viewer_window_get_display(self), &width, &height);
> +
> +    width_zoom = (double) min_width / width;
> +    height_zoom = (double) min_height / height;
> +    zoom = ceil(10 * MAX(width_zoom, height_zoom));

Hmm. I didn't understand this part ...

> +
> +    return MAX(MIN_ZOOM_LEVEL, ZOOM_STEP * zoom);
> +}
> +
>  /*
>   * Local variables:
>   *  c-indent-level: 4
> --
> 2.3.4
>
> _______________________________________________
> virt-tools-list mailing list
> virt-tools-list at redhat.com
> https://www.redhat.com/mailman/listinfo/virt-tools-list


Best Regards,
-- 
Fabiano Fidêncio