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

[Richard Sandiford]

Michel Dänzer <michel at daenzer.net> writes:
>> > 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.
>
> Fair enough.
>
>
>> 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
>
> Is there any drawback to the latter approach for formats where it's
> feasible? If not, it might reduce code duplication somewhat.
>
>> (**) 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
>
> Exactly. :\

Yeah.  I deliberately left those for future work :-)  The 8-bits-per-channel
formats were more interesting when trying out your idea, both because they're
used more and because they have both the "array" and "int" interpretations.
But it was actually because of things like B5G6R5 that I used "int" formats
like RGBA8888 and made .8.8.8.8 an alias of them, rather than the other way
around.  The layout of B5G6R5 on little-endian targets is AIUI:

    76543210 76543210
    GGGBBBBB RRRRRGGG

Reversing the components gives:

    76543210 76543210
    GGGRRRRR BBBBBGGG

But on a big-endian target the "blue first" format is:

    76543210 76543210
    BBBBBGGG GGGRRRRR

and reversing the components gives:

    76543210 76543210
    RRRRRGGG GGGBBBBB

So, unlike for the .8.8.8.8 formats, a plain swizzle doesn't give you
the other endianness.  You need to do something more complicated.
Little-endian support for the big-endian arrangement, and vice-versa,
would be pretty involved.

So in practice I thought we'd want the first two formats on little-endian
targets and the last two on big-endian targets.  I thought that would mean
_replacing_ the current B5G6R5 and R5G6B5 formats with BGR565 and RGB565
formats that match the endian-specific arrangements above.  These two "int"
formats wouldn't be aliases of a target-independent representation.

So the patch was in some ways an experiment to see how easy it would be
to make gallium treat a common format like .8.8.8.8 as an int, in the hope
that if it was easy, things like .5.6.5 would be less of a special case.
And in the end it all seemed pretty natural, although of course that's
from a newbie's perspective.  You both know the code much better than I do.

Thanks,
Richard