[Mesa-dev] [PATCH 03/13] gallium: Introduce 32-bit bytewise format names

[Jose Fonseca]

----- Original Message -----
> On Die, 2013-05-21 at 23:15 -0700, Jose Fonseca wrote:
> > 
> > ----- Original Message -----
> > > Jose Fonseca <jfonseca at vmware.com> writes:
> > > > ----- Original Message -----
> > > >> From: Richard Sandiford <r.sandiford at uk.ibm.com>
> > > >> 
> > > >> RGBA8888 has R at byte 0 and A at byte 3, regardless of platform
> > > >> endianness.
> > > >
> > > > Maybe I'm missing something, but this naming convention seems to me
> > > > the exact opposite of what was decided [1], which is:
> > > >
> > > >  - R at byte 0, ..., and A at byte 3, regardless of platform endianness
> > > >  would be called "R8G8B8A8"
> > > >
> > > >  - R at bit 0, ..., A at bit 24, encoded as integers that match the
> > > >  platform endianness would be called "RGBA8888"
> > > >
> > > > which would be consistent with (as in a superset of) D3D10 format
> > > > naming.
> > > 
> > > Yeah, it's supposed to be that way round in the patches.  RGBA8888 is a
> > > 32-bit int with R in the high 8 bits and A in the low 8 bits.
> > 
> > This is still slightly different than what I expected: gallium was
> > using the convention the components started with least significant bit
> > appear first in name (same as D3D10). That is, RGBA8888 is a 32-bit
> > int with R in the _low_ 8 bits and A in the _high_ 8 bits.
> > 
> > I understand that Mesa formats follow the opposite convention, but
> > between consistency with Mesa vs consistency within gallium I believe
> > that it is better for gallium formats to be consistent among
> > themselves: it is much easier to remember that _all_ gallium formats
> > go from least->most significant bit/byte/word, than to remember which
> > formats are supposed to go from the low->high vs high->low, which
> > would end up forcing developers to either guess wrongly or waste time
> > look up the format implementation.
> 
> I disagree. There is no need or point to even think in terms of
> least/most significant anything (implying a certain byte order) for
> formats such as R8G8B8A8. They're just arrays.

Quite true, but this very patch (gallium: Introduce 32-bit bytewise format names), renames PIPE_FORMAT_B8G8R8A8_UNORM into PIPE_FORMAT_ARGB8888_UNORM, so I don't see how I can avoid this question.   In this case I must insist to leave all array rgba8 formats alone (i.e., not make them endianess dependent) -- actually a good idea regardless (*) --, and add RGBA8888/ABGR8888 endian dependent aliases/alternatives instead.

Anyway, note I was not even talking of matching array formats: I'm talking of matching the other packed formats that this series has not addressed yet (e.g., PIPE_FORMAT_B5G6R5_UNORM) and some packed formats which may never address (e.g., PIPE_FORMAT_R11G11B10_FLOAT, PIPE_FORMAT_Z24_UNORM_S8_UINT), and consistency with D3D10 format naming (e.g., DXGI_FORMAT_B5G6R5_UNORM).

> For packed formats such as RGBA8888, the order used in these patches
> (which is what I suggested in my proposal) matches the order humans use
> for digits of numbers, as well as the Mesa formats. That seems more
> important to me than 'matching' any non-packed formats (which only makes
> sense if one presumes little endian byte order).

I'm sorry I didn't notice this was what you proposed earlier..

However I don't think that consistency with Mesa formats is strong: Mesa formats even have both ways with their *_REV variants.

So I prefer that we keep existing low->high bit/byte/word/etc naming convention for gallium formats.


I do appreciate all the work and thought that went on this series so far, and I really want to get this in.  So here is a summary of what's needed from my POV to get this in mergeable state:

- leave r8g8b8a8 variants alone (ie, as endianess independent)

- fix the util_format_description::is_array == TRUE && util_format_description::is_bitmask == TRUE ambiguity, either:

   - add new rgba8888/argb8888 formats for endianess dependent formats

        - add a new field on util_format_description (e.g., native_endian) for the endianess formats (**)

   - or add rgba8888/argb8888 #define

        - make sure util_format_description::is_array is set for r8g8b8a8 variants, but util_format_description::is_bitmask is not

- low->high bit naming convention

- doc

- squash


Jose


(*) the reason that this is important is that rgba8 variants can both be seen as arithmetically encoded or array, and a lot of llvmpipe/gallivm code treats them as array, so it's confusing/brittle if they become host endianness dependent.

(**) Actually, I'm surprised that formats like PIPE_FORMAT_B5G6R5_UNORM aren't busted on big-endiang without this, as they haven't been converted yet, so they need to be handled precisely as before, right? I suppose everything was busted before, so no net change here