[Mesa-dev] [PATCH 05/29] mesa: Consider internal base format in _mesa_format_convert
Jason Ekstrand
jason at jlekstrand.net
Wed Nov 19 11:43:28 PST 2014
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?
On Tue, Nov 18, 2014 at 1:23 AM, Iago Toral Quiroga <itoral at igalia.com>
wrote:
> 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;
>
Let's put an extra line here so it doesn't get confused with the below if
statement
> + 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]]];
>
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.
> + }
> + 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;
>
Blank line again
+ 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
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20141119/b10b541e/attachment-0001.html>
More information about the mesa-dev
mailing list