-
-
Notifications
You must be signed in to change notification settings - Fork 119
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
Support STL #594
Support STL #594
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, @chrisprice, looks excellent!
use std::path::Path; | ||
use std::{fs::File, path::Path}; | ||
|
||
use anyhow::{anyhow, Result}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since fj-export
is a library, thiserror
would be a more appropriate choice here for error handling. Doesn't really matter though. Good enough for now!
[ | ||
vector.x.into_f32(), | ||
vector.y.into_f32(), | ||
vector.z.into_f32(), | ||
] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This kind of code can be shortened a bit:
[ | |
vector.x.into_f32(), | |
vector.y.into_f32(), | |
vector.z.into_f32(), | |
] | |
vector.components.map(|s| s.into_f32()) |
Not necessary, just noting the possibility.
.map(|triangle: Triangle<3>| triangle.to_parry().normal()) | ||
.collect::<Option<Vec<_>>>() | ||
.ok_or_else(|| anyhow!("Unable to compute normal"))?; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since fj_math::Triangle
is validated on construction, we could add an infallible normal
method to it, which would make this kind of code simpler. Or, as a more incremental step, thanks to that validation the anyhow!
here can probably be replaced with an unreachable!
.
Neither of those need to happen in this pull request though.
Fixes #154