458
2012-05-05 23:51:16 +0000
Quaternion to rotation matrix
2019-12-04 11:38:43 +0000
1
1
1
Unclassified
Eigen
Geometry
3.4 (development)
All
All
CONFIRMED
Normal
Internal Design
---
560
1318
1
strasdat
eigen.nobody
chtz
gael.guennebaud
hauke.heibel
jacob.benoit.1
minorlogic
oldest_to_newest
1858
0
266
strasdat
2012-05-05 23:51:16 +0000
Created attachment 266
patch
I find it quite counter-intuitive that Quaternion::toRotationMatrix only returns an orthogonal matrix, if the quaternion is normalized, but something meaningless
otherwise.
Using a single division, this conversion can be performed for general non-zero quaternions (see patch).
3453
1
strasdat
2014-06-15 22:45:24 +0000
Clarification: That boils down to the question what the invariant of the class Quaternion is.
Especially this comment in the docs made me wonder: "Warning: Operations interpreting the quaternion as rotation have undefined behavior if the quaternion is not normalized".
In other words, different member functions assume different class invariants, e.g. ``toRotationMatrix`` requires a unit quaternion, while other methods don't guarantee that unity will be preserved.
A good long-term solution might be to introduce a class UnitQuaternion with a stronger invariant than Quaternion, so it can compute members such as ``toRotationMatrix``slightly more efficient (without violating the programming by contract principle).
3454
2
chtz
2014-06-15 22:58:27 +0000
Yes. I'd suggest concentrating the meta-discussion about Unit and Non-Unit quaternions at Bug 560 (which blocks other bugs related to that).
For extracting the rotational part of non-unit quaternions your patch is the right solution, it would however cost an unnecessary and (comparatively) expensive division for people who already guarantee unity of their quaternions.
6296
3
minorlogic
2016-09-29 14:33:28 +0000
I prefer additional normalization by "n2".
Here is my "pros"
- It is big performance issue to keep quaternion normalized. Any quaternion multiplication introduce scale drift, and in real usage we mast call renormalization after every operation. Especially when we model some entities updating its state (sceletal, physics, camera etc). The hardest case include conversion to matrix and back assuming the matrix is orthonormal. So normalization anyway is required, but during conversion to matrix it is much cheaper(4mul 1div).
- Even after normalization we can have small scale drift (several epsilons), and additional normalization can remove few bits of error.
- It is very hard to recover, such kind of accuracy loss. Even mature 3d packages introduce scale drift without control. From my experience , it is one of the common error when using quaternions.
"cons"
- Lower performance
- It can hide from user that quaternions are not unit, and introduce more bugs in other math
6301
4
gael.guennebaud
2016-09-29 19:51:48 +0000
I've benchmarked both versions with both clang and gcc, and with both compilers I observe a x2 slow-down. The impact is more significant than expected.
6309
5
minorlogic
2016-09-30 08:05:56 +0000
(In reply to Gael Guennebaud from comment #4)
> I've benchmarked both versions with both clang and gcc, and with both
> compilers I observe a x2 slow-down. The impact is more significant than
> expected.
Hmm... this is very surprising to me. With such a slow-down, im not sure it is way to do.
Then it is better to wait next versions with unit and nonunit policies.
6310
6
chtz
2016-09-30 08:15:11 +0000
If the division is the reason for the x2 slowdown, a compromise might be to do a taylor-based fast normalization if the norm is close to 1, i.e., (pseudo code):
if(abs(n2-1)<sqrt(epsilon))
two_by_squarednorm = 2*(2-x);
else
two_by_squarednorm = 2/x;
and hope that branch prediction always chooses the first.
We could also consider providing a normalizeFast() method (also for arbitrary vectors), which multiplies by 0.5*(3.0-squaredNorm()), if squaredNorm() is close enough to 1.0
6313
7
minorlogic
2016-09-30 08:30:35 +0000
(In reply to Christoph Hertzberg from comment #6)
> We could also consider providing a normalizeFast() method (also for
> arbitrary vectors), which multiplies by 0.5*(3.0-squaredNorm()), if
> squaredNorm() is close enough to 1.0
Method can be very usable on practice, but i propose to use other name. for example "stabilizeUnity" etc. Cause normalizeFast() usualy mean some tricks with inverse sqrt.
6314
8
gael.guennebaud
2016-09-30 10:49:23 +0000
Nope, most of the slowdown comes from squaredNorm even though it is perfectly vectorized.
8902
9
eigen.nobody
2019-12-04 11:38:43 +0000
-- GitLab Migration Automatic Message --
This bug has been migrated to gitlab.com's GitLab instance and has been closed from further activity.
You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.com/libeigen/eigen/issues/458.
266
2012-05-05 23:51:16 +0000
2012-05-06 00:42:46 +0000
patch
quaternion_to_rotation.patch
text/plain
1165
strasdat
LS0tIC90bXAvdG1wbHJzdTlHLW1lbGQvRWlnZW4vc3JjL0dlb21ldHJ5L1F1YXRlcm5pb24uaAor
KysgL2hvbWUvc3RyYXNkYXQvc3JjL1NjYVZpU0xBTS9FWFRFUk5BTC9laWdlbjMuMS9FaWdlbi9z
cmMvR2VvbWV0cnkvUXVhdGVybmlvbi5oCkBAIC01NDUsOCArNTQ1LDggQEAKICAgcmV0dXJuIGRl
cml2ZWQoKTsKIH0KIAotLyoqIENvbnZlcnQgdGhlIHF1YXRlcm5pb24gdG8gYSAzeDMgcm90YXRp
b24gbWF0cml4LiBUaGUgcXVhdGVybmlvbiBpcyByZXF1aXJlZCB0bwotICAqIGJlIG5vcm1hbGl6
ZWQsIG90aGVyd2lzZSB0aGUgcmVzdWx0IGlzIHVuZGVmaW5lZC4KKy8qKiBDb252ZXJ0IHRoZSBx
dWF0ZXJuaW9uIHRvIGEgM3gzIHJvdGF0aW9uIG1hdHJpeC4KKyAgKiBUaGUgcXVhdGVybmlvbiBp
cyBub3QgcmVxdWlyZWQgdG8gYmUgbm9ybWFsaXplZC4KICAgKi8KIHRlbXBsYXRlPGNsYXNzIERl
cml2ZWQ+CiBpbmxpbmUgdHlwZW5hbWUgUXVhdGVybmlvbkJhc2U8RGVyaXZlZD46Ok1hdHJpeDMK
QEAgLTU1OCw5ICs1NTgsMTcgQEAKICAgLy8gaXQgaGFzIHRvIGJlIGlubGluZWQsIGFuZCBzbyB0
aGUgcmV0dXJuIGJ5IHZhbHVlIGlzIG5vdCBhbiBpc3N1ZQogICBNYXRyaXgzIHJlczsKIAotICBj
b25zdCBTY2FsYXIgdHggID0gU2NhbGFyKDIpKnRoaXMtPngoKTsKLSAgY29uc3QgU2NhbGFyIHR5
ICA9IFNjYWxhcigyKSp0aGlzLT55KCk7Ci0gIGNvbnN0IFNjYWxhciB0eiAgPSBTY2FsYXIoMikq
dGhpcy0+eigpOworICBTY2FsYXIgbjIgPSB0aGlzLT5zcXVhcmVkTm9ybSgpOworICBpZiAobjIg
PT0gMCkKKyAgeworICAgIHJlcy5zZXRJZGVudGl0eSgpOworICAgIHJldHVybiByZXM7CisgIH0K
KworICBjb25zdCBTY2FsYXIgdHdvX2J5X3NxdWFyZWRub3JtID0gMi4vbjI7CisgIGNvbnN0IFNj
YWxhciB0eCAgPSB0d29fYnlfc3F1YXJlZG5vcm0qdGhpcy0+eCgpOworICBjb25zdCBTY2FsYXIg
dHkgID0gdHdvX2J5X3NxdWFyZWRub3JtKnRoaXMtPnkoKTsKKyAgY29uc3QgU2NhbGFyIHR6ICA9
IHR3b19ieV9zcXVhcmVkbm9ybSp0aGlzLT56KCk7CiAgIGNvbnN0IFNjYWxhciB0d3ggPSB0eCp0
aGlzLT53KCk7CiAgIGNvbnN0IFNjYWxhciB0d3kgPSB0eSp0aGlzLT53KCk7CiAgIGNvbnN0IFNj
YWxhciB0d3ogPSB0eip0aGlzLT53KCk7Cg==