[Mesa-dev] [PATCH] egl: Fix missing clamping in eglSetDamageRegionKHR
Harish Krupo
harish.krupo.kps at intel.com
Sun Jul 8 03:15:36 UTC 2018
Ilia,
Ilia Mirkin <imirkin at alum.mit.edu> writes:
> On Sat, Jul 7, 2018 at 5:05 PM, Harish Krupo <harish.krupo.kps at intel.com> wrote:
>> Clamp the x and y co-ordinates of the rectangles.
>>
>> Signed-off-by: Harish Krupo <harish.krupo.kps at 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.
Thank you
Regards
Harish Krupo
More information about the mesa-dev
mailing list