Bug 344155

Created attachment 91081 [details]
rawrotation.patch

I put this patch for discussion. Now is the Raw data in half size preview in sync with the thumbnail. But I'm not 100% sure...

Maik,

I'm not sure if your patch is fine about Raw preview rotation support.

Some technical points about RAW :

1/ Raw are mostly TIFF/EP files with private data. RAW are container of data (images + metadata)
2/ Preview in RAW are typically reduced JPEG generated by camera.
3/ Raw image has dedicated metadata. This include Exif orientaion flag.
4/ Preview Image has dedicated metadata also, with another orientation flag.
5/ When you rotate image in camera (while image review on screen), you change orientation flag of preview image, NOT RAW orientation flag.

So, if possible to have an non homogeneous orientation flags in RAW files. This give strange results in digiKam.

I can reproduce the dysfunction with my Sony A77, as explained before.

Gilles

Created attachment 91206 [details]
rotationmatrix.patch

The bug is in libkexiv2, matrix variables for rotate90 and rotate270 are reversed.
Test:
QMatrix m;
m.rotate(90); // m11() == 0, m12() == 1, m21() == -1, m22() == 0
m.rotate(270); // m11() == 0, m12() == -1, m21() == 1, m22() == 0

Git commit 8498b1b0820d4070030f3118159d8674f0dbd0e4 by Gilles Caulier.
Committed on 22/02/2015 at 07:00.
Pushed by cgilles into branch 'master'.

apply patch #91206 from MAik Qualmann to fix rotation matrix issue in libkexiv2
FIXED-IN: 4.8.0

M  +7    -5    libkexiv2/rotationmatrix.cpp
M  +1    -1    libkexiv2/rotationmatrix.h

http://commits.kde.org/libkexiv2/8498b1b0820d4070030f3118159d8674f0dbd0e4

Git commit 54d7aecc0ff400c9acd4739fb723570352d8238a by Gilles Caulier.
Committed on 22/02/2015 at 07:05.
Pushed by cgilles into branch 'frameworks'.

backport commit #8498b1b0820d4070030f3118159d8674f0dbd0e4 from git/master to frameworks branch

M  +5    -7    src/rotationmatrix.cpp

http://commits.kde.org/libkexiv2/54d7aecc0ff400c9acd4739fb723570352d8238a

Git commit 5a716d237a6c379c6300df2b1cc8d25b9ccd9773 by Gilles Caulier.
Committed on 22/02/2015 at 07:09.
Pushed by cgilles into branch 'KDE/4.14'.

backport commit #8498b1b0820d4070030f3118159d8674f0dbd0e4 from git/master to KDE/4.14 branch

M  +4    -4    libkexiv2/rotationmatrix.cpp

http://commits.kde.org/libkexiv2/5a716d237a6c379c6300df2b1cc8d25b9ccd9773

diff --git a/libkexiv2/rotationmatrix.cpp b/libkexiv2/rotationmatrix.cpp
index b96a174..1e4eb6c 100644
--- a/libkexiv2/rotationmatrix.cpp
+++ b/libkexiv2/rotationmatrix.cpp
@@ -65,9 +65,9 @@ namespace KExiv2Iface
    (I did not proof that mathematically, but empirically)

    static const RotationMatrix identity;               //( 1,  0,  0,  1)
-   static const RotationMatrix rotate90;               //( 0, -1,  1,  0)
+   static const RotationMatrix rotate90;               //( 0,  1, -1,  0)
    static const RotationMatrix rotate180;              //(-1,  0,  0, -1)
-   static const RotationMatrix rotate270;              //( 0,  1, -1,  0)
+   static const RotationMatrix rotate270;              //( 0, -1,  1,  0)
    static const RotationMatrix flipHorizontal;         //(-1,  0,  0,  1)
    static const RotationMatrix flipVertical;           //( 1,  0,  0, -1)
    static const RotationMatrix rotate90flipHorizontal; //( 0,  1,  1,  0), first rotate, then flip
@@ -79,9 +79,9 @@ namespace Matrix
 {

 static const RotationMatrix identity               ( 1,  0,  0,  1);
-static const RotationMatrix rotate90               ( 0, -1,  1,  0);
+static const RotationMatrix rotate90               ( 0,  1, -1,  0);
 static const RotationMatrix rotate180              (-1,  0,  0, -1);
-static const RotationMatrix rotate270              ( 0,  1, -1,  0);
+static const RotationMatrix rotate270              ( 0, -1,  1,  0);
 static const RotationMatrix flipHorizontal         (-1,  0,  0,  1);
 static const RotationMatrix flipVertical           ( 1,  0,  0, -1);
 static const RotationMatrix rotate90flipHorizontal ( 0,  1,  1,  0);

My comment is about the original problem and the patch from comment 1:

The complexity has already been explained by Gilles. Another component of the problem is that libraw/dcraw does some intransparent magic of its own and gives us the data readily rotated. Which means that we never know in how far the rotation flags that we can read were actually put in use (maybe the information was taken from some makernotes, or other special knowledge).

Which means that we read a rotation flag via exiv2, and libraw may use another value.

Second part of the problem is that we support "virtual rotation" for RAWs, which means that when rotating in digikam, the flag in digikam's db is changed, but we cannot change the flag in the raw file (because metadata writing to RAWs is not universally supported, and the complexity of the file formats would make it difficult to find the right flag for all formats.
That's the intention of the patched code: Normally, digikam's database contains the exiv2-read rotation value. If we detect that the current code in the database is different, the user must have applied an operation, and this operation B is computed from A and C.

One possible solution was if we would read the rotation flag via libraw instead of exiv2, or see that the two information source gave the same results.
The second problem is that the thumbnail may have a different rotation than the main data. I dont know, we must with several formats and see for each single format where the problem lies.

(In addition I put forward - I'm not a mathematician, so please correct my equations if necessary - that mathematically, the proposal is not correct. 
We have A*B = C. 
By multiplication from the left with A_inv, we get
<=> A_inv*A*B = A_inv *C
<=> I * B = A_inv * C
<=> B = A_inv * C

Assume the proposed "B=C_inv*A" was correct:
B = C_inv*A
<=> C*B = C*C_inv*A
<=> C*B = A
<=> C*B*B_inv = A*B_inv
<=> C = A*B_inv
which is true only if B=B_inv, or B*B_inv = I, which is not true for rotate90 for example.
)