<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Tue, Mar 6, 2018 at 5:06 AM, Pohjolainen, Topi <<a href="mailto:topi.pohjolainen@gmail.com" target="_blank">topi.pohjolainen@gmail.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Fri, Jan 26, 2018 at 05:59:51PM -0800, Jason Ekstrand wrote: > This adds helpers to ISL to convert an isl_color_value to and from > binary data encoded with a given isl_format. The conversion is done > using ISL's built-in format introspection so it's fairly slow as format > conversions go but it should be fine for a single pixel value. In > particular, we can use this to convert clear colors. > > As a side-effect, we now rely on the sRGB helpers in libmesautil so we > need to tweak the build system a bit. All prior uses of src/util in ISL > were header-only. > --- > src/intel/<a href="http://Makefile.compiler.am" rel="noreferrer" target="_blank">Makefile.compiler.am</a> | 2 +- > src/intel/<a href="http://Makefile.isl.am" rel="noreferrer" target="_blank">Makefile.isl.am</a> | 1 + > src/intel/isl/isl.h | 7 ++ > src/intel/isl/isl_format.c | 214 +++++++++++++++++++++++++++++++++++++++++ > src/intel/isl/meson.build | 2 +- > 5 files changed, 224 insertions(+), 2 deletions(-) > > diff --git a/src/intel/<a href="http://Makefile.compiler.am" rel="noreferrer" target="_blank">Makefile.compiler.am</a> b/src/intel/<a href="http://Makefile.compiler.am" rel="noreferrer" target="_blank">Makefile.compiler.am</a> > index 45e7a6c..d5b9920 100644 > --- a/src/intel/<a href="http://Makefile.compiler.am" rel="noreferrer" target="_blank">Makefile.compiler.am</a> > +++ b/src/intel/<a href="http://Makefile.compiler.am" rel="noreferrer" target="_blank">Makefile.compiler.am</a> > @@ -49,8 +49,8 @@ TEST_LIBS = \ > compiler/<a href="http://libintel_compiler.la" rel="noreferrer" target="_blank">libintel_compiler.la</a> \ > common/<a href="http://libintel_common.la" rel="noreferrer" target="_blank">libintel_common.la</a> \ > $(top_builddir)/src/compiler/nir/<a href="http://libnir.la" rel="noreferrer" target="_blank">libnir.la</a> \ > - $(top_builddir)/src/util/<a href="http://libmesautil.la" rel="noreferrer" target="_blank">libmesautil.la</a> \ > $(top_builddir)/src/intel/isl/<a href="http://libisl.la" rel="noreferrer" target="_blank">libisl.la</a> \ > + $(top_builddir)/src/util/<a href="http://libmesautil.la" rel="noreferrer" target="_blank">libmesautil.la</a> \ > $(PTHREAD_LIBS) \ > $(DLOPEN_LIBS) > > diff --git a/src/intel/<a href="http://Makefile.isl.am" rel="noreferrer" target="_blank">Makefile.isl.am</a> b/src/intel/<a href="http://Makefile.isl.am" rel="noreferrer" target="_blank">Makefile.isl.am</a> > index 31273af..8e2cf20 100644 > --- a/src/intel/<a href="http://Makefile.isl.am" rel="noreferrer" target="_blank">Makefile.isl.am</a> > +++ b/src/intel/<a href="http://Makefile.isl.am" rel="noreferrer" target="_blank">Makefile.isl.am</a> > @@ -78,6 +78,7 @@ TESTS += $(check_PROGRAMS) > isl_tests_isl_surf_get_image_offset_test_LDADD = \ > common/<a href="http://libintel_common.la" rel="noreferrer" target="_blank">libintel_common.la</a> \ > isl/<a href="http://libisl.la" rel="noreferrer" target="_blank">libisl.la</a> \ > + $(top_builddir)/src/util/<a href="http://libmesautil.la" rel="noreferrer" target="_blank">libmesautil.la</a> \ > -lm > > # ---------------------------------------------------------------------------- > diff --git a/src/intel/isl/isl.h b/src/intel/isl/isl.h > index 04d0f0b..4806c0c 100644 > --- a/src/intel/isl/isl.h > +++ b/src/intel/isl/isl.h > @@ -1535,6 +1535,13 @@ enum isl_format isl_format_rgb_to_rgba(enum isl_format rgb) ATTRIBUTE_CONST; > enum isl_format isl_format_rgb_to_rgbx(enum isl_format rgb) ATTRIBUTE_CONST; > enum isl_format isl_format_rgbx_to_rgba(enum isl_format rgb) ATTRIBUTE_CONST; > > +void isl_color_value_pack(const union isl_color_value *value, > + enum isl_format format, > + uint32_t *data_out); > +void isl_color_value_unpack(union isl_color_value *value, > + enum isl_format format, > + const uint32_t *data_in); > + > bool isl_is_storage_image_format(enum isl_format fmt); > > enum isl_format > diff --git a/src/intel/isl/isl_format.c b/src/intel/isl/isl_format.c > index 46af175..793c84b 100644 > --- a/src/intel/isl/isl_format.c > +++ b/src/intel/isl/isl_format.c > @@ -24,8 +24,17 @@ > #include <assert.h> > > #include "isl.h" > +#include "isl_priv.h" > #include "common/gen_device_info.h" > > +#include "main/macros.h" /* Needed for MAX3 and MAX2 for format_rgb9e5 */ > +#include "util/format_srgb.h" > +#include "util/format_rgb9e5.h" > +#include "util/format_r11g11b10f.h" > + > +/* Header-only format conversion include */ > +#include "main/format_utils.h" > + > struct surface_format_info { > bool exists; > uint8_t sampling; > @@ -806,3 +815,208 @@ isl_format_rgbx_to_rgba(enum isl_format rgbx) > return rgbx; > } > } > + > +static inline void > +pack_channel(const union isl_color_value *value, unsigned i, > + const struct isl_channel_layout *layout, > + enum isl_colorspace colorspace, > + uint32_t data_out[4]) > +{ > + if (layout->type == ISL_VOID) > + return; > + > + if (colorspace == ISL_COLORSPACE_SRGB) > + assert(layout->type == ISL_UNORM); > + > + uint32_t packed; > + switch (layout->type) { > + case ISL_UNORM: > + if (colorspace == ISL_COLORSPACE_SRGB) { > + if (layout->bits == 8) { > + packed = util_format_linear_float_to_srgb_8unorm(value->f32[i]); > + } else { > + float srgb = util_format_linear_to_srgb_float(value->f32[i]); > + packed = _mesa_float_to_unorm(srgb, layout->bits); > + } > + } else { > + packed = _mesa_float_to_unorm(value->f32[i], layout->bits); > + } > + break; > + case ISL_SNORM: > + packed = _mesa_float_to_snorm(value->f32[i], layout->bits); > + break; > + case ISL_SFLOAT: > + assert(layout->bits == 16 || layout->bits == 32); > + if (layout->bits == 16) { > + packed = _mesa_float_to_half(value->f32[i]); > + } else { > + packed = value->u32[i]; > + } > + break; > + case ISL_UINT: > + packed = MIN(value->u32[i], MAX_UINT(layout->bits)); > + break; > + case ISL_SINT: > + packed = MIN(MAX(value->u32[i], MIN_INT(layout->bits)), > + MAX_INT(layout->bits)); > + break; > + > + default: > + unreachable("Invalid channel type"); > + } > + > + unsigned dword = layout->start_bit / 32; > + unsigned bit = layout->start_bit % 32; > + assert(bit + layout->bits <= 32); > + data_out[dword] |= (packed & MAX_UINT(layout->bits)) << bit; > +} > + > +/** > + * Take an isl_color_value and pack it into the actual bits as specified by > + * the isl_format. This function is very slow for a format conversion > + * function but should be fine for a single pixel worth of data. > + */ > +void > +isl_color_value_pack(const union isl_color_value *value, > + enum isl_format format, > + uint32_t *data_out) > +{ > + const struct isl_format_layout *fmtl = isl_format_get_layout(format); > + assert(fmtl->colorspace == ISL_COLORSPACE_LINEAR || > + fmtl->colorspace == ISL_COLORSPACE_SRGB); > + assert(!isl_format_is_compressed(format)); > + > + memset(data_out, 0, isl_align(fmtl->bpb, 32) / 8); > + > + if (format == ISL_FORMAT_R9G9B9E5_SHAREDEXP) { > + data_out[0] = float3_to_rgb9e5(value->f32); > + return; > + } else if (format == ISL_FORMAT_R11G11B10_FLOAT) { > + data_out[0] = float3_to_r11g11b10f(value->f32); > + return; > + } > + > + pack_channel(value, 0, &fmtl->channels.r, fmtl->colorspace, data_out); > + pack_channel(value, 1, &fmtl->channels.g, fmtl->colorspace, data_out); > + pack_channel(value, 2, &fmtl->channels.b, fmtl->colorspace, data_out); > + pack_channel(value, 3, &fmtl->channels.a, ISL_COLORSPACE_LINEAR, data_out); > + pack_channel(value, 0, &fmtl->channels.l, fmtl->colorspace, data_out); > + pack_channel(value, 0, &fmtl->channels.i, ISL_COLORSPACE_LINEAR, data_out); > + assert(fmtl->channels.p.bits == 0); > +} > + > +/** Extend an N-bit signed integer to 32 bits */ > +static inline int32_t > +sign_extend(int32_t x, unsigned bits) > +{ > + if (bits < 32) { > + unsigned shift = 32 - bits; > + return (x << shift) >> shift; > + } else { > + return x; > + } > +} > + > +static inline void > +unpack_channel(union isl_color_value *value, > + unsigned start, unsigned count, > + const struct isl_channel_layout *layout, > + enum isl_colorspace colorspace, > + const uint32_t *data_in) > +{ > + if (layout->type == ISL_VOID) > + return; > + > + unsigned dword = layout->start_bit / 32; > + unsigned bit = layout->start_bit % 32; > + assert(bit + layout->bits <= 32); > + uint32_t packed = (data_in[dword] >> bit) & MAX_UINT(layout->bits); > + > + union { > + uint32_t u32; > + float f32; > + } unpacked; > + > + if (colorspace == ISL_COLORSPACE_SRGB) > + assert(layout->type == ISL_UNORM); > + > + switch (layout->type) { > + case ISL_UNORM: > + unpacked.f32 = _mesa_unorm_to_float(packed, layout->bits); > + if (colorspace == ISL_COLORSPACE_SRGB) { > + if (layout->bits == 8) { > + unpacked.f32 = util_format_srgb_8unorm_to_linear_float(packed); > + } else { > + float srgb = _mesa_unorm_to_float(packed, layout->bits); > + unpacked.f32 = util_format_srgb_to_linear_float(srgb); > + } > + } else { > + unpacked.f32 = _mesa_unorm_to_float(packed, layout->bits); > + } > + break; > + case ISL_SNORM: > + unpacked.f32 = _mesa_snorm_to_float(sign_extend(packed, layout->bits), > + layout->bits); > + break; > + case ISL_SFLOAT: > + assert(layout->bits == 16 || layout->bits == 32); > + if (layout->bits == 16) { > + unpacked.f32 = _mesa_half_to_float(packed); > + } else { > + unpacked.u32 = packed; > + } > + break; > + case ISL_UINT: > + unpacked.u32 = packed; > + break; > + case ISL_SINT: > + unpacked.u32 = sign_extend(packed, layout->bits); > + break; > + > + default: > + unreachable("Invalid channel type"); > + } > + > + for (unsigned i = 0; i < count; i++) > + value->u32[start + i] = unpacked.u32; > +} > + > +/** > + * Take unpack an isl_color_value from the actual bits as specified by > + * the isl_format. This function is very slow for a format conversion > + * function but should be fine for a single pixel worth of data. > + */ > +void > +isl_color_value_unpack(union isl_color_value *value, > + enum isl_format format, > + const uint32_t data_in[4]) > +{ > + const struct isl_format_layout *fmtl = isl_format_get_layout(format); > + assert(fmtl->colorspace == ISL_COLORSPACE_LINEAR || > + fmtl->colorspace == ISL_COLORSPACE_SRGB); > + assert(!isl_format_is_compressed(format)); > + > + /* Default to opaque black. */ > + memset(value, 0, sizeof(*value)); > + if (isl_format_has_int_channel(format)) { > + value->u32[3] = 1u; > + } else { > + value->f32[3] = 1.0f; > + } > + > + if (format == ISL_FORMAT_R9G9B9E5_SHAREDEXP) { > + rgb9e5_to_float3(data_in[0], value->f32); > + return; > + } else if (format == ISL_FORMAT_R11G11B10_FLOAT) { > + r11g11b10f_to_float3(data_in[0], value->f32); > + return; > + } > + > + unpack_channel(value, 0, 1, &fmtl->channels.r, fmtl->colorspace, data_in); > + unpack_channel(value, 1, 1, &fmtl->channels.g, fmtl->colorspace, data_in); > + unpack_channel(value, 2, 1, &fmtl->channels.b, fmtl->colorspace, data_in); > + unpack_channel(value, 3, 1, &fmtl->channels.a, ISL_COLORSPACE_LINEAR, data_in); > + unpack_channel(value, 0, 3, &fmtl->channels.l, fmtl->colorspace, data_in); > + unpack_channel(value, 0, 4, &fmtl->channels.i, ISL_COLORSPACE_LINEAR, data_in); </div></div>I guess I don't know enough about formats to understand why we leave alpha channel intact for luminance but trash it in case of intensity? </blockquote><div> </div><div>Luminance effectively has a swizzle of RRR1 but intensity is RRRR. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> Other than that this looks good to me. Patches 22 and 23 are: Reviewed-by: Topi Pohjolainen <<a href="mailto:topi.pohjolainen@intel.com">topi.pohjolainen@intel.com</a>> > + assert(fmtl->channels.p.bits == 0); > +} > diff --git a/src/intel/isl/meson.build b/src/intel/isl/meson.build > index 0838c32..e9ec5ba 100644 > --- a/src/intel/isl/meson.build > +++ b/src/intel/isl/meson.build > @@ -95,7 +95,7 @@ if with_tests > 'tests/isl_surf_get_image_offset_test.c', > dependencies : dep_m, > include_directories : [inc_common, inc_intel], > - link_with : [libisl, libintel_common], > + link_with : [libisl, libintel_common, libmesa_util], > ) > ) > endif > -- > 2.5.0.400.gff86faf > > _______________________________________________ > mesa-dev mailing list > <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> > <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </blockquote></div> </div></div>