----- Original Message ----- > Jose Fonseca <jfons...@vmware.com> writes: > >> 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. > > > > I'm afraid it does change.. Because this description is paradoxical for > > formats that can be seen both as "array" and for "int", on big endian. > > Before we didn't have to choose how to interpret formats like r8g8b8a8 > > (we could access them either as 4 x bytes, or a 32bit packed integer, we > > didn't have to care, and unfortunately we didn't care at all and this > > assumption crept into many places), but now we can't (we have to pick > > one). And the mere fact we have to pick one, is a change -- a deep > > reaching change that can easily cause performance/correctness regression > > -- want it or not. > > That wasn't the intention. Even for big endian, we _can_ choose between > accessing formats like .8.8.8.8 as either array or int. That seems like > a useful thing to do. And the patch series doesn't want to change the > choice of which access is used where. It also doesn't want to change > the channel order in util_format or the byte layout of formats that can > be seen as "array". > > Obviously something has to change, and that change was supposed to be > a relatively simple one: when something chooses to access channels > as an int, it uses the new shift field instead of trying to count bits > from the first channel. The change is intended to be much less deep > than the one you suggested later. I don't really see the paradox in > the description above.
I confess that I missed this hunk among the whole series: diff --git a/src/gallium/auxiliary/util/u_format.h b/src/gallium/auxiliary/util/u_format.h index e4b9c36..3a04d89 100644 --- a/src/gallium/auxiliary/util/u_format.h +++ b/src/gallium/auxiliary/util/u_format.h @@ -132,6 +132,7 @@ struct util_format_channel_description unsigned normalized:1; unsigned pure_integer:1; unsigned size:9; /**< bits per channel */ + unsigned shift:16; /** number of bits from lsb */ }; Indeed that solves the paradox I was worried about, and in a neat way. Let me look again into this again... Jose _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
