Repository: mikedh/trimesh

Issue:
triangles_tree gets messed up using deepcopy
Hi mikedh, great job on trimesh! 

Here is my issue:

I have a class which contains a trimesh.Trimesh object. When I use deepcopy on my class, the Trimesh in the copy has its triangles_tree with crazy bounds, on the order of 10^300 or something. I have tried deleting 'triangles_tree' from the new Trimesh's cache before using it (the error was happening when I was using ray intersection) and this seemed to work. This seems a little hacky though, and I was wondering if you had ever encountered this before and if you had a better way of handling this situation.

Thanks!

Discussion:

Patch:
diff --git a/.travis.yml b/.travis.yml
index d6ba0549..9e8f3ff1 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -131,4 +131,5 @@ script:
 - pytest -p no:warnings tests/test_graph.py tests/test_scene.py
 
 after_success:
-- coveralls || true
+# only upload test coverage results on python 3.6
+- if [[ "$PYTHON_VERSION" == "3.6" ]]; then coveralls || true; fi;
diff --git a/trimesh/base.py b/trimesh/base.py
index 9478c678..3caa0099 100644
--- a/trimesh/base.py
+++ b/trimesh/base.py
@@ -849,6 +849,8 @@ class Trimesh(Geometry):
         """
         edges, index = geometry.faces_to_edges(self.faces.view(np.ndarray),
                                                return_index=True)
+        edges.flags['WRITEABLE'] = False
+        index.flags['WRITEABLE'] = False
         self._cache['edges_face'] = index
         return edges
 
@@ -2738,24 +2740,29 @@ class Trimesh(Geometry):
         tree = util.bounds_tree(segment_bounds)
         return tree
 
-    def copy(self):
+    def copy(self, include_cache=False):
         """
         Safely return a copy of the current mesh.
 
-        Copied objects will have emptied caches to avoid memory
-        issues and so may be slow on initial operations until
-        caches are regenerated.
+        By default, copied meshes will have emptied cache
+        to avoid memory issues and so may be slow on initial
+        operations until caches are regenerated.
 
-        Current object will *not* have its cache cleared.
+        Current object will *never* have its cache cleared.
+
+        Parameters
+        ------------
+        include_cache : bool
+          If True, will shallow copy cached data to new mesh
 
         Returns
         ---------
         copied : trimesh.Trimesh
           Copy of current mesh
         """
+        # start with an empty mesh
         copied = Trimesh()
-
-        # copy vertex and face data
+        # always deepcopy vertex and face data
         copied._data.data = copy.deepcopy(self._data.data)
         # copy visual information
         copied.visual = self.visual.copy()
@@ -2766,11 +2773,27 @@ class Trimesh(Geometry):
             copied.center_mass = self.center_mass
         copied._density = self._density
 
-        # make sure cache is set from here
-        copied._cache.clear()
+        # make sure cache ID is set initially
+        copied._cache.verify()
+
+        if include_cache:
+            # shallow copy cached items into the new cache
+            # since the data didn't change here when the
+            # data in the new mesh is changed these items
+            # will be dumped in the new mesh but preserved
+            # in the original mesh
+            copied._cache.cache.update(self._cache.cache)
 
         return copied
 
+    def __deepcopy__(self, *args):
+        # interpret deep copy as "get rid of cached data"
+        return self.copy(include_cache=False)
+
+    def __copy__(self, *args):
+        # interpret shallow copy as "keep cached data"
+        return self.copy(include_cache=True)
+
     def eval_cached(self, statement, *args):
         """
         Evaluate a statement and cache the result before returning.
diff --git a/trimesh/path/segments.py b/trimesh/path/segments.py
index 36baca49..7fa4dc35 100644
--- a/trimesh/path/segments.py
+++ b/trimesh/path/segments.py
@@ -392,7 +392,7 @@ def length(segments, summed=True):
     return norms
 
 
-def resample(segments, maxlen):
+def resample(segments, maxlen, return_index=False):
     """
     Resample line segments until no segment
     is longer than maxlen.
@@ -403,12 +403,17 @@ def resample(segments, maxlen):
       2D line segments
     maxlen : float
       The maximum length of a line segment
+    return_index : bool
+      Return the index of the source segment
 
     Returns
     -------------
     resampled : (m, 2, 3) float
       Line segments where no segment is longer than maxlen
+    index : (m,) int
+      If return_index, the index of segments resampled came from
     """
+    # check arguments
     maxlen = float(maxlen)
     segments = np.asanyarray(segments, dtype=np.float64)
 
@@ -422,6 +427,8 @@ def resample(segments, maxlen):
 
     # save resulting segments
     result = []
+    # save index of original segment
+    index = []
 
     # loop through each count of unique splits needed
     for split in np.unique(splits):
@@ -441,7 +448,17 @@ def resample(segments, maxlen):
         # get indexes to stack polyline into segments
         stack = util.stack_lines(np.arange(split + 1))
         # save the resulting segments
-        result.extend([p[stack] for p in poly])
+        # magical slicing is equivalent to:
+        # > [p[stack] for p in poly]
+        result.extend(poly[:, stack])
+
+        if return_index:
+            # get the original index from the mask
+            index_original = np.nonzero(mask)[0].reshape((-1, 1))
+            # save one entry per split segment
+            index.append((np.ones((len(poly), split),
+                                  dtype=np.int64) *
+                          index_original).ravel())
 
         if tol.strict:
             # check to make sure every start and end point
@@ -459,4 +476,14 @@ def resample(segments, maxlen):
                           length(result),
                           atol=1e-3)
 
+    if return_index:
+        # stack original indexes
+        index = np.concatenate(index)
+        if tol.strict:
+            # index should correspond to result
+            assert len(index) == len(result)
+            # every segment should be represented
+            assert set(index) == set(range(len(segments)))
+        return result, index
+
     return result
diff --git a/trimesh/primitives.py b/trimesh/primitives.py
index 90b8ecc0..51a220c9 100644
--- a/trimesh/primitives.py
+++ b/trimesh/primitives.py
@@ -29,6 +29,10 @@ class _Primitive(Trimesh):
     Mesh is generated lazily when vertices or faces are requested.
     """
 
+    # ignore superclass copy directives
+    __copy__ = None
+    __deepcopy__ = None
+
     def __init__(self, *args, **kwargs):
         super(_Primitive, self).__init__(*args, **kwargs)
         self._data.clear()
@@ -83,13 +87,16 @@ class _Primitive(Trimesh):
         if values is not None:
             log.warning('Primitive face normals are immutable! Not setting!')
 
-    def copy(self):
+    def copy(self, **kwargs):
         """
         Return a copy of the Primitive object.
+
+        Returns
+        -------------
+        copied : object
+          Copy of current primitive
         """
-        result = copy.deepcopy(self)
-        result._cache.clear()
-        return result
+        return copy.deepcopy(self)
 
     def to_mesh(self):
         """
@@ -190,6 +197,19 @@ class _PrimitiveAttributes(object):
             raise ValueError(
                 'Only default attributes {} can be set!'.format(keys))
 
+    def to_kwargs(self):
+        """
+        Return a dict with copies of kwargs for the current
+        Primitive.
+
+        Returns
+        ------------
+        kwargs : dict
+          Arguments to reconstruct current PrimitiveAttributes
+        """
+        return {k: copy.deepcopy(self._data[k])
+                for k in self._defaults.keys()}
+
     def __dir__(self):
         result = sorted(dir(type(self)) +
                         list(self._defaults.keys()))
diff --git a/trimesh/version.py b/trimesh/version.py
index bf5afe7f..0bc4055f 100644
--- a/trimesh/version.py
+++ b/trimesh/version.py
@@ -1,1 +1,1 @@
-__version__ = '3.5.1'
+__version__ = '3.5.2'
diff --git a/trimesh/voxel/ops.py b/trimesh/voxel/ops.py
index aafd2587..62e1c4a4 100644
--- a/trimesh/voxel/ops.py
+++ b/trimesh/voxel/ops.py
@@ -152,7 +152,7 @@ def matrix_to_marching_cubes(matrix, pitch=1.0):
         vertices, faces, normals, vals = meshed
 
     # Return to the origin, add in the pad_width
