[Mesa-dev] [Bug 84566] Unify the format conversion code

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=84566

--- Comment #5 from Iago Toral <itoral at igalia.com> ---
Jason, in your initial implementation of the master function you have this
assert:

if (src_array_format.as_uint && dst_array_format.as_uint) {
   assert(src_array_format.normalized == dst_array_format.normalized);
   (...)
   _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);
}

I think this assertion is wrong, since it prevents conversion from float array
types (which are being generated with normalized=false) to normalized integer
types. Something that is explicitly supported in _mesa_swizzle_and_convert. I
am hitting this assertion while converting from RGB FLOAT to RGBA UBYTE for
example.

Looking at the implementation of texstore_swizzle, which uses
_mesa_swizzle_and_convert (and is the path that the original code takes to do
this conversion in my example), I don't find this kind of checks. What it does
is this:

is_array = _mesa_format_to_array(dstFormat, &dst_type, &dst_components,
                                 rgba2dst, &normalized);
(...)
normalized |= !_mesa_is_enum_format_integer(srcFormat);

This will make it so that normalized is always set to true for float src
formats like RGB FLOAT. 

So assuming that this code is correct I think we would want to do the same in
the master function: remove the assert and set normalized to true if either src
or dst are normalized or if src is a float type.

There is another issue, and that is how we decide if a (glformat,gldatatype)
combination is normalized or not when creating an array format from it. There
are a bunch of functions in glformats.c I could use, like:

GLboolean _mesa_is_enum_format_unorm(GLenum format)
GLboolean _mesa_is_enum_format_snorm(GLenum format)
GLboolean _mesa_is_enum_format_integer(GLenum format)
...

But these do not consider the data type, only the format, so for example
_mesa_is_enum_format_unorm(GL_RGB) will always return true, even if we have a
GL_FLOAT data type. This is the same thing that _mesa_swizzle_and_convert does
when doing:

normalized |= !_mesa_is_enum_format_integer(srcFormat);

but it is inconsistent with your definition of array formats, since all array
formats generated in format_info.c have a normalized setting of 0.

I could also avoid the problem if I simply use I just use
_mesa_is_enum_format_integer(format) when mapping a (glformat,gldatatype) to a
mesa_array_format, like texstore_swizzle does, but as I mentioned, that will
create float array formats with normalized set to true, which is inconsistent
with the array_formats you are generating, so I am not sure about the best path
to take here.

What do you think?

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20141008/5c29de66/attachment.html>