Fast and flexible mesh generation

[jimy-byerley]

This is a proposal for mesh generation routines.
As generation is often faster with raw triangle buffers, I thought it was better to generate the mesh using buffers and then convert in into Mesh

Features

• added a structure called Shape - as there no special restrictions on the geometry and there is no connectivity info - to handle these position and face buffers
• it implements basic mesh generation routines such as triangulation, extrusion, revolution (more would come with time)
• it implements Into<Mesh> for conversion

Note:

I can't find a way to declare the constructor and the methods such that we can do

Shape::new().triangulation([/*...*/]).extrusion([/*...*/]).into()

as well as

let mut shape = Shape::new().extrusion([/*...*/]);
// and one day in long after
shape.revolution([/*...*/], /*...*/)
return shape.into();

I always have an issue with data ownership.

[jimy-byerley]

By the way, Mesh::new() takes flat buffers of type Vec<f64> and Vec<u32> that are consumed by the constructor, even if it has no need to modify it.
Would it be possible to take references instead ?

• slices: &[f64] and &[u32]
• or dynamic owning structure: Cow<[f64]> and Cow<[u32]>

So there will be less copy needed for the Mesh creation from Shape or .obj file

[asny]

I think this is a really nice contribution and valid approach to use raw buffers for this and then convert to halfedge mesh :)

My main concern is that it is not really a part of this library since it is a completely new struct with new functionality only linked to Mesh by a conversion function. So my suggestion is to make the functionality use the Mesh struct, so all input/output is a Mesh (it really is a triangle mesh). Internally, this functionality could then use the raw arrays of vertices and triangles instead of the halfedge data structure (connectivity_info) and then convert at the end. A simple practical solution could be to use the builder pattern also used in MeshBuilder. It could maybe even be a part of MeshBuilder. What do you think?

I was also thinking, it might be a good idea to be able to represent a mesh with different data structures internally and then convert to use the most suited for a specific operation. But this is a major change, so let's wait a bit with that :)

Other minor stuff:

• I have made the Mesh::new() take slices as input as you suggested, but since you chose Vec and Vec<[u32; 3]> for shape, it cannot be used directly in into().
• I must insist that functionality which is purely internal should not be exposed as public functionality.
• Think about naming things with explicit and long names (not displt for example)
• Really nice with tests, but you miss some cases. For example, as far as I can see, the triangulation function only works if the Shape struct is empty.
• About your Note: I think you cannot really do both, except if you implement functions that return &mut Self as well as Self. But I think it is fine to choose one of them.

Again, I think this is really nice and very useful functionality! Thanks for your effort :)

[jimy-byerley]

Thanks for your feedback !

but since you chose Vec and Vec<[u32; 3]> for shape, it cannot be used directly in into().

Yes for sure ... I think that it could be usefull to propose to the team of cgmath to add a trait to cast &[Vector3<T>] to &[[T;3]] and &[T]. It wouldn't be a problem because Vec and slices are contiguous storages and cgmath::Vector fields are aligned.

the triangulation function only works if the Shape struct is empty.

Oops, that was a mistake, I corrected it. If you come to other things like that, warn me !

I think you cannot really do both, except if you implement functions that return &mut Self as well as Self. But I think it is fine to choose one of them.

I was considering implementing all the methods on a Cow<Shape> instead of just Shape, It would be les elegant in the code, but I guess it should solve our problem.

For the concern of exposing the struct's internal, I did that because at contrary to the Mesh that has haflfedge data that must not be corrupted, we have there only unchecked buffers of faces and points, so it seems to me that it could be the messy place, whereas Mesh is the clean one. I mean, at this point, when we are generating a mesh, we can generate a mesh that would be invalid thanks to Mesh just by using the methods of Shape, but that's when we convert it to a Mesh that we would do all the checks.
Since, I think it's a good idea to let the user hack into the buffer, to allow him to create new generation functions.

I was also thinking, it might be a good idea to be able to represent a mesh with different data structures internally and then convert to use the most suited for a specific operation

That's right ! But maybe it could be not as big at it seems: there is no need to use a specific OpaqueMesh structure as long as rust types stay implicit from an operation to an other. Iterators can be a good example of operations that takes a different type each time, each time tailor made for the problem. This way we could use each operation the most suited return type, that would implement the same methods as the other mesh structures.
This way all these structures would have to implement a sort of Mesh trait.

[jimy-byerley]

Maybe we should merge MeshBuilder and Shape together.
for now, Shape was designed to hold data during generation and so not to work as a builder, but that's the only difference.
(Also I prefer the name "shape" to one like SomethingBuilder ... ;)

