[Mesa-dev] [PATCH 4/4] mesa/formats: refactor by globbing on types in switch statement

Nanley Chery nanleychery at gmail.com
Mon Aug 24 15:49:19 PDT 2015 

On Thu, Aug 20, 2015 at 3:52 PM, Nanley Chery <nanleychery at gmail.com> wrote:

> On Thu, Aug 20, 2015 at 11:34 AM, Emil Velikov <emil.l.velikov at gmail.com>
> wrote:
>
>> 2015-08-12 0:07 GMT+01:00 Nanley Chery <nanleychery at gmail.com>:
>> > From: Nanley Chery <nanley.g.chery at intel.com>
>> >
>> > Combine the adjacent cases which have the same GL type in the switch
>> statemnt.
>> >
>> > Signed-off-by: Nanley Chery <nanley.g.chery at intel.com>
>> > ---
>> >  src/mesa/main/formats.c | 152
>> ++++++------------------------------------------
>> >  1 file changed, 17 insertions(+), 135 deletions(-)
>> >
>> > diff --git a/src/mesa/main/formats.c b/src/mesa/main/formats.c
>> > index cb5ad21..9b9d79b 100644
>> > --- a/src/mesa/main/formats.c
>> > +++ b/src/mesa/main/formats.c
>> > @@ -1005,13 +1005,10 @@
>> _mesa_uncompressed_format_to_type_and_comps(mesa_format format,
>> >     case MESA_FORMAT_R8G8B8X8_UNORM:
>> >     case MESA_FORMAT_B8G8R8X8_UNORM:
>> >     case MESA_FORMAT_X8R8G8B8_UNORM:
>> > -      *datatype = GL_UNSIGNED_BYTE;
>> > -      *comps = 4;
>> > -      return;
>> >     case MESA_FORMAT_BGR_UNORM8:
>> >     case MESA_FORMAT_RGB_UNORM8:
>> >        *datatype = GL_UNSIGNED_BYTE;
>> > -      *comps = 3;
>> > +      *comps = _mesa_format_num_components(format);
>> With the datatype aside what is stopping us from using a single
>> _mesa_format_num_components(format) ?
>>
>> I just checked w/ a gtest and we don't get the same output for the
> formats listed below. The "Actual" value is the output of
> _mesa_format_num_components() and the "Expected" value is the output of
> _mesa_uncompressed_format_to_type_and_comps().
>
> mesa_formats.cpp:59: Failure
> Value of: _mesa_format_num_components(f)
>   Actual: 3
> Expected: comps
> Which is: 4
> MESA_FORMAT_B4G4R4X4_UNORM
>
> mesa_formats.cpp:59: Failure
> Value of: _mesa_format_num_components(f)
>   Actual: 3
> Expected: comps
> Which is: 4
> MESA_FORMAT_B5G5R5X1_UNORM
>
> mesa_formats.cpp:59: Failure
> Value of: _mesa_format_num_components(f)
>   Actual: 3
> Expected: comps
> Which is: 4
> MESA_FORMAT_B10G10R10X2_UNORM
>
> mesa_formats.cpp:59: Failure
> Value of: _mesa_format_num_components(f)
>   Actual: 3
> Expected: comps
> Which is: 4
> MESA_FORMAT_R10G10B10X2_UNORM
>
> mesa_formats.cpp:59: Failure
> Value of: _mesa_format_num_components(f)
>   Actual: 0
> Expected: comps
> Which is: 2
> MESA_FORMAT_YCBCR
>
> mesa_formats.cpp:59: Failure
> Value of: _mesa_format_num_components(f)
>   Actual: 0
> Expected: comps
> Which is: 2
> MESA_FORMAT_YCBCR_REV
>
> mesa_formats.cpp:59: Failure
> Value of: _mesa_format_num_components(f)
>   Actual: 3
> Expected: comps
> Which is: 4
> MESA_FORMAT_RGBX_UNORM16
>
> mesa_formats.cpp:59: Failure
> Value of: _mesa_format_num_components(f)
>   Actual: 3
> Expected: comps
> Which is: 4
> MESA_FORMAT_R8G8B8X8_SNORM
>
> mesa_formats.cpp:59: Failure
> Value of: _mesa_format_num_components(f)
>   Actual: 3
> Expected: comps
> Which is: 4
> MESA_FORMAT_RGBX_SNORM16
>
> mesa_formats.cpp:59: Failure
> Value of: _mesa_format_num_components(f)
>   Actual: 3
> Expected: comps
> Which is: 4
> MESA_FORMAT_B8G8R8X8_SRGB
>
> mesa_formats.cpp:59: Failure
> Value of: _mesa_format_num_components(f)
>   Actual: 3
> Expected: comps
> Which is: 4
> MESA_FORMAT_X8R8G8B8_SRGB
>
> mesa_formats.cpp:59: Failure
> Value of: _mesa_format_num_components(f)
>   Actual: 3
> Expected: comps
> Which is: 4
> MESA_FORMAT_R8G8B8X8_SRGB
>
> mesa_formats.cpp:59: Failure
> Value of: _mesa_format_num_components(f)
>   Actual: 3
> Expected: comps
> Which is: 4
> MESA_FORMAT_X8B8G8R8_SRGB
>
> mesa_formats.cpp:59: Failure
> Value of: _mesa_format_num_components(f)
>   Actual: 2
> Expected: comps
> Which is: 1
> MESA_FORMAT_Z32_FLOAT_S8X24_UINT
>
> mesa_formats.cpp:59: Failure
> Value of: _mesa_format_num_components(f)
>   Actual: 3
> Expected: comps
> Which is: 4
> MESA_FORMAT_RGBX_FLOAT16
>
> mesa_formats.cpp:59: Failure
> Value of: _mesa_format_num_components(f)
>   Actual: 3
> Expected: comps
> Which is: 4
> MESA_FORMAT_RGBX_FLOAT32
>
> mesa_formats.cpp:59: Failure
> Value of: _mesa_format_num_components(f)
>   Actual: 3
> Expected: comps
> Which is: 4
> MESA_FORMAT_RGBX_UINT8
>
> mesa_formats.cpp:59: Failure
> Value of: _mesa_format_num_components(f)
>   Actual: 3
> Expected: comps
> Which is: 4
> MESA_FORMAT_RGBX_UINT16
>
> mesa_formats.cpp:59: Failure
> Value of: _mesa_format_num_components(f)
>   Actual: 3
> Expected: comps
> Which is: 4
> MESA_FORMAT_RGBX_UINT32
>
> mesa_formats.cpp:59: Failure
> Value of: _mesa_format_num_components(f)
>   Actual: 3
> Expected: comps
> Which is: 4
> MESA_FORMAT_RGBX_SINT8
>
> mesa_formats.cpp:59: Failure
> Value of: _mesa_format_num_components(f)
>   Actual: 3
> Expected: comps
> Which is: 4
> MESA_FORMAT_RGBX_SINT16
>
> mesa_formats.cpp:59: Failure
> Value of: _mesa_format_num_components(f)
>   Actual: 3
> Expected: comps
> Which is: 4
> MESA_FORMAT_RGBX_SINT32
>
> In fact, for this patch, the following behavioral differences exist:
>
> mesa_formats.cpp:59: Failure
> Value of: _mesa_format_num_components(f)
>   Actual: 3
> Expected: comps
> Which is: 4
> MESA_FORMAT_X8B8G8R8_UNORM
>
> mesa_formats.cpp:59: Failure
> Value of: _mesa_format_num_components(f)
>   Actual: 3
> Expected: comps
> Which is: 4
> MESA_FORMAT_R8G8B8X8_UNORM
>
> mesa_formats.cpp:59: Failure
> Value of: _mesa_format_num_components(f)
>   Actual: 3
> Expected: comps
> Which is: 4
> MESA_FORMAT_B8G8R8X8_UNORM
>
> mesa_formats.cpp:59: Failure
> Value of: _mesa_format_num_components(f)
>   Actual: 3
> Expected: comps
> Which is: 4
> MESA_FORMAT_X8R8G8B8_UNORM
>
> mesa_formats.cpp:59: Failure
> Value of: _mesa_format_num_components(f)
>   Actual: 3
> Expected: comps
> Which is: 4
> MESA_FORMAT_X8B8G8R8_SNORM
>
> I am looking into this issue.
>
> I discussed this issue with Ken and we were able to determine the reason
for this difference.  _mesa_format_num_components() returns the number of
components in a format that have useful data. This is equal to the total
number of components in the format minus the one used for padding (if any).
These padding components have an X in their name. The proper fix for this
is to create a second function, _mesa_format_num_components_with_padding.
This function can then be used instead of hard-coding the number of
components per format (expect for MESA_FORMAT_YCBCR[_REV]).


> Regards,
> Nanley
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150824/f3f0e3b8/attachment-0001.html>

More information about the mesa-dev mailing list