[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-core-devel
Subject: Re: Review Request: Jpeg thumbnailer honouring jpeg rotation info
From: "Jacopo De Simoi" <wilderkde () gmail ! com>
Date: 2009-08-30 21:30:27
Message-ID: 20090830213027.14655.26430 () localhost
[Download RAW message or body]
> On 2009-08-30 17:05:15, Peter Penz wrote:
> > 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 :-)
> >
> >
> - The (!(it == exifData.end())) might be more readable as (it != \
> exifData.end()) (?)
Of course, for some reasons, however the compiler complained when I first \
tried. Now it works.. lsd again?
- Jacopo
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/1444/#review2191
-----------------------------------------------------------
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
>
>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic