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

[Derek Foreman]

On 30/11/15 03:21 PM, Daniel Stone wrote:
> 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?

Rotating a square by 45 degrees will "break" this function.

Since we only have top left and bottom right corner, we can rotate them
so they're both along the same horizontal or vertical line - then the
remaining rectangle has no area at all.

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

Thanks

> Cheers,
> Daniel
>