<html>
    <head>
      <base href="https://bugs.freedesktop.org/" />
    </head>
    <body>
      <p>
        <div>
            <b><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#c46">Comment # 46</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 <span class="vcard"><a class="email" href="mailto:jason@jlekstrand.net" title="Jason Ekstrand <jason@jlekstrand.net>"> <span class="fn">Jason Ekstrand</span></a>
</span></b>
        <pre>(In reply to Iago Toral from <a href="show_bug.cgi?id=84566#c43">comment #43</a>)
<span class="quote">> 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.</span >

My though there was to do the following (in pseudocode)

if (!has_transfer_ops && format != LUMINANCE_ALPHA) {
    mesa_format_convert(dst, src);
} else {

}</pre>
        </div>
      </p>
      <hr>
      <span>You are receiving this mail because:</span>
      
      <ul>
          <li>You are the QA Contact for the bug.</li>
      </ul>
    </body>
</html>