Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Quaternion-to-matrix identity rotated #10

Closed
ghost opened this issue Dec 20, 2017 · 8 comments
Closed

Quaternion-to-matrix identity rotated #10

ghost opened this issue Dec 20, 2017 · 8 comments
Labels

Comments

@ghost
Copy link

ghost commented Dec 20, 2017

I'm seeing the following behavior for quaternion to matrix conversion.

versor rotation = GLM_QUAT_IDENTITY_INIT;
fprintf(stderr, "-- Quaternion --\n%.4f %.4f %.4f %.4f\n\n", rotation[0], rotation[1], rotation[2], rotation[3]);

mat4 rotMatrix = GLM_MAT4_IDENTITY_INIT;
glm_quat_mat4(rotation, rotMatrix);
fprintf(stderr, "-- Matrix --\n%.4f %.4f %.4f %.4f\n%.4f %.4f %.4f %.4f\n%.4f %.4f %.4f %.4f\n%.4f %.4f %.4f %.4f\n", rotMatrix[0][0], rotMatrix[0][1], rotMatrix[0][2], rotMatrix[0][3], rotMatrix[1][0], rotMatrix[1][1], rotMatrix[1][2], rotMatrix[1][3], rotMatrix[2][0], rotMatrix[2][1], rotMatrix[2][2], rotMatrix[2][3], rotMatrix[3][0], rotMatrix[3][1], rotMatrix[3][2], rotMatrix[3][3]);

outputs

-- Quaternion --
0.0000 0.0000 0.0000 1.0000

-- Matrix --
-1.0000 0.0000 0.0000 0.0000
0.0000 -1.0000 0.0000 0.0000
0.0000 0.0000 1.0000 0.0000
0.0000 0.0000 0.0000 1.0000

The identity quaternion is what I expect, but the matrix seems to be flipped 180 degrees on X and Y. I noticed this because everything I render is flipped upside down. Shouldn't rotMatrix[0][0] and rotMatrix[1][1] equal positive 1 instead of negative 1 when using an identity quaternion? Or maybe I don't understand quaternions fully.

Platform:
Linux 64-bit
gcc v7.2.1
cglm v0.3.1

@ghost ghost changed the title Quaternion-to-matrix getting inverted Quaternion-to-matrix identity rotated Dec 20, 2017
@ghost
Copy link
Author

ghost commented Dec 22, 2017

@recp Any ideas on this?

@recp
Copy link
Owner

recp commented Dec 26, 2017

@winduptoy sorry I didn't see this issue until now, fell free to ping me again

I think identity versor (GLM_QUAT_IDENTITY_INIT) is wrong.

GLM_QUAT_IDENTITY_INIT  {0.0f, 0.0f, 0.0f, 1.0f}
versor rotation2;
glm_quat(rotation2, 0, 0, 0, 0);

gives {1.0f, 0.0f, 0.0f, 0.0f} so it must be

GLM_QUAT_IDENTITY_INIT  {1.0f, 0.0f, 0.0f, 0.0f}

but GLM_QUAT_IDENTITY_INIT {0.0f, 0.0f, 0.0f, 0.0f} also seems valid for your case. I'll fix this ASAP, but if you want to fix that, PR is welcome :)

These functions also creates versor for you:

CGLM_INLINE void glm_quat(versor q, float angle, float x, float y, float z);
CGLM_INLINE void glm_quatv(versor q, float angle, vec3 v);

versor rotation;
glm_quat(rotation, glm_deg(45.0f), 1.0f, 0.0f, 0.0f);

/* or  */

glm_quatv(rotation, glm_deg(45.0f), (vec3){1.0f, 0.0f, 0.0f}); 
/* v postrix means it accepts vector parameter */
/* this axis vector maybe stored somewhere-else  */

q is out parameter, I must change its name.

Addition

You don't have to initialize dest (or out) parameters. In your case you dont have to initialize rotMatrix because it is dest/out parameter which means glm_quat_mat4 is already creating matrix for you. But [in, out] parameter may need to be initialized e.g. glm_vec_normalize(vec) vec is in and out, read the header file. I will add docs to quad header too

