----- Original Message -----
> 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.
This is only possible for little endian hosts. For big endian hosts, no format
can be interpreted simultaneously as array or arithmetically packed integer, as
the opposite ordering ensues. For example, 8bits red 8 bits green 8 bits blue 8
bits alpha, on big endian is four bytes with R G B A if seen as array, four
bytes with A B G R if seen as packed integer.
> 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.
Honestly, I think that the mere fact we are going in circles in this thread
implies we're still biting more than we can chew.
I strongly recommend stepping back, and avoiding invasive (or any) semantic
changes. Merely stick to add new things instead of changing the things already
in there. In particular, I suggest on a first stage to do the following:
- introduce _new_ native endianness formats (RGB565, RGBA8888 and friends) as
necessary
- do not alias these with anything
- do not change any of the of existing formats or their interpretation
- introduce a new util_format_description::native_endianess or "packed_integer"
for these new formats.
- implement the new formats where needed (u_format, gallivm, llvmpipe, etc)
- introduce new paths for them "if (format_description->native_endianess) {
... } else { /* precisely the same as before */ }"
- use the new formats in mesa state tracker on big endian hosts
- leave little endian hosts alone until big endian work well
If you do this, we all can be sure that we won't break what's there while
making progress on this issue: that is, it should improve support on big endian
without any regressions on little endian. Therefore we can commit such changes
immediately, one by one as soon as you're done with them, without these long
discussion on "why some change can cause regression somewhere", or "what's the
ideal".
After this is done, and if we are confident on how to do it, we would have a
cleanup stage:
- use the new formats also on little endian
- let dust settle, see if there are any correctness/performance regressions,
and either address them immediately, or flip back and continue using the old
formats on little endian
- remove the old formats like R5G6B5 in favour of the new RGB565 (as we all
agree that we don't need the old native-independent formats)
- _maybe_ remove either RGBA8888/etc or R8G8B8A8/etc -- via aliases or
something else -- (what to do, if anything, is still unclear to me)
We'll get the same end result (proper support big endian), but with much less
grief from everybody.
I really see no other way to make progress on this.
Jose
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
