Skip to content

Fixing Nan errors in Camera Frustum Mesh #1397

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

Merged
merged 4 commits into from
Oct 30, 2024

Conversation

emily-howell
Copy link
Member

Issue was that when normalized, the look vector had a slight floating point imprecision that caused this to try and calculate a new up vector. Because the up vector was being rotated based on that floating point imprecision, it was trying to rotate around a (0,0,0) axis which caused nan values

… point imprecision that caused this to try and calculate a new up vector. Because the up vector was being rotated based on that floating point imprecision, it was trying to rotate around a (0,0,0) axis which caused nan values
@emily-howell emily-howell linked an issue Oct 10, 2024 that may be closed by this pull request
@emily-howell emily-howell self-assigned this Oct 22, 2024
@emily-howell emily-howell requested a review from cyrush October 22, 2024 02:03
@cyrush
Copy link
Member

cyrush commented Oct 22, 2024

@emily-howell can we add a test that exercises one of these cases?

Worried about the divide + mult impact on other cases.

Is it a case where the look at and position are very clost?

Is it possible to fix this before the normalize?

@emily-howell
Copy link
Member Author

For sure. Let me put together some test cases for this

@emily-howell
Copy link
Member Author

@cyrush To answer your above questions:

  1. Q: can we add a test that exercises one of these cases?
    A: I just pushed up changes with a test added

  2. Q: Worried about the divide + mult impact on other cases.
    A: I don't think it would have negatively impacted other cases. Since it was after the normalization step, all of the values being divided and multiplied are [0,1] which helps avoid potential overflow. That being said, I modified the handling of this case to avoid any mult/div/rounding.

  3. Q: Is it a case where the look at and position are very clost?
    A: Not completely, it is a case where the vtkm Normalization of the look vector has some additional floating point values. E.g. instead of normalizing to [0.0, 0.0, 1.0] it is normalizing to [0.0, 0.0, 1.000000000003]. Which is a problem because when you try to find the angle between the forward vector [0.0, 0.0, -1.0] and that look vector you get a non-zero angle. But because it is such a small non zero angle the 3D vector transform of the up vector is trying to transform by essentially 0 degrees which causes nans.

  4. Q: Is it possible to fix this before the normalize?
    A: No because it is a byproduct of that normalization step being imperfect. But there are plenty of ways of handling that imperfection and I pushed up an alternative approach just now that you can look at.

Copy link
Member

@cyrush cyrush left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thanks!

// If the look vector has been rotated by a certain angle, ajust the camera up vector to match
if (angle_between != 0.0) {
// If the look vector has been rotated by a certain angle, adjust the camera up vector to match
if (vtkm::Abs(angle_between) >= 0.001) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, this looks like a robust approach.

@cyrush
Copy link
Member

cyrush commented Oct 30, 2024

Thanks @emily-howell !

@cyrush cyrush merged commit 50c9589 into develop Oct 30, 2024
22 checks passed
@cyrush cyrush deleted the task/10_2024_investigate-nans-in-camera-frustrum-mesh branch October 30, 2024 03:27
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.

investigate nans in camera frustrum mesh
2 participants