[Mesa-dev] [PATCH 2/7] mesa: add compressed_tex_sub_image() helper

[Timothy Arceri]

On 16/05/17 00:12, Nicolai Hähnle wrote:
> On 12.05.2017 06:12, Timothy Arceri wrote:
>> This reduces duplication between the dsa and non-dsa function
>> and will also be used in the following commit to add
>> KHR_no_error support.
>> ---
>>  src/mesa/main/teximage.c | 82 
>> ++++++++++++++++++++++--------------------------
>>  1 file changed, 38 insertions(+), 44 deletions(-)
>>
>> diff --git a/src/mesa/main/teximage.c b/src/mesa/main/teximage.c
>> index 2486704..c95e9e2 100644
>> --- a/src/mesa/main/teximage.c
>> +++ b/src/mesa/main/teximage.c
>> @@ -4522,89 +4522,83 @@ compressed_texture_sub_image(struct gl_context 
>> *ctx, GLuint dims,
>>
>>           /* NOTE: Don't signal _NEW_TEXTURE_OBJECT since we've only 
>> changed
>>            * the texel data, not the texture format, size, etc.
>>            */
>>        }
>>     }
>>     _mesa_unlock_texture(ctx, texObj);
>>  }
>>
>>
>> -void GLAPIENTRY
>> -_mesa_CompressedTexSubImage1D(GLenum target, GLint level, GLint xoffset,
>> -                              GLsizei width, GLenum format,
>> -                              GLsizei imageSize, const GLvoid *data)
>> +static ALWAYS_INLINE void
>> +compressed_tex_sub_image(unsigned dim, GLenum target, GLuint texture,
>> +                         GLint level, GLint xoffset, GLsizei width,
>> +                         GLenum format, GLsizei imageSize, const 
>> GLvoid *data,
>> +                         bool dsa, const char *caller)
>>  {
>>     struct gl_texture_object *texObj;
>>     struct gl_texture_image *texImage;
>>
>>     GET_CURRENT_CONTEXT(ctx);
>>
>> -   if (compressed_subtexture_target_check(ctx, target, 1, format, false,
>> -                                          
>> "glCompressedTexSubImage1D")) {
>> -      return;
>> +   if (dsa) {
>> +      texObj = _mesa_lookup_texture_err(ctx, texture, caller);
>> +      if (!texObj)
>> +         return;
>> +
>> +      target = texObj->Target;
>>     }
>>
>> -   texObj = _mesa_get_current_tex_object(ctx, target);
>> -   if (!texObj)
>> +   if (compressed_subtexture_target_check(ctx, target, dim, format, dsa,
>> +                                          caller)) {
>>        return;
>> +   }
>>
>> -   if (compressed_subtexture_error_check(ctx, 1, texObj, target,
>> -                                         level, xoffset, 0, 0,
>> -                                         width, 1, 1,
>> -                                         format, imageSize, data,
>> -                                         "glCompressedTexSubImage1D")) {
>> +   if (!dsa) {
>> +      texObj = _mesa_get_current_tex_object(ctx, target);
>> +         if (!texObj)
>> +            return;
>> +   }
> 
> I think you can clean the code up a bit here by having
> 
>     if (dsa) {
>        texObj = ...;
>        target = ...;
>     } else {
>        texObj = ...;
>     }
> 
>     if (compressed_subtexture_target_check(...))
>        return;
> 
> When multiple error conditions are present, the OpenGL spec leaves it up 
> to the implementation which one it flags.

Actually no. Switching the order would mean we always report the target 
as no being found. This is no very helpful to the dev and would mean we 
never report that the target is invalid as specified by the spec.

It would also result in a CTS failure [1].

[1] https://bugs.freedesktop.org/show_bug.cgi?id=89311

> 
> Cheers,
> Nicolai
> 
> 
> 
>> +
>> +   if (compressed_subtexture_error_check(ctx, dim, texObj, target, 
>> level,
>> +                                         xoffset, 0, 0, width, 1, 1, 
>> format,
>> +                                         imageSize, data, caller)) {
>>        return;
>>     }
>>
>>     texImage = _mesa_select_tex_image(texObj, target, level);
>>     assert(texImage);
>>
>> -   compressed_texture_sub_image(ctx, 1, texObj, texImage, target, level,
>> +   compressed_texture_sub_image(ctx, dim, texObj, texImage, target, 
>> level,
>>                                  xoffset, 0, 0, width, 1, 1, format, 
>> imageSize,
>>                                  data);
>>  }
>>
>> +
>> +void GLAPIENTRY
>> +_mesa_CompressedTexSubImage1D(GLenum target, GLint level, GLint xoffset,
>> +                              GLsizei width, GLenum format,
>> +                              GLsizei imageSize, const GLvoid *data)
>> +{
>> +   compressed_tex_sub_image(1, target, 0, level, xoffset, width, format,
>> +                            imageSize, data, false,
>> +                            "glCompressedTexSubImage1D");
>> +}
>> +
>>  void GLAPIENTRY
>>  _mesa_CompressedTextureSubImage1D(GLuint texture, GLint level, GLint 
>> xoffset,
>>                                    GLsizei width, GLenum format,
>>                                    GLsizei imageSize, const GLvoid *data)
>>  {
>> -   struct gl_texture_object *texObj;
>> -   struct gl_texture_image *texImage;
>> -
>> -   GET_CURRENT_CONTEXT(ctx);
>> -
>> -   texObj = _mesa_lookup_texture_err(ctx, texture,
>> -                                     "glCompressedTextureSubImage1D");
>> -   if (!texObj)
>> -      return;
>> -
>> -   if (compressed_subtexture_target_check(ctx, texObj->Target, 1, 
>> format, true,
>> -                                          
>> "glCompressedTextureSubImage1D")) {
>> -      return;
>> -   }
>> -
>> -   if (compressed_subtexture_error_check(ctx, 1, texObj, texObj->Target,
>> -                                         level, xoffset, 0, 0,
>> -                                         width, 1, 1,
>> -                                         format, imageSize, data,
>> -                                         
>> "glCompressedTextureSubImage1D")) {
>> -      return;
>> -   }
>> -
>> -   texImage = _mesa_select_tex_image(texObj, texObj->Target, level);
>> -   assert(texImage);
>> -
>> -   compressed_texture_sub_image(ctx, 1, texObj, texImage, 
>> texObj->Target,
>> -                                level, xoffset, 0, 0, width, 1, 1, 
>> format,
>> -                                imageSize, data);
>> +   compressed_tex_sub_image(1, 0, texture, level, xoffset, width,
>> +                            format, imageSize, data, true,
>> +                            "glCompressedTextureSubImage1D");
>>  }
>>
>>
>>  void GLAPIENTRY
>>  _mesa_CompressedTexSubImage2D(GLenum target, GLint level, GLint xoffset,
>>                                GLint yoffset, GLsizei width, GLsizei 
>> height,
>>                                GLenum format, GLsizei imageSize,
>>                                const GLvoid *data)
>>  {
>>     struct gl_texture_object *texObj;
>>
> 
>