On Tue, 7 Jun 2016, Frediano Ziglio wrote:
[...]
> > if (rect_contains(&red_drawable->bbox, other_dest)) {
> > + SpiceRect* candidate_src;
> > int candidate_area = rect_get_area(&red_drawable->bbox);
> > int other_area = rect_get_area(other_dest);
> > /* do not stream drawables that are significantly
> > @@ -265,7 +266,10 @@ static int is_next_stream_frame(DisplayChannel *display,
> > return STREAM_FRAME_NONE;
> > }
> >
> > - if (candidate_area > other_area) {
> > + candidate_src = &red_drawable->u.copy.src_area;
> > + if (candidate_area > other_area ||
> > + candidate_src->right - candidate_src->left !=
> > other_src_width ||
> > + candidate_src->bottom - candidate_src->top !=
> > other_src_height) {
> > is_frame_container = TRUE;
> > }
> > } else {
>
> Not a problem of this patch but this code is confusing.
> It's not clear that other_area is not the area or "other_src",
> perhaps would be better renamed to other_dest_area (I was
> going to suggest to remove the first test on the if as
> a result).
other_area is an area, as in pixel^2. So the '_area' suffix makes sense.
And the 'other_' part makes it clear it corresponds to 'other_dest'.
You can rename it to 'other_dest_area' but I fail to see how it's
clearer. Aren't you going to be confused because it matches the
dest_area field of struct Stream?
--
Francois Gouget <fgouget@xxxxxxxxxxxxxxx>
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel