-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 19/11/14 09:25, Iago Toral wrote: > On Tue, 2014-11-18 at 13:41 -0800, Jason Ekstrand wrote: >> Iago, >> >> Most of this looks pretty good to me. The one primary concern I have >> is in the handling of integer formats. I made the comment in a couple >> of patches, but I'll make it in general here. In a lot of the code, >> when you convert from integer formats to float, you treat them as if >> they are normalized. Can you explain why you are doing this? It >> seems very wrong to me. > > Right, I have been discussing this with Samuel and it does look wrong. > He will change the code and run a new piglit run to verify the changes. > As Iago said, I'm going to do some changes and test them. After that I will reply to the mailing list. >> >> One other issue is that I couldn't actually get it to compile. This >> is probably due to the fact that I always build out-of-tree, so >> sourcedir and builddir are not the same. Not really sure what's going >> on there. > > Mmm... that's weird. I think I remember seeing a patch that added a new > file and could be the source of that issue. We will look into it. > I confirmed that the out-of-tree compilation error is because of the autogeneration of format_pack.c and format_unpack.c files. I'm going to fix it. Thanks! Sam >> >> Other than that, It's looking pretty good. I'll try and get to >> reviewing your second patch series tomorrow. Since my R-B obviously >> doesn't mean much on the code I wrote I'll try and dig up a second >> reviewer as well. > > Yes, that makes sense. > Thanks for looking into these patches so fast! > > Iago > >> --Jason >> >> >> On Tue, Nov 18, 2014 at 12:43 AM, Iago Toral Quiroga >> <ito...@igalia.com> wrote: >> This is the fist of two series of patches to address: >> https://bugs.freedesktop.org/show_bug.cgi?id=84566 >> >> The idea is that we have a lot of format conversion code >> scattered through >> different files in the repository, a lot of that is >> redundant / duplicated, >> so this intends to address that issue. >> >> The goal of this first series is to address auto-generation of >> our pack/unpack >> functions (format_pack.c and format_unpack.c). Currently, we >> have a ton of >> hand-coded pack/unpack functions for lots of formats, but we >> can auto-generate >> most of that code instead, so this series handles this. >> >> This is based on initial work by Jason Ekstrand. >> >> Tested on i965, classic swrast and gallium (radeon, nouveau, >> llvmpipe) without >> regressions. >> >> For software drivers we worked with a trimmed set of piglit >> tests (related to >> format conversion), ~5700 tests selected with the following >> filter: >> >> -t format -t color -t tex -t image -t swizzle -t clamp -t rgb >> -t lum -t pix >> -t fbo -t frame >> >> Summary of the patches: >> * Patches 1-7 are general fixes to the current code that were >> found while >> working on this. >> * Patches 8-16 implement auto-generation of pack/unpack >> functions. >> * Patches 17-20 make use of the auto-generated pack/unpack >> functions in >> various places to simplify the current code. >> >> Notice that some of the fixes in patches 1-7 will become >> obsolete as soon as >> we auto-generate the pack/unpack functions, but we thought it >> would make sense >> to keep them in the patch set anyway since we started from >> that base and they >> should be correct fixes to the currently existing code. >> >> Iago Toral Quiroga (1): >> swrast: Remove unused variable. >> >> Jason Ekstrand (9): >> mesa/format_utils: Fix a bug in one of the format helper >> functions >> mesa: Fix packing/unpacking of MESA_FORMAT_R5G6B5_UNORM >> mesa/colormac: Remove an unused macro >> mesa: Fix A1R5G5B5 packing/unpacking >> mesa/format_utils: Prefix and expose the conversion helper >> functions >> mesa: Add a concept of an array format >> mesa: Add a _mesa_is_format_color_format helper >> mesa: Autogenerate most of format_pack.c >> mesa: Autogenerate format_unpack.c >> >> Samuel Iglesias Gonsalvez (10): >> mesa: Fix get_texbuffer_format(). >> mesa: Fix _mesa_swizzle_and_convert integer conversions to >> clamp >> properly >> mesa: Add _mesa_pack_uint_rgba_row() format conversion >> function >> mesa: Add non-normalized formats support for ubyte packing >> functions >> mesa/format_pack: Add _mesa_pack_int_rgba_row() >> mesa/formats: add new mesa formats and their pack/unpack >> functions. >> mesa: use format conversion functions in swrast >> mesa/pack: use autogenerated format_pack functions >> mesa/main/pack_tmp.h: Add float conversion support >> mesa/pack: refactor _mesa_pack_rgba_span_float() >> >> src/mesa/Makefile.am | 18 + >> src/mesa/Makefile.sources | 4 +- >> src/mesa/main/colormac.h | 3 - >> src/mesa/main/format_convert.py | 71 + >> src/mesa/main/format_info.py | 41 + >> src/mesa/main/format_pack.c | 2994 >> ------------------------ >> src/mesa/main/format_pack.c.mako | 1147 ++++++++++ >> src/mesa/main/format_pack.h | 6 + >> src/mesa/main/format_unpack.c | 4400 >> ------------------------------------ >> src/mesa/main/format_unpack.c.mako | 914 ++++++++ >> src/mesa/main/format_utils.c | 248 +- >> src/mesa/main/format_utils.h | 105 + >> src/mesa/main/formats.c | 193 +- >> src/mesa/main/formats.csv | 13 + >> src/mesa/main/formats.h | 73 + >> src/mesa/main/pack.c | 2111 +++-------------- >> src/mesa/main/pack_tmp.h | 76 +- >> src/mesa/main/run_mako.py | 7 + >> src/mesa/main/teximage.c | 4 +- >> src/mesa/main/texstore.c | 2 +- >> src/mesa/swrast/s_drawpix.c | 3 - >> src/mesa/swrast/s_texfetch.c | 13 + >> src/mesa/swrast/s_texfetch_tmp.h | 1359 +---------- >> 23 files changed, 3222 insertions(+), 10583 deletions(-) >> create mode 100644 src/mesa/main/format_convert.py >> delete mode 100644 src/mesa/main/format_pack.c >> create mode 100644 src/mesa/main/format_pack.c.mako >> delete mode 100644 src/mesa/main/format_unpack.c >> create mode 100644 src/mesa/main/format_unpack.c.mako >> create mode 100644 src/mesa/main/run_mako.py >> >> -- >> 1.9.1 >> >> _______________________________________________ >> mesa-dev mailing list >> mesa-dev@lists.freedesktop.org >> 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 > -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJUbF63AAoJEH/0ujLxfcNDFBwQALGKfvU7Kt7I/+KGus1URYCl fkJ6ivXx3+b/wnZRKfsrnpBVXS6wpze0TiODR1pLnD8Dft+c3t0QL33fKDtPBeO7 r8w4C5yZJ+twvZrWQTskbnC+Q+2H4woFCcYL3VbTvicghakHSoAIDG/0XpHZjZ/Z fI/EZiKXliRJGSM7yNAN2XC6QFeV02RfJzZkIdKI4hTfl4LhAx2RDnrtGLeSPBLK 5LU6v4KQZD2z0vtWkSNG2FJRdjahgpkQmwj7mR9prQ27MX7hRoSNepM5BleHComT Mybv0pHusVXG9oXo7Dh/P+Vru51rjtixPEg9+81C7bURLE3gZRnqlcZKXK/b98tE svbq723suW6sDTgAPBkrOXe6rhvjLGzLoyg+kcXtpg1Z6/JQ8vf+gFG6WplBdgSO RP48t2ERMZCBvppYodZ2I9ZvYgq9FwyGkBJfANyPA3EQ9k1/ZuPTPHXyGEODBDln wsRDN9Oh4UjaqdyzV9jEOIAXbSwHAtx73njUu+F61IGTUKR8y1K3FxL+IUhGKxKJ GopbEzxIG10flgSVkXUmz8cFmUzLBVYFod9lYcoEczpQxWQvXlZbe2mb+lN/6BDR ETNxWo6slzfFYd+/T+iYxeZaUmBpMQ/buRZUn4In9Kgpqqjhre1UbrfE58zoXPOW tOMxMZLKAQtOqzOoHhUA =pw4v -----END PGP SIGNATURE----- _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
