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

[Iago Toral]

El 2014-10-08 07:11, Jason Ekstrand escribió:
> On Oct 8, 2014 6:36 AM, "Iago Toral" <itoral at igalia.com> wrote:
>  >
>  > 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...
> 
> I can give you the configure flags for testing llvmpipe if you'd
> like. 

I am using --with-gallium-drivers=swrast, I think that and having llvm 
should be all, right?

Configure reports that I have llvm, I see sources under 
src/gallium/drivers/llvmpipe/ being built and libllvmpipe.la is being 
generated , so I figure that my configure flags are working fine. My 
problem seems to at runtime, I thought that as far as llvmpipe is built 
it would take priority over softpipe, but for some reason that is not 
happening for me...

> Yes, doing a full piglit run on llvmpipe or swrast requires a
> pretty beefy desktop and still takes quite a while.
> 
>> 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.
> 
> I'm not as worried about llvmpipe because it fails about half the
> tests it attempts anyway.  I would like to know what's going on with
> swrast though.
> 
> Brian, any ideas?
> 
> --Jason
> 
>> 
>  > 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] [1]
>  >>
>  >> Links:
>  >> ------
>  >> [1] http://lists.freedesktop.org/mailman/listinfo/mesa-dev [1]
> 
> 
> Links:
> ------
> [1] http://lists.freedesktop.org/mailman/listinfo/mesa-dev