<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#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 <span class="vcard"><a class="email" href="mailto:itoral@igalia.com" title="Iago Toral <itoral@igalia.com>"> <span class="fn">Iago Toral</span></a>
</span></b>
        <pre>(In reply to Jason Ekstrand from <a href="show_bug.cgi?id=84566#c46">comment #46</a>)
<span class="quote">> (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 {

> }</span >

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>
      </p>
      <hr>
      <span>You are receiving this mail because:</span>
      
      <ul>
          <li>You are the QA Contact for the bug.</li>
      </ul>
    </body>
</html>