On Sat, Mar 28, 2015 at 1:24 PM, Pavel Grunt <pgrunt@xxxxxxxxxx> 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 > --- > src/virt-viewer-window.c | 46 ++++++++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 44 insertions(+), 2 deletions(-) > > diff --git a/src/virt-viewer-window.c b/src/virt-viewer-window.c > index d668f74..205a09a 100644 > --- a/src/virt-viewer-window.c > +++ b/src/virt-viewer-window.c > @@ -67,6 +67,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 gboolean virt_viewer_window_can_zoom_out(VirtViewerWindow *self); > > G_DEFINE_TYPE (VirtViewerWindow, virt_viewer_window, G_TYPE_OBJECT) > > @@ -366,14 +368,16 @@ G_MODULE_EXPORT void > virt_viewer_window_menu_view_zoom_out(GtkWidget *menu G_GNUC_UNUSED, > VirtViewerWindow *self) > { > - virt_viewer_window_set_zoom_level(self, self->priv->zoomlevel - 10); > + if (virt_viewer_window_can_zoom_out(self)) > + virt_viewer_window_set_zoom_level(self, self->priv->zoomlevel - 10); > } > I'm not sure if I prefer this treatment being done here or inside _set_zoom_level(), where you can know if you're zooming in or zooming out as well ... OTOH, I can agree with this here and with some other treatment being moved from _set_zoom_level() specifically to _zoom_in/out() functions ... > G_MODULE_EXPORT void > virt_viewer_window_menu_view_zoom_in(GtkWidget *menu G_GNUC_UNUSED, > VirtViewerWindow *self) > { > - virt_viewer_window_set_zoom_level(self, self->priv->zoomlevel + 10); > + if (self->priv->zoomlevel < MAX_ZOOM_LEVEL) > + virt_viewer_window_set_zoom_level(self, self->priv->zoomlevel + 10); This check is already done on set_zoom_level() ... no? if (zoom_level > 400) zoom_level = 400; What, btw, should use MAX_ZOOM_LEVEL instead of 400 ... Actually, I know it is not related, but on set_zoom_level() I'd like to see some treatment as (with a better debug message): if (priv->zoomlevel == zoom_level) { g_debug ("Maximum/Minimum zoom level reached"); return; } > } > > G_MODULE_EXPORT void > @@ -1467,6 +1471,44 @@ 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; > + g_return_if_fail(VIRT_VIEWER_IS_WINDOW(self)); > + g_return_if_fail(width != NULL); > + g_return_if_fail(height != NULL); > + > + 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 > + *height = 50; > + *width = req.width; I would appreciate a comment here explaining that the minimum size is set to 50x50 (an arbitrary small value) but in the end it will be the top menu's width x 50 > +} > + > +static gboolean > +virt_viewer_window_can_zoom_out(VirtViewerWindow *self) > +{ > + double zoom_level; > + guint min_width = 0, min_height = 0, act_width = 0, act_height = 0; > + > + g_return_val_if_fail(VIRT_VIEWER_IS_WINDOW(self), FALSE); > + if (self->priv->zoomlevel <= MIN_ZOOM_LEVEL || self->priv->display == NULL) > + return FALSE; This treatment is already done on _set_zoom_level() ... maybe we really should move the whole treatment to that function ... > + > + virt_viewer_window_get_minimal_dimensions(self, &min_width, &min_height); > + virt_viewer_display_get_desktop_size(virt_viewer_window_get_display(self), &act_width, &act_height); What does act stands for? actual? I'd use just width and height here ... > + zoom_level = (self->priv->zoomlevel - 10) / 100.0; > + > + return zoom_level * act_width >= min_width && zoom_level *act_height >= min_height; > +} > + > /* > * Local variables: > * c-indent-level: 4 > -- > 2.3.4 > > _______________________________________________ > virt-tools-list mailing list > virt-tools-list@xxxxxxxxxx > https://www.redhat.com/mailman/listinfo/virt-tools-list Let me know your thoughts about moving the _can_zoom_out() and the other checks you have added to the _set_zoom_level() function ... Or would it be a problem when starting virt-viewer setting a zoom-level by the command line? Best Regards, -- Fabiano Fidêncio _______________________________________________ virt-tools-list mailing list virt-tools-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/virt-tools-list
