[Mesa-dev] [PATCH] mesa/glformats: Remove redundant helper _mesa_base_format_component_count

[Eduardo Lima Mitev]

On 10/23/2018 08:17 AM, Tapani Pälli wrote:
> 
> On 10/23/18 8:56 AM, Eduardo Lima Mitev wrote:
>> There exists _mesa_components_in_format() which already includes
>> all cases handled in _mesa_base_format_component_count().
> 
> I guess the idea here was that one function only covers 'base formats'
> and other one all formats. But I guess none of the users verify if 'base
> format' is sane this way? If this is the case and there are no regressions;
> 

Yes, this is the best explanation for adding the helper.
However, looking at the the two uses of this function, it is not
expected that it validates the format as a base format. Also,
_mesa_base_format_component_count() handle mostly base formats anyway.

I ran the patch through Intel CI with no regressions.

> Reviewed-by: Tapani Pälli <tapani.palli at intel.com>
> 

Thanks!

Eduardo

> 
>> ---
>>   src/mesa/drivers/dri/i965/brw_blorp.c |  2 +-
>>   src/mesa/main/glformats.c             | 27 ---------------------------
>>   src/mesa/main/glformats.h             |  3 ---
>>   src/mesa/main/teximage.c              |  4 ++--
>>   4 files changed, 3 insertions(+), 33 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_blorp.c
>> b/src/mesa/drivers/dri/i965/brw_blorp.c
>> index ad3a47ef035..b286b231537 100644
>> --- a/src/mesa/drivers/dri/i965/brw_blorp.c
>> +++ b/src/mesa/drivers/dri/i965/brw_blorp.c
>> @@ -1195,7 +1195,7 @@ set_write_disables(const struct
>> intel_renderbuffer *irb,
>>       * RGB we can treat alpha as not used and write whatever we like
>> into it.
>>       */
>>      const GLenum base_format = irb->Base.Base._BaseFormat;
>> -   const int components =
>> _mesa_base_format_component_count(base_format);
>> +   const int components = _mesa_components_in_format(base_format);
>>      bool disables = false;
>>        assert(components > 0);
>> diff --git a/src/mesa/main/glformats.c b/src/mesa/main/glformats.c
>> index bbeb6034dd7..6cb3435dea2 100644
>> --- a/src/mesa/main/glformats.c
>> +++ b/src/mesa/main/glformats.c
>> @@ -1630,33 +1630,6 @@ _mesa_base_format_has_channel(GLenum
>> base_format, GLenum pname)
>>   }
>>     -/**
>> - * Returns the number of channels/components for a base format.
>> - */
>> -GLint
>> -_mesa_base_format_component_count(GLenum base_format)
>> -{
>> -   switch (base_format) {
>> -   case GL_LUMINANCE:
>> -   case GL_RED:
>> -   case GL_ALPHA:
>> -   case GL_INTENSITY:
>> -   case GL_DEPTH_COMPONENT:
>> -      return 1;
>> -   case GL_RG:
>> -   case GL_LUMINANCE_ALPHA:
>> -   case GL_DEPTH_STENCIL:
>> -      return 2;
>> -   case GL_RGB:
>> -      return 3;
>> -   case GL_RGBA:
>> -      return 4;
>> -   default:
>> -      return -1;
>> -   }
>> -}
>> -
>> -
>>   /**
>>    * If format is a generic compressed format, return the corresponding
>>    * non-compressed format.  For other formats, return the format as-is.
>> diff --git a/src/mesa/main/glformats.h b/src/mesa/main/glformats.h
>> index 5a21317159d..0aefdf50fef 100644
>> --- a/src/mesa/main/glformats.h
>> +++ b/src/mesa/main/glformats.h
>> @@ -119,9 +119,6 @@ _mesa_unpack_format_to_base_format(GLenum format);
>>   extern GLboolean
>>   _mesa_base_format_has_channel(GLenum base_format, GLenum pname);
>>   -extern GLint
>> -_mesa_base_format_component_count(GLenum base_format);
>> -
>>   extern GLenum
>>   _mesa_generic_compressed_format_to_uncompressed_format(GLenum format);
>>   diff --git a/src/mesa/main/teximage.c b/src/mesa/main/teximage.c
>> index d45854bd17f..6805b47c728 100644
>> --- a/src/mesa/main/teximage.c
>> +++ b/src/mesa/main/teximage.c
>> @@ -2407,8 +2407,8 @@ copytexture_error_check( struct gl_context *ctx,
>> GLuint dimensions,
>>        if (_mesa_is_gles(ctx)) {
>>         bool valid = true;
>> -      if (_mesa_base_format_component_count(baseFormat) >
>> -          _mesa_base_format_component_count(rb_base_format)) {
>> +      if (_mesa_components_in_format(baseFormat) >
>> +          _mesa_components_in_format(rb_base_format)) {
>>            valid = false;
>>         }
>>         if (baseFormat == GL_DEPTH_COMPONENT ||
>>
>