[PATCH] New image loader (Softimage PIC)

[Ingo Klöcker]

On Tuesday 28 August 2007 09:42, Bongani Hlope wrote:
> The code bellow should be called convertABGRtoRGA, becaus that is
> what it does. In RGBA, red is the highest nibble so to get red you
> need  (pixel >> 24) & 0xFF
>
> inline unsigned convertRGBAtoABGR(unsigned pixel)
> {
> 	unsigned char r = pixel & 0xFF;
> 	unsigned char g = (pixel >> 8) & 0xFF;
> 	unsigned char b = (pixel >> 16) & 0xFF;
> 	unsigned char a = (pixel >> 24) & 0xFF;
> 	return a | (b << 8) | (g << 16) | (r << 24);
> }

Don't be fooled by the completely irrelevant naming of the temporary 
variables. This method converts RGBA to ABGR and vice versa. So it's 
pretty much irrelevant how the method is called. Both names would be 
equally correct and equally wrong.

I am much more concerned about the usage of the arithmetic shift 
operator on variables of type unsigned char. A char is 8 bit wide so 
for example b << 8 might be 0 for some compilers/on some architectures. 
Or does char always expand to a 32-bit integer when its bits are 
shifted? I guess the answer is "It _should_ on 32+-bit architectures."

I have observed a weird problem with a few signed chars being casted to 
float and back which yielded different results in debug and release 
builds with vc7, so I wouldn't be surprised if the above would still 
break for some compiler.


Regards,
Ingo
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20070829/33e8a2a0/attachment.sig>