On Mon, 30 Nov 2015 13:33:16 -0600
Derek Foreman <[email protected]> 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 <[email protected]>
> ---
>  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 <[email protected]>


Thanks,
pq

Attachment: pgpRtskYpU9to.pgp
Description: OpenPGP digital signature

_______________________________________________
wayland-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Reply via email to