On Tue, 2015-08-18 at 01:41 +0200, Fabiano Fidencio wrote:
>
>
> On Mon, Aug 17, 2015 at 6:08 PM, Pavel Grunt <pgrunt@xxxxxxxxxx> wrote:
> > On Mon, 2015-08-17 at 17:55 +0200, Fabiano Fidêncio wrote:
> > > Coverity says:
> > > Result is not floating-point (UNINTENDED_INTEGER_DIVISION)
> > > interger_division: Dividing integer expressions "preferred->width * 100"
> > > and "zoom", and then converting the integer quotient to type double. Any
> > > remainder, or fractional part of the quotient, is ignored.
> >
> > I think it is better to remove the round(), otherwise you are changing the
> > behavior (which is there since 33614f86db490364339ef69e0eb76f98a4ac8138).
Hey Fabiano,
I just suggested a way how to fix the coverity warning without changing the
behavior.
> I don't see why I am (wrongly) changing the behavior, Pavel.
> Can you give me an example?
you gave the example, sometimes there will be an extra pixel, so I was afraid it
can trigger some extra size allocation events. See
virt_viewer_display_size_allocate() in virt-viewer-display.c and
virt_viewer_display_spice_size_allocate() virt-viewer-display-spice.c and
virt_viewer_session_on_monitor_geometry_changed() in virt-viewer-session.c
virt_viewer_session_on_monitor_geometry_changed() will be called if
virt_viewer_display_get_preferred_size() != GtkAllocation, and the requested
size will be set using virt_viewer_display_get_preferred_monitor_geometry()
ACK if you test it and it is ok (no extra size request). It is also possible
that you are avoiding the extra size request.
>
> Nowadays, using the round or not using the
> round would end up in the same result and I don't think it's the
> expected/correct behavior.
>
> Let's consider: width = 640, NORMAL_ZOOM_LEVEL = 100, zoom = 85.
>
> Current behavior:
> width = round (640*100/85)
> width = round (752)
> width = 752.
>
> Removing the round:
> width = 640*100/85
> width = 752
>
> What I consider the expected behavior:
> width = round (640*100/85)
> width = round (752.94)
> width = 753
>
>
> > Or is the rounding necessary ?
> I do think so.
>
Thanks, it wasn't clear for me that it is about fixing some problem (is there a
"black bar"? - it should be with your example).
Pavel
> >
> > Pavel
> > > ---
> > > src/virt-viewer-display.c | 4 ++--
> > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/src/virt-viewer-display.c b/src/virt-viewer-display.c
> > > index 3efe24c..8431ae4 100644
> > > --- a/src/virt-viewer-display.c
> > > +++ b/src/virt-viewer-display.c
> > > @@ -819,8 +819,8 @@ void
> > > virt_viewer_display_get_preferred_monitor_geometry(VirtViewerDisplay*
> > self,
> > > if (virt_viewer_display_get_zoom(VIRT_VIEWER_DISPLAY(self))) {
> > > guint zoom =
> > > virt_viewer_display_get_zoom_level(VIRT_VIEWER_DISPLAY(self));
> > >
> > > - preferred->width = round(preferred->width * NORMAL_ZOOM_LEVEL /
> > > zoom);
> > > - preferred->height = round(preferred->height * NORMAL_ZOOM_LEVEL /
> > > zoom);
> > > + preferred->width = round(preferred->width * NORMAL_ZOOM_LEVEL /
> > > (double) zoom);
> > > + preferred->height = round(preferred->height * NORMAL_ZOOM_LEVEL /
> > > (double) zoom);
> > > }
> > > }
> > >
> >
>
> Thanks for the review,
> --
> Fabiano Fidêncio
_______________________________________________
virt-tools-list mailing list
virt-tools-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/virt-tools-list
