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

Attempt at implementing Euler angles #595

Draft
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

Strepto
Copy link
Contributor

@Strepto Strepto commented Feb 22, 2023

This uses code from THREEjs for converting to and from Euler, It implements all possible rotation orders, and should handle edge cases around poles.

I have tried to make it as readable as possible in line with other SK math helpers, but feedback is appreciated! Currently the "api" is in Radians, but converting to "Degrees" should be straightforward if wanted.

Actual (unit)tests are needed. Locally I have added a Unit-test lib for this, but not sure how it should be done to best fit the SK way.

Feel free to close this PR if adding Eulers is out of scope for SK, or you are unsure about the implementations fit. I did this without consulting with an issue or similar, but if its not useful i'll keep it in my own toolkit. If its usefull I would like to write more tests, and try to ship this thing :)

Remark: The math is not something I actually understand, but the output seems to be reliable in my testing. Math is ported from THREEjs, and should be compatible license wise.

This uses code from THREEjs for converting to and from Euler
It implements all possible rotation orders, and should
handle edge cases around poles.

I have tried to make it as readable as possible,
but feedback is appreciated!
@maluoi
Copy link
Collaborator

maluoi commented Feb 22, 2023

Hey, thank you for this! I've very much been wanting Quat -> Euler, but every time I tried to nail it down, it failed somewhere. This looks pretty complete!

I do have a few notes about this PR though

  • This is something that belongs in the core, so this should be implemented in C, with C# bindings.
  • I'd prefer not to introduce a whole Euler class for this, ideally it would just be a single method: Vec3 Quat.ToEuler(RotOrder eulerRotationOrder = RotOrder.RollPitchYaw) (RotOrder should be a short name, since users have to type it. Long descriptive names are fine for function parameter names instead.)
  • StereoKit uses degrees for its public API, I don't really expose radians.
  • I'm a little tempted to say the name should be Quat.ToAngles? Euler is the name of a mathematician, rather than a description of what's happening. This is also a mirror to Quat.FromAngles.
  • If you're adding a RotOrder to Quat.ToAngles, it may also be appropriate to add the equivalent in Quat.FromAngles()? I'm not entirely certain how important it is to have the RotOrder here too, this could be overkill as I suspect the vast majority would always use RollPitchYaw. Would love to know your opinion on that!
  • There is actually a Units.rad2deg you can use!

For tests, I've been writing these over here. These all get run with the docs and screenshot generation!

Sorry for throwing a lot of stuff at you here! I'd definitely love to have this functionality in SK, but if you choose not to make these changes, I can still use this as reference later :)

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

Successfully merging this pull request may close these issues.

None yet

2 participants