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

[Pekka Paalanen]

On Mon, 30 Nov 2015 13:33:16 -0600
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.
> + *

Ok. This could be a proper doxygen doc.

> + * 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)

This path is not using matrices anywhere, is it? So it's actually
impossible to have unexpected transformations.

In your transforms branch, you convert weston_surface_to_buffer_rect()
to use a matrix, but even then this warning isn't necessary, because a)
the matrix is read from weston_surface so it better be legal, and b)
you could check the matrix is constrained enough.

This warning is more applicable to weston_matrix_transform_region().

> + */
>  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);
>  
>  	return weston_transformed_rect(surface->width_from_buffer,
>  				       surface->height_from_buffer,

The code is anyway:

Reviewed-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 811 bytes
Desc: OpenPGP digital signature
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20151201/66a8b407/attachment.sig>