[Mesa-dev] [PATCH v4] egl/android: support for EGL_KHR_partial_update
Harish Krupo
harish.krupo.kps at intel.com
Fri Jun 9 11:51:57 UTC 2017
On 06/09/2017 12:58 PM, Tapani Pälli wrote:
>
>
> 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.
>
For now, I will just check if the return value is equal to zero (Eric's
comment). I will also leave the TODO comment so that it can be amended
when proper documentation is available.
>> 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