On 01/12/15 03:48 AM, Pekka Paalanen wrote: > On Mon, 30 Nov 2015 13:33:16 -0600 > Derek Foreman <der...@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 <der...@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().
Ok - for some reason I thought it was possible to have an arbitrary transform there (not that it makes any sense for surface to buffer) I'll remove that comment, and add doxygen. Thanks, Derek >> + */ >> 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.paala...@collabora.co.uk> > > > Thanks, > pq > > > > _______________________________________________ > wayland-devel mailing list > wayland-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/wayland-devel > _______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel