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

[Samuel Pitoiset]

On 05/25/2017 11:51 AM, Timothy Arceri wrote:
> 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?

Not yet, I will make those changes today. :-)

> 
>>
>> 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
>>