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

[Sebastian Scheibner]

> On Oct. 21, 2012, 10:13 p.m., Kevin Krammer wrote:
> > There seem to be two new files in this revision. Any specific reason for those additonal changes?

yes imagewidget.h and imagewidget.cpp. These files are used in the edit contact dialog to set the contact photo.
When the dialog was opened, the imagewidget took the QImage from the contact and stored it. After clicking ok, the QImage was stored back in the contact and because setData is called on the Picture, the raw data is lost and the image was recompressed when the vcard was saved.
I changed them so the imagewidget now stores a Picture with the raw data instead of the QImage.

I missed this when I first made the patch, but as this is also needed to fix this bug, I included it in this review request


> On Oct. 21, 2012, 10:13 p.m., Kevin Krammer wrote:
> > kabc/picture.h, line 117
> > <http://git.reviewboard.kde.org/r/106745/diff/4/?file=91690#file91690line117>
> >
> >     please add
> >     @since 4.10
> >     to all new methods

ok


> On Oct. 21, 2012, 10:13 p.m., Kevin Krammer wrote:
> > kabc/picture.h, line 123
> > <http://git.reviewboard.kde.org/r/106745/diff/4/?file=91690#file91690line123>
> >
> >     doesn't it set the type to "jpeg" or "png"? I.e. without the image/ prefix?

right, missed that.


> On Oct. 21, 2012, 10:13 p.m., Kevin Krammer wrote:
> > kabc/picture.cpp, line 203
> > <http://git.reviewboard.kde.org/r/106745/diff/4/?file=91691#file91691line203>
> >
> >     since mType was already set by setData, you can switch in the type here instead of duplicating the check, no?
> >     or basically
> >     d->mData.save( &buffer, d->mType.toUpper() );
> >

ok


> On Oct. 21, 2012, 10:13 p.m., Kevin Krammer wrote:
> > kabc/picture.cpp, line 242
> > <http://git.reviewboard.kde.org/r/106745/diff/4/?file=91691#file91691line242>
> >
> >     I am not sure it is a good idea to change the format of the serialized picture.
> >     Might be better to keep the stream format, i.e.
> >     - << picture.d->mData;
> >     + << picture.data();

ok, changed that


- Sebastian


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/106745/#review20644
-----------------------------------------------------------


On Oct. 21, 2012, 11:45 p.m., Sebastian Scheibner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/106745/
> -----------------------------------------------------------
> 
> (Updated Oct. 21, 2012, 11:45 p.m.)
> 
> 
> Review request for KDEPIM-Libraries.
> 
> 
> Description
> -------
> 
> 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.
> 
> fixes this bug: https://bugs.kde.org/show_bug.cgi?id=307570
> 
> 
> This addresses bug 307570.
>     http://bugs.kde.org/show_bug.cgi?id=307570
> 
> 
> Diffs
> -----
> 
>   akonadi/contact/editor/imagewidget.h 92df283 
>   akonadi/contact/editor/imagewidget.cpp e1a0527 
>   kabc/picture.h bf9745c 
>   kabc/picture.cpp 0a51c8b 
>   kabc/tests/picturetest.h 2b33e18 
>   kabc/tests/picturetest.cpp b599b48 
>   kabc/vcardparser/testroundtrip.qrc a858dc2 
>   kabc/vcardparser/tests/vcard5.vcf.ref 712b494 
>   kabc/vcardtool.cpp e876e81 
> 
> Diff: http://git.reviewboard.kde.org/r/106745/diff/
> 
> 
> Testing
> -------
> 
> compiled kdepimlibs/kabc with kde 4.9.2
> 
> - created a new contact in kaddressbook and added a photo. exported the contact.
> - changed the photo and exported the contact again
> - the image data in the exported file didn't change
> 
> 
> Thanks,
> 
> Sebastian Scheibner
> 
>

_______________________________________________
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/