[Mesa-dev] [PATCH 1/9] i965: in set_read_rb_tex_image() check _mesa_meta_bind_rb_as_tex_image() did succeed

[Pohjolainen, Topi]

On Thu, Jul 03, 2014 at 11:13:11AM +0300, Juha-Pekka Heikkila wrote:
> Check if _mesa_meta_bind_rb_as_tex_image() did give the texture.
> If no texture was given there is already either
> GL_INVALID_VALUE or GL_OUT_OF_MEMORY error set in context.
> 
> Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila at gmail.com>
> ---
>  src/mesa/drivers/common/meta_blit.c               | 10 +++++++++-
>  src/mesa/drivers/dri/i965/brw_meta_stencil_blit.c | 14 ++++++++++----
>  2 files changed, 19 insertions(+), 5 deletions(-)
> 
> diff --git a/src/mesa/drivers/common/meta_blit.c b/src/mesa/drivers/common/meta_blit.c
> index bbf0c3c..1fd4d1b 100644
> --- a/src/mesa/drivers/common/meta_blit.c
> +++ b/src/mesa/drivers/common/meta_blit.c
> @@ -615,13 +615,21 @@ _mesa_meta_bind_rb_as_tex_image(struct gl_context *ctx,
>                                  GLenum *target)
>  {
>     struct gl_texture_image *texImage;
> +   GLuint tempTex;
>  
>     if (rb->NumSamples > 1)
>        *target = GL_TEXTURE_2D_MULTISAMPLE;
>     else
>        *target = GL_TEXTURE_2D;
>  
> -   _mesa_GenTextures(1, tex);
> +   tempTex = 0;
> +   _mesa_GenTextures(1, &tempTex);
> +   if (tempTex == 0) {
> +      return false;

I think more often "{}" are dropped for single line blocks. In all the
contexts touched in this patch they are and hence I would drop them for
consistency.

> +   }
> +
> +   *tex = tempTex;
> +
>     _mesa_BindTexture(*target, *tex);
>     *texObj = _mesa_lookup_texture(ctx, *tex);
>     texImage = _mesa_get_tex_image(ctx, *texObj, *target, 0);
> diff --git a/src/mesa/drivers/dri/i965/brw_meta_stencil_blit.c b/src/mesa/drivers/dri/i965/brw_meta_stencil_blit.c
> index bdd642b..11ac92b 100644
> --- a/src/mesa/drivers/dri/i965/brw_meta_stencil_blit.c
> +++ b/src/mesa/drivers/dri/i965/brw_meta_stencil_blit.c
> @@ -371,7 +371,7 @@ prepare_vertex_data(void)
>     _mesa_BufferSubData(GL_ARRAY_BUFFER_ARB, 0, sizeof(verts), verts);
>  }
>  
> -static void
> +static bool
>  set_read_rb_tex_image(struct gl_context *ctx, struct fb_tex_blit_state *blit,
>                        GLenum *target)
>  {
> @@ -387,8 +387,10 @@ set_read_rb_tex_image(struct gl_context *ctx, struct fb_tex_blit_state *blit,
>        *target = tex_obj->Target;
>        level = att->TextureLevel;
>     } else {
> -      _mesa_meta_bind_rb_as_tex_image(ctx, rb, &blit->tempTex, &tex_obj,
> -                                      target);
> +      if (!_mesa_meta_bind_rb_as_tex_image(ctx, rb, &blit->tempTex, &tex_obj,
> +                                          target)) {
> +         return false;
> +      }
>     }
>  
>     blit->baseLevelSave = tex_obj->BaseLevel;
> @@ -396,6 +398,7 @@ set_read_rb_tex_image(struct gl_context *ctx, struct fb_tex_blit_state *blit,
>     blit->stencilSamplingSave = tex_obj->StencilSampling;
>     blit->sampler = _mesa_meta_setup_sampler(ctx, tex_obj, *target,
>                                              GL_NEAREST, level);
> +   return true;
>  }
>  
>  static void
> @@ -424,7 +427,9 @@ brw_meta_stencil_blit(struct brw_context *brw,
>     _mesa_DrawBuffer(GL_COLOR_ATTACHMENT0);
>     ctx->DrawBuffer->_Status = GL_FRAMEBUFFER_COMPLETE;
>  
> -   set_read_rb_tex_image(ctx, &blit, &target);
> +   if (!set_read_rb_tex_image(ctx, &blit, &target)) {
> +      goto error;
> +   }
>  
>     _mesa_TexParameteri(target, GL_DEPTH_STENCIL_TEXTURE_MODE,
>                         GL_STENCIL_INDEX);
> @@ -446,6 +451,7 @@ brw_meta_stencil_blit(struct brw_context *brw,
>     _mesa_DrawArrays(GL_TRIANGLE_FAN, 0, 4);
>  
>     _mesa_meta_fb_tex_blit_end(ctx, target, &blit);
> +error:;

Extra ; crept in, and I would include _mesa_meta_fb_tex_blit_end() in the
error path as _mesa_meta_fb_tex_blit_begin() has been called. Currently
there is no difference as nothing is really altered by the time the error
is detected. But that may change in the future and we might forget to update
the error path.  

With these changes:

Reviewed-by: Topi Pohjolainen <topi.pohjolainen at intel.com>

>     _mesa_meta_end(ctx);
>  
>     _mesa_DeleteRenderbuffers(1, &rbo);
> -- 
> 1.8.1.2
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev