[Mesa-dev] [V4 PATCH 0/7] mesa: Naming MESA_FORMATs to a specification

[Marek Olšák]

On Thu, Jan 23, 2014 at 4:17 AM, Mark Mueller <markkmueller at gmail.com> wrote:
> Hi Merek,

I'm Marek.

>
>
>
>
>
> On Wed, Jan 22, 2014 at 2:49 PM, Marek Olšák <maraeo at gmail.com> wrote:
>>
>> Hi Mark,
>>
>> Could you please mention or document somewhere in the code (e.g. in
>> main/formats.h) which _REV formats are incorrect according to you?
>> Sorry if you did so already, I haven't read your other patches yet.
>>
>
> There was not a universally correct solution for some of the _REV
> MESA_FORMATs thus I defaulted to the OGL definition of _REV when eliminating
> the _REV decoration. For the examples below, MESA_FORMAT's use of _REV
> sometimes differs from what is described in the OGL pixel transfer doc,
> which says the following about GL_UNSIGNED_SHORT_5_6_5, for example:
> "This can have a _REV at the end of it. This would mean to reverse the order
> of the components in the data. In "REV" mode, the first component, the one
> that matches the first 5, would go into the last component specified by the
> format."
>
> Using _REV with MESA_FORMATs to have a big and little endian representation
> of a format agrees with the OGL definition most of the time but not for
> formats where each color components isn't equally divided on a byte
> boundary:
>
>    MESA_FORMAT_RGB565, /*                     RRRR RGGG GGGB BBBB */
>
>    MESA_FORMAT_RGB565_REV, /*                     GGGB BBBB RRRR RGGG */
>
>    MESA_FORMAT_ARGB4444, /*                     AAAA RRRR GGGG BBBB */
>
>    MESA_FORMAT_ARGB4444_REV, /*                     GGGG BBBB AAAA RRRR */
>
>    MESA_FORMAT_RGBA5551,        /*                     RRRR RGGG GGBB BBBA
> */
>
>    MESA_FORMAT_ARGB1555, /*                     ARRR RRGG GGGB BBBB */
>
>    MESA_FORMAT_ARGB1555_REV, /*                     GGGB BBBB ARRR RRGG */
>
>
> Searching all of Mesa for the above _REV formats reveals few hits within the
> drivers that are actually employing the endianess interpretation of _REV, vs
> the OGL interpretation. For example radeon_screen.c:
>
>         rgbFormat = _mesa_little_endian() ? MESA_FORMAT_RGB565 :
> MESA_FORMAT_RGB565_REV;
>
>
> vs radeon_pixel_read.c
>
>                 case GL_UNSIGNED_SHORT_5_6_5:
>
>                     return MESA_FORMAT_RGB565;
>
>          case GL_UNSIGNED_SHORT_5_6_5_REV:
>
>                     return MESA_FORMAT_RGB565_REV;
>
>
> In many of the cases that rely on the endianess interpretation,  the _REV
> decoration is ignored anyway.
>
>
> So, based on your request, I should add a comment above the applicable
> format_unpack and format pack functions that says "this function does not
> match the current Mesa definition of <insert format here>". Is that
> acceptable? With the latest patch set I just buried my head in the sand on
> this one.

Yes. If you think the formats are somehow wrong, you can also fix them
or remove them. It's up to you.

>
>
>
>> Also, I have a proposal for SRGB formats. MESA_FORMAT_SRGB_UNORM8 and
>> MESA_FORMAT_SA8B8G8R8_UNORM
>> look weird, because they are not really UNORM and there is also no
>> stencil. :) How about this: MESA_FORMAT_RGB_SRGB8 (denoting an array
>> format of the SRGB type and 8 bits per channel) and
>> MESA_FORMAT_A8B8G8R8_SRGB (denoting a packed format of the SRGB type).
>
>
> I agree it could be better. My reasoning for avoiding what you suggested is
> that it creates an ambiguity between color information and storage type. For
> instance, sRBG color space values could feasibly be stored as floats, half
> floats, snorms, or unorms, could they not? Thus the S is a color component

I don't think so. An sRGB channel is always a byte. You cannot store
both linear and sRGB values in an 8-bit format without losing
precision for either one of them, which I think is why there are sRGB
formats for sRGB values and other formats for linear values. Bigger
texture types do not suffer from this limitation so much. Also, sRGB
is not very well defined outside of the range [0, 1], which leaves
UNORM as the only suitable type.

> modifier, not storage type. Would it be too awkward to use M for stencil
> mask instead of S? Is there a clearer method that doesn't hide storage type
> information?

Yes, M is awkward. Not sure.

Marek