Re: [AD] Status of A5 on OS X (ppc)

[Evert Glebbeek <eglebbk@xxxxxxxxxx>]

On 18 Sep 2008, at 21:08, Elias Pschernig wrote:
> Ok, so right now, all our pixel formats like
> ALLEGRO_PIXEL_FORMAT_ARGB_8888 are not really what they say on big
> endian. Do we want to fix that?
>
> Can you try changing this line in src/opengl/ogl_bitmap.c
>
> {GL_RGBA8, GL_UNSIGNED_INT_8_8_8_8_REV, GL_BGRA}, /* ARGB_8888 */
>
> to this
>
> {GL_RGBA, GL_UNSIGNED_BYTE, GL_BGRA}, /* ARGB_8888 */
>
> instead, then try again with the original example? It makes OpenGL not
> do endian-specific conversion, and I assume it will therefore make the
> original example work (unless some endian-color-conversion takes place
> in between).

I can confirm that this does indeed fix the problem with  
ex_lockbitmap also, and from a quick test with some of the other  
examples it doesn't seem to lead to a regression.
I have to admit I don't really understand the change though. The  
relevant bit that tells OpenGL to not do the conversion is presumably  
the change of GL_UNSIGNED_INT_8_8_8_8_REV to GL_UNSIGNED_BYTE? Is the  
change of GL_RGBA8 to GL_RGBA relevant (I presume not)?

> Somehow I think this also is what we really intended - the pixel  
> format
> specifies the actual layout in memory, not the layout with a 32-bit
> integer.

That is the impression I had, yes, which is part of the reason I  
didn't try to clean up the patch and commit it.
That is a slightly dangerous mindset to have though (in my opinion),  
because it is true on little endian systems, which is probably the  
most common type of machine available at the moment. That makes it  
easy to assume that ARGB also represents the byte order in memory (I  
certainly would do that). For instance, there was a similar bug in  
the TGA loader (outside the scope of the colour conversion functions).

Tthe question is really what "ARGB8888" would mean as a datatype: a  
packed 32 bit integer or an array of 4 bytes with a particular order.  
They are of course the same on a big endian machine, but not on a  
little endian machine
The pixel conversion functions in pixels.c all work on packed  
integers using bitshifts and logical operators. So they do not encode  
for an array of 4 characters. If they are supposed to do that, then  
they are technically wrong.
I should point out that they actually work properly, indicating that  
at least part of the code was written with the (accidental?)  
intention that the format refers to the layout of a 32 bit integer.  
Changing that now may lead to many unexpected bugs and clumsy fixes  
and workarounds (which would be a shame, the code so clean compared  
to some of the A4 code!). I also don't think there's any real reason  
why they would need to be changed: as long as you always refer to  
bits within a 32 bit integer and never treat it as though it's an  
array of 4 8 bit integers you're always safe.

Part of the reason we have a colour type is to hide all that  
confusion from the end user, and I would certainly appreciate Allegro  
taking care of any endianesse problems for me.

> However, I'm unsure about the implications.. we might need another set
> of pixel format conversion functions for big endian systems to get the
> actual memory layout right in all cases then. But if we leave it be,
> there also might be a lot of hidden issues crop up on big endian
> systems.

I know, I'm finding them. :P
I would personally prefer that ARGB8888 would translate into  
0xAARRGGBB, in other words the machine's native 32 bit integer  
format. That's just a preference though and in the end of the day it  
doesn't matter either way, but it does need to be clear and there  
should never be any sort of ambiguity.

Allegro 4 had the makecol32() function partly for this reason.
What would be the main reason the user needs to care about the byte  
ordering in a chunk of memory? The sample code in ex_lockbitmap? In  
that case a function that encodes a quartet of (A, R, G, B) and  
stores them in the indicated location in the buffer could hide the  
details from the end user.

There is also an alternative, but I don't have the birds-eye point of  
view at the moment to see whether this means more work or less:  
Allegro's internals stay as they are, treating 32 bit integers that  
represent a colour as 32 bit integers and using logical operators and  
bitshifts to extract the data. In other words, it means Allegro uses  
whatever the native endian encoding is (as it does now). When  
returning colour components to the user or recieving them from the  
user it communicates in terms of arrays where the first component is  
B, the second is G, the third R and the fourth A.
If that's all your fix does, then cool. It's less work. If there are  
other places where Allegro would have to do such a conversion, it's  
more work.

\end{braindump}
I think that was sortof coherent anyway...

Evert