[Mesa-dev] [PATCH 06/29] mesa: Avoid pack/unpack fast paths if base internal format != base format
Jason Ekstrand
jason at jlekstrand.net
Wed Nov 19 11:57:24 PST 2014
A couple of general comments on this patch:
1) The prerequisites should be moved to before the first patch in the
series and it should be squashed into the patch that introduces the
function. There are one or two more patches which also modify it and those
should also be squashed in.
2) I wonder if giving _mesa_format_convert an internal swizzle wouldn't be
better than a destination internal format. There are a couple of reasons
for this:
a) It's more general. If it doesn't cost us anything extra to do it
that way, we might as well.
b) It removes the GL enum dependency from the _mesa_format_convert
c) I think this implementation misses the case where we download a
texture that is storred in a different format than its base format. For
instance, if you are downloading a GL_RGB texture as GL_RGBA but it's
storred as GL_RGBA. I think with the current implementaion, you'll get the
junk in the alpha component of the texture's backing storage instead of a
constant alpha value of 1.
On Tue, Nov 18, 2014 at 1:23 AM, Iago Toral Quiroga <itoral at igalia.com>
wrote:
> In general, if the dst base internal format and the selected dst format are
> different we can't be sure that directly packing or unpacking to the
> destination
> format will produce correct results. In these cases we probably want to
> fall
> back to other paths (like mesa_swizzle_and_convert) that can handle these
> situations better.
>
> An example of this includes a luminance internal format for which the
> driver
> decided to use something like BGRA. In these case, unpacking to BGRA won't
> match the requirements of the luminance internal format.
>
> In the future we may want to allow these fast paths for specific cases
> where we know the direct pack/unpack functions will do the right thing.
> ---
> src/mesa/main/format_utils.c | 137
> +++++++++++++++++++++++--------------------
> 1 file changed, 72 insertions(+), 65 deletions(-)
>
> diff --git a/src/mesa/main/format_utils.c b/src/mesa/main/format_utils.c
> index 5964689..34c90d9 100644
> --- a/src/mesa/main/format_utils.c
> +++ b/src/mesa/main/format_utils.c
> @@ -331,65 +331,82 @@ _mesa_format_convert(void *void_dst, uint32_t
> dst_format, size_t dst_stride,
> 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 &&
> - !_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 &&
> - _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;
> + /* First we see if we can implement the conversion with a direct pack
> + * or unpack.
> + *
> + * In this case we want to be careful when the dst base format and the
> + * dst format do not match. In these cases a simple pack/unpack to the
> + * dst format from the src format may not match the semantic
> requirements
> + * of the internal base format. For now we decide to be safe and avoid
> + * this path in these scenarios but in the future we may want to enable
> + * it for specific combinations that are known to work.
> + */
> + 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;
> + if (_mesa_get_format_base_format(dst_mesa_format) ==
> dst_internal_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 &&
> + !_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 &&
> + _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;
> }
> - 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 &&
> - !_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 &&
> - _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;
> + /* 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 &&
> + !_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 &&
> + _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;
> }
> - return;
> }
> }
>
> @@ -431,11 +448,6 @@ _mesa_format_convert(void *void_dst, uint32_t
> dst_format, size_t dst_stride,
> * 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;
> 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];
> @@ -580,11 +592,6 @@ _mesa_format_convert(void *void_dst, uint32_t
> dst_format, size_t dst_stride,
> */
> 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;
> if (dst_internal_format !=
> _mesa_get_format_base_format(dst_mesa_format)) {
> _mesa_compute_component_mapping(GL_RGBA, dst_internal_format,
> --
> 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/5cd22cc4/attachment-0001.html>
More information about the mesa-dev
mailing list