Fix quaternion orientation in `glm::decompose` #1012

bosmacs · 2020-05-11T22:11:52Z

Between 0.9.8 and 0.9.9 a change to the implementation of decompose caused the conjugate of the rotation quaternion to be returned. This has also been noted in this Stack Overflow question, in particular this comment. This seems like a regression that ought to be fixed to avoid having to conjugate the result of decompose in the future.

christophe-lunarg · 2020-11-21T21:27:55Z

Thanks for contributing!

thekend · 2021-02-20T15:20:36Z

I think something is wrong with this "fix". Let's say we have a quat Q. Let M = mat4_cast(Q) and let Q2 be the quaternion resulted by decomposing M. We should get the same results with transforming a vec3 by Q, M and Q2. However, it's not the case. It seems that transforming with Q and M are results the same vector but with Q2 we get different result.

`

vec3 transformVec(const mat4& m, const vec3& v)
{
    vec4 vh(v.x, v.y, v.z, 1.0f);
    vh = m * vh;
    return { vh.x / vh.w, vh.y / vh.w, vh.z / vh.w };
}

void testDecompose()
{
    constexpr const vec3 vI = { 1.0f, 0.0f, 0.0f };
    constexpr const vec3 vJ = { 0.0f, 1.0f, 0.0f };
    constexpr const vec3 vK = { 0.0f, 0.0f, 1.0f };

    quat rotQ = angleAxis(radians(45.0f), vec3(0.0f, 1.0f, 0.0f));

    vec3 xQ = rotQ * vI;
    vec3 yQ = rotQ * vJ;
    vec3 zQ = rotQ * vK;

    mat4 rotM = mat4_cast(rotQ);

    vec3 xM = transformVec(rotM, vI);
    vec3 yM = transformVec(rotM, vJ);
    vec3 zM = transformVec(rotM, vK);

    vec3 scale;
    quat rotQ2;
    vec3 trans;
    vec3 skew;
    vec4 proj;
    decompose(rotM, scale, rotQ2, trans, skew, proj);

    vec3 xQ2 = rotQ2 * vI;
    vec3 yQ2 = rotQ2 * vJ;
    vec3 zQ2 = rotQ2 * vK;
}

`

xQ = {0.707106709, 0.00000000, -0.707106829}
xM = {0.707106709, 0.00000000, -0.707106829}
xQ2 = {0.707106709, 0.00000000, 0.707106829}

stephomi · 2021-03-05T21:33:05Z

I Agree, I'm still using the version before the fix as it was working perfectly for my use case.
I only decompose matrices with translation, rotation and scale (non-uniform), but I can't speak for skew/persp.

I don't get all the comments about conjugate in the stackoverflow thread though, maybe it was a side effect somewhere else that has been fixed since?

I would expect a simple decompose/recompose operation to always fall back on the initial matrix, like before:

glm::vec3 scale, trans, skew;
glm::vec4 persp;
glm::quat rot;
bool success = glm::decompose(matrix, scale, rot, trans, skew, persp);

// matrix ~= success
glm::mat4 recompose = glm::translate(trans) * glm::mat4(rot) * glm::scale(scale);

thekend · 2021-03-06T13:59:27Z

Yes, I used this function before and it worked well. Now it's wrong and because I also used it only for matrices composed of scales, rotations and translations, now I use my own simpler decomposeAffineMatrix which decomposes to scale, translation and 3x3 rotation mat and then use glm::quat_cast which seems also working well.

christophe-lunarg · 2021-03-06T16:12:51Z

Do you think we should revert ?
#1061 ?

bosmacs · 2021-03-06T22:23:06Z

I think I've convinced myself that it ought to be reverted. I think the confusion on my (and stack overflow's) part may stem from the column-major indexing of GLSL/glm. In particular, the way Row[i] is constructed, won't it actually contain the ith column of the input matrix? So the way the math is usually written has to be transposed, as it was before.

Revert "Fix quaternion orientation in `glm::decompose`" #1061 #1012

Fix quaternion orientation in glm::decompose

f8bf087

christophe-lunarg merged commit 1cf91a1 into g-truc:master Nov 21, 2020

christophe-lunarg added this to the GLM 0.9.9 milestone Nov 21, 2020

christophe-lunarg self-assigned this Nov 21, 2020

christophe-lunarg added the bug label Nov 21, 2020

thekend mentioned this pull request Mar 6, 2021

matrix decompose(): fix bug #1046

Closed

christophe-lunarg mentioned this pull request Mar 6, 2021

Revert "Fix quaternion orientation in glm::decompose" #1061

Merged

christophe-lunarg added a commit that referenced this pull request Mar 7, 2021

Merge pull request #1061 from g-truc/revert-1012-master

5e243a8

Revert "Fix quaternion orientation in `glm::decompose`" #1061 #1012

perkele1989 mentioned this pull request Apr 6, 2024

decompose error! #941

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix quaternion orientation in `glm::decompose` #1012

Fix quaternion orientation in `glm::decompose` #1012

Fix quaternion orientation in glm::decompose #1012

Fix quaternion orientation in glm::decompose #1012

Conversation

Fix quaternion orientation in `glm::decompose` #1012

Fix quaternion orientation in `glm::decompose` #1012