[Mesa-dev] [Bug 84566] Unify the format conversion code

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=84566

--- Comment #47 from Iago Toral <itoral at igalia.com> ---
(In reply to Jason Ekstrand from comment #46)
> (In reply to Iago Toral from comment #43)
> > 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.

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20141030/61d7045a/attachment-0001.html>