[PATCH weston] compositor: change rounding in weston_surface_to_buffer_rect()

[Daniel Stone]

Hi,

On 30 November 2015 at 19:33, Derek Foreman <derekf at osg.samsung.com> wrote:
> Rounding both corners of the rectangle down can result in a 0
> width/height rectangle before passing to weston_transformed_rect.
>
> This showed up as missing damage in weston-simple-damage (the
> bouncing ball would leave green trails when --use-viewport was
> used)
>
> Also, add a big fat warning for users of the function, since
> some of its operation may not be obvious at a glance.
>
> Signed-off-by: Derek Foreman <derekf at osg.samsung.com>
> ---
>  src/compositor.c | 18 ++++++++++++++++--
>  1 file changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/src/compositor.c b/src/compositor.c
> index 4895bd6..bf59fa8 100644
> --- a/src/compositor.c
> +++ b/src/compositor.c
> @@ -932,6 +932,20 @@ weston_surface_to_buffer(struct weston_surface *surface,
>         *by = floorf(byf);
>  }
>
> +/* Users of weston_surface_to_buffer_rect() need to be
> + * careful - it converts to integer as an intermediate
> + * step, and rounds off at that time - the boundary may
> + * not be exactly as expected.  It works fine when used
> + * for damage tracking since a little extra coverage is
> + * not a problem.
> + *
> + * Also, since the rectangles are specified by 2 corners,
> + * if the input is not axis aligned and the surface to
> + * buffer transform includes a rotation that isn't a
> + * multiple of 90 degrees, the output rectangle won't
> + * have the same area as the input (in fact it could have
> + * none at all)
> + */
>  WL_EXPORT pixman_box32_t
>  weston_surface_to_buffer_rect(struct weston_surface *surface,
>                               pixman_box32_t rect)
> @@ -945,8 +959,8 @@ weston_surface_to_buffer_rect(struct weston_surface *surface,
>         rect.y1 = floorf(yf);
>
>         scaler_surface_to_buffer(surface, rect.x2, rect.y2, &xf, &yf);
> -       rect.x2 = floorf(xf);
> -       rect.y2 = floorf(yf);
> +       rect.x2 = ceilf(xf);
> +       rect.y2 = ceilf(yf);

This seems to make sense, but the comment above is a bit jarring. How
could we go from a non-zero input area to a zeroed output area? I
guess we'd have to be scaling down so hard that the extents
disappeared into the noise?

Reviewed-by: Daniel Stone <daniels at collabora.com>

Cheers,
Daniel