<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#c47">Comment # 47</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>(In reply to Jason Ekstrand from <a href="show_bug.cgi?id=84566#c46">comment #46</a>) > (In reply to Iago Toral from <a href="show_bug.cgi?id=84566#c43">comment #43</a>) > > 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. > > My though there was to do the following (in pseudocode) > > if (!has_transfer_ops && format != LUMINANCE_ALPHA) { > mesa_format_convert(dst, src); > } else { > > } Yes, this is what we have done but I was wondering if we could just handle conversion to luminance inside _mesa_format_convert as a special case to avoid having that if { } else {} in every path that implements format conversion (glTexImage, glGetTexImage, glReadPIxels, etc)... then I noticed that for _mesa_format_convert to do that it would have to know about transferops or at least have a way to know if we have to clamp the result, so it is probably not a good idea.</pre> </div> <hr> You are receiving this mail because: <ul> <li>You are the QA Contact for the bug.</li> </ul> </body> </html>