[PATCH] New image loader (Softimage PIC)

David Faure faure at kde.org
Fri Dec 18 21:06:12 GMT 2009


Hello,

Thanks for your contribution. Some comments below:

On Thursday 17 December 2009, Ruben Lopez wrote:
> +  <mime-type type="image/x-pic">
> +    <comment>Sofrimate PIC image</comment>
> +    <magic priority="80">
> +      <match value="0x5380f634" type="big32" offset="0"/>
> +    </magic>
> +    <glob pattern="*.pic"/>
> +  </mime-type>
 
A mimetype for *.pic files is already defined in kdelibs/mimetypes/kde.xml and 
is called image/x-hdr.
I suppose that's a mistake, and the glob should be removed from image/x-hdr? 
Or is the extension indeed ambiguous between these two image formats?


Not much to say about the image loader itself (except that it could
follow the kdelibs coding style a bit more, especially the if+return in
one line hurts readability given that we're not used to that).
Ah, and pic_io_handler.h (at least) lacks a copyright header.
And don't include <Qt/qimage.h>, I think this breaks on Mac,
use <qimage.h> or <QImage> or <QtGui/QImage> -- plenty of choices available ;)

-- 
David Faure, faure at kde.org, http://www.davidfaure.fr
Sponsored by Nokia to work on KDE, incl. Konqueror (http://www.konqueror.org).




More information about the kde-core-devel mailing list