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

Possible error when setting model matrix #1956

Closed
crbozz opened this issue Aug 2, 2018 · 7 comments
Closed

Possible error when setting model matrix #1956

crbozz opened this issue Aug 2, 2018 · 7 comments
Labels

Comments

@crbozz
Copy link
Contributor

crbozz commented Aug 2, 2018

glm::mat3 rotation_mat(matrix[0][0] / new_scale.x,

I'm facing a problem when trying to set model matrix to a scene object and it seems the statement starting at the line above is the reason. When setting model matrix for a scene object (using setModelMatrix(Matrix4f mat)) position and scale are correct but rotation is not. I tried to calculate position, scale and rotation at Java side (using JOML) and then apply them (using Java transform API methods) and it worked fine. Then I tried to change the above statement as below and it also worked:

glm::mat3 rotation_mat(matrix[0][0] / new_scale.x, matrix[0][1] / new_scale.x, matrix[0][2] / new_scale.x, matrix[1][0] / new_scale.y, matrix[1][1] / new_scale.y, matrix[1][2] / new_scale.y, matrix[2][0] / new_scale.z, matrix[2][1] / new_scale.z, matrix[2][2] / new_scale.z);

Is this really and issue or maybe there is something else to consider when setting model matrix?

@liaxim liaxim added the bug label Aug 7, 2018
@liaxim
Copy link
Contributor

liaxim commented Aug 8, 2018

Can you share code showing the problem?
Could it be a row vs column major confusion somewhere in the code?

@crbozz
Copy link
Contributor Author

crbozz commented Aug 8, 2018

The problem was detected at the new A.R. demo we are working on: https://github.com/sidia-dev-team/GearVRf-Demos/tree/feature_arpet

More specifically in the method starting here: https://github.com/sidia-dev-team/GearVRf-Demos/blob/83a0821c57f57e9e53df07532f65dc67a3b77c25/gvr-arpet/app/src/main/java/org/gearvrf/arpet/PlaneHandler.java#L58

I replaced the call to setModelMatrix by some other operations that I supposed should have same results.

I will make some changes to this demo so that you can easily visualize the problem.

@crbozz
Copy link
Contributor Author

crbozz commented Aug 9, 2018

@crbozz
Copy link
Contributor Author

crbozz commented Aug 9, 2018

I also created a simpler application to demonstrate the problem:

https://github.com/crbozz/TransformTest

@liaxim
Copy link
Contributor

liaxim commented Aug 17, 2018

It does look to be row-vector vs column-vector issue. Please check #1969.

@crbozz
Copy link
Contributor Author

crbozz commented Aug 20, 2018

I tested your fix and it worked fine!

@liaxim
Copy link
Contributor

liaxim commented Sep 19, 2018

Fix has been merged

@liaxim liaxim closed this as completed Sep 19, 2018
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

2 participants