Why does the tesseract_geometry::mesh constructor take an Eigen::VectorXi for the triangles?

[AlexGisi]

The mesh constructor takes an integer vector of variable length (triangles) to represent the mesh's triangles. The constructor describes this as

A vector of face indices where the first index indicates the number of vertices associated with the face followed by the vertex index in parameter vertices. For example a triangle has three vertices so there should be four inputs where the first should be 3 indicating there are three vertices that define this face followed by three indices.

The constructor then checks that the mesh is triangular. Combining this with the fact triangles is suggestively named, would it be more intuitive and efficient to store the triangles in a std::vector<Eigen::3i>>, analogous to the vertices?

[Levi-Armstrong]

A triangle mesh is a special case for a polygon mesh which it inherits from reducing code duplication.

[marip8]

To clarify, I think both @AlexGisi and I are confused about the input structure of the face integers. It seems like the constructor for the polygon mesh currently takes an Nx1 vector of integers, but what size is N for an arbitrary polygon mesh? For the Mesh case, you make the assumption that the polygons are triangles, so N = n_vertices * 4, but you can't make that assumption for an arbitrary polygon mesh.

I think what @AlexGisi is suggesting is that we:

1. Rename the Mesh class to TriangleMesh since you are making the assumption that all polygons in the mesh are triangles
2. Update the constructor to PolygonMesh to take face definitions as a std::vector<Eigen::VectorXi>, where each face can be an actual arbitrarily sized polygon

[Levi-Armstrong]

Rename the Mesh class to TriangleMesh since you are making the assumption that all polygons in the mesh are triangles

I don't have any objections to this.

Update the constructor to PolygonMesh to take face definitions as a std::vectorEigen::VectorXi, where each face can be an actual arbitrarily sized polygon

I would need to verify, but I am almost certain that the data types were specifically used because they are directly compatible with components which consume or produce the object to avoid type conversions. If this is the case, I would be against changing the storage type.

[rjoomen]

Rename the Mesh class to TriangleMesh since you are making the assumption that all polygons in the mesh are triangles

Sounds good to me too. Could we maybe also derive SDFMesh from TriangleMesh instead of PolygonMesh, as it currently duplicates the check for triangularity?

I would need to verify, but I am almost certain that the data types were specifically used because they are directly compatible with components which consume or produce the object to avoid type conversions. If this is the case, I would be against change the storage type.

This type of storage is compatible with PCL: 1) a list of vertices and 2) a polygon list consisting of each time a size (number of vertices of the polygon) followed by the specified number of indices into the vertex list. Not very logical, I agree. I'm not sure if the way it is used currently indeed has efficiency benefits, that should be investigated indeed. Or you could simply add another constructor with the std::vectorEigen::VectorXi datatype and convert.