On Mon 09 Nov 2015, Jason Ekstrand wrote:
> On Mon, Nov 9, 2015 at 2:24 PM, Chad Versace <chad.vers...@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.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
