El 2014-10-07 21:46, Jason Ekstrand escribió:
On Oct 7, 2014 12:04 PM, "Iago Toral Quiroga" <ito...@igalia.com>
wrote:
 >
 > There is a comment warning about the fact that this is not doing
what we
 > expect, so fix it. This should be doing the same as
unpack_B5G6R5_UNORM
 > but swapping B and R.
 > ---
 >
 > Jason started some work to auto-generate the format_pack.c and
 > format_unpack.c files which I think has this fixed. I am continuing
his
 > work on this at the moment, but I guess it might make sense to fix
this in
 > the current code too while that work is on-going.

Not much time to reply right now, but I seem to recall there being a
bit more to fix here.  What about packing and swrast's texel fetch
implementation.  Are those OK for this format?

No piglit regressions observed.

On what drivers?  It might be good to test on swrast and llvmpipe. 
I'm not super-concerned there but we should at least try not to break
it.
 --Jason

That was on Intel, but you are right, at least classic swrast has some regressions with this.

For reference, I did not see any regressions on Gallium Softpipe but I could only run a small subset of tests with this driver (-t texture -t color -t format) or otherwise it would hog my CPU and eventually crash my system. Could not test with llvmpipe, for some reason, even when I am bulding with llvm and I see the llvmpipe sources being built Mesa insists in using the softpipe driver at runtime...

Since there are regressions on swrast at least I guess we should just drop this patch until we have a proper fix for all drivers.

Iago


 >  src/mesa/main/format_unpack.c | 10 +++-------
 >  1 file changed, 3 insertions(+), 7 deletions(-)
 >
 > diff --git a/src/mesa/main/format_unpack.c
b/src/mesa/main/format_unpack.c
 > index d5628a9..11b028c 100644
 > --- a/src/mesa/main/format_unpack.c
 > +++ b/src/mesa/main/format_unpack.c
 > @@ -207,16 +207,12 @@ 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++) {
 > -      GLuint t = (s[i] >> 8) | (s[i] << 8); /* byte swap */
 > -      dst[i][RCOMP] = UBYTE_TO_FLOAT( ((t >> 8) & 0xf8) | ((t
13) & 0x7) );
 > -      dst[i][GCOMP] = UBYTE_TO_FLOAT( ((t >> 3) & 0xfc) | ((t
  9) & 0x3) );
 > -      dst[i][BCOMP] = UBYTE_TO_FLOAT( ((t << 3) & 0xf8) | ((t
  2) & 0x7) );
 > +      dst[i][RCOMP] = ((s[i]      ) & 0x1f) * (1.0F /
31.0F);
 > +      dst[i][GCOMP] = ((s[i] >> 5 ) & 0x3f) * (1.0F / 63.0F);
 > +      dst[i][BCOMP] = ((s[i] >> 11) & 0x1f) * (1.0F / 31.0F);
 >        dst[i][ACOMP] = 1.0F;
 >     }
 >  }
 > --
 > 1.9.1
 >
 > _______________________________________________
 > mesa-dev mailing list
 > mesa-dev@lists.freedesktop.org
 > http://lists.freedesktop.org/mailman/listinfo/mesa-dev [1]

Links:
------
[1] http://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to