<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#c43">Comment # 43</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:itoral@igalia.com" title="Iago Toral <itoral@igalia.com>"> Iago Toral</a> <pre>Jason, we are running into some issues when attempting to use _mesa_format_convert for glReadPixels and glGetTexImage. Generally, one thing that is different in this case is that the current implementation never attempts direct conversions from src to dst (except for a couple of specific cases that had been written ad-hoc). These functions do the conversion in 3 steps: 1) unpack to RGBA (float or uint). 2) Rebase (to consider the base format of the source pixel data). 3) Pack to dst (this not only packs, also handles type conversion, transferops, semantics specific to things like luminance formats, type conversions, etc). So we are hitting various issues when attempting to replace this logic with _mesa_format_convert: 1) You mentioned that things like transferops should be handled by the client. To achieve this we have _mesa_apply_rgba_transfer_ops, which requires an RGBA format, so converting (in the client) to RGBA first is required in these cases so they can use this function. I suppose this is okay since this is what the current implementation is doing in all cases right now, with or without transferops. 2) So far we have been focusing on pixel uploads. I mentioned then that we needed to add a new parameter to _mesa_format_convert to consider the base internal format we are converting to. Well, now we have the same situation but with the format we are converting from, and the semantics are different (in the sense that we need to know if we have a base format for the source or the destination in order to know what we need to do, like computing the right swizzle transform). I suppose that we could add another parameter to _mesa_format_convert and all the logic necessary to do the right thing in that case too. This, will complicate the implementation a bit I think, but I guess is the most consistent option. That said, the alternative would be to always transform to RGBA first, then call _mesa_rebase_rgba_* as the current code does... since the current code always convert to RGBA first we would not be losing performance and we would not have to add all the logic to _mesa_format_convert to account for a source base format. It would be less consistent though since _mesa_format_convert would support base formats for the dst but not for the src. 3) Luminance formats have special requirements. A conversion to Luminance from RGBA requires to do L=R+G+B for example. This is something that _mesa_format_convert cannot achieve at the moment, because neither pack/unpack functions nor _mesa_swizzle_and_convert do this kind of operation, so I wonder what is the right thing to do here. We could run another pass that computes these values after the conversion. We could do this inside _mesa_format_convert or in the client, after calling _mesa_format_convert I suppose there are no other options. The current code does another pass specifically for this, right before packing to the dst. 4) Step 3 does a lot of clamping work even after handling transfer ops, (right after packing to the dst). I am not sure that pack/unpack functions and _mesa_swizzle_and_convert will do the thing we need in all cases, but right now I don't have a specific example, so we can burn that bridge once we get there.</pre> </div> <hr> You are receiving this mail because: <ul> <li>You are the QA Contact for the bug.</li> </ul> </body> </html>