-    vertices = np.subtract(vertices, pad_width)
+    vertices = np.subtract(vertices, pad_width * pitch)
     # create the mesh
     mesh = Trimesh(vertices=vertices,
                    faces=faces,


Test patch:
diff --git a/tests/generic.py b/tests/generic.py
index a5afc82d..ee7f9c72 100644
--- a/tests/generic.py
+++ b/tests/generic.py
@@ -7,6 +7,7 @@ from distutils.spawn import find_executable
 import os
 import sys
 import json
+import copy
 import time
 import shutil
 import timeit
diff --git a/tests/test_copy.py b/tests/test_copy.py
new file mode 100644
index 00000000..91ec8269
--- /dev/null
+++ b/tests/test_copy.py
@@ -0,0 +1,59 @@
+"""
+Copy meshes and make sure they do what we expect.
+"""
+try:
+    from . import generic as g
+except BaseException:
+    import generic as g
+
+
+class CopyTests(g.unittest.TestCase):
+
+    def test_copy(self):
+        for mesh in g.get_meshes(raise_error=True):
+            if not isinstance(mesh, g.trimesh.Trimesh):
+                continue
+            start = {mesh.md5(), mesh.crc()}
+
+            # make sure some stuff is populated
+            mesh.kdtree
+            mesh.triangles_tree
+            mesh.face_adjacency_angles
+            mesh.facets
+            assert 'triangles_tree' in mesh._cache
+            assert len(mesh._cache) > 0
+
+            # if you cache c-objects then deepcopy the mesh
+            # it randomly segfaults
+            copy_count = 200
+            for i in range(copy_count):
+                copied = mesh.copy(include_cache=False)
+                assert len(copied._cache) == 0
+                assert len(mesh._cache) > 0
+
+                # deepcopy should clear the cache
+                copied = g.copy.deepcopy(mesh)
+                assert len(copied._cache) == 0
+                assert len(mesh._cache) > 0
+
+                # regular copy should try to preserve the cache
+                copied = g.copy.copy(mesh)
+                assert len(copied._cache) == len(mesh._cache)
+                # the triangles_tree should be the SAME OBJECT
+                assert id(copied.triangles_tree) == id(
+                    mesh.triangles_tree)
+
+                # cache should be same data in different object
+                assert id(copied._cache.cache) != id(mesh._cache.cache)
+                assert id(copied._cache) != id(mesh._cache)
+                # identifier shouldn't change
+                assert g.np.allclose(copied.identifier,
+                                     mesh.identifier)
+
+            # ...still shouldn't have changed anything
+            assert start == {mesh.md5(), mesh.crc()}
+
+
+if __name__ == '__main__':
+    g.trimesh.util.attach_to_log()
+    g.unittest.main()
diff --git a/tests/test_mesh.py b/tests/test_mesh.py
index 824e7553..6039923c 100644
--- a/tests/test_mesh.py
+++ b/tests/test_mesh.py
@@ -107,14 +107,23 @@ class MeshTests(g.unittest.TestCase):
             assert len(mesh.vertices) == len(mesh.vertex_defects)
             assert len(mesh.principal_inertia_components) == 3
 
+            # collect list of cached properties that are writeable
+            writeable = []
             # we should have built up a bunch of stuff into
-            # our cache, so make sure all numpy arrays cached are
-            # finite
+            # our cache, so make sure all numpy arrays cached
+            # are read-only and not crazy
             for name, cached in mesh._cache.cache.items():
                 # only check numpy arrays
                 if not isinstance(cached, g.np.ndarray):
                     continue
 
+                # nothing in the cache should be writeable
+                if cached.flags['WRITEABLE']:
+                    # TODO
+                    # eventually make all cached arrays read-only
+                    # then raise a ValueError here
+                    writeable.append(name)
+
                 # only check int, float, and bool
                 if cached.dtype.kind not in 'ibf':
                     continue
@@ -133,29 +142,16 @@ class MeshTests(g.unittest.TestCase):
                     raise ValueError('inf values in %s/%s',
                                      file_name, name)
 
-            # some memory issues only show up when you copy the mesh a bunch
-            # specifically, if you cache c- objects then deepcopy the mesh this
-            # generally segfaults randomly
-            copy_count = 200
-            g.log.info('Attempting to copy mesh %d times', copy_count)
-            for i in range(copy_count):
-                copied = mesh.copy()
-                assert copied.is_empty == mesh.is_empty
-                # t = copied.triangles_tree
-                c = copied.kdtree  # NOQA
-                copied.apply_transform(
-                    g.trimesh.transformations.rotation_matrix(
-                        g.np.degrees(i),
-                        [0, 1, 1]))
-            g.log.info('Multiple copies done')
-
-            if not g.np.allclose(copied.identifier,
-                                 mesh.identifier):
-                raise ValueError('copied identifier changed!')
-
             # ...still shouldn't have changed anything
             assert start == {mesh.md5(), mesh.crc()}
 
+            # log the names of properties we need to make read-only
+            if len(writeable) > 0:
+                # TODO : all cached values should be read-only
+                g.log.error(
+                    'cached properties writeable: {}'.format(
+                        ', '.join(writeable)))
+
 
 if __name__ == '__main__':
     g.trimesh.util.attach_to_log()
diff --git a/tests/test_segments.py b/tests/test_segments.py
index a7d081fa..a2520d5c 100644
--- a/tests/test_segments.py
+++ b/tests/test_segments.py
@@ -75,6 +75,13 @@ class SegmentsTest(g.unittest.TestCase):
         # make sure overall length hasn't changed
         assert g.np.isclose(length(res), length(seg))
 
+        # now try with indexes returned
+        res, index = resample(seg, maxlen=maxlen, return_index=True)
+        # check lengths of the resampled result
+        assert (length(res, summed=False) < maxlen).all()
+        # make sure overall length hasn't changed
+        assert g.np.isclose(length(res), length(seg))
+
 
 if __name__ == '__main__':
     g.trimesh.util.attach_to_log()