Ilia,
Ilia Mirkin <imir...@alum.mit.edu> writes:
> On Sat, Jul 7, 2018 at 11:15 PM, Harish Krupo
> <harish.krupo....@intel.com> wrote:
>> Ilia,
>>
>> Ilia Mirkin <imir...@alum.mit.edu> writes:
>>
>>> On Sat, Jul 7, 2018 at 5:05 PM, Harish Krupo <harish.krupo....@intel.com>
>>> wrote:
>>>> Clamp the x and y co-ordinates of the rectangles.
>>>>
>>>> Signed-off-by: Harish Krupo <harish.krupo....@intel.com>
>>>> ---
>>>> src/egl/main/eglapi.c | 23 +++++++++--------------
>>>> 1 file changed, 9 insertions(+), 14 deletions(-)
>>>>
>>>> diff --git a/src/egl/main/eglapi.c b/src/egl/main/eglapi.c
>>>> index c110349119..c7d07c4fde 100644
>>>> --- a/src/egl/main/eglapi.c
>>>> +++ b/src/egl/main/eglapi.c
>>>> @@ -1320,9 +1320,7 @@ eglSwapBuffersWithDamageKHR(EGLDisplay dpy,
>>>> EGLSurface surface,
>>>> }
>>>>
>>>> /**
>>>> - * If the width of the passed rect is greater than the surface's
>>>> - * width then it is clamped to the width of the surface. Same with
>>>> - * height.
>>>> + * Clamp the rectangles so that they lie within the surface.
>>>> */
>>>>
>>>> static void
>>>> @@ -1334,17 +1332,14 @@ _eglSetDamageRegionKHRClampRects(_EGLDisplay*
>>>> disp, _EGLSurface* surf,
>>>> EGLint surf_width = surf->Width;
>>>>
>>>> for (i = 0; i < (4 * n_rects); i += 4) {
>>>> - EGLint x, y, rect_width, rect_height;
>>>> - x = rects[i];
>>>> - y = rects[i + 1];
>>>> - rect_width = rects[i + 2];
>>>> - rect_height = rects[i + 3];
>>>> -
>>>> - if (rect_width > surf_width - x)
>>>> - rects[i + 2] = surf_width - x;
>>>> -
>>>> - if (rect_height > surf_height - y)
>>>> - rects[i + 3] = surf_height - y;
>>>> + EGLint x, y;
>>>> + x = CLAMP(rects[i], 0, surf_width);
>>>> + y = CLAMP(rects[i + 1], 0, surf_height);
>>>> +
>>>> + rects[i] = x;
>>>> + rects[i + 1] = y;
>>>> + rects[i + 2] = CLAMP(rects[i + 2], x, surf_width);
>>>> + rects[i + 3] = CLAMP(rects[i + 3], y, surf_height);
>>>
>>> I believe the previous code for clamping the width/height was correct.
>>> Note that this might end up with 0-width/height rects, hopefully the
>>> downstream code can handle it.
>>>
>>
>> Thanks for the review.
>> The spec says:
>> "Rectangles that lie (partially) outside of the current surface
>> dimensions (as queryable via the EGL_WIDTH and EGL_HEIGHT
>> attributes) will be clamped to the current surface dimensions."
>>
>> I believe this also includes regions < 0?
>> Also, I observed that the rects passed by deqp also include negative
>> values. Even in the previous method we could end up with 0 width-height.
>> Example: {surf_width, surf_height, X, X}, for any value of X we would
>> have clamped it to 0-width/height rect.
>> Yes, the downstream should handle these 0 area cases.
>
> Right, so that all has to be handled cleanly. The value in rects is:
>
> <rects> is a pointer to a list of values describing the rectangles. The
> list
> should consist of <n_rects> groups of four values, with each group
> representing a single rectangle in surface coordinates in the form {x, y,
> width, height}.
>
> So it's unclear why the lower bound on [2] and [3] are x/y in your
> version. If x is -1 and width is 2, what should the outcome be? x = 0
> and width = 1, I'd guess?
>
Okay, I see the problem. Converting the width/height to a co-ordinate
and then clamping the value in [0, surf_width/surf_height] and then
converting it back to width/height should do it.
Will send a v2 with the change.
Thank you
Regards
Harish Krupo
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
