ranahanocka / MeshCNN

Convolutional Neural Network for 3D meshes in PyTorch
MIT License
1.56k stars 314 forks source link

Asking for clarification of general assumptions and further input on code questions #84

Open NaskyD opened 4 years ago

NaskyD commented 4 years ago

First: Thanks for sharing your work with all of us!

I have questions about the code, but also would appreaciate further note on some assumptions I have built up by now and need verification. (I am terribly sorry for this wall of text. I simply try to be as precise in my questions as I try to condence everything as much as possible)

General understanding - assumptions: (1.1)Do you have experience with "sticked-together", but not properly connected geometry? The decimation process seems to mess up the meshes really bad in case of low resolution parts and high resolution parts, which are part of the same object, but not connected as a mesh structure. Hence, the resulting meshes are almost always non-manifold very fast (low decimation level).

Hence the follow-up: As mentioned in other threads (eg. #83, #43) the meshes have to be manifold to adhere to the 4-ring convention for convolution, meaning two incident faces. However, I was wondering, since your code for pooling is actually checking conditions accordingly, isn't it actually robust against adding "wrong" edges as features and is as such robust against non-manifold input?

(1.2) In #43 it is mentioned, that "small connected but isolated components" can lead to trigger the assert(len(vertex) ==1) error. You mentioned them being like isolated pyramid structures. Is this the only potentail reason leading to that assert failing? I already added preprocessing to my meshes removing any isolated geometry (using blender api). However, I still get this issue. In case I am wrong with 1.1 and the code being not robust against non-manifold geometry like I mentioned, could this also lead to this assert failing?

Code specific/implementation realated questions: (2.1)How can it be, that say edge e1 is neighbour of e7 and such is presented in its 1-ring structure, but e1 is not present in the 1-ring structure of e7. While debugging this occured to me, unfortunately. Maybe I am missing something here.

(2.2)The edge-side list is used for indexing the neighbours and as far as I understood so far, index 0 and 1 correspond to one side, while 2 and 3 correspond to the other side. What is the meaning of a 1,0,1,0 side entry though? Is there something wrong, maybe the again an issue with geometry not being manifold? Or is that an valid entry?

(2.3)What are the __union_groups for exactly? My guess so far is that they simply represent the tensor holding the edge features in the end for torch to use? Why are they called "groups"?

(2.4)As asked in #43 I have my problems understanding __get_invalids(). The __redirect_edges() and __union_groups() calls do actually change up the mesh structure as the original function call simply wants to 'get' some information returned, doesn't it? Do these calls already perform the cleanup of "bad" (valence 3) vertices you mentioned in #43? I haven't fully understood what both are actually doing with the mesh structure and what for. image And while on this topic, isn't this picture actually missing one vertex in the middle of the collapsed edge? So far I though one vertex gets added there, however, it seems the vertex on the right gets moved instead. This would mean an edge collapse operation takes 5 edges and outputs 1, instead of the 2 that are mentioned in the paper. And as a result I actually don't understand the image and why the right sight would actually lead to non-manifold geometry when inserting the additional vertex.

(2.5) In case get_invalids() does change the mesh's structure as mentioned in 2.4, doesn't this lead to the following potential problem: __clean_side(..., side=0) does some changes, however, `clean_side(..., 2)` actually detects that the edge is not allowed to get pooled. Does this lead to a valid state?

(2.6) Why is there a member __merge_edge, which gets updated in __pool_edge() only and not used elsewhere? It might simply be some left over code not used anymore, or I am missing something, which then again would be interesting to understand on my side.

I am very thankful for any input, great work!

ranahanocka commented 4 years ago

(1.1)Do you have experience with "sticked-together", but not properly connected geometry? The decimation process seems to mess up the meshes really bad in case of low resolution parts and high resolution parts, which are part of the same object, but not connected as a mesh structure. Hence, the resulting meshes are almost always non-manifold very fast (low decimation level).

Right. We tried to avoid these cases, but it should be possible to modify the code to handle them. But, if each closed component is watertight, it should not become non-manifold. If I remember correctly, the problem is that one component will be reduced to a trapezoid (4 triangular faces), and then maybe the network tries to collapse an edge there. If this is indeed the problem, you can add a check to see if the shape is a trapezoid, and if it is, a collapse should not be allowed.

(1.2) In #43 it is mentioned, that "small connected but isolated components" can lead to trigger the assert(len(vertex) ==1) error. You mentioned them being like isolated pyramid structures.

Yeah so I think this is exactly the place the code will fail for a pyramid. I think that it will enter 'remove-triplete' (it detects that it's something to remove), yet, remove-triplete should not execute, since it will result in deleting the pyramid entirely.

Is this the only potentail reason leading to that assert failing?

The other case this can happen is if the input geometry is non-manifold.

I already added preprocessing to my meshes removing any isolated geometry (using blender api). However, I still get this issue.

Did you run 'degenerate dissolve' with blender as well?

In case I am wrong with 1.1 and the code being not robust against non-manifold geometry like I mentioned, could this also lead to this assert failing?

Different pytorch versions won't be the reason for the edge collapse difficulties.

(2.1)How can it be, that say edge e1 is neighbour of e7 and such is presented in its 1-ring structure, but e1 is not present in the 1-ring structure of e7. While debugging this occured to me, unfortunately. Maybe I am missing something here.

if e1 is a neighbor of e7 then e7 must be a neigbhor of e1. This is exactly the manifold criteria. If this is happening it sounds like your mesh is non-manifold. Open your meshes in meshlab to view non-manifold geometry.

(2.2)The edge-side list is used for indexing the neighbours and as far as I understood so far, index 0 and 1 correspond to one side, while 2 and 3 correspond to the other side. What is the meaning of a 1,0,1,0 side entry though? Is there something wrong, maybe the again an issue with geometry not being manifold? Or is that an valid entry?

I think it is OK if an edge has 0101 (although maybe I didn't think about it enough). But, the sides tell us where a particular edge is located in it's "gemm neighbor". For example, if we care about edge number 6 (call it e6), we know that the 4 neighbors are given by gemm_edges[e6, :], which a 4-d list[e6_n0, e6_n1, e6_n2, e6_n3]. Now let's consider the first neighbor of e6, which is e6_n0, it's gemm entry is gemm_edges[e6_n0, :] has four neighbors as well. How do we know where e6 is in gemm_edges[e6_n0, :]? That's where sides comes in. Sides tells us that here: sides[e6_n0, 0].

(2.3)What are the __union_groups for exactly? My guess so far is that they simply represent the tensor holding the edge features in the end for torch to use? Why are they called "groups"?

This is the book keeping that is used for unpooling the features back to the original input mesh resolution (remembering the history of collapses). Basically new edge features were created by 'union'ing edge feature vectors.

(2.4)As asked in #43 I have my problems understanding get_invalids(). The __redirect_edges() and union_groups() calls do actually change up the mesh structure as the original function call simply wants to 'get' some information returned, doesn't it? Do these calls already perform the cleanup of "bad" (valence 3) vertices you mentioned in #43? I haven't fully understood what both are actually doing with the mesh structure and what for.

The get invalids just looks for these invalid edges and returns their indices. as you can see here: first get_invalids is called and then remove tripelet removes the invalids. https://github.com/ranahanocka/MeshCNN/blob/feaa6c17607b84bb7334dca06747a63e72216aa0/models/layers/mesh_pool.py#L77-L79

isn't this picture actually missing one vertex in the middle of the collapsed edge? So far I though one vertex gets added there, however, it seems the vertex on the right gets moved instead. This would mean an edge collapse operation takes 5 edges and outputs 1, instead of the 2 that are mentioned in the paper.

I am not sure I follow. We are talking about these 5 edges right? Consider the light green edge, and it's 4 neighbors drawn with colors (5 edges). When we collapse, the light green edge is deleted (becomes the vertex on the right), and the purple & blue become a single edge, and the green & yellow become a single edge. We removed a total of 3 edges in the deletion of 1 edge.

image

(2.5) In case __get_invalids() does change the mesh's structure as mentioned

It doesn't change it

(2.6) Why is there a member __merge_edge

Correct, this does nothing. I should probably remove it to avoid confusion