<div dir="ltr"><div><div><div><div>Iago, </div>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. </div>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. </div>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. </div>--Jason </div><div class="gmail_extra"> <div class="gmail_quote">On Tue, Nov 18, 2014 at 12:43 AM, Iago Toral Quiroga <<a href="mailto:itoral@igalia.com" target="_blank">itoral@igalia.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">This is the fist of two series of patches to address: <a href="https://bugs.freedesktop.org/show_bug.cgi?id=84566" target="_blank">https://bugs.freedesktop.org/show_bug.cgi?id=84566</a> 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 <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> <a href="http://lists.freedesktop.org/mailman/listinfo/mesa-dev" target="_blank">http://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </blockquote></div> </div>