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

Quat::to_euler outputs bigger than PI * 2 at certain angles #500

Closed
cybersoulK opened this issue Mar 24, 2024 · 13 comments
Closed

Quat::to_euler outputs bigger than PI * 2 at certain angles #500

cybersoulK opened this issue Mar 24, 2024 · 13 comments
Assignees

Comments

@cybersoulK
Copy link

when using Quat::to_euler(glam::EulerRot::XYZ);
The object is spinning on the Y axis, once in a while at a 90 degrees transition, it outputs an angle that is > PI * 2

Format is Euler angles and then corresponding Quat:

(-3.1415925, -1.542782, -3.1415925) . Quat(-0.0003244644, -0.7200991, -0.0003244644, 0.69387114)
(-3.1415925, -1.5510916, -3.1415925) . Quat(-0.0003244644, -0.7200991, -0.0003244644, 0.69387114)
(-3.1415925, -1.5594077, -3.1415925) . Quat(-0.00010815916, -0.7114648, -0.00010815916, 0.7027218)

(-6.283185, -1.5677276, 0.0) . Quat(-0.7114118, 0.008691366, 0.7026667, -0.008799534) <----------

(-0.0, -1.5655829, -0.0) . Quat(0.0, -0.7027204, 0.0, 0.71146613)
(-0.0, -1.557269, -0.0) . Quat(0.0, -0.6938671, 0.0, 0.7201031)
(-0.0, -1.5489539, -0.0) . Quat(0.0, -0.6938671, 0.0, 0.7201031)

Another example:

(-0.0, 1.5640393, -0.0) . Quat(0.0, 0.7027204, 0.0, 0.71146613)
(6.283185, 1.5693315, 0.0) . Quat(0.71146613, -3.0716883e-8, 0.7027204, -3.1099173e-8).  <-------
(-3.1415925, 1.5609698, -3.1415925) . Quat(-0.017489437, -0.7112498, -0.017489437, -0.70250404)

Here is a similar 90 degrees transition, but this time the angle is within PI

(-0.0, 1.559818, -0.0) . Quat(0.0, 0.7027204, 0.0, 0.71146613)
(0.0, 1.5681219, 0.0) . Quat(0.0, 0.7027204, 0.0, 0.71146613) <------ it's fine
(-3.1415925, 1.5651652, -3.1415925) . Quat(-0.017489437, -0.7112498, -0.017489437, -0.70250404)

Is this a bug or intended to happen?
i also realized that if i use YXZ instead with the same object rotation, i don't have the same issue.

@bitshifter
Copy link
Owner

Hey, I haven't had a chance to look at this.

The euler conversion code was contributed by someone so I unfortunately don't have any additional insight into how it works.

Prior to that being contributed I was actually planning on adopting a solution from another more mature math library. I may still do that. In any case I'll try look at why this is happening before taking the nuclear option.

@bitshifter
Copy link
Owner

bitshifter commented May 18, 2024

I wasn't able to repro this with those quaternions:

fn main() {
    dbg!(Quat::from_xyzw(-0.7114118, 0.008691366, 0.7026667, -0.008799534).to_euler(EulerRot::XYZ));
    dbg!(Quat::from_xyzw(0.71146613, -3.0716883e-8, 0.7027204, -3.1099173e-8).to_euler(EulerRot::XYZ));
}
[src/main.rs:91:5] Quat::from_xyzw(-0.7114118, 0.008691366, 0.7026667,
        -0.008799534).to_euler(EulerRot::XYZ) = (
    3.1168556,
    -1.5584388,
    7.5300115e-8,
)
[src/main.rs:92:5] Quat::from_xyzw(0.71146613, -3.0716883e-8, 0.7027204,
        -3.1099173e-8).to_euler(EulerRot::XYZ) = (
    -3.1415925,
    1.5584291,
    -0.0,
)

I was testing on x86_64 so it would be using SSE2, were you seeing this on x86_64 or was it another architecture?

Edit: I just realised I was on a branch that meant sse2 would not be used, so that could be why I didn't see the same thing.
Edit2: Can't repro on main either.

@bitshifter
Copy link
Owner

@cybersoulK there were some fixes to to_euler that were introduced in 0.24.2 in September 2023, is it possible that you were using an older version of glam when you were seeing this?

@cybersoulK
Copy link
Author

cybersoulK commented May 18, 2024

@bitshifter

image

