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

Tue Aug 25 11:35:01 PDT 2015

On Tue, Aug 25, 2015 at 7:42 AM, Emil Velikov <emil.l.velikov at gmail.com>
wrote:

> 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 ;-)
>
> I was thinking along the same lines.

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