-
-
Notifications
You must be signed in to change notification settings - Fork 111
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
Added CPUMeshGenerator #188
base: master
Are you sure you want to change the base?
Conversation
/// Generates the uv buffer. | ||
pub uvs: Option<&'t mut (dyn FnMut(P) -> [f32; 2] + 't)>, | ||
/// Generates the color buffer. | ||
pub colors: Option<&'t mut (dyn FnMut(P) -> [u8; 4] + 't)>, |
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.
I am not sure if it is OK to use dynamic dispatch here. Another possible way without introducing 5 type parameters in everywhere is to create a trait and use associated types, but that feels like too much work for a simple use case.
I suppose the compiler can inline the dynamic dispatch anyway, and even if it doesn't, users shouldn't call mesh generation functions in hot paths anyway.
src/core/cpu_mesh.rs
Outdated
tangents: None, | ||
uvs: None, | ||
colors: None, | ||
}); |
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.
I was a bit lazy here, didn't verify if the cylinder and cone functions are really the same except with the (1.0 - v.x)
multiplier.
Can you elaborate a bit on the motivation for this? I'm a bit concerned that it adds extra complexity. Also, it is a bit orthogonal to the focus of this crate. All the fields in CPUMesh are public so you can easily add custom uv coordinates or advanced mesh generation features outside this crate. |
For example, I would like to generate a sphere mesh like this: CPUMesh::sphere_gen(
"sphere",
angle_subdivisions,
CPUMeshGenerator {
positions: &mut |v| [v.x, v.y, v.z],
normals: Some(&mut |v| [v.x, v.y, v.z]),
tangents: None,
uvs: Some(&mut |v| [v.theta / PI, v.phi / PI / 2.]),
colors: None,
},
); If I were to derive the |
Sorry for the late reply, I caught the flu 🤧 Isn't the problem just that there are no uv coordinates generated in CPUMesh::sphere then? We can easily add that. And if your texture doesn't fit the generated coordinates then you can use a texture transform. Or just create a sphere the way you like in another tool, export it and load it into three-d. |
Texture transform only works with linear transformations right? Sometimes textures are not continuously joined, e.g. some might split the texture into multiple disjoint grids and arrange them horizontally or some other discontinuous way. Actually I generated my own sphere mesh anyway, just thought this might be a useful addition to the API. |
Yeah texture transform is only linear and yeah there are a million different use-cases that are not supported with regards to mesh generation, you are absolutely right. However, the point of I really hope you understand my reasoning and are not offended by this 🙂 Anyway, I will see if I can incorporate some of it to generate meshes internally because I really like the reusing of information. |
This allows users to create modified meshes (especially custom UV definitions) reusing information like angle/theta/phi instead of deducing them from the position buffer.