On Jan 8, 2015 11:54 PM, "Samuel Iglesias Gonsálvez" <<a href="mailto:siglesias@igalia.com">siglesias@igalia.com</a>> wrote: > > On Thursday, January 08, 2015 10:13:11 AM Jason Ekstrand wrote: > > On Wed, Jan 7, 2015 at 11:20 PM, Iago Toral Quiroga <<a href="mailto:itoral@igalia.com">itoral@igalia.com</a>> > > > > wrote: > > > From: Samuel Iglesias Gonsalvez <<a href="mailto:siglesias@igalia.com">siglesias@igalia.com</a>> > > > > > > This will be used to unify code in pack.c. > > > > > > v2: > > > - Modify pack_int_*() function generator to use c.datatype() and > > > > > > f.datatype() > > > > > > Signed-off-by: Samuel Iglesias Gonsalvez <<a href="mailto:siglesias@igalia.com">siglesias@igalia.com</a>> > > > --- > > > > > > src/mesa/main/format_pack.h | 3 ++ > > > src/mesa/main/format_pack.py | 121 > > > > > > +++++++++++++++++++++++++++++++++++++++++++ > > > > > > 2 files changed, 124 insertions(+) > > > > > > diff --git a/src/mesa/main/format_pack.h b/src/mesa/main/format_pack.h > > > index 1582ad1..6087fc3 100644 > > > --- a/src/mesa/main/format_pack.h > > > +++ b/src/mesa/main/format_pack.h > > > @@ -68,6 +68,9 @@ extern gl_pack_ubyte_stencil_func > > > > > > _mesa_get_pack_ubyte_stencil_func(mesa_format format); > > > > > > +extern void > > > +_mesa_pack_int_rgba_row(mesa_format format, GLuint n, > > > + const GLint src[][4], void *dst); > > > > > > extern void > > > _mesa_pack_float_rgba_row(mesa_format format, GLuint n, > > > > > > diff --git a/src/mesa/main/format_pack.py b/src/mesa/main/format_pack.py > > > index 3240d24..8d605be 100644 > > > --- a/src/mesa/main/format_pack.py > > > +++ b/src/mesa/main/format_pack.py > > > @@ -213,6 +213,99 @@ pack_uint_${f.short_name()}(const GLuint src[4], void > > > *dst) > > > > > > } > > > %endfor > > > > > > +/* int packing functions */ > > > + > > > +%for f in rgb_formats: > > > + %if <a href="http://f.name">f.name</a> in ('MESA_FORMAT_R9G9B9E5_FLOAT', > > > 'MESA_FORMAT_R11G11B10_FLOAT'): > > > + <% continue %> > > > + %elif f.is_compressed(): > > > + <% continue %> > > > + %endif > > > > Aren't the integer pack/unpack functions for non-normalized integer formats > > only? I think this batch of functions needs to be reworked. Bunch of > > comments below. I'm sorry if I haven't noticed and commented on this > > earlier. > > --Jason > > > > Ok, no problem. I did not notice too because later we remove this function. We do? Sorry, I must have missed that. > > I will fix all the issues and send a new version to the mailing list as > suggested. > > Thanks for your review, > > Sam > > > > + > > > +static inline void > > > +pack_int_${f.short_name()}(const GLint src[4], void *dst) > > > +{ > > > + %for (i, c) in enumerate(f.channels): > > > + <% i = f.swizzle.inverse()[i] %> > > > + %if c.type == 'x': > > > + <% continue %> > > > + %endif > > > + > > > + ${c.datatype()} ${<a href="http://c.name">c.name</a>} = > > > + %if not f.is_normalized(): > > > + %if c.type == parser.FLOAT and c.size == 32: > > > + INT_TO_FLOAT(src[${i}]); > > > + %elif c.type == parser.FLOAT and c.size == 16: > > > + _mesa_float_to_half(INT_TO_FLOAT(src[${i}])); > > > > Why are we converting to float for a non-normalized destination? > > > > > + %else: > > > + (${c.datatype()}) src[${i}]; > > > + %endif > > > + %elif c.type == parser.UNSIGNED: > > > + %if f.colorspace == 'srgb' and <a href="http://c.name">c.name</a> in 'rgb': > > > + util_format_linear_to_srgb_8unorm(src[${i}]); > > > > We shouldn't have SRGB for non-normlized formats > > > > > + %else: > > > + CLAMP(src[${i}], 0, MAX_UINT(${c.size})); > > > > We have signed_to_unsigned etc. functions for this. If not, we should add > > them. > > > > > + %endif > > > + %elif c.type == parser.SIGNED: > > > + CLAMP(src[${i}], 0, MAX_UINT(${c.size})); > > > > Same here. > > > > > + %elif c.type == parser.FLOAT: > > > + %if c.size == 32: > > > + _mesa_snorm_to_float(src[${i}], 8); > > > + %elif c.size == 16: > > > + _mesa_snorm_to_half(src[${i}], 8); > > > > Again, why are we doing snorm? > > > > > + %else: > > > + <% assert False %> > > > + %endif > > > + %else: > > > + <% assert False %> > > > + %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}] = ${<a href="http://c.name">c.name</a>}; > > > + %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(${<a href="http://c.name">c.name</a>}, ${c.shift}, ${c.size}); > > > + %endfor > > > + (*(${f.datatype()} *)dst) = d; > > > + %else: > > > + <% assert False %> > > > + %endif > > > +} > > > +%endfor > > > + > > > +static inline void > > > +pack_int_r9g9b9e5_float(const GLint src[4], void *dst) > > > +{ > > > + GLuint *d = (GLuint *) dst; > > > + GLfloat rgb[3]; > > > + rgb[0] = _mesa_snorm_to_float(src[RCOMP], 8); > > > + rgb[1] = _mesa_snorm_to_float(src[GCOMP], 8); > > > + rgb[2] = _mesa_snorm_to_float(src[BCOMP], 8); > > > + *d = float3_to_rgb9e5(rgb); > > > +} > > > + > > > +static inline void > > > +pack_int_r11g11b10_float(const GLint src[4], void *dst) > > > +{ > > > + GLuint *d = (GLuint *) dst; > > > + GLfloat rgb[3]; > > > + rgb[0] = _mesa_snorm_to_float(src[RCOMP], 8); > > > + rgb[1] = _mesa_snorm_to_float(src[GCOMP], 8); > > > + rgb[2] = _mesa_snorm_to_float(src[BCOMP], 8); > > > + *d = float3_to_r11g11b10f(rgb); > > > +} > > > + > > > > > > /* float packing functions */ > > > > > > %for f in rgb_formats: > > > @@ -396,6 +489,34 @@ _mesa_pack_uint_rgba_row(mesa_format format, GLuint > > > n, > > > > > > } > > > > > > /** > > > > > > + * Pack a row of GLint rgba[4] values to the destination. > > > + */ > > > +void > > > +_mesa_pack_int_rgba_row(mesa_format format, GLuint n, > > > + const GLint src[][4], void *dst) > > > +{ > > > + GLuint i; > > > + GLubyte *d = dst; > > > + > > > + switch (format) { > > > +%for f in rgb_formats: > > > + %if f.is_compressed(): > > > + <% continue %> > > > + %endif > > > + > > > + case ${<a href="http://f.name">f.name</a>}: > > > + for (i = 0; i < n; ++i) { > > > + pack_int_${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 > > > > > > -- > > > 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>