I'm not sure calling queryImage with an unsupported attribute is legal,
thus I think a small check doesn't hurt.
It'd give
if (offsets) {
offsets[0] = 0;
if (dri2_dpy->image->base.version >= 13) {
EGLint img_offset = 0;
bool ret = dri2_dpy->image->queryImage(dri2_img->dri_image,
__DRI_IMAGE_ATTRIB_OFFSET,
&img_offset);
if (ret)
offsets[0] = img_offset;
}
}
return EGL_TRUE;
or alternatively, if you think not being able to give the offset
indicates an error,
if (offsets) {
offsets[0] = 0;
if (dri2_dpy->image->base.version >= 13) {
EGLint img_offset = 0;
bool ret = dri2_dpy->image->queryImage(dri2_img->dri_image,
__DRI_IMAGE_ATTRIB_OFFSET,
&img_offset);
if (!ret)
return EGL_FALSE;
offsets[0] = img_offset;
}
}
return EGL_TRUE;
Axel Davy
On 09/09/2016 17:34, Weng, Chuanbo wrote:
The comments from Emil can be summarized into the following code:
if (offsets) {
offsets[0] = 0;
EGLint img_offset = 0;
bool ret = dri2_dpy->image->queryImage(dri2_img->dri_image,
__DRI_IMAGE_ATTRIB_OFFSET, &img_offset);
if(ret == true)
offsets[0] = img_offset;
}
return EGL_TRUE;
Emil, is it right?
-----Original Message-----
From: Emil Velikov [mailto:emil.l.veli...@gmail.com]
Sent: Friday, September 9, 2016 8:32 PM
To: Weng, Chuanbo <chuanbo.w...@intel.com>
Cc: ML mesa-dev <mesa-dev@lists.freedesktop.org>; Axel Davy <axel.d...@ens.fr>
Subject: Re: [PATCH v2 2/3] egl: return corresponding offset of EGLImage
instead of 0.
On 9 September 2016 at 08:58, Chuanbo Weng <chuanbo.w...@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.w...@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
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
