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

[Timothy Arceri]

On 25/05/17 19:19, Samuel Pitoiset wrote:
> v2: - add bind_image_textures() helper

Thanks, looks good.

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

Are there also piglit updates to go with the no error mesa patches you 
have sent so far?

> 
> Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
> ---
>   src/mapi/glapi/gen/ARB_multi_bind.xml |  2 +-
>   src/mesa/main/shaderimage.c           | 78 +++++++++++++++++++++--------------
>   src/mesa/main/shaderimage.h           |  4 ++
>   3 files changed, 52 insertions(+), 32 deletions(-)
> 
> diff --git a/src/mapi/glapi/gen/ARB_multi_bind.xml b/src/mapi/glapi/gen/ARB_multi_bind.xml
> index f42eaa28e9..d58c2708cb 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 494125346b..5e31d4e822 100644
> --- a/src/mesa/main/shaderimage.c
> +++ b/src/mesa/main/shaderimage.c
> @@ -658,31 +658,12 @@ _mesa_BindImageTexture(GLuint unit, GLuint texture, GLint level,
>      bind_image_texture(ctx, texObj, unit, level, layered, layer, access, format);
>   }
>   
> -void GLAPIENTRY
> -_mesa_BindImageTextures(GLuint first, GLsizei count, const GLuint *textures)
> +static ALWAYS_INLINE void
> +bind_image_textures(struct gl_context *ctx, GLuint first, GLuint count,
> +                    const GLuint *textures, bool no_error)
>   {
> -   GET_CURRENT_CONTEXT(ctx);
>      int i;
>   
> -   if (!ctx->Extensions.ARB_shader_image_load_store) {
> -      _mesa_error(ctx, GL_INVALID_OPERATION, "glBindImageTextures()");
> -      return;
> -   }
> -
> -   if (first + count > ctx->Const.MaxImageUnits) {
> -      /* The ARB_multi_bind spec says:
> -       *
> -       *    "An INVALID_OPERATION error is generated if <first> + <count>
> -       *     is greater than the number of image units supported by
> -       *     the implementation."
> -       */
> -      _mesa_error(ctx, GL_INVALID_OPERATION,
> -                  "glBindImageTextures(first=%u + count=%d > the value of "
> -                  "GL_MAX_IMAGE_UNITS=%u)",
> -                  first, count, ctx->Const.MaxImageUnits);
> -      return;
> -   }
> -
>      /* Assume that at least one binding will be changed */
>      FLUSH_VERTICES(ctx, 0);
>      ctx->NewDriverState |= ctx->DriverFlags.NewImageUnits;
> @@ -712,13 +693,13 @@ _mesa_BindImageTextures(GLuint first, GLsizei count, const GLuint *textures)
>         struct gl_image_unit *u = &ctx->ImageUnits[first + i];
>         const GLuint texture = textures ? textures[i] : 0;
>   
> -      if (texture != 0) {
> -         struct gl_texture_object *texObj;
> +      if (texture) {
> +         struct gl_texture_object *texObj = u->TexObj;
>            GLenum tex_format;
>   
> -         if (!u->TexObj || u->TexObj->Name != texture) {
> +         if (!texObj || texObj->Name != texture) {
>               texObj = _mesa_lookup_texture_locked(ctx, texture);
> -            if (!texObj) {
> +            if (!no_error && !texObj) {
>                  /* The ARB_multi_bind spec says:
>                   *
>                   *    "An INVALID_OPERATION error is generated if any value
> @@ -731,8 +712,6 @@ _mesa_BindImageTextures(GLuint first, GLsizei count, const GLuint *textures)
>                              "object)", i, texture);
>                  continue;
>               }
> -         } else {
> -            texObj = u->TexObj;
>            }
>   
>            if (texObj->Target == GL_TEXTURE_BUFFER) {
> @@ -740,8 +719,8 @@ _mesa_BindImageTextures(GLuint first, GLsizei count, const GLuint *textures)
>            } else {
>               struct gl_texture_image *image = texObj->Image[0][0];
>   
> -            if (!image || image->Width == 0 || image->Height == 0 ||
> -                image->Depth == 0) {
> +            if (!no_error && (!image || image->Width == 0 ||
> +                              image->Height == 0 || image->Depth == 0)) {
>                  /* The ARB_multi_bind spec says:
>                   *
>                   *    "An INVALID_OPERATION error is generated if the width,
> @@ -758,7 +737,8 @@ _mesa_BindImageTextures(GLuint first, GLsizei count, const GLuint *textures)
>               tex_format = image->InternalFormat;
>            }
>   
> -         if (!_mesa_is_shader_image_format_supported(ctx, tex_format)) {
> +         if (!no_error &&
> +             !_mesa_is_shader_image_format_supported(ctx, tex_format)) {
>               /* The ARB_multi_bind spec says:
>                *
>                *   "An INVALID_OPERATION error is generated if the internal
> @@ -786,3 +766,39 @@ _mesa_BindImageTextures(GLuint first, GLsizei count, const GLuint *textures)
>   
>      _mesa_HashUnlockMutex(ctx->Shared->TexObjects);
>   }
> +
> +void GLAPIENTRY
> +_mesa_BindImageTextures_no_error(GLuint first, GLsizei count,
> +                                 const GLuint *textures)
> +{
> +   GET_CURRENT_CONTEXT(ctx);
> +
> +   bind_image_textures(ctx, first, count, textures, true);
> +}
> +
> +void GLAPIENTRY
> +_mesa_BindImageTextures(GLuint first, GLsizei count, const GLuint *textures)
> +{
> +   GET_CURRENT_CONTEXT(ctx);
> +
> +   if (!ctx->Extensions.ARB_shader_image_load_store) {
> +      _mesa_error(ctx, GL_INVALID_OPERATION, "glBindImageTextures()");
> +      return;
> +   }
> +
> +   if (first + count > ctx->Const.MaxImageUnits) {
> +      /* The ARB_multi_bind spec says:
> +       *
> +       *    "An INVALID_OPERATION error is generated if <first> + <count>
> +       *     is greater than the number of image units supported by
> +       *     the implementation."
> +       */
> +      _mesa_error(ctx, GL_INVALID_OPERATION,
> +                  "glBindImageTextures(first=%u + count=%d > the value of "
> +                  "GL_MAX_IMAGE_UNITS=%u)",
> +                  first, count, ctx->Const.MaxImageUnits);
> +      return;
> +   }
> +
> +   bind_image_textures(ctx, first, count, textures, false);
> +}
> diff --git a/src/mesa/main/shaderimage.h b/src/mesa/main/shaderimage.h
> index b2b22bbf86..6a9e3d67e9 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
>