<div dir="ltr">A couple of specific comments are below. More generally, why are you only considering the base format on two cases? Do we never use it for anything else? <div><div class="gmail_extra"> <div class="gmail_quote">On Tue, Nov 18, 2014 at 1:23 AM, Iago Toral Quiroga <<a href="mailto:itoral@igalia.com" target="_blank">itoral@igalia.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Add a dst_internal_format parameter to _mesa_format_convert, that represents the base internal format for texture/pixel uploads, so we can do the right thing when the driver has selected a different internal format for the target dst format. --- src/mesa/main/format_utils.c | 65 +++++++++++++++++++++++++++++++++++++++++++- src/mesa/main/format_utils.h | 2 +- 2 files changed, 65 insertions(+), 2 deletions(-) diff --git a/src/mesa/main/format_utils.c b/src/mesa/main/format_utils.c index fc59e86..5964689 100644 --- a/src/mesa/main/format_utils.c +++ b/src/mesa/main/format_utils.c @@ -303,7 +303,7 @@ _mesa_compute_component_mapping(GLenum inFormat, GLenum outFormat, GLubyte *map) 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) + size_t width, size_t height, GLenum dst_internal_format) { uint8_t *dst = (uint8_t *)void_dst; uint8_t *src = (uint8_t *)void_src; @@ -422,6 +422,36 @@ _mesa_format_convert(void *void_dst, uint32_t dst_format, size_t dst_stride, if (src_array_format.as_uint && dst_array_format.as_uint) { assert(src_array_format.normalized == dst_array_format.normalized); + /* If the base format of our dst is not the same as the provided base + * format it means that we are converting to a different format + * than the one originally requested by the client. This can happen when + * the internal base format requested is not supported by the driver. + * In this case we need to consider the requested internal base format to + * compute the correct swizzle operation from src to dst. We will do this + * by computing the swizzle transform src->rgba->base->rgba->dst instead + * of src->rgba->dst. + */ + mesa_format dst_mesa_format; + if (dst_format & MESA_ARRAY_FORMAT_BIT) + dst_mesa_format = _mesa_format_from_array_format(dst_format); + else + dst_mesa_format = dst_format; </blockquote><div> </div><div>Let's put an extra line here so it doesn't get confused with the below if statement </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> + if (dst_internal_format != _mesa_get_format_base_format(dst_mesa_format)) { + /* Compute src2rgba as src->rgba->base->rgba */ + uint8_t rgba2base[4], base2rgba[4], swizzle[4]; + _mesa_compute_component_mapping(GL_RGBA, dst_internal_format, rgba2base); + _mesa_compute_component_mapping(dst_internal_format, GL_RGBA, base2rgba); + + for (i = 0; i < 4; i++) { + if (base2rgba[i] > MESA_FORMAT_SWIZZLE_W) + swizzle[i] = base2rgba[i]; + else + swizzle[i] = src2rgba[rgba2base[base2rgba[i]]]; </blockquote><div> </div><div>This doesn't work for composing three swizzles. If you get a ZERO or ONE in rgba2base, you'll read the wrong memory from src2rgba. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> + } + memcpy(src2rgba, swizzle, sizeof(src2rgba)); + } + + /* Compute src2dst from src2rgba */ for (i = 0; i < 4; i++) { if (rgba2dst[i] > MESA_FORMAT_SWIZZLE_W) { src2dst[i] = rgba2dst[i]; @@ -539,9 +569,42 @@ _mesa_format_convert(void *void_dst, uint32_t dst_format, size_t dst_stride, src += src_stride; } } else { + /* For some conversions, doing src->rgba->dst is not enough and we + * need to consider the base internal format. In these cases a + * swizzle operation is required to match the semantics of the base + * internal format requested: src->rgba->swizzle->rgba->dst. + * + * We can detect these cases by checking if the swizzle transform + * for base->rgba->base is 0123. If it is not, then we need + * to do the swizzle operation (need_convert = true). + */ + GLubyte rgba2base[4], base2rgba[4], map[4]; + bool need_convert = false; + mesa_format dst_mesa_format; + if (dst_format & MESA_ARRAY_FORMAT_BIT) + dst_mesa_format = _mesa_format_from_array_format(dst_format); + else + dst_mesa_format = dst_format; </blockquote><div> </div><div>Blank line again </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> + if (dst_internal_format != + _mesa_get_format_base_format(dst_mesa_format)) { + _mesa_compute_component_mapping(GL_RGBA, dst_internal_format, + base2rgba); + _mesa_compute_component_mapping(dst_internal_format, GL_RGBA, + rgba2base); + for (i = 0; i < 4; ++i) { + map[i] = base2rgba[rgba2base[i]]; + if (map[i] != i) + need_convert = true; + } + } + for (row = 0; row < height; ++row) { _mesa_unpack_rgba_row(src_format, width, src, tmp_float + row * width); + if (need_convert) + _mesa_swizzle_and_convert(tmp_float + row * width, GL_FLOAT, 4, + tmp_float + row * width, GL_FLOAT, 4, + map, false, width); src += src_stride; } } diff --git a/src/mesa/main/format_utils.h b/src/mesa/main/format_utils.h index 4237ad3..29ab4a0 100644 --- a/src/mesa/main/format_utils.h +++ b/src/mesa/main/format_utils.h @@ -158,6 +158,6 @@ _mesa_compute_component_mapping(GLenum inFormat, GLenum outFormat, GLubyte *map) 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); + size_t width, size_t height, GLenum dst_internal_format); #endif -- 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" target="_blank">http://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </blockquote></div> </div></div></div>