STORM-IRIT / Radium-Engine

Research 3D Engine for rendering, animation and processing
https://storm-irit.github.io/Radium-Engine/
Apache License 2.0
100 stars 50 forks source link

TriangleMesh vs PolygonMesh #360

Closed hoshiryu closed 5 years ago

hoshiryu commented 6 years ago

Currently, the mesh classes in Core TriangleMesh and TopologicalMesh exhibit interoperability issues. On the one hand, a TriangleMesh is meant to store only triangular faces, though other polygonal faces can be stored in it without disrupting the attributes management. On the other hand, a TopologicalMesh is meant to store arbitrary polygonal faces, due to its inheritance from OpenMesh::PolyMeshT. However, if we want to deal with triangle-only meshes, we need to inherit from OpenMesh::TriMeshT since the topological operations (e.g. edge split) are not processed the same way, and may lead to complete rewrite of those for the PolyMeshT version to behave the expected way.

The shortest way would be to only consider triangles, and never deal with other polygons. This would only imply to change the inheritance of TopologicalMesh to OpenMesh::TriMeshT. However, this also implies to always triangulate input polygonal meshes and work only on the triangles. This can be a problem when applying several Catmull-Clark subdivision passes since the faces regularity will be lost. This is also a problem when skinning an animated character, where a nice triangulation for rendering might depend on the polygon shape after deformation.

A more viable solution would be to actually implement the difference between triangle-only meshes and polygonal meshes; and also add a triangulation conversion. Thus, the TriangleMesh class would only store triangles and a PolyMesh class would store any polygons (with a rather subtle implementation difference). Similarly, a TopologicalTriMesh class would manage the topology for triangles only, and a TopologicalPolyMesh class would deal with other polygons.

dlyr commented 6 years ago

Well I think TriangleMesh could be renamed Mesh Then the conversion back and forth TopologicalMesh should keep polygons And the Render (or Engine) Mesh should triangulate One implementation issue is to keep a list of triangle, and a list of other poly, to ease the work fo RenderMesh

nmellado commented 6 years ago

After a discussion offline, we decided to keep the Core::TriangleMesh class as is, just to rename it as Core::Mesh. The motivation behind this choice is that keeping Mesh to store triangles and polys as few downsides.

On the other hand, keeping a polytopologicalmesh prevents to use some functionalities of OpenMesh when processing triangle-only meshes. To mitigate this issue, we decided to have a TopologicalTriMesh and a TopologicalPolyMesh as you mentioned. These structures will be both converted from/to a Core::Mesh. Note that the conversion from Core::Mesh to TopologicalTriMesh will require a PolygonConvertionStrategy, to describe what to do with the polygons that are potentially stored in Core::Mesh. The default strategy will be IgnorePolygons, I let you guess its behavior ;).

Is that all clear ?

dlyr commented 6 years ago

Clear for me ;)

dlyr commented 6 years ago

Btw, it's also in refactor discussion i think, but how do we name mesh classes do Core::Mesh and Engine::Mesh can co exists, or one could be called CoreMesh ...

nmellado commented 6 years ago

May I rephrase your question as follow:

As namespace are supposed to protect types and methods names, do we want to keep the Core, Io and Engine namespaces, or just stick to Ra ?

If the answer is yes, then we can have Ra::Core::Mesh and Ra::Engine::Mesh. Ortherwise, we can do Ra::CoreMesh and Ra::EngineMesh.

nmellado commented 5 years ago

Any comment/suggestion ?

MathiasPaulin commented 5 years ago

I'm for keeping Core, IO and Engine namespaces as all the classes in these namespace are packed in different libraries. For the conversion from Core::Mesh to TopologicalTriMesh, the PolygonConvertionStrategymight define a functor used to triangulate the polygon. 2 "standard strategy" will be implemented (IgnorePolygons and PolyToTriangleFan with an evident sementic as said by @nmellado. The user could, for sure, also develop its own strategy to manage the polygons.

stale[bot] commented 5 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.