Repository: mikedh/trimesh

Issue:
Null vector normals makes GLB export unreadable by external viewers
GLB/GLTF exports unitize by default the vertex normals of the mesh.

However, this normalization is not safe regarding null normal vectors.

This causes an issue when reading the exported GLB/GLTF from external viewers (windows 3D viewer for exemple) the null normals have been unitized to NaN vectors.

Example code illustrating the problem
```python
import numpy as np
import trimesh

# Define the vertices
vertices = np.array([
    [0, 0, 0],  # Vertex 0
    [1, 0, 0],  # Vertex 1
    [0, 1, 0],  # Vertex 2
    [1, 1, 0],  # Vertex 3
    [0, 0, 1],  # Vertex 4
    [1, 0, 1]   # Vertex 5
])

# Define the faces
faces = np.array([
    [0, 1, 2],  # Triangle 0
    [1, 3, 2],  # Triangle 1
    [0, 1, 4]   # Triangle 2
])

# Create the mesh
mesh = trimesh.Trimesh(vertices=vertices, faces=faces)

# Set the normal of the first vertex to null
modified_normals = mesh.vertex_normals.copy()
modified_normals[0] = [0,0,0]

mesh.vertex_normals = modified_normals

# Export the mesh to GLB
mesh.export("test_glb_export_null_normals.glb")

```
The exported tree looks like this, with NaN values as min/max values in the vertex normals buffer
```json
{"scene":0,"scenes":[{"nodes":[0]}],"asset":{"version":"2.0","generator":"https://github.com/mikedh/trimesh"},"accessors":[{"componentType":5125,"type":"SCALAR","bufferView":0,"count":9,"max":[4],"min":[0]},{"componentType":5126,"type":"VEC3","byteOffset":0,"bufferView":1,"count":5,"max":[1.0,1.0,1.0],"min":[0.0,0.0,0.0]},{"componentType":5126,"count":5,"type":"VEC3","byteOffset":0,"bufferView":2,"max":[NaN,NaN,NaN],"min":[NaN,NaN,NaN]}],"meshes":[{"name":"geometry_0","extras":{"processed":true},"primitives":[{"attributes":{"POSITION":1,"NORMAL":2},"indices":0,"mode":4}]}],"nodes":[{"name":"world","children":[1]},{"name":"geometry_0","mesh":0}],"buffers":[{"byteLength":156}],"bufferViews":[{"buffer":0,"byteOffset":0,"byteLength":36},{"buffer":0,"byteOffset":36,"byteLength":60},{"buffer":0,"byteOffset":96,"byteLength":60}]}
```

I would recommend having a null safe unitize operation, where null vectors are kept as is in the exported GLB/GLTF.

Discussion:

Patch:
diff --git a/pyproject.toml b/pyproject.toml
index acdcf1b3..ec9f97aa 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -5,7 +5,7 @@ requires = ["setuptools >= 61.0", "wheel"]
 [project]
 name = "trimesh"
 requires-python = ">=3.7"
-version = "4.1.2"
+version = "4.1.3"
 authors = [{name = "Michael Dawson-Haggerty", email = "mikedh@kerfed.com"}]
 license = {file = "LICENSE.md"}
 description = "Import, export, process, analyze and view triangular meshes."
@@ -112,7 +112,12 @@ all = ["trimesh[easy,recommend,test]"]
 
 [tool.ruff]
 target-version = "py37"
+line-length = 90
+
+
 # See https://github.com/charliermarsh/ruff#rules for error code definitions.
