[Mesa-dev] [PATCH 4/7] i965/fs: Use image_format_info for doing image_load_store workarounds

[Chad Versace]

On Mon 09 Nov 2015, Jason Ekstrand wrote:
> On Mon, Nov 9, 2015 at 2:24 PM, Chad Versace <chad.versace at intel.com> wrote:
> > On Wed 04 Nov 2015, Jason Ekstrand wrote:
> >> ---
> >>  .../drivers/dri/i965/brw_fs_surface_builder.cpp    | 157 ++++++++++++++-------
> >>  1 file changed, 106 insertions(+), 51 deletions(-)


> >> +      inline unsigned
> >> +      get_format_num_components(uint32_t brw_format)
> >> +      {
> >> +         if (brw_image_format_info[brw_format].green_bits == 0) {
> >> +            return 1;
> >> +         } else if (brw_image_format_info[brw_format].blue_bits == 0) {
> >> +            return 2;
> >> +         } else {
> >> +            return 4;
> >> +         }
> >
> > This function needs a case for alpha. Either
> 
> Actually, it doesn't...  All of the brw_fs_surface_builder code
> assumes image formats which are all power-of-two so RGB isn't valid.
> Maybe I should say something?

Yes, then a comment or assertion is in order.

> --Jason
> 
> >         } else if (brw_image_format_info[brw_format].alpha_bits == 0) {
> >                 return 3;
> >         }
> >
> > or
> >
> >         } else {
> >                 assert(brw_image_format_info[brw_format].alpha_bits > 0);
> >                 return 4;
> >         }
> >
> >
> > The rest of the patch looked good to me.