<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/122873/">https://git.reviewboard.kde.org/r/122873/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On March 13th, 2015, 5:23 p.m. CET, <b>David Edmundson</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Do you have commit access?</pre>
</blockquote>
<p>On March 13th, 2015, 5:28 p.m. CET, <b>Martin Kyral</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Unfortunatelly not. I made 2 patches for gwenview so far, both committed by Aurelien on my behalf.</p></pre>
</blockquote>
<p>On March 18th, 2015, 2:36 p.m. CET, <b>Martin Kyral</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Could you, please, commit it?</p></pre>
</blockquote>
<p>On March 22nd, 2015, 5:01 p.m. CET, <b>Aurélien Gâteau</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The changes done by the Qt patch do not affect only RAW images: JPEGs are not displayed correctly anymore since they rotated once by Qt and then again by Gwenview. I guess some of the special-handling code for JPEG files can simply be removed. Could you look into this?</p></pre>
</blockquote>
<p>On March 23rd, 2015, 11:22 a.m. CET, <b>Martin Kyral</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">It's strange, but RAW images and the previews dumped from RAW using dcraw -e are the only images affected by the double rotation. I tried removing the jpeg special-handling code unconditionally but it actually breaks the rest (I tried with the images from the gwenview tests and with images from https://github.com/recurser/exif-orientation-examples.git) so I decided to implement the fix as is in this patch. However, as I am stating in the description - the previews saved by dcraw -e (technically exactly the same data as the RAW handling code extracts) are not fixed here and this case needs to be further elaborated (probably using better detection from exif).</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">It seems that there are two disctinct ways of lossless rotation of jpeg. I fiddled with some images in geeqie and gwenview with interesting results. While rotating jpeg in geeqie does not alter file size at all, rotating the same image in gwenview produces different file sizes for portrait and landscape orientation of the same image (but the file does not shrink upon multiple rotations so the data are not recompressed, thus the rotation is losless). When I rotated the image in geeqie and in gwenview I ended up with image which was displayed improperly by one of the viewers, maybe both.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Unfortunatelly I am not familiar enough with the JPEG format nor the EXIF metadata so I am unable now to see the cause.</p></pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">My guess regarding the difference you see between Geeqie and Gwenview is that Gwenview losslessly rotates the pixels and resets the orientation flag to 1, whereas I assume Geeqie only changes the orientation flag. The way Gwenview behaves can produce weird results if the dimensions of the image are not multiples of 8, which may be the reason why your image was displayed improperly.</p></pre>
<br />
<p>- Aurélien</p>
<br />
<p>On March 13th, 2015, 5:21 p.m. CET, Martin Kyral wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
<div>Review request for Gwenview.</div>
<div>By Martin Kyral.</div>
<p style="color: grey;"><i>Updated March 13, 2015, 5:21 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
gwenview
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Displaying of RAW images in gwenview was broken by https://bugreports.qt.io/browse/QTBUG-37946. I am fixing (or rather workarounding it) by blocking EXIF rotation iff the loaded image is RAW.</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Tested with various RAW files, used previously to test the RAW loading support itself & with some jpeg files (including https://github.com/recurser/exif-orientation-examples.git).</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Seems OK.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Nevertheless, the fix is not complete as jpeg files extracted from the RAWs using 'dcraw -e' are still mis-rotated.</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>lib/document/loadingdocumentimpl.cpp <span style="color: grey">(cc8bea9)</span></li>
<li>lib/thumbnailprovider/thumbnailgenerator.h <span style="color: grey">(4571832)</span></li>
<li>lib/thumbnailprovider/thumbnailgenerator.cpp <span style="color: grey">(54875f5)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/122873/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>