mat4 rotMatrix;
glm_quat_mat4(rotation, rotMatrix);

would be enough, but since function is inline I think compiler will ignore identity initialization (I hope)

Also I would use built-in print utils to print them, it would save you to write huge printf :)

Print matrix: glm_mat4_print(rotMatrix, stderr);
Print Versor: glm_versor_print(rotation2, stderr);
Print Vec3: glm_vec3_print(vec, stderr)
Print Vec4: glm_vec4_print(vec, stderr)

Important

Some resources / tools / libraries may use x, y, z, w order but cglm uses w, x, y, z order to represent quaternion: [angle, axis]

So you may see Quaternion.Identity = [0, 0, 0, 1] in MSDN or Unity or somewhere else, but it is [1, 0, 0, 0] in cglm and some other libraries

I doesn't matter for me, if it must be x, y, z, w then we can change this.

EDIT:

[major change] by starting v0.4.0, quaternions are stored as [x, y, z, w], it was [w, x, y, z] in v0.3.5 and earlier versions

@recp recp added the bug label Dec 26, 2017
@ghost
Copy link
Author

ghost commented Dec 26, 2017

Ah thank you for pointing out the print functions! I must have missed those. I'd like to help you with documentation soon.

I did not realize that cglm used w, x, y, z, I simply assumed it was x, y, z, w. I don't really have a preference, but I think it would be important to document this in a very visible place.

Thank you! Hope you had a good holiday wherever in the world you may be.

@recp
Copy link
Owner

recp commented Dec 26, 2017

I'd like to help you with documentation soon.

That would be wonderful

I did not realize that cglm used w, x, y, z, I simply assumed it was x, y, z, w. I don't really have a preference, but I think it would be important to document this in a very visible place.

I'm confused now, probably most people may assume that is x, y, z, w. For C++ classes probably most people accessing those values by ctor or its name like quat->x, quat->w so it doesn't matter for them I think, but since we have stored them as array order is matter. versor is stored as vec4 (aligned) because it is perfectly fits with SIMD instuctions to optimize them. Maybe documentation would prevent any confusion here

Thank you! Hope you had a good holiday wherever in the world you may be.

Thank you very much, I hope you too.

recp added a commit that referenced this issue Dec 26, 2017
Fixed GLM_QUAT_IDENTITY_INIT (wxyz) instead of (xyzw). Addresses #10.
@recp
Copy link
Owner

recp commented Dec 26, 2017

Fixed by: #11

@recp recp closed this as completed Dec 26, 2017
@ghost
Copy link
Author

ghost commented Aug 16, 2018

@recp Sorry to bring this up again, I hope this isn't just a silly issue with my understanding.

You said in the comment above that cglm uses w, x, y, z order for the quaternion where w, the first component, is the real part of the quaternion.

So, if you take a look at the current implementation of glm_quat_init() you will see this:

void glm_quat_init(versor q, float x, float y, float z, float w) {
  q[0] = x;
  q[1] = y;
  q[2] = z;
  q[3] = w;
}

Does this go against what you said earlier? I realize that the arguments are the same order as the components in the quaternion and the name doesn't really matter, but if I have a mental model of things being stored as w, x, y, z and then I go read the documentation to find out that the function signature is x, y, z, w, I would assume that the argument named w would be placed at q[0] instead of q[3]. The document says that w is the real part (last argument) but that doesn't line up with what you have presented above. Does that make sense?

@recp
Copy link
Owner

recp commented Aug 16, 2018

Do not worry, you are welcome to ask anything!, it helps to improve this library.

cglm was used wxyz layout/order until this pull request: #43 and version v0.4.0. Check the last comment in PR ;)

Currently it must use xyzw and now quaternion api is more powerful than before. So last element is w now, it is real part yes.

If there is a bug please let me know or try to fix it by a PR :)

PS: I added a note about this change to my previous comment for the future. Also it is important to follow "Note for previous versions" section in README. It should help to migrate to new version[s].

@ghost
Copy link
Author

ghost commented Aug 17, 2018

Awesome, thanks for explanation. You're improving the library faster than I can keep up!

As always, thank you for this wonderful library and your excellent work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant