[Mesa-dev] [PATCH v2 2/3] egl: return corresponding offset of EGLImage instead of 0.

[Emil Velikov]

On 9 September 2016 at 08:58, Chuanbo Weng <chuanbo.weng at intel.com> wrote:
> The offset should not always be 0. For example, if EGLImage is
> created from a 2D texture with EGL_GL_TEXTURE_LEVEL=1, then the
> offset should be the actual start of miplevel 1 in bo.
>
> v2: Add version check of __DRIimageExtension implementation
> (Suggested by Axel Davy).
>
Just to elaborate what Axel was saying from another perspective:

With current master the offset is explicitly zeroed, while with the
(v2) patch you'll _only_ set the value when you have v13 driver.
Thus using the loader + v12 driver you'll get a regression since a)
the offset will remain garbage and b) the function
(dri2_export_dma_buf_image_mesa) will fail.


> Signed-off-by: Chuanbo Weng <chuanbo.weng at intel.com>
> ---
>  src/egl/drivers/dri2/egl_dri2.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
> index 859612f..c529e55 100644
> --- a/src/egl/drivers/dri2/egl_dri2.c
> +++ b/src/egl/drivers/dri2/egl_dri2.c
> @@ -2249,6 +2249,7 @@ dri2_export_dma_buf_image_mesa(_EGLDriver *drv, _EGLDisplay *disp, _EGLImage *im
>     struct dri2_egl_image *dri2_img = dri2_egl_image(img);
>
>     (void) drv;
> +   EGLint img_offset = 0;
>
>     /* rework later to provide multiple fds/strides/offsets */
>     if (fds)
> @@ -2259,8 +2260,14 @@ dri2_export_dma_buf_image_mesa(_EGLDriver *drv, _EGLDisplay *disp, _EGLImage *im
>        dri2_dpy->image->queryImage(dri2_img->dri_image,
>                                   __DRI_IMAGE_ATTRIB_STRIDE, strides);
>
> -   if (offsets)
> +   if (offsets){
>        offsets[0] = 0;
> +      if(dri2_dpy->image->base.version >= 13){
Please move the variable declaration in local scope and add space between ){

Functionality wise we don't need the version check, esp. since you
want to set the offset only when queryImage() succeeds...

> +         dri2_dpy->image->queryImage(dri2_img->dri_image,
> +             __DRI_IMAGE_ATTRIB_OFFSET, &img_offset);
... which you don't seem to be checking here, so you'll effectively
store/use garbage.

So unless Alex feels strongly for the version check I'd omit it, and
fix the rest of this patch.

-Emil