[PATCH] New image loader (Softimage PIC)

[Ruben Lopez]

Aaron J. Seigo wrote:
> On Thursday 30 August 2007, Ruben Lopez wrote:
>   
>> 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).
>>     
>
> for Qt code you can use quint32 and friends, yes =)
>
>   
>> 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?
>>     
>
> you mean in picReadHeader? that looks ok to me, as you use ntoh* to pull the 
> values out of the file and into the in-memory variable..
>
>   
>> 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?
>>     
>
> you're doing the right thing right here =)
>
>   
>> 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.
>>     
>
> the kfile plugins have been moved to "strigi-analyzer" but the idea is the 
> same, i suppose. 
>
> so, i looked at the source tarball a bit more just now (only after having sent 
> the last email.. meh, bad aaron, bad!) and i see it's very small and indeed a 
> user-invisible plugin. so upon further examination, i'm fine with having it 
> in kdegraphics itself, at trunk/KDE/kdegraphics/imageio/pic/. the contents of 
> the share/ and src/ directories should both go in there.
>
> it also occurred to me while reading through the tarball that it's kde3 code 
> =) kde3 is no longer accepting new features / code, but you could certainly 
> offer this version in branches/extragear/kde3/graphics.
>
> going forward, we need to transition it to kde4, which includes:
>
> - the mimetype .desktop file needs to be transitioned to the shared mimetype 
> xml[1]; it would be great to move this upstream[2] as well. i could help here 
> if needed.
>
> - moving the build to cmake. that's a 5 minute job and i could also do that 
> for you if you'd like.
>
> - port and make sure it compiles. this is the part that entails some work, and 
> entails:
>
>   - porting kfile_pic.* to a strigi-analyzer. this is pretty easy, and the 
> results should go into kdegraphics/strigi-analyzer/pic/ ... it can share the 
> pic_read.* files with the imageio plugin
>   - rather than kimigio_* factories, this needs to be a QImageIOPlugin and a 
> QImageIOHandler needs to be written around pic_read and pic_write. 
> essentially, canRead, read and write need to be implemented; there are a 
> number of other virtuals that may or may not be interesting. read and write 
> should be pretty easy, as they essentially can be ported verbatin from the 
> current kimgio_pic_read and kimgio_pic_write.
>
> so ... do you have a kde4 install around? are you interested in / able to do 
> the porting (either with assistance or on your own)? it would be cool to get 
> this into kde4, though there is a bit of work to do. the majority of the 
> work, indeed the hard bits (the format support itself), is already done so 
> it's just the final bits to tie up.
>   
I don't have any kde4 installation, and I plan to wait before we have it 
in testing/production to do the port. Unfortunatelly I can't dedicate 
the needed time right now. Softimage|XSI is certificated for Fedora Core 
distributions with KDE3. Porting it to KDE4 right now wouldn't benefit 
either us or any other potential user/contributor.

In the meantime, I will upload the code to some web to help other KDE3 
XSI users.

Thanks for your help. This mail will be very helpful for doing the port 
when the moment comes.

Regards.