On 17 October 2017 at 11:29, Eric Engestrom <eric.engest...@imgtec.com> wrote:
> On Monday, 2017-10-16 16:04:10 +0000, Emil Velikov wrote:
>> From: Emil Velikov <emil.veli...@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.veli...@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?
>
Right, I've assumed that __DRI_IMAGE_FORMAT_NONE is the default case
which doesn't seems to be the case ...
The whole __DRI_IMAGE_FORMAT handling seems to need some work, but the
first one would be to fixup driGLFormatToImageFormat()
Patch coming in v2.
>> + *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);
>
Thought about it, then decided again it... not sure why. Will fix.
>> +
>> 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)`?)
>
The use of _mesa_error() is a bug in the original code. It generates a
GL error, whereas a EGL one is expected.
Thus programs thoroughly checking the error states will be mislead/lied to.
I'll add an explicit note in the commit message about it.
Thanks
Emil
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
