Handle aiScene ownership with shared_ptr custom deleters

dart/dynamics/ArrowShape.cpp

@@ -252,10 +252,9 @@ ShapePtr ArrowShape::clone() const
   new_shape->mHead = mHead;
   new_shape->mProperties = mProperties;
 
-  new_shape->mMesh = new_scene;
-  new_shape->mMeshUri = mMeshUri;
+  new_shape->setMesh(
+      new_scene, MeshOwnership::Copied, mMeshUri, mResourceRetriever);
   new_shape->mMeshPath = mMeshPath;
-  new_shape->mResourceRetriever = mResourceRetriever;
   new_shape->mDisplayList = mDisplayList;
   new_shape->mScale = mScale;
   new_shape->mColorMode = mColorMode;
@@ -387,7 +386,7 @@ void ArrowShape::instantiate(std::size_t resolution)
     face->mIndices[2] = 2 * resolution;
   }
 
-  mMesh = scene;
+  setMesh(scene, MeshOwnership::Manual, common::Uri(), nullptr);
 
   // setColor(mColor);
   // TODO(JS)

dart/dynamics/MeshShape.cpp

@@ -141,6 +141,25 @@ MeshShape::MeshShape(
     const Eigen::Vector3d& scale,
     const aiScene* mesh,
     const common::Uri& path,
+    common::ResourceRetrieverPtr resourceRetriever,
+    MeshOwnership ownership)
+  : Shape(MESH),
+    mMesh(nullptr),
+    mMeshOwnership(MeshOwnership::None),
+    mDisplayList(0),
+    mColorMode(MATERIAL_COLOR),
+    mAlphaMode(BLEND),
+    mColorIndex(0)
+{
+  setMesh(mesh, ownership, path, std::move(resourceRetriever));
+  setScale(scale);
+}
+
+//==============================================================================
+MeshShape::MeshShape(
+    const Eigen::Vector3d& scale,
+    std::shared_ptr<const aiScene> mesh,
+    const common::Uri& path,
     common::ResourceRetrieverPtr resourceRetriever)
   : Shape(MESH),
     mMesh(nullptr),
