ranahanocka / MeshCNN

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

fix bug when calculating angles from faces #13

Closed bertjiazheng closed 5 years ago

bertjiazheng commented 5 years ago

Correct me if I am wrong.

bertjiazheng commented 5 years ago

Hi @ranahanocka,

I make this PR more clear.

The function angles_from_faces computes the dihedral angle between the two faces:

  1. select two edges from the same face
  2. compute normal by the cross product of two edges
  3. obtain the dihedral angle by the inner product of the two normals.

However, when computing the normals of two faces normals in step 1, the _edgea and _edgeb are from two different faces _edgefaces[:, 0] and _edgefaces[:, 1], which make the normal mistakenly computed.

Looking forward to hearing from you!

Best, Jia

ranahanocka commented 5 years ago

Hi @bertjiazheng

Thanks a lot for the PR! :)

I started checking into this briefly. A short update:

Note that this function is not used to extract the input features to the network (extracting features happens in extract_features and uses different methods (dihedral_angle, symmetric_opposite_angles and symmetric_ratios ).

So, _the angles_fromfaces function is only used to decide which edges to flip. The idea is that we tried to avoid flipping edges at sharp corners since it distorts the mesh. I remember we had some issues / noticed some odd behavior with this, but I thought we fixed them.

Anyway, I am going to take a look now at exporting some different edge flipped meshes and see if I can remember what is going on.

bertjiazheng commented 5 years ago

Hi @ranahanocka

Yes, the _angles_fromfaces only affect the edge flipping operation.

The vertices of _edgea (or _edgeb) are from two different faces _edgefaces[:, 0] and _edgefaces[:, 1], and normals[0] and normals[1] are exactly the same. So the dot only has two different results:

Best, Jia

ranahanocka commented 5 years ago

Hi @bertjiazheng ,

Nice catch - indeed this is a bug. Thanks for the PR :+1:

Here is how an augmented mesh looked when applying edge flips (before): image and now with the fix: image