[Mesa-dev] [PATCH] mesa/main: Fix unpack_R5G6B5_UNORM.

[Iago Toral]

El 2014-10-07 21:46, Jason Ekstrand escribió:
> On Oct 7, 2014 12:04 PM, "Iago Toral Quiroga" <itoral at 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 at lists.freedesktop.org
>  > http://lists.freedesktop.org/mailman/listinfo/mesa-dev [1]
> 
> Links:
> ------
> [1] http://lists.freedesktop.org/mailman/listinfo/mesa-dev