On Sep 16, 2014 6:54 PM, "Dave Airlie" <<a href="mailto:airlied@gmail.com">airlied@gmail.com</a>> wrote: > > On 17 September 2014 09:56, Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> wrote: > > Got a couple comments below. Other than that, > > Reviewed-by: Jason Ekstrand <<a href="mailto:jason.ekstrand@intel.com">jason.ekstrand@intel.com</a>> > > > > On Mon, Sep 15, 2014 at 11:28 PM, Dave Airlie <<a href="mailto:airlied@gmail.com">airlied@gmail.com</a>> wrote: > >> > >> From: Richard Sandiford <<a href="mailto:rsandifo@linux.vnet.ibm.com">rsandifo@linux.vnet.ibm.com</a>> > >> > >> This means that each 8888 SRGB format has a reversed counterpart, > >> which is necessary for handling big-endian mesa<->gallium mappings. > >> > >> Signed-off-by: Richard Sandiford <<a href="mailto:rsandifo@linux.vnet.ibm.com">rsandifo@linux.vnet.ibm.com</a>> > >> Signed-off-by: Dave Airlie <<a href="mailto:airlied@redhat.com">airlied@redhat.com</a>> > >> --- > >> src/mesa/drivers/dri/i965/brw_surface_formats.c | 1 + > >> src/mesa/main/format_pack.c | 60 > >> +++++++++++++++++++++++++ > >> src/mesa/main/format_unpack.c | 42 +++++++++++++++++ > >> src/mesa/main/formats.c | 16 +++++++ > >> src/mesa/main/formats.csv | 3 ++ > >> src/mesa/main/formats.h | 3 ++ > >> src/mesa/main/texformat.c | 10 +++++ > >> src/mesa/swrast/s_texfetch.c | 3 ++ > >> src/mesa/swrast/s_texfetch_tmp.h | 24 ++++++++++ > >> 9 files changed, 162 insertions(+) > >> > >> diff --git a/src/mesa/drivers/dri/i965/brw_surface_formats.c > >> b/src/mesa/drivers/dri/i965/brw_surface_formats.c > >> index b726c27..21b8adb 100644 > >> --- a/src/mesa/drivers/dri/i965/brw_surface_formats.c > >> +++ b/src/mesa/drivers/dri/i965/brw_surface_formats.c > >> @@ -517,6 +517,7 @@ brw_format_for_mesa_format(mesa_format mesa_format) > >> [MESA_FORMAT_B5G5R5X1_UNORM] = BRW_SURFACEFORMAT_B5G5R5X1_UNORM, > >> [MESA_FORMAT_R8G8B8X8_SNORM] = 0, > >> [MESA_FORMAT_R8G8B8X8_SRGB] = 0, > >> + [MESA_FORMAT_X8B8G8R8_SRGB] = 0, > > > > > > Any particular reason you go out of your way to add X8B8G8R8_SRGB and not > > the other two? > > no good one, I'll fix that up. > > >> +static void > >> +pack_float_X8B8G8R8_SRGB(const GLfloat src[4], void *dst) > >> +{ > >> + GLuint *d = (GLuint *) dst; > >> + GLubyte r = util_format_linear_float_to_srgb_8unorm(src[RCOMP]); > >> + GLubyte g = util_format_linear_float_to_srgb_8unorm(src[GCOMP]); > >> + GLubyte b = util_format_linear_float_to_srgb_8unorm(src[BCOMP]); > >> + *d = PACK_COLOR_8888(r, g, b, 127); > > > > > > That should be a max of 255, these aren't SNORM formats > > These two are taken from equivalent code in > pack_float_B8G8R8X8_SRGB > > that is currently in mesa, so I should send a patch to fix those up as well? Yes, unless I'm going crazy. (Which is always a possibility.) --Jason > > Thanks, > Dave.