Re: [virt-tools-list] [PATCH virt-viewer 2/2] display: Move variable definitions to block where are used

2016-02-02 Thread Eduardo Lima (Etrunko) 
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

Re: [virt-tools-list] [PATCH virt-viewer 2/2] display: Move variable definitions to block where are used

2016-02-02 Thread Pavel Grunt 
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

[virt-tools-list] [PATCH virt-viewer 2/2] display: Move variable definitions to block where are used

2016-01-27 Thread Pavel Grunt 
---
 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