Repository: mikedh/trimesh

Issue:
Garbled glb/gltf export
Thank you for this great library.

### Incorrect export

In a larger scene, I noticed that some objects were garbled after export to .glb/.gltf. But the data in the Trimesh objects was correct. 

Some of the objects in question were simple (but different) cube-like meshes. Here is a screenshot of one of the objects that got exported incorrectly (left) and the correct mesh (right):
![garbled-glb-export](https://user-images.githubusercontent.com/2477826/125739329-977144c4-07f9-440c-ada4-b1f41db224b0.png)

The vertices are correct, but the indices are exported incorrectly.

### Cause

After some debugging, I tracked it down to the hashing of the faces array, used to deduplicate buffers in gltf (introduced in https://github.com/mikedh/trimesh/pull/1182).

The affected objects had (in my case) the same number of vertices, and thus similar, but different, definitions of their faces, e.g.:

```python
faces1 = np.array([0, 1, 2, 0, 3, 1, 0, 2, 4, 0, 4, 5, 5, 6, 3, 5, 3, 0, 7, 1, 3, 7,
    3, 6, 4, 2, 1, 4, 1, 7, 5, 4, 7, 6, 5, 7], dtype=np.int64).reshape(-1, 3)
faces2 = np.array([0, 1, 2, 0, 3, 1, 2, 4, 0, 5, 4, 2, 6, 3, 0, 6, 0, 4, 6, 1, 3, 6,
    7, 1, 2, 7, 5, 2, 1, 7, 4, 5, 7, 6, 4, 7], dtype=np.int64).reshape(-1 ,3)
```

Using the default `fast_hash` (`zlib.adler32`, when `xxhash` is not installed), they hash to the same value:

```python
>>> zlib.adler32(faces1.tobytes()) == zlib.adler32(faces2.tobytes())
True
```
So only one of the index buffers got exported, and the other meshes incorrectly shared the same buffer.

### Possible solutions

The easiest workaround is to just install `xxhash`, which fixes this. Maybe `xxhash` should be promoted to a hard dependency? For the latest release, there seem to be [wheels for Linux, macOS and Windows](https://pypi.org/project/xxhash/#files), so maybe the comment https://github.com/mikedh/trimesh/blob/73a41c3659b7962a07c8ebd509bca2455010e34f/trimesh/caching.py#L25-L27 is not valid anymore?

Another fix would be to use a better hash function as a slower, but correct, default, like for example md5.
Adler-32 is not well suited for small buffers, like in my example. Quoting [Wikipedia](https://en.wikipedia.org/wiki/Adler-32#Weakness):
> Adler-32 is weak for short messages because the sum A does not wrap.

Discussion:

Patch:
diff --git a/trimesh/bounds.py b/trimesh/bounds.py
index 5bb80d65..56ef89b9 100644
--- a/trimesh/bounds.py
+++ b/trimesh/bounds.py
@@ -219,7 +219,7 @@ def oriented_bounds(obj,
 
         # make sure transform isn't mangling triangles
         # by reversing windings on triangles
-        if np.isclose(np.trace(flip[:3, :3]), 0.0):
+        if not np.isclose(np.linalg.det(flip[:3, :3]), 1.0):
             flip[:3, :3] = np.dot(flip[:3, :3], -np.eye(3))
 
         # apply the flip to the OBB transform
diff --git a/trimesh/caching.py b/trimesh/caching.py
index 90247edd..ea7b68d3 100644
--- a/trimesh/caching.py
+++ b/trimesh/caching.py
@@ -9,7 +9,7 @@ and clearing cached values based on those changes.
 import numpy as np
 
 import zlib
-import hashlib
+from hashlib import md5
 
 from functools import wraps
 
@@ -21,13 +21,22 @@ try:
 except ImportError:
     from collections import Mapping
 
+# xxhash is roughly 5x faster than zlib.adler32 and
+# up to 30x faster than MD5, but is only
+# packaged in easy wheels on linux (`pip install xxhash`)
 try:
-    # xxhash is roughly 5x faster than zlib.adler32 but is only
-    # packaged in easy wheels on linux (`pip install xxhash`)
-    # so we keep it a soft dependency
-    import xxhash
-except ImportError:
-    xxhash = None
+    # newest version of algorithm
+    from xxhash import xxh3_64_intdigest as fast_hash
+except BaseException:
+    try:
+        # older version of the algorithm
+        from xxhash import xxh64_intdigest as fast_hash
+    except BaseException:
+        log.debug('falling back to MD5 hashing: ' +
+                  '`pip install xxhash`' +
+                  ' for 30x faster cache checks')
+        # use MD5 as a fallback hashing algorithm
+        def fast_hash(a): return int(md5(a).hexdigest(), 16)
 
 
 def tracked_array(array, dtype=None):
@@ -166,16 +175,14 @@ class TrackedArray(np.ndarray):
         """
         if self._modified_m or not hasattr(self, '_hashed_md5'):
             if self.flags['C_CONTIGUOUS']:
-                hasher = hashlib.md5(self)
-                self._hashed_md5 = hasher.hexdigest()
+                self._hashed_md5 = md5(self).hexdigest()
             else:
                 # the case where we have sliced our nice
                 # contiguous array into a non- contiguous block
                 # for example (note slice *after* track operation):
                 # t = util.tracked_array(np.random.random(10))[::-1]
-                contiguous = np.ascontiguousarray(self)
-                hasher = hashlib.md5(contiguous)
-                self._hashed_md5 = hasher.hexdigest()
+                self._hashed_md5 = md5(
+                    np.ascontiguousarray(self)).hexdigest()
         self._modified_m = False
         return self._hashed_md5
 
@@ -202,7 +209,7 @@ class TrackedArray(np.ndarray):
         self._modified_c = False
         return self._hashed_crc
 
-    def _xxhash(self):
+    def fast_hash(self):
         """
         An xxhash.b64 hash of the array.
 
@@ -216,14 +223,15 @@ class TrackedArray(np.ndarray):
         # these functions are called millions of times so everything helps
         if self._modified_x or not hasattr(self, '_hashed_xx'):
             if self.flags['C_CONTIGUOUS']:
-                self._hashed_xx = xxhash.xxh64(self).intdigest()
+                self._hashed_xx = fast_hash(self)
             else:
                 # the case where we have sliced our nice
                 # contiguous array into a non- contiguous block
                 # for example (note slice *after* track operation):
                 # t = util.tracked_array(np.random.random(10))[::-1]
-                self._hashed_xx = xxhash.xxh64(np.ascontiguousarray(self)).intdigest()
+                self._hashed_xx = fast_hash(np.ascontiguousarray(self))
         self._modified_x = False
+
         return self._hashed_xx
 
     def __hash__(self):
@@ -353,13 +361,6 @@ class TrackedArray(np.ndarray):
         super(self.__class__, self).__setslice__(*args,
                                                  **kwargs)
 
-    if xxhash is None:
-        # otherwise use our fastest CRC
-        fast_hash = crc
-    else:
-        # if xxhash is installed use it
-        fast_hash = _xxhash
-
 
 class Cache(object):
     """
@@ -637,11 +638,10 @@ class DataStore(Mapping):
         md5 : str
           MD5 of data in hexadecimal
         """
-        hasher = hashlib.md5()
-        for key in sorted(self.data.keys()):
-            hasher.update(self.data[key].md5().encode('utf-8'))
-        md5 = hasher.hexdigest()
-        return md5
+        hasher = md5(''.join(
+            self.data[key].md5()
+            for key in sorted(self.data.keys())).encode('utf-8'))
+        return hasher.hexdigest()
 
     def crc(self):
         """
@@ -670,7 +670,7 @@ class DataStore(Mapping):
         return fast
 
 
-def _fast_crc(count=25):
+def _fast_crc(count=10):
     """
     On certain platforms/builds zlib.adler32 is substantially
     faster than zlib.crc32, but it is not consistent across
@@ -711,9 +711,3 @@ def _fast_crc(count=25):
 # get the fastest CRC32 available on the
 # current platform when trimesh is imported
 crc32 = _fast_crc()
-
-# add a shortcut for fast hashing
-if xxhash is None:
-    fast_hash = crc32
-else:
-    def fast_hash(x): return xxhash.xxh64(x).intdigest()
diff --git a/trimesh/exchange/ply.py b/trimesh/exchange/ply.py
index de7b8499..050ed6ae 100644
--- a/trimesh/exchange/ply.py
+++ b/trimesh/exchange/ply.py
@@ -525,16 +525,18 @@ def elements_to_kwargs(elements,
             # incorrectly is a ValueError
             pass
 
-    # If texture coordinates are defined with vertices
-    if (texcoord is None and
-        'texture_u' in elements['vertex']['data'] and
-            'texture_v' in elements['vertex']['data']):
-        texture_u = elements['vertex']['data']['texture_u']
-        texture_v = elements['vertex']['data']['texture_v']
-        texcoord = np.stack((
-            texture_u[faces.reshape(-1)],
-            texture_v[faces.reshape(-1)]), axis=-1).reshape(
-                (faces.shape[0], -1))
+    if texcoord is None:
+        try:
+            # If texture coordinates are defined with vertices
+            t_u = elements['vertex']['data']['texture_u']
+            t_v = elements['vertex']['data']['texture_v']
+            texcoord = np.stack((
+                t_u[faces.reshape(-1)],
+                t_v[faces.reshape(-1)]), axis=-1).reshape(
+                    (faces.shape[0], -1))
+        except (ValueError, KeyError):
+            # if the fields didn't exist
+            pass
 
     if faces is not None:
         shape = np.shape(faces)
diff --git a/trimesh/path/exchange/load.py b/trimesh/path/exchange/load.py
index 0b51cf8d..054efae6 100644
--- a/trimesh/path/exchange/load.py
+++ b/trimesh/path/exchange/load.py
@@ -42,12 +42,11 @@ def load_path(file_obj, file_type=None, **kwargs):
             file_obj, file_type=file_type))
     elif util.is_string(file_obj):
         # strings passed are evaluated as file file_objects
-        with open(file_obj, 'rb') as file_file_obj:
+        with open(file_obj, 'rb') as f:
             # get the file type from the extension
             file_type = os.path.splitext(file_obj)[-1][1:].lower()
             # call the loader
-            kwargs.update(path_loaders[file_type](
-                file_file_obj, file_type=file_type))
+            kwargs.update(path_loaders[file_type](f, file_type=file_type))
     elif util.is_instance_named(file_obj, 'Polygon'):
         # convert from shapely polygons to Path2D
         kwargs.update(misc.polygon_to_path(file_obj))
diff --git a/trimesh/version.py b/trimesh/version.py
index 615d9f18..cb63a1c4 100644
--- a/trimesh/version.py
+++ b/trimesh/version.py
@@ -1,1 +1,1 @@
-__version__ = '3.9.21'
+__version__ = '3.9.23'


Test patch:
diff --git a/tests/generic.py b/tests/generic.py
index dcec5efe..1862b995 100644
--- a/tests/generic.py
+++ b/tests/generic.py
@@ -185,6 +185,16 @@ def get_mesh(file_name, *args, **kwargs):
 def get_path(file_name):
     """
     Get the absolute location of a referenced model file.
+
+    Parameters
+    ------------
+    file_name : str
+      Name of model relative to `repo/models`
+
+    Returns
+    ------------
+    full : str
+      Full absolute path to model.
     """
     return os.path.abspath(
         os.path.join(dir_models, file_name))
diff --git a/tests/test_bounds.py b/tests/test_bounds.py
index 03e00414..2baea4e7 100644
--- a/tests/test_bounds.py
+++ b/tests/test_bounds.py
@@ -29,6 +29,9 @@ class BoundsTest(g.unittest.TestCase):
                 box_ext = m.bounding_box_oriented.primitive.extents.copy()
                 box_t = m.bounding_box_oriented.primitive.transform.copy()
 
+                # determinant of rotation should be 1.0
+                assert g.np.isclose(g.np.linalg.det(box_t[:3, :3]), 1.0)
+
                 m.apply_transform(g.np.linalg.inv(box_t))
 
                 test = m.bounds / (box_ext / 2.0)
diff --git a/tests/test_cache.py b/tests/test_cache.py
index 0f0f110f..a1e88a8f 100644
--- a/tests/test_cache.py
+++ b/tests/test_cache.py
@@ -43,9 +43,12 @@ class CacheTest(g.unittest.TestCase):
 
         # xxhash should be faster than CRC and MD5
         # it is sometimes slower on Windows/Appveyor TODO: figure out why
-        if g.trimesh.caching.xxhash is not None and g.platform.system() == 'Linux':
-            assert xt < mt
-            assert xt < ct
+        try:
+            import xxhash  # NOQA
+        except BaseException:
+            return
+        assert xt < mt
+        assert xt < ct
 
     def test_track(self):
         """
@@ -219,6 +222,22 @@ class CacheTest(g.unittest.TestCase):
         # should still be in the cache
         assert len(m.edges_face) == e_len
 
+    def test_simple_collision(self):
+        faces1 = g.np.array([0, 1, 2, 0, 3, 1, 0,
+                             2, 4, 0, 4, 5, 5, 6,
+                             3, 5, 3, 0, 7, 1, 3,
+                             7, 3, 6, 4, 2, 1, 4,
+                             1, 7, 5, 4, 7, 6, 5, 7],
+                            dtype=g.np.int64).reshape(-1, 3)
+        faces2 = g.np.array([0, 1, 2, 0, 3, 1, 2,
+                             4, 0, 5, 4, 2, 6, 3,
+                             0, 6, 0, 4, 6, 1, 3,
+                             6, 7, 1, 2, 7, 5, 2,
+                             1, 7, 4, 5, 7, 6, 4, 7],
+                            dtype=g.np.int64).reshape(-1, 3)
+        fast_hash = g.trimesh.caching.fast_hash
+        assert fast_hash(faces1) != fast_hash(faces2)
+
 
 if __name__ == '__main__':
     g.trimesh.util.attach_to_log()
diff --git a/tests/test_minimal.py b/tests/test_minimal.py
index a0657f07..8562e752 100644
--- a/tests/test_minimal.py
+++ b/tests/test_minimal.py
@@ -42,6 +42,11 @@ class MinimalTest(unittest.TestCase):
                 assert len(m.face_adjacency.shape) == 2
                 assert len(m.vertices.shape) == 2
 
+                # make sure hash changes
+                initial = m._data.fast_hash()
+                m.vertices[:, 0] += 1.0
+                assert m._data.fast_hash() != initial
+
 
 if __name__ == '__main__':
     trimesh.util.attach_to_log()
diff --git a/tests/test_pathlib.py b/tests/test_pathlib.py
index 5444f744..73586067 100644
--- a/tests/test_pathlib.py
+++ b/tests/test_pathlib.py
@@ -39,6 +39,35 @@ class PathTest(g.unittest.TestCase):
         # mesh should be the same after exporting
         assert g.np.isclose(m.volume, r.volume)
 
+    def test_full_filetype(self):
+        """
+        Test loading with file types specified as
+        the full filename, not just extension.
+        """
+        file_name = g.get_path('unit_cube.STL')
+        with open(file_name, 'rb') as f:
+            # check `load_mesh`
+            mesh = g.trimesh.load_mesh(file_obj=file_name,
+                                       file_type=file_name)
+            assert g.np.isclose(mesh.volume, 1.0)
+            f.seek(0)
+            mesh = g.trimesh.load(file_obj=file_name,
+                                  file_type=file_name)
+            assert g.np.isclose(mesh.volume, 1.0)
+
+        file_name = g.get_path('2D/1002_tray_bottom.DXF')
+        with open(file_name, 'rb') as f:
+            # check load_path
+            path = g.trimesh.load_path(file_obj=file_name,
+                                       file_type=file_name)
+            assert len(path.entities) == 46
+
+            f.seek(0)
+            # check `load`
+            path = g.trimesh.load(file_obj=file_name,
+                                  file_type=file_name)
+            assert len(path.entities) == 46
+
 
 if __name__ == '__main__':
     g.trimesh.util.attach_to_log()