Surely a Cow can allow us to merge them.

[asny]

For the concern of exposing the struct's internal, I did that because at contrary to the Mesh that has haflfedge data that must not be corrupted, we have there only unchecked buffers of faces and points, so it seems to me that it could be the messy place, whereas Mesh is the clean one. I mean, at this point, when we are generating a mesh, we can generate a mesh that would be invalid thanks to Mesh just by using the methods of Shape, but that's when we convert it to a Mesh that we would do all the checks.
Since, I think it's a good idea to let the user hack into the buffer, to allow him to create new generation functions.

I have to disagree, the internal functionality can be (a bit) messy, that is fine, but the interface should not! If the user wants to hack away at raw buffers, there is nothing stopping them, they can do so and then construct a mesh from raw buffers. Don’t get me wrong, I think the functionality is great and very well suited for tri-mesh, but as it is now, the functionality is not integrated and does not follow the same standards.

That's right ! But maybe it could be not as big at it seems: there is no need to use a specific OpaqueMesh structure as long as rust types stay implicit from an operation to an other. Iterators can be a good example of operations that takes a different type each time, each time tailor made for the problem. This way we could use each operation the most suited return type, that would implement the same methods as the other mesh structures.
This way all these structures would have to implement a sort of Mesh trait.

I think that sounds like a really big task since you have to implement all algorithms for all mesh types. Also, it goes against my principle of keeping stuff explicit as much as possible. Making things generic that does not need to be will always cause trouble. I think it was a bad idea from my side :)

Maybe we should merge MeshBuilder and Shape together.
for now, Shape was designed to hold data during generation and so not to work as a builder, but that's the only difference.
(Also I prefer the name "shape" to one like SomethingBuilder ... ;)

I definitely think MeshBuilder and Shape are very similar, the only difference as I see it is that the generation functionality can take an existing mesh as input also? And that can maybe be solved by generating a new mesh and then merging the existing and the new mesh. If you can merge the generation functionality into mesh builder and make a clean interface, then I will be happy to merge.

[jimy-byerley]

If the user wants to hack away at raw buffers, there is nothing stopping them, they can do so and then construct a mesh from raw buffers

That's right, but just to be sure:
In what way one can hack into a raw buffer, that would create problems for the created Mesh ? (except put points that doesn't exist for face vertices and put more that 2 faces an edge, these two can be checked easily at convertion time or at generation time).

If you can merge the generation functionality into mesh builder

I'm not sure of how to to this in a smart way:

• The MeshBuilder is using Option for its buffers, whereas Shape expect existing already initialized buffers.
• MeshBuilder is using consuming self methods so I would have to use the same pattern for generation methods. It makes it difficult to call generation methods multiple times and then convert to Mesh.

So maybe I misunderstood:

And that can maybe be solved by generating a new mesh and then merging the existing and the new mesh

Do you mean calling

mymesh.merge_with( MeshBuilder::new().generation_method(/* ... */).build().unwrap() )

each time we want to generate a portion of the mesh ?

[jimy-byerley]

What do you think of this more checked and safe implementation: jimy-byerley@e56cda1

[asny]

In what way one can hack into a raw buffer, that would create problems for the created Mesh ? (except put points that doesn't exist for face vertices and put more that 2 faces an edge, these two can be checked easily at convertion time or at generation time).

My point is that there is no need to add "hacking into raw buffers"-functionality to tri-mesh. You can already hack away at raw buffers and call a mesh builder with those buffers and create a Mesh. Why do you want to add hacky functionality to tri-mesh?

The MeshBuilder is using Option for its buffers, whereas Shape expect existing already initialized buffers.

A simple example: MeshBuilder::new().triangulate(outline).build() where triangulate is your triangulate function which creates index and position buffers for the meshbuilder class.

MeshBuilder is using consuming self methods so I would have to use the same pattern for generation methods. It makes it difficult to call generation methods multiple times and then convert to Mesh.

No, not at all. Right now it can only make one mesh at a time, but there is no reason why it shouldn't be able to add several meshes like: MeshBuilder::new().triangulate(outline).extrusion(line, displacement).build(). The only reason not to have it is maybe that it is exactly the same as creating two separate meshes and call append or merge_with. Also, right now you have a merge method for Shape, which is duplicate code since it is already a function in Mesh.

Do you mean calling
mymesh.merge_with( MeshBuilder::new().generation_method(/* ... */).build().unwrap() )
each time we want to generate a portion of the mesh ?

Yes exactly. Or if you don't want to merge them, you can call append.