[Mesa-dev] [PATCH] mesa: Correctly print glTexImage dimensions

[Tapani Pälli]

On 01/29/2018 12:05 PM, Elie Tournier wrote:
> On Fri, Jan 26, 2018 at 02:34:03PM +0200, Tapani Pälli wrote:
>> Hi;
>>
>> On 01/25/2018 05:18 PM, Elie Tournier wrote:
>>> texture_format_error_check_gles() displays error like "glTexImage%dD".
>>> This patch just replace the %d by the correct dimension.
>>>
>>> Signed-off-by: Elie Tournier <elie.tournier at collabora.com>
>>> ---
>>>    src/mesa/main/teximage.c | 13 ++++++-------
>>>    1 file changed, 6 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/src/mesa/main/teximage.c b/src/mesa/main/teximage.c
>>> index e5f8bb0718..cc329e6410 100644
>>> --- a/src/mesa/main/teximage.c
>>> +++ b/src/mesa/main/teximage.c
>>> @@ -1787,7 +1787,6 @@ texture_formats_agree(GLenum internalFormat,
>>>     * \param format pixel data format given by the user.
>>>     * \param type pixel data type given by the user.
>>>     * \param internalFormat internal format given by the user.
>>> - * \param dimensions texture image dimensions (must be 1, 2 or 3).
>>>     * \param callerName name of the caller function to print in the error message
>>>     *
>>>     * \return true if a error is found, false otherwise
>>> @@ -1796,8 +1795,7 @@ texture_formats_agree(GLenum internalFormat,
>>>     */
>>>    static bool
>>>    texture_format_error_check_gles(struct gl_context *ctx, GLenum format,
>>> -                                GLenum type, GLenum internalFormat,
>>> -                                GLuint dimensions, const char *callerName)
>>> +                                GLenum type, GLenum internalFormat, const char *callerName)
>>>    {
>>>       GLenum err = _mesa_es3_error_check_format_and_type(ctx, format, type,
>>>                                                          internalFormat);
>>> @@ -1911,9 +1909,11 @@ texture_error_check( struct gl_context *ctx,
>>>        * Formats and types that require additional extensions (e.g., GL_FLOAT
>>>        * requires GL_OES_texture_float) are filtered elsewhere.
>>>        */
>>> +   char bufCallerName[20];
>>> +   snprintf(bufCallerName, 20, "glTexImage%dD", dimensions);
>>
>> I don't think this is going to work, reason is that this error check is used
>> both from glTexImage and glTexSubImage. For example _mesa_TexSubImage2D
>> passes "glTexSubImage2D" as callerName like this:
>>
>> _mesa_TexSubImage2D
>> texsubimage_err
>> texsubimage_error_check
>> texture_format_error_check_gles
>>
> Hello Tapani,
> 
> I'm not sure to understand your comment.
> This patch just call texture_format_error_check_gles() with callerName parameter set correctly.
> Previously, callerName was "glTexImage%dD" but we didn't display the dimension.
> So I use snprintf in order to print the image dimension.

Sorry, now I noticed this works just fine. I was blind before and 
thought you did the snprintf in texture_format_error_check_gles.

> I should probably split this patch in 2:
> * Remove unused "dimensions" parameter in texture_format_error_check_gles.
> * Set callerName with the correct dimension.

I'm fine having these on same patch;
Reviewed-by: Tapani Pälli <tapani.palli at intel.com>

>>
>>>       if (_mesa_is_gles(ctx) &&
>>> -       texture_format_error_check_gles(ctx, format, type, internalFormat,
>>> -                                       dimensions, "glTexImage%dD")) {
>>> +       texture_format_error_check_gles(ctx, format, type,
>>> +                                       internalFormat, bufCallerName)) {
>>>          return GL_TRUE;
>>>       }
>>> @@ -2234,8 +2234,7 @@ texsubimage_error_check(struct gl_context *ctx, GLuint dimensions,
>>>        */
>>>       if (_mesa_is_gles(ctx) &&
>>>           texture_format_error_check_gles(ctx, format, type,
>>> -                                       internalFormat,
>>> -                                       dimensions, callerName)) {
>>> +                                       internalFormat, callerName)) {
>>>          return GL_TRUE;
>>>       }
>>>