[Mesa-dev] [PATCH] egl/android: Fix error condition for EGL_ANDROID_image_native_buffer

[Chia-I Wu]

On Wed, May 8, 2013 at 12:15 AM, Chad Versace
<chad.versace at linux.intel.com> wrote:
> On 05/07/2013 01:19 AM, Chia-I Wu wrote:
>>
>> On Tue, May 7, 2013 at 3:49 PM, Pohjolainen, Topi
>> <topi.pohjolainen at intel.com> wrote:
>>>
>>> On Mon, May 06, 2013 at 02:23:52PM -0700, Chad Versace wrote:
>>>>
>>>> Emit EGL_BAD_CONTEXT if the user passes a context to
>>>> eglCreateImageKHR(type=EGL_ANDROID_image_native_buffer).
>>>>
>>>>  From the EGL_ANDROID_image_native_buffer spec:
>>>>    * If <target> is EGL_NATIVE_BUFFER_ANDROID and <ctx> is not
>>>>      EGL_NO_CONTEXT, the error EGL_BAD_CONTEXT is generated.
>>>>
>>>> Note: This is a candidate for the stable branches.
>>>> CC: Tapani Pälli <tapani.palli at intel.com>
>>>> Signed-off-by: Chad Versace <chad.versace at linux.intel.com>
>>>> ---
>>>>   src/egl/drivers/dri2/platform_android.c | 16 ++++++++++++++--
>>>>   1 file changed, 14 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/src/egl/drivers/dri2/platform_android.c
>>>> b/src/egl/drivers/dri2/platform_android.c
>>>> index cee4035..ed50907 100644
>>>> --- a/src/egl/drivers/dri2/platform_android.c
>>>> +++ b/src/egl/drivers/dri2/platform_android.c
>>>> @@ -337,7 +337,7 @@ droid_swap_buffers(_EGLDriver *drv, _EGLDisplay
>>>> *disp, _EGLSurface *draw)
>>>>   }
>>>>
>>>>   static _EGLImage *
>>>> -dri2_create_image_android_native_buffer(_EGLDisplay *disp,
>>>> +dri2_create_image_android_native_buffer(_EGLDisplay *disp, _EGLContext
>>>> *ctx,
>>>>                                           struct ANativeWindowBuffer
>>>> *buf)
>>>>   {
>>>>      struct dri2_egl_display *dri2_dpy = dri2_egl_display(disp);
>>>> @@ -346,6 +346,18 @@ dri2_create_image_android_native_buffer(_EGLDisplay
>>>> *disp,
>>>>      uint32_t offsets[3], strides[3], handles[3], tmp;
>>>>      EGLint format;
>>>>
>>>> +   if (ctx != NULL) {
>>>
>>>
>>> I did a similar check for the 'EGL_LINUX_DMA_BUF_EXT'. Technically
>>> 'eglapi.c::eglCreateImageKhr()' does a lookup of the context via
>>> '_eglLookupContext()' and does a "translation" of 'EGL_NO_CONTEXT' also
>>> (from
>>> NULL to NULL). Hence I chose to do the check there. But would it be
>>> better for
>>> me to do it also in the driver side as the target is valid only for linux
>>> platforms anyway?
>>
>> I will suggest do the check in the driver for the moment.  eglapi.[ch]
>> should be generated ultimately, IMHO, and adding extension-specific
>> there will make the switch harder.
>
>
> Topi, I chose to place the check in platform_android.c because this is a
> platform-specific extension. There's no sense in wasting cycles in
> platform-common
> code for a platform-specific check.
>
> For dma_buf_import, I think you put the check in the right place. That
> extension is
> not confined to any of Wayland, X11, nor Android, so it makes sense to put
> the check
> in the common code to reduce code duplication.
>
> Chia-I, do you have any concrete plans to generate eglapi.c? What would that
> generated file look like?
No, I don't yet.  The generated file probably still needs to perform
display locking and EGL resource lookup.  But parameter checks would
need to be moved elsewhere.

--
olv at LunarG.com