<div dir="ltr"> <div class="gmail_extra"> <div class="gmail_quote">On Wed, Dec 3, 2014 at 2:20 PM, Ian Romanick <<a href="mailto:idr@freedesktop.org" target="_blank">idr@freedesktop.org</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">One question below... <div><div class="h5"> On 12/01/2014 03:04 AM, Iago Toral Quiroga wrote: > From: Jason Ekstrand <<a href="mailto:jason.ekstrand@intel.com">jason.ekstrand@intel.com</a>> > > Aparently, the packing/unpacking functions for these formats have differed > from the format description in formats.h. Instead of fixing this, people > simply left a comment saying it was broken. Let's actually fix it for > real. > > Signed-off-by: Jason Ekstrand <<a href="mailto:jason.ekstrand@intel.com">jason.ekstrand@intel.com</a>> > --- > src/mesa/main/format_pack.c | 10 ++-------- > src/mesa/main/format_unpack.c | 10 ++-------- > src/mesa/main/formats.c | 12 ++++++------ > src/mesa/main/texstore.c | 2 +- > src/mesa/swrast/s_texfetch_tmp.h | 8 ++++---- > 5 files changed, 15 insertions(+), 27 deletions(-) > > diff --git a/src/mesa/main/format_pack.c b/src/mesa/main/format_pack.c > index 31c9f77..20d2b1a 100644 > --- a/src/mesa/main/format_pack.c > +++ b/src/mesa/main/format_pack.c > @@ -458,17 +458,11 @@ pack_row_float_B5G6R5_UNORM(GLuint n, const GLfloat src[][4], void *dst) > } > > > -/* > - * MESA_FORMAT_R5G6B5_UNORM > - * Warning: these functions do not match the current Mesa definition > - * of MESA_FORMAT_R5G6B5_UNORM. > - */ > - > static void > pack_ubyte_R5G6B5_UNORM(const GLubyte src[4], void *dst) > { > GLushort *d = ((GLushort *) dst); > - *d = PACK_COLOR_565_REV(src[RCOMP], src[GCOMP], src[BCOMP]); > + *d = PACK_COLOR_565(src[BCOMP], src[GCOMP], src[RCOMP]); > } > > static void > @@ -479,7 +473,7 @@ pack_float_R5G6B5_UNORM(const GLfloat src[4], void *dst) > UNCLAMPED_FLOAT_TO_UBYTE(r, src[RCOMP]); > UNCLAMPED_FLOAT_TO_UBYTE(g, src[GCOMP]); > UNCLAMPED_FLOAT_TO_UBYTE(b, src[BCOMP]); > - *d = PACK_COLOR_565_REV(r, g, b); > + *d = PACK_COLOR_565(b, g, r); > } > > static void > diff --git a/src/mesa/main/format_unpack.c b/src/mesa/main/format_unpack.c > index d5628a9..f5ab966 100644 > --- a/src/mesa/main/format_unpack.c > +++ b/src/mesa/main/format_unpack.c > @@ -207,9 +207,6 @@ unpack_B5G6R5_UNORM(const void *src, GLfloat dst[][4], GLuint n) > static void > unpack_R5G6B5_UNORM(const void *src, GLfloat dst[][4], GLuint n) > { > - /* Warning: this function does not match the current Mesa definition > - * of MESA_FORMAT_R5G6B5_UNORM. > - */ > const GLushort *s = ((const GLushort *) src); > GLuint i; > for (i = 0; i < n; i++) { </div></div>This removes the comment, but it doesn't change the code. Was the code actually correct before? </blockquote><div> </div><div>Looking at mesa master on cgit, it looks like it's wrong. It won't matter because the later patches autogenerate this fill and do so correctly. </div><div>--Jason </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> <div class="HOEnZb"><div class="h5"> > @@ -2764,16 +2761,13 @@ unpack_ubyte_B5G6R5_UNORM(const void *src, GLubyte dst[][4], GLuint n) > static void > unpack_ubyte_R5G6B5_UNORM(const void *src, GLubyte dst[][4], GLuint n) > { > - /* Warning: this function does not match the current Mesa definition > - * of MESA_FORMAT_R5G6B5_UNORM. > - */ > const GLushort *s = ((const GLushort *) src); > GLuint i; > for (i = 0; i < n; i++) { > GLuint t = (s[i] >> 8) | (s[i] << 8); /* byte swap */ > - dst[i][RCOMP] = EXPAND_5_8((t >> 11) & 0x1f); > + dst[i][RCOMP] = EXPAND_5_8( t & 0x1f); > dst[i][GCOMP] = EXPAND_6_8((t >> 5 ) & 0x3f); > - dst[i][BCOMP] = EXPAND_5_8( t & 0x1f); > + dst[i][BCOMP] = EXPAND_5_8((t >> 11) & 0x1f); > dst[i][ACOMP] = 0xff; > } > } > diff --git a/src/mesa/main/formats.c b/src/mesa/main/formats.c > index 58c32e2..1315d36 100644 > --- a/src/mesa/main/formats.c > +++ b/src/mesa/main/formats.c > @@ -1462,14 +1462,14 @@ _mesa_format_matches_format_and_type(mesa_format mesa_format, > return format == GL_RGB && type == GL_UNSIGNED_BYTE && littleEndian; > > case MESA_FORMAT_B5G6R5_UNORM: > - return format == GL_RGB && type == GL_UNSIGNED_SHORT_5_6_5 && !swapBytes; > + return ((format == GL_RGB && type == GL_UNSIGNED_SHORT_5_6_5) || > + (format == GL_BGR && type == GL_UNSIGNED_SHORT_5_6_5_REV)) && > + !swapBytes; > > case MESA_FORMAT_R5G6B5_UNORM: > - /* Some of the 16-bit MESA_FORMATs that would seem to correspond to > - * GL_UNSIGNED_SHORT_* are byte-swapped instead of channel-reversed, > - * according to formats.h, so they can't be matched. > - */ > - return GL_FALSE; > + return ((format == GL_BGR && type == GL_UNSIGNED_SHORT_5_6_5) || > + (format == GL_RGB && type == GL_UNSIGNED_SHORT_5_6_5_REV)) && > + !swapBytes; > > case MESA_FORMAT_B4G4R4A4_UNORM: > return format == GL_BGRA && type == GL_UNSIGNED_SHORT_4_4_4_4_REV && > diff --git a/src/mesa/main/texstore.c b/src/mesa/main/texstore.c > index f858cef..3ac8bcf 100644 > --- a/src/mesa/main/texstore.c > +++ b/src/mesa/main/texstore.c > @@ -923,7 +923,7 @@ _mesa_texstore_rgb565(TEXSTORE_PARAMS) > } > else { > for (col = 0; col < srcWidth; col++) { > - dstUS[col] = PACK_COLOR_565_REV( srcUB[0], srcUB[1], srcUB[2] ); > + dstUS[col] = PACK_COLOR_565( srcUB[2], srcUB[1], srcUB[0] ); > srcUB += 3; > } > } > diff --git a/src/mesa/swrast/s_texfetch_tmp.h b/src/mesa/swrast/s_texfetch_tmp.h > index 7ff30f6..23db48d 100644 > --- a/src/mesa/swrast/s_texfetch_tmp.h > +++ b/src/mesa/swrast/s_texfetch_tmp.h > @@ -417,10 +417,10 @@ FETCH(R5G6B5_UNORM)(const struct swrast_texture_image *texImage, > GLint i, GLint j, GLint k, GLfloat *texel) > { > const GLushort *src = TEXEL_ADDR(GLushort, texImage, i, j, k, 1); > - const GLushort s = (*src >> 8) | (*src << 8); /* byte swap */ > - texel[RCOMP] = UBYTE_TO_FLOAT( ((s >> 8) & 0xf8) | ((s >> 13) & 0x7) ); > - texel[GCOMP] = UBYTE_TO_FLOAT( ((s >> 3) & 0xfc) | ((s >> 9) & 0x3) ); > - texel[BCOMP] = UBYTE_TO_FLOAT( ((s << 3) & 0xf8) | ((s >> 2) & 0x7) ); > + const GLushort s = *src; > + texel[RCOMP] = ((s ) & 0x1f) * (1.0F / 31.0F); > + texel[GCOMP] = ((s >> 5 ) & 0x3f) * (1.0F / 63.0F); > + texel[BCOMP] = ((s >> 11) & 0x1f) * (1.0F / 31.0F); > texel[ACOMP] = 1.0F; > } > > </div></div><div class="HOEnZb"><div class="h5">_______________________________________________ mesa-dev mailing list <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> <a href="http://lists.freedesktop.org/mailman/listinfo/mesa-dev" target="_blank">http://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </div></div></blockquote></div> </div></div>