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

[Chad Versace]

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?