[Mesa-dev] [PATCH 01/29] mesa: Add an implementation of a master convert function.
Iago Toral
itoral at igalia.com
Thu Nov 20 01:48:02 PST 2014
On Wed, 2014-11-19 at 11:28 -0800, Jason Ekstrand wrote:
> By and large, this looks good to me. Most of my comments are cosmetic
> or suggestions for added documentation. There is one issue that I
> think is subtly wrong with integer format conversion but that should
> be easy to fix.
>
> --Jason
>
> On Tue, Nov 18, 2014 at 1:23 AM, Iago Toral Quiroga
> <itoral at igalia.com> wrote:
> From: Jason Ekstrand <jason.ekstrand at intel.com>
>
> v2 by Iago Toral <itoral at igalia.com>:
>
> - When testing if we can directly pack we should use the src
> format to check
> if we are packing from an RGBA format. The original code
> used the dst format
> for the ubyte case by mistake.
> - Fixed incorrect number of bits for dst, it was computed
> using the src format
> instead of the dst format.
> - If the dst format is an array format, check if it is signed.
> We were only
> checking this for the case where it was not an array format,
> but we need
> to know this in both scenarios.
> - Fixed incorrect swizzle transform for the cases where we
> convert between
> array formats.
> - Compute is_signed and bits only once and for the dst format.
> We were
> computing these for the src format too but they were
> overwritten by the
> dst values immediately after.
> - Be more careful when selecting the integer path.
> Specifically, check that
> both src and dst are integer types. Checking only one of
> them should suffice
> since OpenGL does not allow conversions between normalized
> and integer types,
> but putting extra care here makes sense and also makes the
> actual requirements
> for this path more clear.
> - The format argument for pack functions is the destination
> format we are
> packing to, not the source format (which has to be RGBA).
> - Expose RGBA8888_* to other files. These will come in handy
> when in need to
> test if a given array format is RGBA or in need to pass RGBA
> formats to
> mesa_format_convert.
>
> v3 by Samuel Iglesias <siglesias at igalia.com>:
>
> - Add an RGBA8888_INT definition.
> ---
> src/mesa/main/format_utils.c | 378
> +++++++++++++++++++++++++++++++++++++++++++
> src/mesa/main/format_utils.h | 10 ++
> src/mesa/main/formats.h | 15 +-
> 3 files changed, 399 insertions(+), 4 deletions(-)
>
> diff --git a/src/mesa/main/format_utils.c
> b/src/mesa/main/format_utils.c
> index fcbbba4..c3815cb 100644
> --- a/src/mesa/main/format_utils.c
> +++ b/src/mesa/main/format_utils.c
> @@ -25,6 +25,384 @@
> #include "format_utils.h"
> #include "glformats.h"
> #include "macros.h"
> +#include "format_pack.h"
> +#include "format_unpack.h"
> +
> +mesa_array_format RGBA8888_FLOAT = {{
> + MESA_ARRAY_FORMAT_TYPE_FLOAT,
> + 0,
> + 4,
> + 0, 1, 2, 3,
> + 0, 1
> +}};
> +
> +mesa_array_format RGBA8888_UBYTE = {{
> + MESA_ARRAY_FORMAT_TYPE_UBYTE,
> + 1,
> + 4,
> + 0, 1, 2, 3,
> + 0, 1
> +}};
> +
> +mesa_array_format RGBA8888_UINT = {{
> + MESA_ARRAY_FORMAT_TYPE_UINT,
> + 0,
> + 4,
> + 0, 1, 2, 3,
> + 0, 1
> +}};
> +
> +mesa_array_format RGBA8888_INT = {{
> + MESA_ARRAY_FORMAT_TYPE_INT,
> + 0,
> + 4,
> + 0, 1, 2, 3,
> + 0, 1
> +}};
> +
> +static void
> +invert_swizzle(uint8_t dst[4], const uint8_t src[4])
> +{
> + int i, j;
> +
> + dst[0] = MESA_FORMAT_SWIZZLE_NONE;
> + dst[1] = MESA_FORMAT_SWIZZLE_NONE;
> + dst[2] = MESA_FORMAT_SWIZZLE_NONE;
> + dst[3] = MESA_FORMAT_SWIZZLE_NONE;
> +
> + for (i = 0; i < 4; ++i)
> + for (j = 0; j < 4; ++j)
> + if (src[j] == i && dst[i] ==
> MESA_FORMAT_SWIZZLE_NONE)
> + dst[i] = j;
> +}
> +
> +static GLenum
> +gl_type_for_array_format_datatype(enum
> mesa_array_format_datatype type)
> +{
> + switch (type) {
> + case MESA_ARRAY_FORMAT_TYPE_UBYTE:
> + return GL_UNSIGNED_BYTE;
> + case MESA_ARRAY_FORMAT_TYPE_USHORT:
> + return GL_UNSIGNED_SHORT;
> + case MESA_ARRAY_FORMAT_TYPE_UINT:
> + return GL_UNSIGNED_INT;
> + case MESA_ARRAY_FORMAT_TYPE_BYTE:
> + return GL_BYTE;
> + case MESA_ARRAY_FORMAT_TYPE_SHORT:
> + return GL_SHORT;
> + case MESA_ARRAY_FORMAT_TYPE_INT:
> + return GL_INT;
> + case MESA_ARRAY_FORMAT_TYPE_HALF:
> + return GL_HALF_FLOAT;
> + case MESA_ARRAY_FORMAT_TYPE_FLOAT:
> + return GL_FLOAT;
> + default:
> + assert(!"Invalid datatype");
> + return GL_NONE;
> + }
> +}
>
>
> We should probably just make _mesa_swizzle_and_convert take these
> instead of the GL types. That way we can completely remove GL from
> the format conversion code. I'm fine if that's a separate patch on
> top.
Ok, I will try this.
Just out of curiosity: is there any gain in avoiding the GL types in the
conversion code?
>
> Also, it would be good to have a nice descriptive docstring on the
> function below.
>
Oh, right!
>
> +
> +void
> +_mesa_format_convert(void *void_dst, uint32_t dst_format,
> size_t dst_stride,
> + void *void_src, uint32_t src_format,
> size_t src_stride,
> + size_t width, size_t height)
> +{
> + uint8_t *dst = (uint8_t *)void_dst;
> + uint8_t *src = (uint8_t *)void_src;
> + mesa_array_format src_array_format, dst_array_format;
> + uint8_t src2dst[4], src2rgba[4], rgba2dst[4], dst2rgba[4];
> + GLenum src_gl_type, dst_gl_type, common_gl_type;
> + bool normalized, dst_integer, src_integer, is_signed;
> + uint8_t (*tmp_ubyte)[4];
> + float (*tmp_float)[4];
> + uint32_t (*tmp_uint)[4];
> + int i, bits;
> + size_t row;
> +
> + if (src_format & MESA_ARRAY_FORMAT_BIT) {
> + src_array_format.as_uint = src_format;
> + } else {
> + assert(_mesa_is_format_color_format(src_format));
> + src_array_format.as_uint =
> _mesa_format_to_array_format(src_format);
> + }
> +
> + if (dst_format & MESA_ARRAY_FORMAT_BIT) {
> + dst_array_format.as_uint = dst_format;
> + } else {
> + assert(_mesa_is_format_color_format(dst_format));
> + dst_array_format.as_uint =
> _mesa_format_to_array_format(dst_format);
> + }
> +
> + /* Handle the cases where we can directly unpack */
> + if (!(src_format & MESA_ARRAY_FORMAT_BIT)) {
> + if (dst_array_format.as_uint == RGBA8888_FLOAT.as_uint)
> {
> + for (row = 0; row < height; ++row) {
> + _mesa_unpack_rgba_row(src_format, width,
> + src, (float (*)[4])dst);
> + src += src_stride;
> + dst += dst_stride;
> + }
> + return;
> + } else if (dst_array_format.as_uint ==
> RGBA8888_UBYTE.as_uint) {
> + assert(!_mesa_is_format_integer_color(src_format));
> + for (row = 0; row < height; ++row) {
> + _mesa_unpack_ubyte_rgba_row(src_format, width,
> + src, (uint8_t
> (*)[4])dst);
> + src += src_stride;
> + dst += dst_stride;
> + }
> + return;
> + } else if (dst_array_format.as_uint ==
> RGBA8888_UINT.as_uint) {
> + assert(_mesa_is_format_integer_color(src_format));
> + for (row = 0; row < height; ++row) {
> + _mesa_unpack_uint_rgba_row(src_format, width,
> + src, (uint32_t
> (*)[4])dst);
> + src += src_stride;
> + dst += dst_stride;
> + }
> + return;
> + }
> + }
> +
> + /* Handle the cases where we can directly pack */
> + if (!(dst_format & MESA_ARRAY_FORMAT_BIT)) {
> + if (src_array_format.as_uint == RGBA8888_FLOAT.as_uint)
> {
> + for (row = 0; row < height; ++row) {
> + _mesa_pack_float_rgba_row(dst_format, width,
> + (const float
> (*)[4])src, dst);
> + src += src_stride;
> + dst += dst_stride;
> + }
> + return;
> + } else if (src_array_format.as_uint ==
> RGBA8888_UBYTE.as_uint) {
> + assert(!_mesa_is_format_integer_color(dst_format));
> + for (row = 0; row < height; ++row) {
> + _mesa_pack_ubyte_rgba_row(dst_format, width,
> + (const uint8_t
> (*)[4])src, dst);
> + src += src_stride;
> + dst += dst_stride;
> + }
> + return;
> + } else if (src_array_format.as_uint ==
> RGBA8888_UINT.as_uint) {
> + assert(_mesa_is_format_integer_color(dst_format));
> + for (row = 0; row < height; ++row) {
> + _mesa_pack_uint_rgba_row(dst_format, width,
> + (const uint32_t
> (*)[4])src, dst);
> + src += src_stride;
> + dst += dst_stride;
> + }
> + return;
> + }
> + }
> +
> + /* Handle conversions between array formats */
> + normalized = false;
> + if (src_array_format.as_uint) {
> + src_gl_type =
> gl_type_for_array_format_datatype(src_array_format.type);
> +
> + src2rgba[0] = src_array_format.swizzle_x;
> + src2rgba[1] = src_array_format.swizzle_y;
> + src2rgba[2] = src_array_format.swizzle_z;
> + src2rgba[3] = src_array_format.swizzle_w;
> +
> + normalized = src_array_format.normalized;
> + }
> +
> + if (dst_array_format.as_uint) {
> + dst_gl_type =
> gl_type_for_array_format_datatype(dst_array_format.type);
> +
> + dst2rgba[0] = dst_array_format.swizzle_x;
> + dst2rgba[1] = dst_array_format.swizzle_y;
> + dst2rgba[2] = dst_array_format.swizzle_z;
> + dst2rgba[3] = dst_array_format.swizzle_w;
> +
> + invert_swizzle(rgba2dst, dst2rgba);
> +
> + normalized |= dst_array_format.normalized;
> + }
> +
> + if (src_array_format.as_uint && dst_array_format.as_uint)
> {
> + assert(src_array_format.normalized ==
> dst_array_format.normalized);
> +
> + for (i = 0; i < 4; i++) {
> + if (rgba2dst[i] > MESA_FORMAT_SWIZZLE_W) {
> + src2dst[i] = rgba2dst[i];
> + } else {
> + src2dst[i] = src2rgba[rgba2dst[i]];
> + }
> + }
> +
> + for (row = 0; row < height; ++row) {
> + _mesa_swizzle_and_convert(dst, dst_gl_type,
> dst_array_format.num_channels,
> + src, src_gl_type,
> src_array_format.num_channels,
> + src2dst, normalized,
> width);
> + src += src_stride;
> + dst += dst_stride;
> + }
> + return;
> + }
> +
> + /* At this point, we're fresh out of fast-paths and we
> need to convert
> + * to float, uint32, or, if we're lucky, uint8.
> + */
> + dst_integer = false;
> + src_integer = false;
> +
> + if (src_array_format.array_format_bit) {
> + if (!(src_array_format.type &
> MESA_ARRAY_FORMAT_TYPE_IS_FLOAT) &&
> + !src_array_format.normalized)
> + src_integer = true;
> + } else {
> + switch (_mesa_get_format_datatype(src_format)) {
> + case GL_UNSIGNED_INT:
> + case GL_INT:
> + src_integer = true;
> + break;
> + }
> + }
> +
> + if (dst_array_format.array_format_bit) {
> + if (!(dst_array_format.type &
> MESA_ARRAY_FORMAT_TYPE_IS_FLOAT) &&
> + !dst_array_format.normalized)
> + dst_integer = true;
> + is_signed = dst_array_format.as_uint &
> MESA_ARRAY_FORMAT_TYPE_IS_SIGNED;
> + bits = 8 *
> _mesa_array_format_datatype_size(dst_array_format.type);
> + } else {
>
>
> It's worth a quick comment here as to why we only need to look at the
> destination in order to determine the signedness of the intermediate
> storage. What it comes down to is this: If the destination format is
> signed but the source is unsigned, then we don't loose any data by
> converting to a signed format above and beyond the precision that we
> loose in the conversion itself. If the destination is unsigned then,
> by using an unsigned intermediate format, we make the conversion
> function that converts from the source to the intermediate format take
> care of truncating at zero. The exception here is if the intermediate
> format is float, in which case the first conversion will leave it
> signed and the second conversion will truncate at zero.
Sure, I'll add a comment.
>
> + switch (_mesa_get_format_datatype(dst_format)) {
> + case GL_UNSIGNED_NORMALIZED:
> + is_signed = false;
> + break;
> + case GL_SIGNED_NORMALIZED:
> + is_signed = true;
> + break;
> + case GL_FLOAT:
> + is_signed = true;
> + break;
> + case GL_UNSIGNED_INT:
> + is_signed = false;
> + dst_integer = true;
> + break;
> + case GL_INT:
> + is_signed = true;
> + dst_integer = true;
> + break;
> + }
> + bits = _mesa_get_format_max_bits(dst_format);
> + }
> +
> + assert((src_integer && dst_integer) || (!src_integer && !
> dst_integer));
>
>
> Wouldn't src_integer == dst_integer be a bit more clear here?
Right, I guess I was a bit thick when I wrote that :)
>
> +
> + if (src_integer && dst_integer) {
> + tmp_uint = malloc(width * height * sizeof(*tmp_uint));
>
>
> There's another quick comment that would be good here. Namely that
> the [un]packing functions for unsigned datatypes treat the 32-bit
> integer array as signed for signed formats and as unsigned for
> unsigned formats. This is a bit of a problem if we ever convert from
> a signed to an unsigned format because the unsigned packing function
> doesn't know that the input is signed and will treat it as unsigned
> and not do the trunctation. The thing that saves us here is that all
> of the packed formats are unsigned, so we can just always use
> _mesa_swizzle_and_convert for signed formats and it is aware of the
> truncation problem. This may be worth an assertion somewhere.
Sure, that's helpful.
>
>
> + common_gl_type = is_signed ? GL_INT : GL_UNSIGNED_INT;
> +
> + if (src_format & MESA_ARRAY_FORMAT_BIT) {
>
>
> In light of the above coment, I think we want to use "if
> (src_array_format)" to ensure that we use _mesa_swizzle_and_convert
> for all signed source formats
I will try this.
>
> + for (row = 0; row < height; ++row) {
> + _mesa_swizzle_and_convert(tmp_uint + row * width,
> common_gl_type, 4,
> + src, src_gl_type,
> +
> src_array_format.num_channels,
> + src2rgba, normalized,
> width);
> + src += src_stride;
> + }
> + } else {
> + for (row = 0; row < height; ++row) {
> + _mesa_unpack_uint_rgba_row(src_format, width,
> + src, tmp_uint + row *
> width);
> + src += src_stride;
> + }
> + }
> +
> + if (dst_format & MESA_ARRAY_FORMAT_BIT) {
>
>
> This one's fine because, at this point, we have already done the
> truncation if the source is signed but the destination is unsigned.
I'll add a commment to clarify this too.
>
> + for (row = 0; row < height; ++row) {
> + _mesa_swizzle_and_convert(dst, dst_gl_type,
> +
> dst_array_format.num_channels,
> + tmp_uint + row * width,
> common_gl_type, 4,
> + rgba2dst, normalized,
> width);
> + dst += dst_stride;
> + }
> + } else {
> + for (row = 0; row < height; ++row) {
> + _mesa_pack_uint_rgba_row(dst_format, width,
> + (const uint32_t
> (*)[4])tmp_uint + row * width, dst);
> + dst += dst_stride;
> + }
> + }
> +
> + free(tmp_uint);
> + } else if (is_signed || bits > 8) {
> + tmp_float = malloc(width * height *
> sizeof(*tmp_float));
> +
> + if (src_format & MESA_ARRAY_FORMAT_BIT) {
> + for (row = 0; row < height; ++row) {
> + _mesa_swizzle_and_convert(tmp_float + row *
> width, GL_FLOAT, 4,
> + src, src_gl_type,
> +
> src_array_format.num_channels,
> + src2rgba, normalized,
> width);
> + src += src_stride;
> + }
> + } else {
> + for (row = 0; row < height; ++row) {
> + _mesa_unpack_rgba_row(src_format, width,
> + src, tmp_float + row *
> width);
> + src += src_stride;
> + }
> + }
> +
> + if (dst_format & MESA_ARRAY_FORMAT_BIT) {
> + for (row = 0; row < height; ++row) {
> + _mesa_swizzle_and_convert(dst, dst_gl_type,
> +
> dst_array_format.num_channels,
> + tmp_float + row *
> width, GL_FLOAT, 4,
> + rgba2dst, normalized,
> width);
> + dst += dst_stride;
> + }
> + } else {
> + for (row = 0; row < height; ++row) {
> + _mesa_pack_float_rgba_row(dst_format, width,
> + (const float
> (*)[4])tmp_float + row * width, dst);
> + dst += dst_stride;
> + }
> + }
> +
> + free(tmp_float);
> + } else {
> + tmp_ubyte = malloc(width * height *
> sizeof(*tmp_ubyte));
> +
> + if (src_format & MESA_ARRAY_FORMAT_BIT) {
> + for (row = 0; row < height; ++row) {
> + _mesa_swizzle_and_convert(tmp_ubyte + row *
> width, GL_UNSIGNED_BYTE, 4,
> + src, src_gl_type,
> +
> src_array_format.num_channels,
> + src2rgba, normalized,
> width);
> + src += src_stride;
> + }
> + } else {
> + for (row = 0; row < height; ++row) {
> + _mesa_unpack_ubyte_rgba_row(src_format, width,
> + src, tmp_ubyte + row
> * width);
> + src += src_stride;
> + }
> + }
> +
> + if (dst_format & MESA_ARRAY_FORMAT_BIT) {
> + for (row = 0; row < height; ++row) {
> + _mesa_swizzle_and_convert(dst, dst_gl_type,
> +
> dst_array_format.num_channels,
> + tmp_ubyte + row *
> width, GL_UNSIGNED_BYTE, 4,
> + rgba2dst, normalized,
> width);
> + dst += dst_stride;
> + }
> + } else {
> + for (row = 0; row < height; ++row) {
> + _mesa_pack_ubyte_rgba_row(dst_format, width,
> + (const uint8_t
> (*)[4])tmp_ubyte + row * width, dst);
> + dst += dst_stride;
> + }
> + }
> +
> + free(tmp_ubyte);
> + }
> +}
>
> static const uint8_t map_identity[7] = { 0, 1, 2, 3, 4, 5,
> 6 };
> static const uint8_t map_3210[7] = { 3, 2, 1, 0, 4, 5, 6 };
> diff --git a/src/mesa/main/format_utils.h
> b/src/mesa/main/format_utils.h
> index 010d137..4d348cc 100644
> --- a/src/mesa/main/format_utils.h
> +++ b/src/mesa/main/format_utils.h
> @@ -33,6 +33,11 @@
>
> #include "imports.h"
>
> +extern mesa_array_format RGBA8888_FLOAT;
> +extern mesa_array_format RGBA8888_UBYTE;
> +extern mesa_array_format RGBA8888_UINT;
> +extern mesa_array_format RGBA8888_INT;
> +
> /* Only guaranteed to work for BITS <= 32 */
> #define MAX_UINT(BITS) ((BITS) == 32 ? UINT32_MAX : ((1u <<
> (BITS)) - 1))
> #define MAX_INT(BITS) ((int)MAX_UINT((BITS) - 1))
> @@ -147,4 +152,9 @@ _mesa_swizzle_and_convert(void *dst,
> GLenum dst_type, int num_dst_channels,
> const void *src, GLenum src_type,
> int num_src_channels,
> const uint8_t swizzle[4], bool
> normalized, int count);
>
> +void
> +_mesa_format_convert(void *void_dst, uint32_t dst_format,
> size_t dst_stride,
> + void *void_src, uint32_t src_format,
> size_t src_stride,
> + size_t width, size_t height);
> +
> #endif
> diff --git a/src/mesa/main/formats.h b/src/mesa/main/formats.h
> index bba5bae..7a792d4 100644
> --- a/src/mesa/main/formats.h
> +++ b/src/mesa/main/formats.h
> @@ -81,7 +81,7 @@ enum {
> MESA_FORMAT_SWIZZLE_NONE = 6,
> };
>
> -enum mesa_array_format_datatype {
> +enum mesa_array_format_datatype {
> MESA_ARRAY_FORMAT_TYPE_UBYTE = 0x0,
> MESA_ARRAY_FORMAT_TYPE_USHORT = 0x1,
> MESA_ARRAY_FORMAT_TYPE_UINT = 0x2,
> @@ -90,11 +90,12 @@ enum mesa_array_format_datatype {
> MESA_ARRAY_FORMAT_TYPE_INT = 0x6,
> MESA_ARRAY_FORMAT_TYPE_HALF = 0xd,
> MESA_ARRAY_FORMAT_TYPE_FLOAT = 0xe,
> -
> - MESA_ARRAY_FORMAT_TYPE_IS_SIGNED = 0x4,
> - MESA_ARRAY_FORMAT_TYPE_IS_FLOAT = 0x8,
> };
>
> +#define MESA_ARRAY_FORMAT_TYPE_IS_SIGNED 0x4
> +#define MESA_ARRAY_FORMAT_TYPE_IS_FLOAT 0x8
> +#define MESA_ARRAY_FORMAT_BIT 0x80000000
> +
> typedef union {
> struct {
> enum mesa_array_format_datatype type:4;
> @@ -130,6 +131,12 @@ _mesa_ilog2(unsigned x)
> (((SIGNED) << 2 ) & MESA_ARRAY_FORMAT_TYPE_IS_SIGNED) |
> \
> (((IS_FLOAT) << 3 ) & MESA_ARRAY_FORMAT_TYPE_IS_FLOAT)
>
> +static inline int
> +_mesa_array_format_datatype_size(enum
> mesa_array_format_datatype type)
> +{
> + return 1 << (type & 0x3);
> +}
> +
> /**
> * Mesa texture/renderbuffer image formats.
> */
>
>
> Let's squash these changes into the pach where we first introduce
> array formats.
Ok.
>
> --
> 1.9.1
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
>
More information about the mesa-dev
mailing list