[Mesa-dev] [PATCH 08/10] i965: implement DRIImage::createImageFromRenderbuffer2

[Eric Engestrom]

On Monday, 2017-10-16 16:04:10 +0000, Emil Velikov wrote:
> From: Emil Velikov <emil.velikov at collabora.com>
> 
> The new entry point has a way to feedback the error. Thus we no longer
> need to call _mesa_error() but instead we can pass the correct value.
> 
> Signed-off-by: Emil Velikov <emil.velikov at collabora.com>
> ---
>  src/mesa/drivers/dri/i965/intel_screen.c | 31 ++++++++++++++++++++++++++-----
>  1 file changed, 26 insertions(+), 5 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/intel_screen.c b/src/mesa/drivers/dri/i965/intel_screen.c
> index ea04a72e860..7cbb5e3b060 100644
> --- a/src/mesa/drivers/dri/i965/intel_screen.c
> +++ b/src/mesa/drivers/dri/i965/intel_screen.c
> @@ -474,8 +474,9 @@ intel_create_image_from_name(__DRIscreen *dri_screen,
>  }
>  
>  static __DRIimage *
> -intel_create_image_from_renderbuffer(__DRIcontext *context,
> -				     int renderbuffer, void *loaderPrivate)
> +intel_create_image_from_renderbuffer2(__DRIcontext *context,
> +				      int renderbuffer, void *loaderPrivate,
> +				      unsigned *error)
>  {
>     __DRIimage *image;
>     struct brw_context *brw = context->driverPrivate;
> @@ -485,15 +486,17 @@ intel_create_image_from_renderbuffer(__DRIcontext *context,
>  
>     rb = _mesa_lookup_renderbuffer(ctx, renderbuffer);
>     if (!rb) {
> -      _mesa_error(ctx, GL_INVALID_OPERATION, "glRenderbufferExternalMESA");
> +      *error = __DRI_IMAGE_ERROR_BAD_PARAMETER;
>        return NULL;
>     }

[there]

>  
>     irb = intel_renderbuffer(rb);
>     intel_miptree_make_shareable(brw, irb->mt);
>     image = calloc(1, sizeof *image);
> -   if (image == NULL)
> +   if (image == NULL) {
> +      *error = __DRI_IMAGE_ERROR_BAD_ALLOC;
>        return NULL;
> +   }
>  
>     image->internal_format = rb->InternalFormat;
>     image->format = rb->Format;
> @@ -508,12 +511,29 @@ intel_create_image_from_renderbuffer(__DRIcontext *context,
>     image->height = rb->Height;
>     image->pitch = irb->mt->surf.row_pitch;
>     image->dri_format = driGLFormatToImageFormat(image->format);
> +   if (image->dri_format == __DRI_IMAGE_FORMAT_NONE) {

__DRI_IMAGE_FORMAT_NONE can only come from MESA_FORMAT_NONE; did you
mean `== 0`? Or both?

> +      *error = __DRI_IMAGE_ERROR_BAD_PARAMETER;
> +      brw_bo_unreference(irb->mt->bo);
> +      free(image);
> +      return NULL;
> +   }

You can move this check before the calloc and ref ([there] above), so
that the error path becomes just setting `error` and returning NULL.

You'll want a temp var to avoid calling driGLFormatToImageFormat()
twice:
	uint32_t dri_format = driGLFormatToImageFormat(rb->Format);

> +
>     image->has_depthstencil = irb->mt->stencil_mt? true : false;
>  
>     rb->NeedsFinishRenderTexture = true;
> +   *error = __DRI_IMAGE_ERROR_SUCCESS;
>     return image;
>  }
>  
> +static __DRIimage *
> +intel_create_image_from_renderbuffer(__DRIcontext *context,
> +				     int renderbuffer, void *loaderPrivate)
> +{
> +   unsigned error;
> +   return intel_create_image_from_renderbuffer2(context, renderbuffer,
> +                                                loaderPrivate, &error);

	if (error == __DRI_IMAGE_ERROR_BAD_PARAMETER)
		_mesa_error(ctx, GL_INVALID_OPERATION, "glRenderbufferExternalMESA");

(maybe even `if (error != __DRI_IMAGE_ERROR_SUCCESS)`?)

> +}
> +
>  static __DRIimage *
>  intel_create_image_from_texture(__DRIcontext *context, int target,
>                                  unsigned texture, int zoffset,
> @@ -1289,7 +1309,7 @@ intel_from_planar(__DRIimage *parent, int plane, void *loaderPrivate)
>  }
>  
>  static const __DRIimageExtension intelImageExtension = {
> -    .base = { __DRI_IMAGE, 16 },
> +    .base = { __DRI_IMAGE, 17 },
>  
>      .createImageFromName                = intel_create_image_from_name,
>      .createImageFromRenderbuffer        = intel_create_image_from_renderbuffer,
> @@ -1312,6 +1332,7 @@ static const __DRIimageExtension intelImageExtension = {
>      .queryDmaBufFormats                 = intel_query_dma_buf_formats,
>      .queryDmaBufModifiers               = intel_query_dma_buf_modifiers,
>      .queryDmaBufFormatModifierAttribs   = intel_query_format_modifier_attribs,
> +    .createImageFromRenderbuffer2       = intel_create_image_from_renderbuffer2,
>  };
>  
>  static uint64_t
> -- 
> 2.14.1
>