[Kde-pim] Review Request: Don't always recompress contact photos when saving

[Kevin Krammer]

Hi Sebastian,

thank you for working on a fix to this nasty oversight.

I have a couple of general thoughts though.

On Saturday, 2012-10-06, Sebastian Scheibner wrote:

> When changing a contact with a photo, the photo is recompressed when saving
> (even though the photo wasn't changed). That's because the photo is loaded
> into a QImage and when the contact is saved, the QImage is compressed as
> JPEG again (or PNG if it contains an alpha-channel, which most don't).
> 
> This patch stores the raw base64 photo data from the vcard as a QByteArray
> in KABC::Picture. If the photo is not changed, this QByteArray is simply
> used when saving the contact, so there's no recompression and quality
> loss. If the photo was changed it gets compressed as JPEG as before.

I am a bit concerned about the asymmetry of the two getter/setters invovled.
Calling setRawData() will make rawData() and data() return the expected 
results, calling setData() will only make data() return the expected result, 
while rawData() will return "there is no image data in this picture object".

One idea would be to make the setters always clear the other field and the 
getters update their field if only the other one is present.

Or maybe we could just always encode as PNG, thus solving the bug by not 
applying lossy compression?

In any case I'd recommend you add a unit test that shows that setting raw data 
allows retrieval of image data and that saving one picture with raw data and 
one picture with image data results in the same vcard output (of course 
assuming that the raw data has been created from the image and uses the same 
encoding format).

Cheers,
Kevin
-- 
Kevin Krammer, KDE developer, xdg-utils developer
KDE user support, developer mentoring
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 190 bytes
Desc: This is a digitally signed message part.
URL: <http://mail.kde.org/pipermail/kde-pim/attachments/20121006/4e517037/attachment.sig>
-------------- next part --------------
_______________________________________________
KDE PIM mailing list kde-pim at kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/