[Mesa-dev] [PATCH 00/20] Auto-generate pack/unpack functions

Tue Nov 18 13:41:06 PST 2014

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.

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.

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.
--Jason

On Tue, Nov 18, 2014 at 12:43 AM, Iago Toral Quiroga <itoral at 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 at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20141118/2af0705f/attachment-0001.html>