+
+[tool.ruff.lint]
 select = [
     # "ANN", # annotations
     "B", # bugbear
@@ -121,7 +126,7 @@ select = [
     "F", # flakes
     "I", # import sorting
     "RUF100", # meta
-    "U", # upgrade
+    "UP", # upgrade
     "W", # style warnings
     "YTT", # sys.version
 ]
@@ -133,9 +138,4 @@ ignore = [
   "E501", # Line too long ({width} > {limit} characters)
   "B904", # raise ... from err
   "B905", # zip() without an explicit strict= parameter
-  "ANN101", # type hint for `self`
-  "ANN002", # type hint for *args
-  "ANN003", # type hint for **kwargs
 ]
-line-length = 90
-
diff --git a/trimesh/__init__.py b/trimesh/__init__.py
index a200c7af..327680aa 100644
--- a/trimesh/__init__.py
+++ b/trimesh/__init__.py
@@ -17,6 +17,7 @@ from . import (
     creation,
     curvature,
     decomposition,
+    exceptions,
     geometry,
     graph,
     grouping,
@@ -59,9 +60,15 @@ try:
     from . import path
 except BaseException as E:
     # raise a useful error if path hasn't loaded
-    from .exceptions import ExceptionWrapper
+    path = exceptions.ExceptionWrapper(E)
+
+
+try:
+    from . import voxel
+except BaseException as E:
+    # requires non-minimal imports
+    voxel = exceptions.ExceptionWrapper(E)
 
-    path = ExceptionWrapper(E)
 
 __all__ = [
     "PointCloud",
@@ -108,4 +115,5 @@ __all__ = [
     "units",
     "utilScene",
     "voxel",
+    "exceptions",
 ]
diff --git a/trimesh/exchange/gltf.py b/trimesh/exchange/gltf.py
index 72de7203..f851fdb3 100644
--- a/trimesh/exchange/gltf.py
+++ b/trimesh/exchange/gltf.py
@@ -7,14 +7,15 @@ as GL_TRIANGLES, and trimesh.Path2D/Path3D as GL_LINES
 """
 
 import base64
-import collections
 import json
+from collections import OrderedDict, defaultdict, deque
 
 import numpy as np
 
 from .. import rendering, resources, transformations, util, visual
 from ..caching import hash_fast
 from ..constants import log, tol
+from ..typed import NDArray
 from ..util import unique_name
 from ..visual.gloss import specular_to_pbr
 
@@ -484,7 +485,7 @@ def _buffer_append(ordered, data):
 
     Parameters
     ----------
-    od : collections.OrderedDict
+    od : OrderedDict
       Keyed like { hash : data }
     data : bytes
       To be stored
@@ -505,19 +506,19 @@ def _buffer_append(ordered, data):
     return len(ordered) - 1
 
 
-def _data_append(acc, buff, blob, data):
+def _data_append(acc: OrderedDict, buff: OrderedDict, blob: dict, data: NDArray):
     """
     Append a new accessor to an OrderedDict.
 
     Parameters
     ------------
-    acc : collections.OrderedDict
+    acc
       Collection of accessors, will be mutated in-place
-    buff : collections.OrderedDict
+    buff
       Collection of buffer bytes, will be mutated in-place
-    blob : dict
+    blob
       Candidate accessor
-    data : numpy.array
+    data
       Data to fill in details to blob
 
     Returns
@@ -562,6 +563,11 @@ def _data_append(acc, buff, blob, data):
     # see if we're an array, matrix, etc
     kind = blob["type"]
 
+    if tol.strict:
+        # in unit tests make sure everything we're trying to export
+        # is finite, which also checks for accidental NaN values
+        assert np.isfinite(data).all()
+
     if kind == "SCALAR":
         # is probably (n, 1)
         blob["count"] = int(np.prod(data.shape))
@@ -665,7 +671,7 @@ def _create_gltf_structure(
         "scene": 0,
         "scenes": [{"nodes": [0]}],
         "asset": {"version": "2.0", "generator": "https://github.com/mikedh/trimesh"},
-        "accessors": collections.OrderedDict(),
+        "accessors": OrderedDict(),
         "meshes": [],
         "images": [],
         "textures": [],
@@ -689,7 +695,7 @@ def _create_gltf_structure(
     # store materials as {hash : index} to avoid duplicates
     mat_hashes = {}
     # store data from geometries
-    buffer_items = collections.OrderedDict()
+    buffer_items = OrderedDict()
 
     # map the name of each mesh to the index in tree['meshes']
     mesh_index = {}
@@ -922,10 +928,7 @@ def _append_mesh(
     ):
         # store vertex normals if requested
         if unitize_normals:
-            normals = mesh.vertex_normals.copy()
-            norms = np.linalg.norm(normals, axis=1)
-            if not util.allclose(norms, 1.0, atol=1e-4):
-                normals /= norms.reshape((-1, 1))
+            normals = util.unitize(mesh.vertex_normals)
         else:
             # we don't have to copy them since
             # they aren't being altered
@@ -978,7 +981,7 @@ def _build_views(buffer_items):
 
     Parameters
     --------------
-    buffer_items : collections.OrderedDict
+    buffer_items : OrderedDict
       Buffers to build views for
 
     Returns
@@ -1461,9 +1464,9 @@ def _read_buffers(
         else:
             materials = _parse_materials(header, views=views, resolver=resolver)
 
-    mesh_prim = collections.defaultdict(list)
+    mesh_prim = defaultdict(list)
     # load data from accessors into Trimesh objects
-    meshes = collections.OrderedDict()
+    meshes = OrderedDict()
 
     # keep track of how many times each name has been attempted to
     # be inserted to avoid a potentially slow search through our
@@ -1661,9 +1664,9 @@ def _read_buffers(
     names[base_frame] = base_frame
 
     # visited, kwargs for scene.graph.update
-    graph = collections.deque()
+    graph = deque()
     # unvisited, pairs of node indexes
-    queue = collections.deque()
+    queue = deque()
 
     if "scene" in header:
         # specify the index of scenes if specified
diff --git a/trimesh/voxel/__init__.py b/trimesh/voxel/__init__.py
index 83599cd2..11297598 100644
--- a/trimesh/voxel/__init__.py
+++ b/trimesh/voxel/__init__.py
@@ -1,3 +1,4 @@
+from . import creation
 from .base import VoxelGrid
 
-__all__ = ["VoxelGrid"]
+__all__ = ["VoxelGrid", "creation"]


Test patch:
diff --git a/tests/generic.py b/tests/generic.py
index b3fa46fc..88f4c92e 100644
--- a/tests/generic.py
+++ b/tests/generic.py
@@ -543,7 +543,7 @@ def check_fuze(fuze):
     fuze.visual.material.copy()
 
 
-def wrapload(exported, file_type, **kwargs):
+def roundtrip(exported, file_type, **kwargs):
     """
     Reload an exported byte blob into a mesh.
 
diff --git a/tests/test_export.py b/tests/test_export.py
index a18565a2..ac13a1b9 100644
--- a/tests/test_export.py
+++ b/tests/test_export.py
@@ -200,7 +200,7 @@ class ExportTest(g.unittest.TestCase):
     def test_obj(self):
         m = g.get_mesh("textured_tetrahedron.obj", process=False)
         export = m.export(file_type="obj")
-        reconstructed = g.wrapload(export, file_type="obj", process=False)
+        reconstructed = g.roundtrip(export, file_type="obj", process=False)
         # test that we get at least the same number of normals and texcoords out;
         # the loader may reorder vertices, so we shouldn't check direct
         # equality
@@ -243,7 +243,7 @@ class ExportTest(g.unittest.TestCase):
         export = source.export(file_type="glb")
 
         # re- load the file as a trimesh.Scene object again
-        loaded = g.wrapload(export, file_type="glb")
+        loaded = g.roundtrip(export, file_type="glb")
 
         # the scene should be identical after export-> import cycle
         assert g.np.allclose(loaded.extents / source.extents, 1.0)
diff --git a/tests/test_gltf.py b/tests/test_gltf.py
index 2261f8c1..3e00b798 100644
--- a/tests/test_gltf.py
+++ b/tests/test_gltf.py
@@ -1032,6 +1032,30 @@ class GLTFTest(g.unittest.TestCase):
         # export with a postprocessor
         s.export(file_type="glb", tree_postprocessor=post)
 
+    def test_unitize_normals_null_values(self):
+        # Create the mesh
+        mesh = g.trimesh.Trimesh(
+            vertices=[[0, 0, 0], [1, 0, 0], [0, 1, 0], [1, 1, 0], [0, 0, 1], [1, 0, 1]],
+            faces=[[0, 1, 2], [1, 3, 2], [0, 1, 4]],
+        )
+
+        # Set the normal of the first vertex to null
+        modified_normals = mesh.vertex_normals.copy()
+        modified_normals[0] = [0, 0, 0]
+
+        mesh.vertex_normals = modified_normals
+
+        # Export the mesh
+        export = mesh.export(file_type="glb", unitize_normals=True)
+        reimported_mesh = list(
+            g.trimesh.load(
+                g.trimesh.util.wrap_as_stream(export), file_type="glb"
+            ).geometry.values()
+        )[0]
+
+        # Check that the normals are still null
+        assert g.np.allclose(reimported_mesh.vertex_normals[0], [0, 0, 0])
+
 
 if __name__ == "__main__":
     g.trimesh.util.attach_to_log()
diff --git a/tests/test_obj.py b/tests/test_obj.py
index 01aac6b8..8000ff32 100644
--- a/tests/test_obj.py
+++ b/tests/test_obj.py
@@ -11,7 +11,7 @@ class OBJTest(g.unittest.TestCase):
         # this should test the non-vectorized load path
         m = g.get_mesh("rabbit.obj")
         assert len(m.faces) == 1252
-        rec = g.wrapload(m.export(file_type="obj"), file_type="obj")
+        rec = g.roundtrip(m.export(file_type="obj"), file_type="obj")
         assert g.np.isclose(m.area, rec.area)
 
     def test_no_img(self):
@@ -25,14 +25,14 @@ class OBJTest(g.unittest.TestCase):
         assert m.visual.uv.min() > -1e-5
         # check to make sure it's not all zeros
         assert m.visual.uv.ptp() > 0.5
-        rec = g.wrapload(m.export(file_type="obj"), file_type="obj")
+        rec = g.roundtrip(m.export(file_type="obj"), file_type="obj")
         assert g.np.isclose(m.area, rec.area)
 
     def test_trailing(self):
         # test files with texture and trailing slashes
         m = g.get_mesh("jacked.obj")
         assert len(m.visual.uv) == len(m.vertices)
-        rec = g.wrapload(m.export(file_type="obj"), file_type="obj")
+        rec = g.roundtrip(m.export(file_type="obj"), file_type="obj")
         assert g.np.isclose(m.area, rec.area)
 
     def test_obj_groups(self):
@@ -66,7 +66,7 @@ class OBJTest(g.unittest.TestCase):
 
         assert mesh.is_watertight
         assert mesh.is_winding_consistent
-        rec = g.wrapload(mesh.export(file_type="obj"), file_type="obj")
+        rec = g.roundtrip(mesh.export(file_type="obj"), file_type="obj")
         assert g.np.isclose(mesh.area, rec.area)
 
     def test_obj_multiobj(self):
@@ -242,7 +242,7 @@ class OBJTest(g.unittest.TestCase):
                 raise ValueError("cannot export empty")
             elif "points" in empty_file:
                 export = e.export(file_type="ply")
-                reconstructed = g.wrapload(export, file_type="ply")
+                reconstructed = g.roundtrip(export, file_type="ply")
 
                 # result should be a point cloud instance
                 assert isinstance(e, g.trimesh.PointCloud)
@@ -258,7 +258,7 @@ class OBJTest(g.unittest.TestCase):
 
     def test_no_uv(self):
         mesh = g.get_mesh("box.obj")
-        rec = g.wrapload(mesh.export(file_type="obj"), file_type="obj")
+        rec = g.roundtrip(mesh.export(file_type="obj"), file_type="obj")
         assert g.np.isclose(mesh.area, rec.area)
 
     def test_no_uv_but_mtl(self):
diff --git a/tests/test_ply.py b/tests/test_ply.py
index dad2d8ff..4747b9ff 100644
--- a/tests/test_ply.py
+++ b/tests/test_ply.py
@@ -19,7 +19,7 @@ class PlyTest(g.unittest.TestCase):
         assert m.visual.face_colors.ptp(axis=0).max() > 0
 
         export = m.export(file_type="ply")
-        reconstructed = g.wrapload(export, file_type="ply")
+        reconstructed = g.roundtrip(export, file_type="ply")
 
         assert reconstructed.visual.kind == "face"
 
@@ -31,7 +31,7 @@ class PlyTest(g.unittest.TestCase):
         assert m.visual.vertex_colors.ptp(axis=0).max() > 0
 
         export = m.export(file_type="ply")
-        reconstructed = g.wrapload(export, file_type="ply")
+        reconstructed = g.roundtrip(export, file_type="ply")
         assert reconstructed.visual.kind == "vertex"
 
         assert g.np.allclose(reconstructed.visual.vertex_colors, m.visual.vertex_colors)
@@ -90,7 +90,7 @@ class PlyTest(g.unittest.TestCase):
         m.vertex_attributes["test_nd_attribute"] = test_nd_attribute
 
         export = m.export(file_type="ply")
-        reconstructed = g.wrapload(export, file_type="ply")
+        reconstructed = g.roundtrip(export, file_type="ply")
 
         vertex_attributes = reconstructed.metadata["_ply_raw"]["vertex"]["data"]
         result_1d = vertex_attributes["test_1d_attribute"]
@@ -110,7 +110,7 @@ class PlyTest(g.unittest.TestCase):
         m.face_attributes["test_nd_attribute"] = test_nd_attribute
 
         export = m.export(file_type="ply")
-        reconstructed = g.wrapload(export, file_type="ply")
+        reconstructed = g.roundtrip(export, file_type="ply")
 
         face_attributes = reconstructed.metadata["_ply_raw"]["face"]["data"]
         result_1d = face_attributes["test_1d_attribute"]
@@ -133,19 +133,19 @@ class PlyTest(g.unittest.TestCase):
 
     def test_ascii_color(self):
         mesh = g.trimesh.creation.box()
-        en = g.wrapload(mesh.export(file_type="ply", encoding="ascii"), file_type="ply")
+        en = g.roundtrip(mesh.export(file_type="ply", encoding="ascii"), file_type="ply")
         assert en.visual.kind is None
 
         color = [255, 0, 0, 255]
         mesh.visual.vertex_colors = color
 
         # try exporting and reloading raw
-        eb = g.wrapload(mesh.export(file_type="ply"), file_type="ply")
+        eb = g.roundtrip(mesh.export(file_type="ply"), file_type="ply")
 
         assert g.np.allclose(eb.visual.vertex_colors[0], color)
         assert eb.visual.kind == "vertex"
 
-        ea = g.wrapload(mesh.export(file_type="ply", encoding="ascii"), file_type="ply")
+        ea = g.roundtrip(mesh.export(file_type="ply", encoding="ascii"), file_type="ply")
         assert g.np.allclose(ea.visual.vertex_colors, color)
         assert ea.visual.kind == "vertex"
 
@@ -171,7 +171,7 @@ class PlyTest(g.unittest.TestCase):
             elif "points" in empty_file:
                 # create export
                 export = e.export(file_type="ply")
-                reconstructed = g.wrapload(export, file_type="ply")
+                reconstructed = g.roundtrip(export, file_type="ply")
 
                 # result should be a point cloud instance
                 assert isinstance(e, g.trimesh.PointCloud)
diff --git a/tests/test_points.py b/tests/test_points.py
index 716d0a06..4c28953b 100644
--- a/tests/test_points.py
+++ b/tests/test_points.py
@@ -222,7 +222,7 @@ class PointsTest(g.unittest.TestCase):
         assert hash(p.visual) != initial
 
         # test exporting a pointcloud to a PLY file
-        r = g.wrapload(p.export(file_type="ply"), file_type="ply")
+        r = g.roundtrip(p.export(file_type="ply"), file_type="ply")
         assert r.vertices.shape == p.vertices.shape
         # make sure colors survived the round trip
         assert g.np.allclose(r.colors, p.colors)