[Mesa-dev] [PATCH 4/4] mesa/formats: refactor by globbing on types in switch statement
Emil Velikov
emil.l.velikov at gmail.com
Tue Aug 25 07:42:16 PDT 2015
Hi Nanley,
On 24 August 2015 at 23:49, Nanley Chery <nanleychery at gmail.com> wrote:
> 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]).
>
I wouldn't bother with this for the time being. That is unless others
insist ;-)
Thanks
Emil
More information about the mesa-dev
mailing list