[Mesa-dev] [PATCH 5/5] mesa: add KHR_no_error support for glBindImageTextures()

[Timothy Arceri]

On 24/05/17 07:45, Samuel Pitoiset wrote:
> Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
> ---
>   src/mapi/glapi/gen/ARB_multi_bind.xml |  2 +-
>   src/mesa/main/shaderimage.c           | 43 +++++++++++++++++++++++++++++++++++
>   src/mesa/main/shaderimage.h           |  4 ++++
>   3 files changed, 48 insertions(+), 1 deletion(-)
> 
> diff --git a/src/mapi/glapi/gen/ARB_multi_bind.xml b/src/mapi/glapi/gen/ARB_multi_bind.xml
> index f42eaa28e96..d58c2708cb2 100644
> --- a/src/mapi/glapi/gen/ARB_multi_bind.xml
> +++ b/src/mapi/glapi/gen/ARB_multi_bind.xml
> @@ -35,7 +35,7 @@
>           <param name="samplers" type="const GLuint *"/>
>       </function>
>   
> -    <function name="BindImageTextures">
> +    <function name="BindImageTextures" no_error="true">
>           <param name="first" type="GLuint"/>
>           <param name="count" type="GLsizei"/>
>           <param name="textures" type="const GLuint *"/>
> diff --git a/src/mesa/main/shaderimage.c b/src/mesa/main/shaderimage.c
> index 494125346b4..df2ad5a8195 100644
> --- a/src/mesa/main/shaderimage.c
> +++ b/src/mesa/main/shaderimage.c
> @@ -659,6 +659,49 @@ _mesa_BindImageTexture(GLuint unit, GLuint texture, GLint level,
>   }
>   
>   void GLAPIENTRY
> +_mesa_BindImageTextures_no_error(GLuint first, GLsizei count,
> +                                 const GLuint *textures)
> +{
> +   GET_CURRENT_CONTEXT(ctx);
> +   int i;
> +
> +   FLUSH_VERTICES(ctx, 0);
> +   ctx->NewDriverState |= ctx->DriverFlags.NewImageUnits;
> +
> +   _mesa_HashLockMutex(ctx->Shared->TexObjects);
> +
> +   for (i = 0; i < count; i++) {
> +      struct gl_image_unit *u = &ctx->ImageUnits[first + i];
> +      const GLuint texture = textures ? textures[i] : 0;
> +
> +      if (texture) {
> +         struct gl_texture_object *texObj = u->TexObj;
> +         GLenum tex_format;
> +
> +         if (!texObj || texObj->Name != texture) {
> +            texObj = _mesa_lookup_texture_locked(ctx, texture);
> +         }
> +
> +         if (texObj->Target == GL_TEXTURE_BUFFER) {
> +            tex_format = texObj->BufferObjectFormat;
> +         } else {
> +            struct gl_texture_image *image = texObj->Image[0][0];
> +
> +            tex_format = image->InternalFormat;
> +         }
> +
> +         set_image_binding(u, texObj, 0,
> +                           _mesa_tex_target_is_layered(texObj->Target),
> +                           0, GL_READ_WRITE, tex_format);
> +      } else {
> +         set_image_binding(u, NULL, 0, GL_FALSE, 0, GL_READ_ONLY, GL_R8);
> +      }
> +   }

In order to avoid code duplication I've been creating helpers e.g

static ALWAYS_INLINE void
bind_image_textures(..., bool no_error)

That way we can do a no_error check on error checks and we can avoid 
duplicating all this code. ALWAYS_INLINE will set compiler flags to 
force inlining so the no_error check will get optimised away.

Personally I think it looks a bit messy, but IMO its also better than 
the alternative which is the no_error function getting out of sync with 
the regular function after code updates.

For now patches 1-4 are:

Reviewed-by: Timothy Arceri <tarceri at itsqueeze.com>

> +
> +   _mesa_HashUnlockMutex(ctx->Shared->TexObjects);
> +}
> +
> +void GLAPIENTRY
>   _mesa_BindImageTextures(GLuint first, GLsizei count, const GLuint *textures)
>   {
>      GET_CURRENT_CONTEXT(ctx);
> diff --git a/src/mesa/main/shaderimage.h b/src/mesa/main/shaderimage.h
> index b2b22bbf863..6a9e3d67e92 100644
> --- a/src/mesa/main/shaderimage.h
> +++ b/src/mesa/main/shaderimage.h
> @@ -90,6 +90,10 @@ _mesa_BindImageTexture(GLuint unit, GLuint texture, GLint level,
>                          GLenum format);
>   
>   void GLAPIENTRY
> +_mesa_BindImageTextures_no_error(GLuint first, GLsizei count,
> +                                 const GLuint *textures);
> +
> +void GLAPIENTRY
>   _mesa_BindImageTextures(GLuint first, GLsizei count, const GLuint *textures);
>   
>   #ifdef __cplusplus
>