[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