[Mesa-dev] [PATCH v2 16/23] mesa: Add _mesa_pack_uint_rgba_row() format conversion function

[Iago Toral]

On Mon, 2014-12-01 at 11:14 -0800, Jason Ekstrand wrote:
> 
> 
> On Mon, Dec 1, 2014 at 3:04 AM, Iago Toral Quiroga <itoral at igalia.com>
> wrote:
>         From: Samuel Iglesias Gonsalvez <siglesias at igalia.com>
>         
>         We will use this later on to handle uint conversion scenarios
>         in a master
>         convert function.
>         
>         v2:
>         - Modify pack_uint_*() function generation to use c.datatype()
>         and
>           f.datatype().
>         - Remove UINT_TO_FLOAT() macro usage from pack_uint*()
>         - Remove "if not f.is_normalized()" conditional as
>         pack_uint*()
>           functions are only autogenerated for non normalized formats.
>         
>         Signed-off-by: Samuel Iglesias Gonsalvez
>         <siglesias at igalia.com>
>         ---
>          src/mesa/main/format_pack.c.mako | 82
>         ++++++++++++++++++++++++++++++++++++++++
>          src/mesa/main/format_pack.h      |  3 ++
>          2 files changed, 85 insertions(+)
>         
>         diff --git a/src/mesa/main/format_pack.c.mako
>         b/src/mesa/main/format_pack.c.mako
>         index aced58d..7feb3f8 100644
>         --- a/src/mesa/main/format_pack.c.mako
>         +++ b/src/mesa/main/format_pack.c.mako
>         @@ -149,6 +149,56 @@ pack_ubyte_r11g11b10_float(const GLubyte
>         src[4], void *dst)
>             *d = float3_to_r11g11b10f(rgb);
>          }
>         
>         +/* uint packing functions */
>         +
>         +%for f in rgb_formats:
>         +   %if not f.is_int():
>         +      <% continue %>
>         +   %elif f.is_normalized():
>         +      <% continue %>
>         +   %elif f.is_compressed():
>         +      <% continue %>
>         +   %endif
>         +
>         +static inline void
>         +pack_uint_${f.short_name()}(const GLuint src[4], void *dst)
>         +{
>         +   %for (i, c) in enumerate(f.channels):
>         +      <% i = f.swizzle.inverse()[i] %>
>         +      %if c.type == 'x':
>         +         <% continue %>
>         +      %endif
>         +
>         +      ${c.datatype()} ${c.name} =
>         +      %if c.type == parser.FLOAT and c.size == 16:
>         +         _mesa_float_to_half(src[${i}]);

Jason, I think this float handling shouldn't be here.... we only allow
packing/unpacking between integer types, right? In that case maybe we
should add an assertion to catch these other cases.

>         +      %else:
>         +         (${c.datatype()}) src[${i}];
> 
> 
> I think we're missing the clamping here.  We should probably use the
> same clamping functions that we cooked up for swizzle_and_convert.
> We'll have to be careful though because the source gets interpreted as
> signed vs. unsigned depending on the destination format.

So you mean that we should use _mesa_unsigned_to_unsigned when c.type ==
parser.UNSIGNED and _mesa_signed_to_signed when c.type == parser.SIGNED.

If there is nothing wrong with what I suggest above, then I think this
is how it should look like in the end:

${c.datatype()} ${c.name} =
%if c.type == parser.SIGNED:
   _mesa_signed_to_signed(src[${i}], ${c.size});
%elif c.type == parser.UNSIGNED:      
   _mesa_unsigned_to_unsigned(src[${i}], ${c.size});
%else:
   assert(!"Invalid type: only integer types are allowed");
%endif

>  
>         +      %endif
>         +   %endfor
>         +
>         +   %if f.layout == parser.ARRAY:
>         +      ${f.datatype()} *d = (${f.datatype()} *)dst;
>         +      %for (i, c) in enumerate(f.channels):
>         +         %if c.type == 'x':
>         +            <% continue %>
>         +         %endif
>         +         d[${i}] = ${c.name};
>         +      %endfor
>         +   %elif f.layout == parser.PACKED:
>         +      ${f.datatype()} d = 0;
>         +      %for (i, c) in enumerate(f.channels):
>         +         %if c.type == 'x':
>         +            <% continue %>
>         +         %endif
>         +         d |= PACK(${c.name}, ${c.shift}, ${c.size});
>         +      %endfor
>         +      (*(${f.datatype()} *)dst) = d;
>         +   %else:
>         +      <% assert False %>
>         +   %endif
>         +}
>         +%endfor
>         
>          /* float packing functions */
>         
>         @@ -297,6 +347,38 @@ _mesa_pack_ubyte_rgba_row(mesa_format
>         format, GLuint n,
>          }
>         
>          /**
>         + * Pack a row of GLuint rgba[4] values to the destination.
>         + */
>         +void
>         +_mesa_pack_uint_rgba_row(mesa_format format, GLuint n,
>         +                          const GLuint src[][4], void *dst)
>         +{
>         +   GLuint i;
>         +   GLubyte *d = dst;
>         +
>         +   switch (format) {
>         +%for f in rgb_formats:
>         +   %if not f.is_int():
>         +      <% continue %>
>         +   %elif f.is_normalized():
>         +      <% continue %>
>         +   %elif f.is_compressed():
>         +      <% continue %>
>         +   %endif
>         +
>         +   case ${f.name}:
>         +      for (i = 0; i < n; ++i) {
>         +         pack_uint_${f.short_name()}(src[i], d);
>         +         d += ${f.block_size() / 8};
>         +      }
>         +      break;
>         +%endfor
>         +   default:
>         +      assert(!"Invalid format");
>         +   }
>         +}
>         +
>         +/**
>           * Pack a row of GLfloat rgba[4] values to the destination.
>           */
>          void
>         diff --git a/src/mesa/main/format_pack.h
>         b/src/mesa/main/format_pack.h
>         index 2577def..1582ad1 100644
>         --- a/src/mesa/main/format_pack.h
>         +++ b/src/mesa/main/format_pack.h
>         @@ -77,6 +77,9 @@ extern void
>          _mesa_pack_ubyte_rgba_row(mesa_format format, GLuint n,
>                                    const GLubyte src[][4], void *dst);
>         
>         +extern void
>         +_mesa_pack_uint_rgba_row(mesa_format format, GLuint n,
>         +                         const GLuint src[][4], void *dst);
>         
>          extern void
>          _mesa_pack_ubyte_rgba_rect(mesa_format format, GLuint width,
>         GLuint height,
>         --
>         1.9.1
>         
>         _______________________________________________
>         mesa-dev mailing list
>         mesa-dev at lists.freedesktop.org
>         http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 
>