[PATCH] New image loader (Softimage PIC)

Ruben Lopez r.lopez at bren.es
Thu Aug 30 09:19:47 BST 2007


Ingo Klöcker wrote:
> 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.
>   
I have changed these unsigned char to unsigned. This should fix the 
problem on most architectures. Anyway, when integrating this code with 
kdelibs or kdegraphics configure (CMake?) system, maybe I could use more 
portable types (like uint32 or something like that).
About the float problems, where did you identified them? I have scanned 
all the code searching for floats and they are only in the PICHeader 
structure. These float fields are only accessed once in the code, and 
with double constant values. I have changed this constants to float to 
avoid warnings in VC. Well, it is also readed from the QIODevice, but I 
read the whole PICHeader structure as a block. Can this yield to endian 
problems?

As soon as I can test the new code (I miss scons on this machine) I will 
send the new version. I have also cleaned it up for dead code and 
spanish comments...

Btw, is there any doc online explaining the procedure for submitting a 
patch for kdelibs or kdegraphics? If submitted to kdegraphics, what is 
the right folder for the KImgIO plugins? I have seen a folder for KFile 
plugins, but I miss the other one.

Thanks.




More information about the kde-core-devel mailing list