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

[Mark Mueller]

Hi Merek,





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.



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 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?

 Mark
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20140122/697228ee/attachment.html>