Refactor vtkmeshwrapper such that low level mesh operations are in the mesh class

[HeavenlyBerserker]

vtkmeshwrapper does not use any mesh class functionality, leading to the possibility of redundant functions in libs/mesh/mesh and libs/optimize/ParticleSystem/vtkmeshwrapper.

[cchriste]

I think removing trimeshwrapper was also part of this, sadly. But adding functions to Mesh that can give you a TriMesh was also discussed if needed.

[akenmorris]

TriMeshWrapper can be removed at this point.

[HeavenlyBerserker]

Oh, can we hold this off until after the next release? I'm working on constraints and making tons of modifications and I don't wanna undo anything you guys refactor. Shireen told me to make this issue and not assign anybody as of yet. Sorry, I should have mentioned that.

[cchriste]

Oh, can we hold this off until after the next release? I'm working on constraints and making tons of modifications and I don't wanna undo anything you guys refactor. Shireen told me to make this issue and not assign anybody as of yet. Sorry, I should have mentioned that.

What kind of changes are you making? Do any of them related to issue #935? It is meant to create an independent Field class to be used by Meshes. The main question I have is whether Fields composed of 1, 3, and 9-component tuples are reasonable or if we would prefer a more generalized container, requiring a more complicated interface.

[cchriste]

@HeavenlyBerserker Can you please answer the question above about the use of fields in the MeshWrappers? Thanks!

[cchriste]

This is a reasonable place to start to understand how and where things should be split. @archanasri and I have been working on the geodesic distance function, but there are other components of these classes. @HeavenlyBerserker can you please share a summary (bullet list) of the components/features of these wrapper classes? And also please confirm we can dump TriMeshWrapper.

So far I have:

• compute geodesic distance on a mesh
• ?

[HeavenlyBerserker]

Firstly, we can dump Trimesh for sue. Karthik tried it out and was not working for our purposes. All the functionality that is used is in VtkMesh.

I only added some methods to mesh that are needed for free-form constraint operation and eventually visualizations. I have not touched many of the other methods. Please see #1422 for more information.

I have used the current fields' functionality in mesh.h and I am still not sure why an independent fields class is necessary. I think vtk itself provides good fields' functionality which is easy to use with a simple wrapper to the GetCellData()->GetArray() and GetPointData()->GetArray() routines. We can just have a function that returns a vtkDataArray regardless of what the tuple looks like. I think having a class can be overkill unless we are going to use this extensively. However, if we do decide to make one, I'd suggest having 1, 3, and 9-component tuples.

The VtkMeshWrapper seems to be used in optimize.cpp as the class that contains mesh domain objects. It includes the following functionality:

• Compute geodesic distances bw vertices
• Test whether a vertex is within a certain geodesic distance of another
• Geodesic walks
• Sampling normals and grads at points
• Snapping points onto mesh
• Getting mesh bounds

Most auxiliary and private methods are used for the above main functionality. There are some preprocessing steps in some of these methods that allow faster queries during execution. I have copied ComputeBarycentricCoordinates and GetIGLMesh almost verbatim to mesh.cpp since it's needed for optimized geodesic distance and gradient computation. I think most methods can be copy-pasted to mesh. This would allow the mesh domain, visualizations, and grooming to use the same mesh class so that it is easier to maintain in the future, although it'd be a pain to refactor presently.

Another way is to have the mesh class itself and have a meshdomain.h class with a mesh.h class member to include operations that are not necessary to groom nor visualizing, but necessary for the mesh domain.

[akenmorris]

Yes, we can drop TriMeshWrapper. It was the first Mesh implementation for the Optimize by Oleks. It had a number of drawbacks and I re-implemented it using VTK as VTKMeshWrapper. I only kept TriMeshWrapper around in case we ran into bugs and needed to compare the results between the two. We haven't run into any such problems, so we can remove it now.

[cchriste]

I have used the current fields' functionality in mesh.h and I am still not sure why an independent fields class is necessary. I think vtk itself provides good fields' functionality which is easy to use with a simple wrapper to the GetCellData()->GetArray() and GetPointData()->GetArray() routines. We can just have a function that returns a vtkDataArray regardless of what the tuple looks like. I think having a class can be overkill unless we are going to use this extensively. However, if we do decide to make one, I'd suggest having 1, 3, and 9-component tuples.

Let's discuss this in more detail in #935. It may not be necessary, but regardless will simply be a consolidation of various functions from here and already in Mesh.

The VtkMeshWrapper seems to be used in optimize.cpp as the class that contains mesh domain objects. It includes the following functionality:

• Compute geodesic distances bw vertices
• Test whether a vertex is within a certain geodesic distance of another
• Geodesic walks
• Sampling normals and grads at points
• Snapping points onto mesh
• Getting mesh bounds

This is very helpful!

Most auxiliary and private methods are used for the above main functionality. There are some preprocessing steps in some of these methods that allow faster queries during execution. I have copied ComputeBarycentricCoordinates and GetIGLMesh almost verbatim to mesh.cpp since it's needed for optimized geodesic distance and gradient computation. I think most methods can be copy-pasted to mesh. This would allow the mesh domain, visualizations, and grooming to use the same mesh class so that it is easier to maintain in the future, although it'd be a pain to refactor presently.

Copying isn't acceptable. In my understanding, many of the preprocessing methods here are no longer necessary since it turned out that simply caching the lookup of the source point when computing geodesic distances across an entire mesh was a sufficient optimization.

Another way is to have the mesh class itself and have a meshdomain.h class with a mesh.h class member to include operations that are not necessary to groom nor visualizing, but necessary for the mesh domain.

More details will be helpful. There is a MeshUtils class that might be pertinent. We should talk about this idea in more detail since MeshDomain sounds a lot like MeshWrapper from a high level.

[HeavenlyBerserker]

The reason this issue was brought up is because I use two of the functions that Karthik implemented in vtkmeshwrapper in mesh, and Shireen suggested keeping a single data structure for meshes anywhere in the code. In some sense, both vtkmeshwrapper and mesh are wrappers for the vtk mesh data structure, right?

Copying isn't acceptable.

I agree, but I would have had to refactor the whole vtkmeshwrapper for just the two functions. Refactoring this would have taken a few days, probably.

[cchriste]

Found in Libs/Optimize/VtkMeshWrapper.h/cpp.

There are many aspects to this issue. Breaking it up into individual issues may be helpful.