-
Notifications
You must be signed in to change notification settings - Fork 725
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
mat4 decomposition #305
base: master
Are you sure you want to change the base?
mat4 decomposition #305
Conversation
these new removal/decomposition functions allow to trasform some objects with their own translation/scaling/rotation while they are involved in a scene graph
these new removal/decomposition functions allow to trasform some objects with their own translation/scaling/rotation while they are involved in a scene graph
src/gl-matrix/mat4.js
Outdated
@@ -1240,6 +1240,138 @@ export function fromQuat(out, q) { | |||
return out; | |||
} | |||
|
|||
/** | |||
* Removes the transation component from the transformation matrix. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should this be translation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes thanks
src/gl-matrix/mat4.js
Outdated
* Removes the scaling component from the transformation matrix. | ||
* This is equivalent to (but much faster than): | ||
* | ||
* mat4.getTransation(t, mat); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
translation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes thanks
|
||
/** | ||
* Removes the scaling component from the transformation matrix. | ||
* This is equivalent to (but much faster than): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you have tests to support this claim? would be nice to have a look at them too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you are right ... but the errors you have report are comments ... all unit tests work ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe they do, but regarding performance, how can I know that this option is faster than the alternative written on the comment?
src/gl-matrix/mat4.js
Outdated
* Removes the rotation component from the transformation matrix. | ||
* This is equivalent to (but much faster than): | ||
* | ||
* mat4.getTransation(t, mat); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
translation :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes thanks
spec/gl-matrix/mat4-spec.js
Outdated
mat4.fromRotationTranslationScale(matA, q, t, s); | ||
mat4.removeRotation(matB, matA); | ||
result = quat.setAxisAngle(q, [0, 1, 0], 0.7); | ||
console.log( result ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you mind removing this? I suppose it's just some debugging code.
spec/gl-matrix/mat4-spec.js
Outdated
result = quat.setAxisAngle(q, [0, 1, 0], 0.7); | ||
console.log( result ); | ||
quat.normalize(result, mat4.getRotation(result, matB) ); | ||
console.log( result ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same goes here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorry thanks
let s = vec3.fromValues(5, 6, 7); | ||
mat4.fromRotationTranslationScale(matA, q, t, s); | ||
mat4.removeScaling(matB, matA); | ||
result = vec3.fromValues(5, 6, 7); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the point of this?
let t = vec3.fromValues(1, 2, 3); | ||
let s = vec3.fromValues(5, 6, 7); | ||
mat4.fromRotationTranslationScale(matA, q, t, s); | ||
mat4.removeScaling(matB, matA); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
result = mat4.removeScaling(out, matA);
would be better, since that allows you to check more things.
- You can check if it returns the
result
- If it sets
out
to the correct value - If matA is untouched
That sounds like a good use case. However, I'm not entirely sure how they would be used. Would you mind typing up some pseudo code/an explanation? A link is also fine. |
OK ... later ... |
these new removal/decomposition functions allow to trasform some objects with their own translation/scaling/rotation while they are involved in a scene graph