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

Sphere intersection not correct #97

Open
athorwall opened this issue Jul 31, 2018 · 3 comments
Open

Sphere intersection not correct #97

athorwall opened this issue Jul 31, 2018 · 3 comments

Comments

@athorwall
Copy link

athorwall commented Jul 31, 2018

https://github.com/rustgd/collision-rs/blob/master/src/volume/sphere.rs#L61 seems to be incorrect. Specifically the computation of tca:

let tca = l.dot(r.direction);

where tca is intended to be the length of the projection of l onto r.direction is incorrect - it needs to be divided by the magnitude of r.direction:

let tca = l.dot(r.direction) / r.direction.magnitude();
@athorwall athorwall changed the title Sphere intersection Sphere intersection not correct Jul 31, 2018
@athorwall
Copy link
Author

athorwall commented Jul 31, 2018

I just noticed that the tests always normalize ray directions, which would address this - is it just always assumed that ray directions are normalized, through the whole library? If so it's unfortunate that it's possible to construct a non-normalized Ray.

@Rhuagh
Copy link
Collaborator

Rhuagh commented Jul 31, 2018

All intersection tests assume Ray direction is normalized, to avoid a hidden square root computation. Rays can be created without normalized direction because of the same reason.

We should probably document this behavior more clearly.

@kvark
Copy link
Collaborator

kvark commented Jul 31, 2018

@Rhuagh we can also debug_assert on normalization in the code, where appropriate

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

3 participants