Michel Dänzer <mic...@daenzer.net> writes:
> On Fre, 2013-05-24 at 09:11 -0700, Jose Fonseca wrote:
>>
>> I agree that with non-array formats, like B5G6R5 and R5G6B5, replacing
>> them with endian-variant BGR565 and RGB565 makes a lot of sense (as
>> the swapped version will probably never be needed).
>>
>> But I'm not sure about RGBA8 variants...
>>
>> - On one hand, it is often more efficient to read/write them as 32bit
>> integers than as an array of bytes.
>>
>> - On the other hand it is easier to think of then as an array of
>> bytes than an integer quantity.
>>
>> One thing is clear -- a given format can't be both -- either it is
>> endianess-variant packed color or a endianness-invariant array color.
>
> Actually, I think it should be possible to make the RGBA8 variants
> usable as either an array of bytes or a packed integer, given the right
> setup of definitions and aliases (which would differ between little and
> big endian hosts). But I'm not sure offhand what would be the best way
> to achieve that for the util_format stuff.
Yeah, that's what the patch was trying to do. Even though the formats
were defined as "int"s, the int layout was extra information on top of
what's already there. The int information didn't change or replace the
array information.
So the idea is that the array nature of the formats doesn't really change.
R8G8B8A8 was originally renamed to ABGR8888 on little endian (with R8G8B8A8
as a #defined alias)[*], but the util_format description was the same as
before. I.e. quoting the comment change:
/**
- * Input channel description.
+ * Input channel description, in the order XYZW.
*
* Only valid for UTIL_FORMAT_LAYOUT_PLAIN formats.
+ *
+ * If each channel is accessed as an individual N-byte value, X is always
+ * at the lowest address in memory, Y is always next, and so on. For all
+ * currently-defined formats, the N-byte value has native endianness.
...this gives the "array" layout for all plain formats for which that
makes sense, even "int" ones, and the patch doesn't change that.
The order of the channels and the values of the fields are the same
as before. All that changes is that we have a new "shift" field that
counts the number of bits between the lsb of the int and the lsb of the
channel. FWIW, the rest of the comment was:
+ *
+ * If instead a group of channels is accessed as a single N-byte value,
+ * the order of the channels within that value depends on endianness.
+ * For big-endian targets, X is the most significant subvalue,
+ * otherwise it is the least significant one.
+ *
+ * For example, if X is 8 bits and Y is 24 bits, the memory order is:
+ *
+ * 0 1 2 3
+ * little-endian: X Yl Ym Yu (l = lower, m = middle, u = upper)
+ * big-endian: X Yu Ym Yl
+ *
+ * If X is 5 bits, Y is 5 bits, Z is 5 bits and W is 1 bit, the layout is:
+ *
+ * 0 1
+ * msb lsb msb lsb
+ * little-endian: YYYXXXXX WZZZZZYY
+ * big-endian: XXXXXYYY YYZZZZZW
*/
struct util_format_channel_description channel[4];
I think the unpatched code already accepts that it doesn't make much
sense to load and store individual components of a 565 format.
The code loads or stores all channels as a single 16-bit value instead.
(Please correct me if I'm wrong about that.) That's why the first
paragraph of the comment was talking about "N-byte" value rather than
"N-bit" value.
In other words, the single channel ordering that is used in the unpatched
code is well-defined from both an "array" and "int" perspective, provided
that the "array" case is restricted to formats where the components are
full bytes in width. The "int" ordering is endian-dependent, but that
fact is limited to u_format_pack.py and u_format_parse.py. No runtime
code cares about the endian difference, because it's all wrapped up
in the "shift" field.
Sorry, I realise what I'm about to say is going to take up even more
your time on this, but: I think Michael's original suggestion was to
follow mesa names and numbering for int formats. I'm happy to change
it to use "low bit first" numbering for int formats instead, but I think
it'd be a mistake to keep the name "PIPE_FORMAT_ARGB8888" for "alpha in
the low byte". It looks too similar to the mesa name. How about just
sticking "INT_" at the beginning of a name that gives int ordering rather
than array ordering? E.g. PIPE_FORMAT_INT_A8R8G8B8 (alpha in the low byte).
That's just one idea though. Please suggest something better. :-)
[*] I'm going to change it to be the other way around, in line
with your comments.
Thanks,
Richard
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
