[Mesa-dev] [PATCH v4] egl/android: support for EGL_KHR_partial_update

Tapani Pälli tapani.palli at intel.com
Fri Jun 9 07:28:38 UTC 2017



On 06/08/2017 04:53 PM, Emil Velikov wrote:
> Hi all,
> 
> On 8 June 2017 at 13:06, Harish Krupo <harish.krupo.kps at intel.com> wrote:
>> Hi Tapani,
>>
>>
>> On 06/08/2017 04:32 PM, Tapani Pälli wrote:
>>>
>>> Harish, please find small addition below.
>>>
>>> I noticed we still have one crasher in tests when this extension is
>>> enabled, it's related to buffer age so I've sent a fix separately here:
>>>
>>> https://lists.freedesktop.org/archives/mesa-dev/2017-June/158391.html
>>>
>>> On 06/07/2017 07:11 PM, Harish Krupo wrote:
>>>>
>>>> This patch adds support for the EGL_KHR_partial_update extension for
>>>> android platform. It passes 36/37 tests in dEQP for
>>>> EGL_KHR_partial_update.
>>>> 1 test not supported.
>>>>
>>>> v2: add fallback for eglSetDamageRegionKHR (Tapani)
>>>>
>>>> v3: The native_window_set_surface_damage call is available only from
>>>>       Android version 6.0. Reintroduce the ANDROID_VERSION guard and
>>>>       advertise extension only if version is >= 6.0. (Emil Velikov)
>>>>
>>>> v4: use newly introduced ANDROID_API_LEVEL guard rather than
>>>>       ANDROID_VERSION guard to advertise the extension.The extension
>>>>       is advertised only if ANDROID_API_LEVEL >= 23 (Android 6.0 or
>>>>       greater). Add fallback function for platforms other than Android.
>>>>       Fix possible math overflow. (Emil Velikov)
>>>>       Return immediately when n_rects is 0. Place function's entrypoint
>>>>       in alphabetical order. (Eric Engestrom)
>>>>
>>>> Signed-off-by: Harish Krupo <harish.krupo.kps at intel.com>
>>>> ---
>>>>    src/egl/drivers/dri2/egl_dri2.c             |  9 ++++
>>>>    src/egl/drivers/dri2/egl_dri2.h             |  4 ++
>>>>    src/egl/drivers/dri2/egl_dri2_fallbacks.h   |  8 +++
>>>>    src/egl/drivers/dri2/platform_android.c     | 41 ++++++++++++++
>>>>    src/egl/drivers/dri2/platform_drm.c         |  1 +
>>>>    src/egl/drivers/dri2/platform_surfaceless.c |  1 +
>>>>    src/egl/drivers/dri2/platform_wayland.c     |  1 +
>>>>    src/egl/drivers/dri2/platform_x11.c         |  2 +
>>>>    src/egl/drivers/dri2/platform_x11_dri3.c    |  1 +
>>>>    src/egl/main/eglapi.c                       | 83
>>>> +++++++++++++++++++++++++++++
>>>>    src/egl/main/eglapi.h                       |  2 +
>>>>    src/egl/main/egldisplay.h                   |  1 +
>>>>    src/egl/main/eglentrypoint.h                |  1 +
>>>>    src/egl/main/eglfallbacks.c                 |  1 +
>>>>    src/egl/main/eglsurface.c                   |  9 ++++
>>>>    src/egl/main/eglsurface.h                   | 12 +++++
>>>>    16 files changed, 177 insertions(+)
>>>>
>>>> diff --git a/src/egl/drivers/dri2/egl_dri2.c
>>>> b/src/egl/drivers/dri2/egl_dri2.c
>>>> index d31a0bf8e0..a1d72166df 100644
>>>> --- a/src/egl/drivers/dri2/egl_dri2.c
>>>> +++ b/src/egl/drivers/dri2/egl_dri2.c
>>>> @@ -1512,6 +1512,14 @@ dri2_swap_buffers_region(_EGLDriver *drv,
>>>> _EGLDisplay *dpy, _EGLSurface *surf,
>>>>    }
>>>>    static EGLBoolean
>>>> +dri2_set_damage_region(_EGLDriver *drv, _EGLDisplay *dpy, _EGLSurface
>>>> *surf,
>>>> +                       EGLint *rects, EGLint n_rects)
>>>> +{
>>>> +   struct dri2_egl_display *dri2_dpy = dri2_egl_display(dpy);
>>>> +   return dri2_dpy->vtbl->set_damage_region(drv, dpy, surf, rects,
>>>> n_rects);
>>>> +}
>>>> +
>>>> +static EGLBoolean
>>>>    dri2_post_sub_buffer(_EGLDriver *drv, _EGLDisplay *dpy, _EGLSurface
>>>> *surf,
>>>>                         EGLint x, EGLint y, EGLint width, EGLint height)
>>>>    {
>>>> @@ -3141,6 +3149,7 @@ _eglBuiltInDriverDRI2(const char *args)
>>>>       dri2_drv->base.API.SwapBuffers = dri2_swap_buffers;
>>>>       dri2_drv->base.API.SwapBuffersWithDamageEXT =
>>>> dri2_swap_buffers_with_damage;
>>>>       dri2_drv->base.API.SwapBuffersRegionNOK = dri2_swap_buffers_region;
>>>> +   dri2_drv->base.API.SetDamageRegion = dri2_set_damage_region;
>>>>       dri2_drv->base.API.PostSubBufferNV = dri2_post_sub_buffer;
>>>>       dri2_drv->base.API.CopyBuffers = dri2_copy_buffers,
>>>>       dri2_drv->base.API.QueryBufferAge = dri2_query_buffer_age;
>>>> diff --git a/src/egl/drivers/dri2/egl_dri2.h
>>>> b/src/egl/drivers/dri2/egl_dri2.h
>>>> index 449016093a..ba7a7be57b 100644
>>>> --- a/src/egl/drivers/dri2/egl_dri2.h
>>>> +++ b/src/egl/drivers/dri2/egl_dri2.h
>>>> @@ -118,6 +118,10 @@ struct dri2_egl_display_vtbl {
>>>>                                              _EGLSurface *surface,
>>>>                                              const EGLint *rects, EGLint
>>>> n_rects);
>>>> +   EGLBoolean (*set_damage_region)(_EGLDriver *drv, _EGLDisplay *dpy,
>>>> +                                   _EGLSurface *surface,
>>>> +                                   const EGLint *rects, EGLint n_rects);
>>>> +
>>>>       EGLBoolean (*swap_buffers_region)(_EGLDriver *drv, _EGLDisplay *dpy,
>>>>                                         _EGLSurface *surf, EGLint
>>>> numRects,
>>>>                                         const EGLint *rects);
>>>> diff --git a/src/egl/drivers/dri2/egl_dri2_fallbacks.h
>>>> b/src/egl/drivers/dri2/egl_dri2_fallbacks.h
>>>> index 67a9c5034a..d8363c9bdd 100644
>>>> --- a/src/egl/drivers/dri2/egl_dri2_fallbacks.h
>>>> +++ b/src/egl/drivers/dri2/egl_dri2_fallbacks.h
>>>> @@ -95,6 +95,14 @@ dri2_fallback_copy_buffers(_EGLDriver *drv,
>>>> _EGLDisplay *dpy,
>>>>       return EGL_FALSE;
>>>>    }
>>>> +static inline EGLBoolean
>>>> +dri2_fallback_set_damage_region(_EGLDriver *drv, _EGLDisplay *dpy,
>>>> +                                _EGLSurface *surf,
>>>> +                                const EGLint *rects, EGLint n_rects)
>>>> +{
>>>> +   return EGL_FALSE;
>>>> +}
>>>> +
>>>>    static inline EGLint
>>>>    dri2_fallback_query_buffer_age(_EGLDriver *drv, _EGLDisplay *dpy,
>>>>                                   _EGLSurface *surf)
>>>> diff --git a/src/egl/drivers/dri2/platform_android.c
>>>> b/src/egl/drivers/dri2/platform_android.c
>>>> index 055dc0f996..303e19052f 100644
>>>> --- a/src/egl/drivers/dri2/platform_android.c
>>>> +++ b/src/egl/drivers/dri2/platform_android.c
>>>> @@ -656,6 +656,39 @@ droid_swap_buffers(_EGLDriver *drv, _EGLDisplay
>>>> *disp, _EGLSurface *draw)
>>>>       return EGL_TRUE;
>>>>    }
>>>> +static EGLBoolean
>>>> +droid_set_damage_region(_EGLDriver *drv,
>>>> +                        _EGLDisplay *disp,
>>>> +                        _EGLSurface *draw, const EGLint* rects, EGLint
>>>> n_rects)
>>>> +{
>>>> +   struct dri2_egl_display *dri2_dpy = dri2_egl_display(disp);
>>>> +   struct dri2_egl_surface *dri2_surf = dri2_egl_surface(draw);
>>>> +   android_native_rect_t* droid_rects = NULL;
>>>> +
>>>> +   if (n_rects == 0)
>>>> +      return EGL_TRUE;
>>>> +
>>>> +   droid_rects = calloc(n_rects, sizeof(android_native_rect_t));
> In the very unlikely case that this fails, we might want to throw a
> EGL_BAD_ALLOC and return EGL_FALSE.
> The error is not explicitly documented, but I'm out of ideas how to
> handle otherwise. Returning EGL_TRUE is an option, but does not sound
> that appealing.
> 
>>>> +   EGLint num_drects;
>>>> +
>>>> +   for (num_drects = 0; num_drects < n_rects; num_drects++) {
>>>> +      EGLint i = num_drects * 4;
>>>> +      droid_rects[num_drects].left = rects[i];
>>>> +      droid_rects[num_drects].bottom = rects[i + 1];
>>>> +      droid_rects[num_drects].right = rects[i] + rects[i + 2];
>>>> +      droid_rects[num_drects].top = rects[i + 1] + rects[i + 3];
>>>> +   }
>>>> +
>>>> +   /*
>>>> +    * XXX/TODO: Need to check the return value of this function.
>>>> +    */
>>>
>>>
>>> Why not just do it now then, return EGL_FALSE if return value != 0, I
>>> don't think anything else is required here.
>>>
>>
>>
>> Wasn't sure about the return values of the perform method. Window.h
>> documents only -ENOENT. Sure, I will make this change.
>> I will send the patch immediately after the fix for buffer age is merged.
>>
> FWIW I suggested keeping this as TODO, since it's not obvious if the
> function returns 0, 1 or other on success.
> There was an idea to prod the Android people and updating this as
> answers/documentation appears.
> Don't have a strong preference either way - as long as you guys are
> happy with the route taken.

OK I see, then TODO is just fine, Harish can decide which route to take 
:) I noticed also that Android framework itself has a call to this that 
does not check for the return value.

> That aside: I think droid_set_damage_region() should be wrapped in #if
> ANDROID_API_LEVEL >= 23/#endif, otherwise it will fail to build.
> 
> With the build/calloc bits sorted
> Reviewed-by: Emil Velikov <emil.velikov at collabora.com>
> 
> -Emil
> 


More information about the mesa-dev mailing list