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

[Nicolai Hähnle]

On 16.05.2017 04:19, Timothy Arceri wrote:
>
>
> 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].

Okay, that's fair enough. You can add my R-b to this patch as well.

Cheers,
Nicolai


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


-- 
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.