On Thursday, January 31, 2013 10:31:33 AM Chad Versace wrote:
> > +#include "egl/main/eglcurrent.h"
> >
> >
> > +static __DRIimage *
> > +intel_create_image_from_texture(__DRIcontext *context, int target,
> > + unsigned texture, int zoffset,
> > + int level,
> > + void *loaderPrivate)
> > +{
>
> This function directly calls _eglError(). Nowhere else
> in the i965 driver are direct calls made to GLX or EGL. If,
> in the future, we wish to use this function when a GLXContext
> is current, the behavior would be undefined.
See last comment below.
>
> To fix this, I would add an out parameter `int *dri_error` to
> __DRIimageExtensionRec::createImageFromTexture. Then
> dri2_create_image_khr_texture() would inspect the DRI error
> code and emit the matching EGL error. This is exactly what the DRI interface
> does for DRIdri2ExtensionRec::CreateContext, which is implemented by
> intelCreateContext().
>
> However, dri_interface.h does not define any appropriate error codes for
> EGL_BAD_{ALLOC,PARAMETER,MATCH}. So you would need to define
> new ones. I would name them to match the EGL names, such
> as __DRI_ERROR_BAD_{ALLOC,PARAMETER,MATCH}. Since the set of
> possible error codes is part of a function's ABI, the
> __DRIimageExtensionRec::createImageFromTexture documentation
> should explain what that set is.
Sounds reasonable.
>
> > + __DRIimage *image;
> > + struct intel_context *intel = context->driverPrivate;
> > + struct gl_texture_object *obj;
> > + struct intel_texture_object *iobj;
> > + GLuint face = 0;
> > +
> > + obj = _mesa_lookup_texture(&intel->ctx, texture);
> > + if (!obj || obj->Target != target) {
> > + _eglError(EGL_BAD_PARAMETER, __func__);
> > + return NULL;
> > + }
> > +
> > + if (target == GL_TEXTURE_CUBE_MAP)
> > + face = zoffset;
> > +
> > + _mesa_test_texobj_completeness(&intel->ctx, obj);
> > + iobj = intel_texture_object(obj);
> > + if (!obj->_BaseComplete || (level > 0 && !obj->_MipmapComplete)) {
> > + _eglError(EGL_BAD_PARAMETER, __func__);
> > + return NULL;
> > + }
> > +
> > + if (level < obj->BaseLevel || level > obj->_MaxLevel) {
> > + _eglError(EGL_BAD_MATCH, __func__);
> > + return NULL;
> > + }
> > +
> > + if (target == GL_TEXTURE_3D && obj->Image[face][level]->Depth <
> > zoffset) { + _eglError(EGL_BAD_MATCH, __func__);
> > + return NULL;
> > + }
> > + image = calloc(1, sizeof *image);
> > + if (image == NULL) {
> > + _eglError(EGL_BAD_ALLOC, __func__);
> > + return NULL;
> > + }
> > +
> > + image->internal_format = obj->Image[face][level]->InternalFormat;
> > + image->format = obj->Image[face][level]->TexFormat;
> > + image->data = loaderPrivate;
> > + if (iobj->mt->stencil_mt ||
> > + !intel_setup_image_from_mipmap_tree(intel, image, iobj->mt, level,
> > zoffset)) { + _mesa_error(&intel->ctx, GL_INVALID_OPERATION,
> > __func__);
>
> This function emits a mixture of EGL and GL errors. Is that allowed by the
> extension spec?
Bummer, you are right. Spec says the GL_INVALID_OPERATION should be
implemented on the OES_EGL_Image side of the wall. Anyway, I think I can move
the page alignment surface address checks from the image-to-texture to the
texture-from-image side. The only thing I foresee here is we need another
field in DRIimage to indicate if the EGLimages representing them are
depth/stencil textures.
Well back to the drawing board I guess. But I think this should take a couple
of changes in patches 2 and 8 at least.
>
> > + free(image);
> > + return NULL;
> > + }
> > + image->dri_format = intel_dri_format(image->format);
> > + if (image->dri_format == MESA_FORMAT_NONE) {
> > + fprintf(stderr, "%s: Cannot make EGL image from invalid format.\n",
> > __func__); + free(image);
>
> An error code needs to be emitted here, or the EGL client might get
> confused. Perhaps __DRI_ERROR_BAD_PARAMETER?
>
> > + return NULL;
> >
> > }
>
> Removing the calls to _eglError fixes the Android build.
Yup, earlier it was in the back of my head that calling _eglError to error out
in driver code doesn't seem to sound good at all. But it worked quite well on
normal non-Android build. So i tried poking around Android compiler flags to
try to pacify its complaint. Anyway, I'll implement the *dri_error suggestion
you had above.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
