On Wed 04 Nov 2015, Jason Ekstrand wrote: > --- > .../drivers/dri/i965/brw_fs_surface_builder.cpp | 157 > ++++++++++++++------- > 1 file changed, 106 insertions(+), 51 deletions(-) > > diff --git a/src/mesa/drivers/dri/i965/brw_fs_surface_builder.cpp > b/src/mesa/drivers/dri/i965/brw_fs_surface_builder.cpp > index 31ecb5b..d841ffe 100644 > --- a/src/mesa/drivers/dri/i965/brw_fs_surface_builder.cpp > +++ b/src/mesa/drivers/dri/i965/brw_fs_surface_builder.cpp > @@ -22,6 +22,9 @@ > */ > > #include "brw_fs_surface_builder.h" > +#include "intel_mipmap_tree.h" > +#include "brw_state.h" > +#include "brw_image_load_store.h" > #include "brw_fs.h" > > using namespace brw; > @@ -192,32 +195,80 @@ namespace { > * Return the per-channel bitfield widths for a given image format. > */ > inline color_u > - get_bit_widths(mesa_format format) > + get_bit_widths(uint32_t brw_format) > { > - return color_u(_mesa_get_format_bits(format, GL_RED_BITS), > - _mesa_get_format_bits(format, GL_GREEN_BITS), > - _mesa_get_format_bits(format, GL_BLUE_BITS), > - _mesa_get_format_bits(format, GL_ALPHA_BITS)); > + return color_u(brw_image_format_info[brw_format].red_bits, > + brw_image_format_info[brw_format].green_bits, > + brw_image_format_info[brw_format].blue_bits, > + brw_image_format_info[brw_format].alpha_bits); > } > > /** > * Return the per-channel bitfield shifts for a given image format. > */ > inline color_u > - get_bit_shifts(mesa_format format) > + get_bit_shifts(uint32_t brw_format) > { > - const color_u widths = get_bit_widths(format); > + const color_u widths = get_bit_widths(brw_format); > return color_u(0, widths.r, widths.r + widths.g, > widths.r + widths.g + widths.b); > } > > + inline unsigned > + get_format_bytes(uint32_t brw_format) > + { > + return (brw_image_format_info[brw_format].red_bits + > + brw_image_format_info[brw_format].green_bits + > + brw_image_format_info[brw_format].blue_bits + > + brw_image_format_info[brw_format].alpha_bits) / 8;
For general formats, this calculation is wrong. I assume this function is used only for RGBA-formats, though, which makes the function correct. I have the same comment for other hunks in this patch too. > + } > + > + 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 } 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