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

fix(f32): is_normalized #532

Closed
wants to merge 1 commit into from
Closed

fix(f32): is_normalized #532

wants to merge 1 commit into from

Conversation

P-Asta
Copy link

@P-Asta P-Asta commented Jun 27, 2024

No description provided.

@P-Asta
Copy link
Author

P-Asta commented Jun 27, 2024

Error correction of this way
image

@bitshifter
Copy link
Owner

Can you please provide a text description for this and update the tests to include the problem you are fixing

@P-Asta
Copy link
Author

P-Asta commented Jun 28, 2024

Can you please provide a text description for this and update the tests to include the problem you are fixing

This code eliminates the error that occurs when the length of vec2 is smaller than the minimum value of f32.

@P-Asta
Copy link
Author

P-Asta commented Jun 28, 2024

While playing with bevy_xpbd, an error occurred, so I searched for it and estimated the cause.

@bitshifter
Copy link
Owner

bitshifter commented Jun 28, 2024

it would help if you can provide a working repro, in text, not an image. so by that I mean a vec2 which is normalized but fails the current is_normalized check

@bitshifter
Copy link
Owner

I don't really think this change makes sense, at least it is inefficient because the code now calculates length_squared twice, adds an if statement and it makes the check potentially too narrow, but I would like to better understand the problem you are trying to fix.

@bitshifter bitshifter closed this Jul 3, 2024
@bitshifter
Copy link
Owner

Closing this, if you can provide some example code showing how is_normalized is doing something unexpected please open an issue.

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.

2 participants