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

[Jason Ekstrand]

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.
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]
>>
>> Links:
>> ------
>> [1] http://lists.freedesktop.org/mailman/listinfo/mesa-dev
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20141007/ce953f06/attachment.html>