@@ -150,7 +169,7 @@ MeshShape::MeshShape(
     mAlphaMode(BLEND),
     mColorIndex(0)
 {
-  setMesh(mesh, path, std::move(resourceRetriever));
+  setMesh(std::move(mesh), path, std::move(resourceRetriever));
   setScale(scale);
 }
 
@@ -163,22 +182,7 @@ MeshShape::~MeshShape()
 //==============================================================================
 void MeshShape::releaseMesh()
 {
-  if (!mMesh)
-    return;
-
-  switch (mMeshOwnership) {
-    case MeshOwnership::Imported:
-      aiReleaseImport(const_cast<aiScene*>(mMesh));
-      break;
-    case MeshOwnership::Copied:
-      aiFreeScene(const_cast<aiScene*>(mMesh));
-      break;
-    case MeshOwnership::None:
-    default:
-      break;
-  }
-
-  mMesh = nullptr;
+  mMesh.reset();
   mMeshOwnership = MeshOwnership::None;
 }
 
@@ -198,7 +202,7 @@ const std::string& MeshShape::getStaticType()
 //==============================================================================
 const aiScene* MeshShape::getMesh() const
 {
-  return mMesh;
+  return mMesh.get();
 }
 
 //==============================================================================
@@ -237,7 +241,11 @@ void MeshShape::setMesh(
     const std::string& path,
     common::ResourceRetrieverPtr resourceRetriever)
 {
-  setMesh(mesh, common::Uri(path), std::move(resourceRetriever));
+  setMesh(
+      mesh,
+      MeshOwnership::Imported,
+      common::Uri(path),
+      std::move(resourceRetriever));
 }
 
 //==============================================================================
@@ -246,15 +254,95 @@ void MeshShape::setMesh(
     const common::Uri& uri,
     common::ResourceRetrieverPtr resourceRetriever)
 {
-  if (mesh == mMesh) {
+  setMesh(mesh, MeshOwnership::Imported, uri, std::move(resourceRetriever));
+}
+
+//==============================================================================
+namespace {
+
+std::shared_ptr<const aiScene> makeMeshHandle(
+    const aiScene* mesh, MeshShape::MeshOwnership ownership)
+{
+  if (!mesh)
+    return nullptr;
+
+  switch (ownership) {
+    case MeshShape::MeshOwnership::Imported:
+      return std::shared_ptr<const aiScene>(mesh, [](const aiScene* scene) { //
+        aiReleaseImport(const_cast<aiScene*>(scene));
+      });
+    case MeshShape::MeshOwnership::Copied:
+      return std::shared_ptr<const aiScene>(mesh, [](const aiScene* scene) {
+        aiFreeScene(const_cast<aiScene*>(scene));
+      });
+    case MeshShape::MeshOwnership::Manual:
+      return std::shared_ptr<const aiScene>(mesh, [](const aiScene* scene) {
+        delete const_cast<aiScene*>(scene);
+      });
+    case MeshShape::MeshOwnership::Custom:
+    case MeshShape::MeshOwnership::None:
+    default:
+      return std::shared_ptr<const aiScene>(
+          mesh, [](const aiScene*) { /* no-op */ });
+  }
+}
+
+} // namespace
+
+//==============================================================================
+void MeshShape::setMesh(
+    const aiScene* mesh,
+    MeshOwnership ownership,
+    const common::Uri& uri,
+    common::ResourceRetrieverPtr resourceRetriever)
+{
+  if (mesh == mMesh.get() && ownership == mMeshOwnership) {
     // Nothing to do.
     return;
   }
 
   releaseMesh();
 
-  mMesh = mesh;
-  mMeshOwnership = mesh ? MeshOwnership::Imported : MeshOwnership::None;
+  mMesh = makeMeshHandle(mesh, ownership);
+  mMeshOwnership = mesh ? ownership : MeshOwnership::None;
+
+  if (!mMesh) {
+    mMeshUri.clear();
+    mMeshPath.clear();
+    mResourceRetriever = nullptr;
+    return;
+  }
+
+  mMeshUri = uri;
+
+  if (uri.mScheme.get_value_or("file") == "file" && uri.mPath) {
+    mMeshPath = uri.getFilesystemPath();
+  } else if (resourceRetriever) {
+    DART_SUPPRESS_DEPRECATED_BEGIN
+    mMeshPath = resourceRetriever->getFilePath(uri);
+    DART_SUPPRESS_DEPRECATED_END
+  } else {
+    mMeshPath.clear();
+  }
+
+  mResourceRetriever = std::move(resourceRetriever);
+
+  incrementVersion();
+}
+
+//==============================================================================
+void MeshShape::setMesh(
+    std::shared_ptr<const aiScene> mesh,
+    const common::Uri& uri,
+    common::ResourceRetrieverPtr resourceRetriever)
+{
+  if (mesh == mMesh && mMeshOwnership == MeshOwnership::Custom) {
+    return;
+  }
+
+  releaseMesh();
+  mMesh = std::move(mesh);
+  mMeshOwnership = mMesh ? MeshOwnership::Custom : MeshOwnership::None;
 
   if (!mMesh) {
     mMeshUri.clear();
@@ -363,8 +451,7 @@ ShapePtr MeshShape::clone() const
   aiScene* new_scene = cloneMesh();
 
   auto new_shape = std::make_shared<MeshShape>(
-      mScale, new_scene, mMeshUri, mResourceRetriever);
-  new_shape->mMeshOwnership = MeshOwnership::Copied;
+      mScale, new_scene, mMeshUri, mResourceRetriever, MeshOwnership::Copied);
   new_shape->mMeshPath = mMeshPath;
   new_shape->mDisplayList = mDisplayList;
   new_shape->mColorMode = mColorMode;
@@ -418,7 +505,7 @@ aiScene* MeshShape::cloneMesh() const
     return nullptr;
 
   aiScene* new_scene = nullptr;
-  aiCopyScene(mMesh, &new_scene);
+  aiCopyScene(mMesh.get(), &new_scene);
   return new_scene;
 }
 

dart/dynamics/MeshShape.hpp

@@ -39,6 +39,7 @@
 
 #include <assimp/scene.h>
 
+#include <memory>
 #include <string>
 
 namespace dart {
@@ -47,6 +48,15 @@ namespace dynamics {
 class DART_API MeshShape : public Shape
 {
 public:
+  enum class MeshOwnership
+  {
+    None,
+    Imported, // from aiImportFile* family; free with aiReleaseImport
+    Copied,   // from aiCopyScene; free with aiFreeScene
+    Manual,   // from manual new aiScene; free with delete
+    Custom    // managed externally via shared_ptr deleter
+  };
+
   enum ColorMode
   {
     MATERIAL_COLOR = 0, ///< Use the colors specified by the Mesh's material
@@ -82,6 +92,15 @@ class DART_API MeshShape : public Shape
       const Eigen::Vector3d& scale,
       const aiScene* mesh,
       const common::Uri& uri = "",
+      common::ResourceRetrieverPtr resourceRetriever = nullptr,
+      MeshOwnership ownership = MeshOwnership::Imported);
+
+  /// Constructor that accepts a shared_ptr so callers can supply a custom
+  /// deleter for aiScene.
+  MeshShape(
+      const Eigen::Vector3d& scale,
+      std::shared_ptr<const aiScene> mesh,
+      const common::Uri& uri = "",
       common::ResourceRetrieverPtr resourceRetriever = nullptr);
 
   /// Destructor.
@@ -106,11 +125,24 @@ class DART_API MeshShape : public Shape
       const std::string& path = "",
       common::ResourceRetrieverPtr resourceRetriever = nullptr);
 
+  /// Sets the mesh pointer with explicit ownership semantics.
+  void setMesh(
+      const aiScene* mesh,
+      MeshOwnership ownership,
+      const common::Uri& path,
+      common::ResourceRetrieverPtr resourceRetriever = nullptr);
+
   void setMesh(
       const aiScene* mesh,
       const common::Uri& path,
       common::ResourceRetrieverPtr resourceRetriever = nullptr);
 
+  /// Sets the mesh using a shared_ptr so callers can provide a custom deleter.
+  void setMesh(
+      std::shared_ptr<const aiScene> mesh,
+      const common::Uri& path = "",
+      common::ResourceRetrieverPtr resourceRetriever = nullptr);
+
   /// Returns URI to the mesh as std::string; an empty string if unavailable.
   std::string getMeshUri() const;
   // TODO(DART 7): Replace with getMeshUri2().
@@ -192,16 +224,9 @@ class DART_API MeshShape : public Shape
 
   aiScene* cloneMesh() const;
 
-  enum class MeshOwnership
-  {
-    None,
-    Imported, // from aiImportFile* family; free with aiReleaseImport
-    Copied    // from aiCopyScene; free with aiFreeScene
-  };
-
   void releaseMesh();
 
-  const aiScene* mMesh;
+  std::shared_ptr<const aiScene> mMesh;
   MeshOwnership mMeshOwnership{MeshOwnership::None};
 
   /// URI the mesh, if available).

dart/dynamics/MetaSkeleton.hpp

@@ -130,7 +130,7 @@ class DART_API MetaSkeleton : public common::Subject
   /// Deprecated BodyNode list getter kept for downstream consumers (e.g.,
   /// gz-physics) until they migrate away from it.
   DART_DEPRECATED(6.13)
-  virtual const std::vector<BodyNode*>& getBodyNodes() = 0;
+  virtual std::vector<BodyNode*>& getBodyNodes() = 0;
 
   /// Deprecated BodyNode list getter kept for downstream consumers (e.g.,
   /// gz-physics) until they migrate away from it.

dart/dynamics/ReferentialSkeleton.cpp

@@ -139,7 +139,7 @@ static std::vector<T2>& convertVector(
 }
 
 //==============================================================================
-const std::vector<BodyNode*>& ReferentialSkeleton::getBodyNodes()
+std::vector<BodyNode*>& ReferentialSkeleton::getBodyNodes()
 {
   return convertVector<BodyNodePtr, BodyNode*>(mBodyNodes, mRawBodyNodes);
 }

dart/dynamics/ReferentialSkeleton.hpp

@@ -109,7 +109,7 @@ class ReferentialSkeleton : public MetaSkeleton
   const BodyNode* getBodyNode(const std::string& name) const override;
 
   // Documentation inherited
-  const std::vector<BodyNode*>& getBodyNodes() override;
+  std::vector<BodyNode*>& getBodyNodes() override;
 
   // Documentation inherited
   const std::vector<const BodyNode*>& getBodyNodes() const override;

dart/dynamics/Skeleton.cpp

@@ -954,7 +954,7 @@ static std::vector<const T*>& convertToConstPtrVector(
 }
 
 //==============================================================================
-const std::vector<BodyNode*>& Skeleton::getBodyNodes()
+std::vector<BodyNode*>& Skeleton::getBodyNodes()
 {
   return mSkelCache.mBodyNodes;
 }

dart/dynamics/Skeleton.hpp

@@ -359,7 +359,7 @@ class DART_API Skeleton
   /// Deprecated list getter retained for backward compatibility until
   /// gz-physics migrates.
   DART_DEPRECATED(6.13)
-  const std::vector<BodyNode*>& getBodyNodes() override;
+  std::vector<BodyNode*>& getBodyNodes() override;
 
   /// Deprecated list getter retained for backward compatibility until
   /// gz-physics migrates.

dart/gui/InteractiveFrame.cpp

@@ -445,7 +445,12 @@ void InteractiveFrame::createStandardVisualizationShapes(
     scene->mRootNode = node;
 
     std::shared_ptr<dart::dynamics::MeshShape> shape(
-        new dart::dynamics::MeshShape(Eigen::Vector3d::Ones(), scene));
+        new dart::dynamics::MeshShape(
+            Eigen::Vector3d::Ones(),
+            scene,
+            common::Uri(),
+            nullptr,
+            dart::dynamics::MeshShape::MeshOwnership::Manual));
     shape->setColorMode(dart::dynamics::MeshShape::COLOR_INDEX);
 
     Eigen::Isometry3d tf(Eigen::Isometry3d::Identity());
@@ -529,7 +534,12 @@ void InteractiveFrame::createStandardVisualizationShapes(
     scene->mRootNode = node;
 
     std::shared_ptr<dart::dynamics::MeshShape> shape(
-        new dart::dynamics::MeshShape(Eigen::Vector3d::Ones(), scene));
+        new dart::dynamics::MeshShape(
+            Eigen::Vector3d::Ones(),
+            scene,
+            common::Uri(),
+            nullptr,
+            dart::dynamics::MeshShape::MeshOwnership::Manual));
     shape->setColorMode(dart::dynamics::MeshShape::COLOR_INDEX);
 
     Eigen::Isometry3d tf(Eigen::Isometry3d::Identity());