Review Request: Jpeg thumbnailer honouring jpeg rotation info

[Peter Penz]

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


I cannot comment on the CMakeList-stuff, but the patch looks fine. Some minor nitpicking:

- I'd replace the following method signature:
    const QTransform JpegCreator::orientationMatrix(const int exifOrientation) const
by 
    QTransform JpegCreator::orientationMatrix(int exifOrientation) const
as a passing by value is done and using 'const' only restricts the caller of the method or the implementation inside the method in an unnecessary way (not in this example, but as a general guideline).

- The (!(it == exifData.end())) might be more readable as (it != exifData.end()) (?)

These are just suggestions :-)



- Peter


On 2009-08-30 12:58:47, Jacopo De Simoi wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/1444/
> -----------------------------------------------------------
> 
> (Updated 2009-08-30 12:58:47)
> 
> 
> Review request for kdelibs.
> 
> 
> Summary
> -------
> 
> This patch makes the jpeg thumbnailer honor jpeg rotation infos stored in exif metadata. 
> The method is quite simple, although I don't like ifdefs, this time they seem to me to be necessary.
> It's my first nontrivial CMake modification; please check that I did not do something stupid there.
> Also, the orientationMatrix method could be ifdeffed if you feel it is necessary; I just didn't want to add another ifdef.
> 
> I am not sure if this is a good solution performance-wise; please comment on that if you have better ideas.
> 
> 
> Diffs
> -----
> 
>   branches/KDE/4.3/kdebase/runtime/kioslave/thumbnail/CMakeLists.txt 1016603 
>   branches/KDE/4.3/kdebase/runtime/kioslave/thumbnail/jpegcreator.h 1016603 
>   branches/KDE/4.3/kdebase/runtime/kioslave/thumbnail/jpegcreator.cpp 1016603 
> 
> Diff: http://reviewboard.kde.org/r/1444/diff
> 
> 
> Testing
> -------
> 
> Works good with a *clean* .thumbnails directory. Cached thumbnails are indeed a problem; not sure how to solve this issue yet.
> 
> 
> Screenshots
> -----------
> 
> Dolphin showing correctly rotated jpegs
>   http://reviewboard.kde.org/r/1444/s/192/
> 
> 
> Thanks,
> 
> Jacopo
> 
>