[Mesa-dev] [PATCH] intel: implement create image from texture (v8)

[Chad Versace]

On 01/31/2013 09:35 AM, Abdiel Janulgue wrote:
> Save miptree level info to DRIImage:
> - Appropriately-aligned base offset pointing to the image
> - Additional x/y adjustment offsets from above.
> 
> In non-tile-aligned surface cases where resolving back to the original image
> located in mip-levels higher than the base level proves problematic due to
> offset alignment issues, report INVALID_OPERATION as per spec wording.
> 
> v8: Bump intelImageExtension version 
> 
> Reviewed-by: Eric Anholt <eric at anholt.net> (v6)
> Signed-off-by: Abdiel Janulgue <abdiel.janulgue at linux.intel.com>
> ---
>  src/mesa/drivers/dri/intel/intel_screen.c |  181 +++++++++++++++++++++++++----
>  1 file changed, 160 insertions(+), 21 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/intel/intel_screen.c b/src/mesa/drivers/dri/intel/intel_screen.c
> index b39bc46..0e11554 100644
> --- a/src/mesa/drivers/dri/intel/intel_screen.c
> +++ b/src/mesa/drivers/dri/intel/intel_screen.c
> @@ -31,11 +31,13 @@


> +#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.

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.

> +   __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?

> +      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.