trimesh icon indicating copy to clipboard operation
trimesh copied to clipboard

Strange behaviour when setting face_normals in Trimesh object

Open domef opened this issue 7 months ago • 2 comments

The setter property face_normals of triemesh.Trimesh is very strange. It essentialy checks if the normals you passed to the function are the same to the ones it computes with its own algorithm, but it checks only the first 20 vertices and if they are not the same it skips the assignment (https://github.com/mikedh/trimesh/blob/bcbc8f14739a2cd60f2130d20617cf0bfd54ef62/trimesh/base.py#L440C1-L446C19).

This is very clear with this example:

import trimesh
import numpy as np

mesh = trimesh.creation.icosphere()
face_normals = mesh.face_normals
new_face_normals = face_normals.copy()
new_face_normals *= -1
mesh.face_normals = new_face_normals
print(np.allclose(mesh.face_normals, face_normals))  # True
print(np.allclose(mesh.face_normals, new_face_normals))  # False

mesh = trimesh.creation.icosphere()
face_normals = mesh.face_normals
new_face_normals = face_normals.copy()
new_face_normals[20:] *= -1
mesh.face_normals = new_face_normals
print(np.allclose(mesh.face_normals, face_normals))  # False
print(np.allclose(mesh.face_normals, new_face_normals))  # True

So my question is: what's the point of assigning a value only if it is equal to a fixed precomputed value? Isn't it like not assigning anything at all?

domef avatar Apr 16 '25 23:04 domef

Hey, yeah agreed that they probably shouldn't be settable and just be strictly defined as "the cross product of the edges" to avoid confusion from functions that expect them to be perpendicular.

It's something of a legacy behavior since computing the face normals is often the slowest thing in a profile, and some import file formats (like STL) include them. And doing a check on the whole array isn't that much faster than recomputing, hence the slightly janky "only check the first 20" behavior:

In [27]: m
Out[27]: <trimesh.Trimesh(vertices.shape=(163842, 3), faces.shape=(327680, 3))>

In [30]: %timeit trimesh.triangles.normals(m.triangles)
27.4 ms ± 14.5 μs per loop (mean ± std. dev. of 7 runs, 10 loops each)

In [31]: n = m.face_normals.copy()

# check that a given array of face normals is perpendicular to one edge of a triangle
In [32]: %timeit trimesh.util.diagonal_dot(n, np.diff(m.vertices[m.edges[::3]], axis=1).reshape((-1, 3)))
16.9 ms ± 6.11 μs per loop (mean ± std. dev. of 7 runs, 100 loops each)

mikedh avatar Apr 17 '25 00:04 mikedh

Thanks for the answer. I think that trimesh could precomputes the face normals, but I also think that they should be overridable by the user.

domef avatar Apr 17 '25 09:04 domef