--nextPart1286876.aKvYNmQnHD
Content-Type: text/plain;
  charset="utf-8"
Content-Transfer-Encoding: quoted-printable
Content-Disposition: inline

On Thursday 30 August 2007, Ruben Lopez wrote:
> Ingo Kl=C3=B6cker 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 =3D pixel & 0xFF;
> >> 	unsigned char g =3D (pixel >> 8) & 0xFF;
> >> 	unsigned char b =3D (pixel >> 16) & 0xFF;
> >> 	unsigned char a =3D (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 =3D)

> 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 th=
e=20
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 =3D)

> If submitted to kdegraphics, what is=20
> 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=
=20
same, i suppose.=20

so, i looked at the source tarball a bit more just now (only after having s=
ent=20
the last email.. meh, bad aaron, bad!) and i see it's very small and indeed=
 a=20
user-invisible plugin. so upon further examination, i'm fine with having it=
=20
in kdegraphics itself, at trunk/KDE/kdegraphics/imageio/pic/. the contents =
of=20
the share/ and src/ directories should both go in there.

it also occurred to me while reading through the tarball that it's kde3 cod=
e=20
=3D) kde3 is no longer accepting new features / code, but you could certain=
ly=20
offer this version in branches/extragear/kde3/graphics.

going forward, we need to transition it to kde4, which includes:

=2D the mimetype .desktop file needs to be transitioned to the shared mimet=
ype=20
xml[1]; it would be great to move this upstream[2] as well. i could help he=
re=20
if needed.

=2D moving the build to cmake. that's a 5 minute job and i could also do th=
at=20
for you if you'd like.

=2D port and make sure it compiles. this is the part that entails some work=
, and=20
entails:

  - porting kfile_pic.* to a strigi-analyzer. this is pretty easy, and the=
=20
results should go into kdegraphics/strigi-analyzer/pic/ ... it can share th=
e=20
pic_read.* files with the imageio plugin
  - rather than kimigio_* factories, this needs to be a QImageIOPlugin and =
a=20
QImageIOHandler needs to be written around pic_read and pic_write.=20
essentially, canRead, read and write need to be implemented; there are a=20
number of other virtuals that may or may not be interesting. read and write=
=20
should be pretty easy, as they essentially can be ported verbatin from the=
=20
current kimgio_pic_read and kimgio_pic_write.

so ... do you have a kde4 install around? are you interested in / able to d=
o=20
the porting (either with assistance or on your own)? it would be cool to ge=
t=20
this into kde4, though there is a bit of work to do. the majority of the=20
work, indeed the hard bits (the format support itself), is already done so=
=20
it's just the final bits to tie up.

[1]=20
http://standards.freedesktop.org/shared-mime-info-spec/shared-mime-info-spe=
c-latest.html
[2] http://www.freedesktop.org/wiki/Software/shared-mime-info

=2D-=20
Aaron J. Seigo
humru othro a kohnu se
GPG Fingerprint: 8B8B 2209 0C6F 7C47 B1EA  EE75 D6B7 2EB1 A7F1 DB43

KDE core developer sponsored by Trolltech

--nextPart1286876.aKvYNmQnHD
Content-Type: application/pgp-signature

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)

iD8DBQBG1w/K1rcusafx20MRAkUwAKCtqPc6YDP9gg1BtFhBsxVqRrAA6ACgqPz7
uD5j63c2xbak0zBoXpx2ALY=
=zZas
-----END PGP SIGNATURE-----

--nextPart1286876.aKvYNmQnHD--