[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
fabiano at fidencio.org
Tue Mar 31 07:55:06 UTC 2015
On Mon, Mar 30, 2015 at 11:28 PM, Fabiano Fidêncio <fabiano at fidencio.org> wrote:
> 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:
After discussing a bit with Pavel on IRC I saw I am wrong here :-)
We cannot check for priv->display right in the beginning of the
function and early return. It affects the case when set the zoom level
by command line and I just didn't realize that before. My bad here.
>
> 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
--
Fabiano Fidêncio
More information about the virt-tools-list
mailing list