On Oct 8, 2014 6:36 AM, "Iago Toral" <<a href="mailto:itoral@igalia.com">itoral@igalia.com</a>> wrote: > > El 2014-10-07 21:46, Jason Ekstrand escribió: > >> On Oct 7, 2014 12:04 PM, "Iago Toral Quiroga" <<a href="mailto:itoral@igalia.com">itoral@igalia.com</a>> >> 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 >> > <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> >> > <a href="http://lists.freedesktop.org/mailman/listinfo/mesa-dev">http://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> [1] >> >> Links: >> ------ >> [1] <a href="http://lists.freedesktop.org/mailman/listinfo/mesa-dev">http://lists.freedesktop.org/mailman/listinfo/mesa-dev</a>