facebookresearch / pytorch3d

PyTorch3D is FAIR's library of reusable components for deep learning with 3D data
https://pytorch3d.org/
Other
8.67k stars 1.3k forks source link

Wrong documentation #433

Closed CharlesNord closed 3 years ago

CharlesNord commented 3 years ago

https://github.com/facebookresearch/pytorch3d/blob/194b29fb6c1796cc66cb6ef7c8fa5c7099086186/pytorch3d/renderer/mesh/rasterize_meshes.py#L98

https://github.com/facebookresearch/pytorch3d/blob/194b29fb6c1796cc66cb6ef7c8fa5c7099086186/pytorch3d/renderer/blending.py#L136

the pix_dists or dists is not the distance between the pixel and the face it belongs to.

According to section 3.2 of [1],

d(i,j)is the closest distance from pi to fj’s edge

in which fj means the jth triangle and pi is the ith vertex. So, here pix_dists should be the minimum distance between the pixel and the triangle's three edges, where the triangle is the face that covers the pixel.

The code in point_triangle_distance:

https://github.com/facebookresearch/pytorch3d/blob/194b29fb6c1796cc66cb6ef7c8fa5c7099086186/pytorch3d/renderer/mesh/rasterize_meshes.py#L545

also follows the description of paper [1]

[1] Shichen Liu et al, 'Soft Rasterizer: A Differentiable Renderer for Image-based 3D Reasoning'

gkioxari commented 3 years ago

During rasterization, a ray starting at pixel (x, y) can hit numerous 3D faces. The point of intersection of the ray and the face can be within the face, i.e. the space spanned by the three vertices, or outside of the face (because we blur the face, see blur_radius) to make rendering differentiable. If the ray intersects the exterior of the face, then the distance is the distance of the pixel (x, y) to the nearest edge of the face.

If the ray intersects the interior of the face, then the distance in the rasterizer is the negative distance from the nearest edge.

I don't understand where the error is in the documentation however?

CharlesNord commented 3 years ago

During rasterization, a ray starting at pixel (x, y) can hit numerous 3D faces. The point of intersection of the ray and the face can be within the face, i.e. the space spanned by the three vertices, or outside of the face (because we blur the face, see blur_radius) to make rendering differentiable. If the ray intersects the exterior of the face, then the distance is the distance of the pixel (x, y) to the nearest edge of the face.

If the ray intersects the interior of the face, then the distance in the rasterizer is the negative distance from the nearest edge.

I don't understand where the error is in the documentation however?

Yeah, what you discribe is exactly what I mean. But here you redefined the distance of a point to a face. Usually when we say the distance from a point to a face, we mean the length of he perpendicular from the point to the plane of the face (since the pixel is in the triangle, this value is 0 ), not the distance to the edge of the face. Screenshot from 2020-11-10 13-32-27

gkioxari commented 3 years ago

Your figure is correct but note that in the rasterizer, the dists output is the distance of a point to the face in 2D. The point is the (x, y) pixel and the face is the projected face to the plane. The distance is not in 3D. So the blue line that you have marked in the figure is not pertinent.

CharlesNord commented 3 years ago

Your figure is correct but note that in the rasterizer, the dists output is the distance of a point to the face in 2D. The point is the (x, y) pixel and the face is the projected face to the plane. The distance is not in 3D. So the blue line that you have marked in the figure is not pertinent.

Yeah, you're right. I know the blue line is not pertinent, I just want to show you one misunderstanding of the ambiguous documents may lead to. Both the "2D distance" and the "squared distance between the pixel and the face" in the documentations are not well-defined and commonly accepted as the green line in my figure above. On the contrary, the distance between the pixel and the face is commonly considered as the length of the blue line above.

gkioxari commented 3 years ago

Ok I see your point! We will try to clarify this in the documentation! Thank you for bringing it up!