diff --git a/src/python/magnum/meshtools.cpp b/src/python/magnum/meshtools.cpp index 7b8faa6..72bc7ff 100644 --- a/src/python/magnum/meshtools.cpp +++ b/src/python/magnum/meshtools.cpp @@ -40,6 +40,9 @@ #include #include +#include "Corrade/PythonBindings.h" +#include "Magnum/Trade/PythonBindings.h" + #include "corrade/EnumOperators.h" #include "magnum/bootstrap.h" @@ -129,10 +132,16 @@ void meshtools(py::module_& m) { return MeshTools::duplicate(mesh); }, "Duplicate indexed mesh data", py::arg("mesh")) .def("filter_except_attributes", [](const Trade::MeshData& mesh, const std::vector attributes) { - return MeshTools::filterExceptAttributes(mesh, attributes); + /* If the mesh already has an owner, use that instead to avoid + long reference chains */ + py::object meshOwner = pyObjectHolderFor(mesh).owner; + return Trade::pyDataHolder(MeshTools::filterExceptAttributes(mesh, attributes), meshOwner.is_none() ? py::cast(mesh) : std::move(meshOwner)); }, "Filter a mesh to contain everything except the selected subset of named attributes", py::arg("mesh"), py::arg("attributes")) .def("filter_only_attributes", [](const Trade::MeshData& mesh, const std::vector attributes) { - return MeshTools::filterOnlyAttributes(mesh, attributes); + /* If the mesh already has an owner, use that instead to avoid + long reference chains */ + py::object meshOwner = pyObjectHolderFor(mesh).owner; + return Trade::pyDataHolder(MeshTools::filterOnlyAttributes(mesh, attributes), meshOwner.is_none() ? py::cast(mesh) : std::move(meshOwner)); }, "Filter a mesh to contain only the selected subset of named attributes", py::arg("mesh"), py::arg("attributes")) .def("generate_indices", [](const Trade::MeshData& mesh) { if(mesh.primitive() != MeshPrimitive::LineStrip && diff --git a/src/python/magnum/test/test_meshtools.py b/src/python/magnum/test/test_meshtools.py index 8f20271..50d52d3 100644 --- a/src/python/magnum/test/test_meshtools.py +++ b/src/python/magnum/test/test_meshtools.py @@ -24,6 +24,7 @@ # import os +import sys import unittest from magnum import * @@ -106,23 +107,61 @@ class GenerateIndices(unittest.TestCase): class FilterAttributes(unittest.TestCase): def test_only(self): mesh = primitives.cube_solid() + mesh_refcount = sys.getrefcount(mesh) self.assertEqual(mesh.attribute_count(), 2) self.assertTrue(mesh.has_attribute(trade.MeshAttribute.NORMAL)) # Currently it doesn't blow up if unknown attributes are listed filtered = meshtools.filter_only_attributes(mesh, [trade.MeshAttribute.TEXTURE_COORDINATES, trade.MeshAttribute.NORMAL]) + filtered_refcount = sys.getrefcount(filtered) self.assertEqual(filtered.attribute_count(), 1) self.assertTrue(filtered.has_attribute(trade.MeshAttribute.NORMAL)) + self.assertEqual(sys.getrefcount(mesh), mesh_refcount + 1) + self.assertIs(filtered.owner, mesh) + + # Subsequent filtering will still reference the original mesh, not the + # intermediates + filtered2 = meshtools.filter_only_attributes(filtered, [trade.MeshAttribute.NORMAL]) + self.assertEqual(filtered.attribute_count(), 1) + self.assertTrue(filtered.has_attribute(trade.MeshAttribute.NORMAL)) + self.assertEqual(sys.getrefcount(filtered), filtered_refcount) + self.assertEqual(sys.getrefcount(mesh), mesh_refcount + 2) + self.assertIs(filtered2.owner, mesh) + + del filtered + self.assertEqual(sys.getrefcount(mesh), mesh_refcount + 1) + + del filtered2 + self.assertEqual(sys.getrefcount(mesh), mesh_refcount) def test_except(self): mesh = primitives.cube_solid() + mesh_refcount = sys.getrefcount(mesh) self.assertEqual(mesh.attribute_count(), 2) self.assertTrue(mesh.has_attribute(trade.MeshAttribute.NORMAL)) # Currently it doesn't blow up if unknown attributes are listed filtered = meshtools.filter_except_attributes(mesh, [trade.MeshAttribute.TEXTURE_COORDINATES, trade.MeshAttribute.NORMAL]) + filtered_refcount = sys.getrefcount(filtered) self.assertEqual(filtered.attribute_count(), 1) - self.assertFalse(filtered.has_attribute(trade.MeshAttribute.NORMAL)) + self.assertTrue(filtered.has_attribute(trade.MeshAttribute.POSITION)) + self.assertEqual(sys.getrefcount(mesh), mesh_refcount + 1) + self.assertIs(filtered.owner, mesh) + + # Subsequent filtering will still reference the original mesh, not the + # intermediates + filtered2 = meshtools.filter_except_attributes(filtered, [trade.MeshAttribute.NORMAL]) + self.assertEqual(filtered.attribute_count(), 1) + self.assertTrue(filtered.has_attribute(trade.MeshAttribute.POSITION)) + self.assertEqual(sys.getrefcount(filtered), filtered_refcount) + self.assertEqual(sys.getrefcount(mesh), mesh_refcount + 2) + self.assertIs(filtered2.owner, mesh) + + del filtered + self.assertEqual(sys.getrefcount(mesh), mesh_refcount + 1) + + del filtered2 + self.assertEqual(sys.getrefcount(mesh), mesh_refcount) class Interleave(unittest.TestCase): def test(self):