Re: [virt-tools-list] [PATCH virt-viewer 2/2] display: Move variable definitions to block where are used
On Tue, 2016-02-02 at 09:38 -0200, Eduardo Lima (Etrunko) wrote: > On 01/27/2016 03:03 PM, Pavel Grunt wrote: > > --- > > src/virt-viewer-display.c | 18 +++--- > > 1 file changed, 7 insertions(+), 11 deletions(-) > > > > diff --git a/src/virt-viewer-display.c b/src/virt-viewer-display.c > > index 72ec56a..d1b088e 100644 > > --- a/src/virt-viewer-display.c > > +++ b/src/virt-viewer-display.c > > @@ -501,11 +501,6 @@ virt_viewer_display_size_allocate(GtkWidget > > *widget, > > GtkBin *bin = GTK_BIN(widget); > > VirtViewerDisplay *display = VIRT_VIEWER_DISPLAY(widget); > > VirtViewerDisplayPrivate *priv = display->priv; > > -GtkAllocation child_allocation; > > -gint width, height; > > -gint border_width; > > -double desktopAspect; > > -double actualAspect; > > GtkWidget *child = gtk_bin_get_child(bin); > > > > g_debug("Allocated %dx%d", allocation->width, allocation- > > >height); > > @@ -519,14 +514,15 @@ virt_viewer_display_size_allocate(GtkWidget > > *widget, > > return; > > #endif > > > > -desktopAspect = (double)priv->desktopWidth / (double)priv- > > >desktopHeight; > > - > > if (child && gtk_widget_get_visible(child)) { > > -border_width = > > gtk_container_get_border_width(GTK_CONTAINER(display)); > > +GtkAllocation child_allocation; > > +gint border_width = > > gtk_container_get_border_width(GTK_CONTAINER(display)); > > + > > +gint width = MAX(1, allocation->width - 2 * > > border_width); > > +gint height = MAX(1, allocation->height - 2 * > > border_width); > > > > -width = MAX(1, allocation->width - 2 * border_width); > > -height = MAX(1, allocation->height - 2 * border_width); > > -actualAspect = (double)width / (double)height; > > +double desktopAspect = (double) priv->desktopWidth / > > (double) priv->desktopHeight; > > +double actualAspect = (double) width / (double) height; > > > > if (actualAspect > desktopAspect) { > > child_allocation.width = round(height * > > desktopAspect); > > > > > Looks good, but what do you think about removing this huge if block > and > maybe merging this "child" check together with the one on the > beginning > of the function, something like: > > if (priv->desktopWidth == 0 || > priv->desktopHeight == 0 || > !child || !gtk_widget_get_visible(child)) > sure, works for me. I will post updated patch. Pavel > > ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] [PATCH virt-viewer 2/2] display: Move variable definitions to block where are used
On 01/27/2016 03:03 PM, Pavel Grunt wrote: > --- > src/virt-viewer-display.c | 18 +++--- > 1 file changed, 7 insertions(+), 11 deletions(-) > > diff --git a/src/virt-viewer-display.c b/src/virt-viewer-display.c > index 72ec56a..d1b088e 100644 > --- a/src/virt-viewer-display.c > +++ b/src/virt-viewer-display.c > @@ -501,11 +501,6 @@ virt_viewer_display_size_allocate(GtkWidget *widget, > GtkBin *bin = GTK_BIN(widget); > VirtViewerDisplay *display = VIRT_VIEWER_DISPLAY(widget); > VirtViewerDisplayPrivate *priv = display->priv; > -GtkAllocation child_allocation; > -gint width, height; > -gint border_width; > -double desktopAspect; > -double actualAspect; > GtkWidget *child = gtk_bin_get_child(bin); > > g_debug("Allocated %dx%d", allocation->width, allocation->height); > @@ -519,14 +514,15 @@ virt_viewer_display_size_allocate(GtkWidget *widget, > return; > #endif > > -desktopAspect = (double)priv->desktopWidth / (double)priv->desktopHeight; > - > if (child && gtk_widget_get_visible(child)) { > -border_width = > gtk_container_get_border_width(GTK_CONTAINER(display)); > +GtkAllocation child_allocation; > +gint border_width = > gtk_container_get_border_width(GTK_CONTAINER(display)); > + > +gint width = MAX(1, allocation->width - 2 * border_width); > +gint height = MAX(1, allocation->height - 2 * border_width); > > -width = MAX(1, allocation->width - 2 * border_width); > -height = MAX(1, allocation->height - 2 * border_width); > -actualAspect = (double)width / (double)height; > +double desktopAspect = (double) priv->desktopWidth / (double) > priv->desktopHeight; > +double actualAspect = (double) width / (double) height; > > if (actualAspect > desktopAspect) { > child_allocation.width = round(height * desktopAspect); > Looks good, but what do you think about removing this huge if block and maybe merging this "child" check together with the one on the beginning of the function, something like: if (priv->desktopWidth == 0 || priv->desktopHeight == 0 || !child || !gtk_widget_get_visible(child)) -- Eduardo de Barros Lima (Etrunko) Software Engineer - RedHat etru...@redhat.com ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
[virt-tools-list] [PATCH virt-viewer 2/2] display: Move variable definitions to block where are used
--- src/virt-viewer-display.c | 18 +++--- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/src/virt-viewer-display.c b/src/virt-viewer-display.c index 72ec56a..d1b088e 100644 --- a/src/virt-viewer-display.c +++ b/src/virt-viewer-display.c @@ -501,11 +501,6 @@ virt_viewer_display_size_allocate(GtkWidget *widget, GtkBin *bin = GTK_BIN(widget); VirtViewerDisplay *display = VIRT_VIEWER_DISPLAY(widget); VirtViewerDisplayPrivate *priv = display->priv; -GtkAllocation child_allocation; -gint width, height; -gint border_width; -double desktopAspect; -double actualAspect; GtkWidget *child = gtk_bin_get_child(bin); g_debug("Allocated %dx%d", allocation->width, allocation->height); @@ -519,14 +514,15 @@ virt_viewer_display_size_allocate(GtkWidget *widget, return; #endif -desktopAspect = (double)priv->desktopWidth / (double)priv->desktopHeight; - if (child && gtk_widget_get_visible(child)) { -border_width = gtk_container_get_border_width(GTK_CONTAINER(display)); +GtkAllocation child_allocation; +gint border_width = gtk_container_get_border_width(GTK_CONTAINER(display)); + +gint width = MAX(1, allocation->width - 2 * border_width); +gint height = MAX(1, allocation->height - 2 * border_width); -width = MAX(1, allocation->width - 2 * border_width); -height = MAX(1, allocation->height - 2 * border_width); -actualAspect = (double)width / (double)height; +double desktopAspect = (double) priv->desktopWidth / (double) priv->desktopHeight; +double actualAspect = (double) width / (double) height; if (actualAspect > desktopAspect) { child_allocation.width = round(height * desktopAspect); -- 2.5.0 ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list