<html> <head> <base href="https://bugs.freedesktop.org/" /> </head> <body> <div> <a class="bz_bug_link bz_status_NEW " title="NEW - Unify the format conversion code" href="https://bugs.freedesktop.org/show_bug.cgi?id=84566#c6">Comment # 6</a> on <a class="bz_bug_link bz_status_NEW " title="NEW - Unify the format conversion code" href="https://bugs.freedesktop.org/show_bug.cgi?id=84566">bug 84566</a> from <a class="email" href="mailto:jason@jlekstrand.net" title="Jason Ekstrand <jason@jlekstrand.net>"> Jason Ekstrand</a> <pre>(In reply to Iago Toral from <a href="show_bug.cgi?id=84566#c5">comment #5</a>) > 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. Yes, I believe that is correct. While I implemented the master function, it didn't get much testing. I'ts mostly just a prototype, so bugs like that aren't surprising. > 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); This should be correct. The way you detect things on the GL side is usually that the GL_RGBA type formats are normalized while the GL_RGBA_INTEGER type formats are not. The only issue here is when you should clamp floating-point. What we probably want is a boolean "clamp" parameter that indicates that the input should be clamped to [0, 1] before converting. Whether a float-float conversion should be clampped depends on where the conversion is being done. > 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. The generation is quite possibly wrong there. Again, the branches I pushed were very thrown together and not a full, tested, implementation. > 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. I think that's perfectly reasonable if we fix the generation format_info code.</pre> </div> <hr> You are receiving this mail because: <ul> <li>You are the QA Contact for the bug.</li> </ul> </body> </html>