I updated my own fork at the time, to make sure it happened in the main.
I ran the same tests you provided, and i get the same values as you, (apple m1 pro) (at the exact same fork that i had the issues in)

I am not sure why, maybe a println is missing some floating numbers? i will investigate further, do you think it's possible some floating numbers might be missing from my initial debug message?

@cybersoulK
Copy link
Author

so, when i run your tests using the following commit, which was 24.0,
glam = { git = "https://github.com/bitshifter/glam-rs", rev = "249023c06db3796aebba0c75143e042043e2a5c2" }

i still have the correct values, so something else must be at play here

@bitshifter
Copy link
Owner

bitshifter commented May 18, 2024

Hrm, you weren't using libm or fast-math features?

It could be the printing losing precision, not really sure about that.

@cybersoulK
Copy link
Author

cybersoulK commented May 18, 2024

i don't see these 2 features enabled. I just ran the tests within the same project, to check that, but the output was correct.

Within 1 or 2 days, I should be able to test the same exact scenario in the game, to see if it glitches, and i can capture the Quat values.
Any advice to print them without losing precision?

@bitshifter
Copy link
Owner

I think debug printing (i.e. "{:?}") should be sufficient, but you can always print more precision if you want to be sure.

I did a little experiment here https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=472f213957510840510e2d5abc92ba07 based on https://randomascii.wordpress.com/2013/02/07/float-precision-revisited-nine-digit-float-portability/ and although you can print more decimals it's still the same float. You could always print as hex to be sure.

@cybersoulK
Copy link
Author

cybersoulK commented May 19, 2024

dbg!(glam::Quat::from_xyzw(0.0, 0.70599556, 0.0, -0.7082163).to_euler(glam::EulerRot::XYZ));
dbg!(glam::Quat::from_xyzw(0.0, 0.70554054, 0.0, -0.7086696).to_euler(glam::EulerRot::XYZ));
dbg!(glam::Quat::from_xyzw(0.0, 0.70813525, 0.0, -0.7060769).to_euler(glam::EulerRot::XYZ));
dbg!(glam::Quat::from_xyzw(0.0, 0.70819116, 0.0, -0.70602083).to_euler(glam::EulerRot::XYZ));

dbg!(glam::Quat::from_xyzw(0.0, -0.70860523, 0.0, -0.7056052).to_euler(glam::EulerRot::XYZ));
dbg!(glam::Quat::from_xyzw(0.0, -0.706111, 0.0, -0.7081012).to_euler(glam::EulerRot::XYZ));

dbg!(glam::Quat::from_xyzw(0.5765686, 0.4102857, -0.5761997, -0.4089355).to_euler(glam::EulerRot::XYZ));

dbg!(glam::Quat::from_xyzw(0.052489556, 0.7064261, -0.053010665, -0.70384437).to_euler(glam::EulerRot::XYZ));
  glam::Quat::from_xyzw(0.052489556, 0.7064261, -0.053010665,
      -0.70384437).to_euler(glam::EulerRot::XYZ) = (
  6.1328373,
  -1.5670617,
  0.0,
)

...

I found these! can you test them in x86?

@bitshifter
Copy link
Owner

yep, that reproduces the issue on x86_64 as well, thanks!

@bitshifter
Copy link
Owner

It appears that these results are coming from change to the Euler conversion code to fix another issue, this if block in particular https://github.com/bitshifter/glam-rs/blob/main/src/euler.rs#L81-L92.

The original conversion code was contributed and based on a code listing from this blog post https://web.archive.org/web/20210506232447/http://bediyap.com/programming/convert-quaternion-to-euler-rotations/. That post doesn't have any background on where it came from.

Prior to this being contributed I was planning to implement something based on https://github.com/AcademySoftwareFoundation/Imath/blob/main/src/Imath/ImathEuler.h

The Imath code is based on this paper from Graphics Gems IV:

Shoemake, Ken, Euler Angle Conversion, p. 222-229, code: p. 225-228, euler_angle/.

There have been a few issues around the Euler conversion code and I wonder given its unclear origins if it would be easier to rewrite it using Shoemake's method which appears to be widely used since 1994.

In any case, leaving this here as a reminder for when I have time to look into it more.

@bitshifter bitshifter self-assigned this Jul 12, 2024
@bitshifter
Copy link
Owner

fixed in #537

@cybersoulK
Copy link
Author

Awesome! My previous failing case works well now.

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

No branches or pull requests

2 participants