[Mesa-dev] [PATCH v2 7/8] egl: Add EGL_WAYLAND_PLANE_WL attribute

[Kristian Høgsberg]

On Wed, Jul 11, 2012 at 5:27 AM, Gwenole Beauchesne <gb.devel at gmail.com> wrote:
> Hi,
>
> 2012/7/9 Kristian Høgsberg <krh at bitplanet.net>:
>
>> +   switch (buffer->format) {
>> +   case WL_DRM_FORMAT_ARGB8888:
>> +   case WL_DRM_FORMAT_XRGB8888:
>> +      if (plane > 0) {
>> +         _eglError(EGL_BAD_PARAMETER, "dri2_create_image_wayland_wl_buffer");
>> +         return NULL;
>> +      }
>> +      width = buffer->buffer.width;
>> +      height = buffer->buffer.height;
>> +      format = buffer->driver_format;
>> +      offset = buffer->offset[0];
>> +      stride = buffer->stride[0];
>> +      cpp = 4;
>
> For all of those, I'd suggest a map table or the use of some format
> descriptor. e.g.
>
> struct dri_image_descriptor {
>     unsigned int        num_planes; /* Max number of planes */
>     struct {
>         unsigned int    format;     /* __DRI_IMAGE_FORMAT_xxx */
>         unsigned char   cpp;        /* Number of components per pixel */
>         unsigned char   w_shift;    /* Plane width = Image width >> w_shift */
>         unsigned char   h_shift;    /* Plane height = Image height >> h_shift */
>     }                   planes[3];
> };

True, that's more concise.  I implemented this, but I also needed an
'index' field in the plane struct.  For YUYV, we map the same packed
buffer to two different __DRIimages, so we need to be able to override
what buffer->stride[n] and buffer->offset[n] we use to create the
__DRIimage.  Other than that it works well.

Kristian

> static const struct dri_image_descriptor dri_image_argb8888_desc = {
>     1, { { __DRI_IMAGE_FORMAT_ARGB8888, 4, 0, 0 }, } };
> static const struct dri_image_descriptor dri_image_xrgb8888_desc = {
>     1, { { __DRI_IMAGE_FORMAT_XRGB8888, 4, 0, 0 }, } };
> static const struct dri_image_descriptor dri_image_nv12_desc = {
>     2, { { __DRI_IMAGE_FORMAT_R8,      1, 0, 0 },
>          { __DRI_IMAGE_FORMAT_GR88,    2, 1, 1 }, } };
> [...]
>
> That way, we could also re-use the same descriptors for other
> non-Wayland buffers.
>
> switch (buffer->format) {
> case WL_DRM_FORMAT_ARGB8888:
>     m = &image_argb8888_desc;
>     break;
> }
>
> and thus reducing to
>
> if (plane >= m->num_planes) {
>     _eglError(EGL_BAD_PARAMETER, "dri2_create_image_wayland_wl_buffer");
>     return NULL;
> }
> width = buffer->buffer.width >> m->planes[plane].w_shift;
> height = buffer->buffer.height >> m->planes[plane].h_shift;
> cpp = m->planes[plane].cpp;
>
> BTW, could you please also add YUV 4:2:2 and 4:4:4 back? That's useful
> to JPEG decoded surfaces. :)
>
> Thanks,
> Gwenole.