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

[Samuel Pitoiset]

On 05/25/2017 06:41 AM, Timothy Arceri wrote:
> 
> 
> 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.

I do agree, it's not perfect but it looks better.

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