Skip to content

Commit ee5be94

Browse files
authored
Handle aiScene ownership with shared_ptr custom deleters (#2285)
1 parent 1988da0 commit ee5be94

File tree

13 files changed

+392
-44
lines changed

13 files changed

+392
-44
lines changed

dart/dynamics/ArrowShape.cpp

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -252,10 +252,9 @@ ShapePtr ArrowShape::clone() const
252252
new_shape->mHead = mHead;
253253
new_shape->mProperties = mProperties;
254254

255-
new_shape->mMesh = new_scene;
256-
new_shape->mMeshUri = mMeshUri;
255+
new_shape->setMesh(
256+
new_scene, MeshOwnership::Copied, mMeshUri, mResourceRetriever);
257257
new_shape->mMeshPath = mMeshPath;
258-
new_shape->mResourceRetriever = mResourceRetriever;
259258
new_shape->mDisplayList = mDisplayList;
260259
new_shape->mScale = mScale;
261260
new_shape->mColorMode = mColorMode;
@@ -387,7 +386,7 @@ void ArrowShape::instantiate(std::size_t resolution)
387386
face->mIndices[2] = 2 * resolution;
388387
}
389388

390-
mMesh = scene;
389+
setMesh(scene, MeshOwnership::Manual, common::Uri(), nullptr);
391390

392391
// setColor(mColor);
393392
// TODO(JS)

dart/dynamics/MeshShape.cpp

Lines changed: 112 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,25 @@ MeshShape::MeshShape(
141141
const Eigen::Vector3d& scale,
142142
const aiScene* mesh,
143143
const common::Uri& path,
144+
common::ResourceRetrieverPtr resourceRetriever,
145+
MeshOwnership ownership)
146+
: Shape(MESH),
147+
mMesh(nullptr),
148+
mMeshOwnership(MeshOwnership::None),
149+
mDisplayList(0),
150+
mColorMode(MATERIAL_COLOR),
151+
mAlphaMode(BLEND),
152+
mColorIndex(0)
153+
{
154+
setMesh(mesh, ownership, path, std::move(resourceRetriever));
155+
setScale(scale);
156+
}
157+
158+
//==============================================================================
159+
MeshShape::MeshShape(
160+
const Eigen::Vector3d& scale,
161+
std::shared_ptr<const aiScene> mesh,
162+
const common::Uri& path,
144163
common::ResourceRetrieverPtr resourceRetriever)
145164
: Shape(MESH),
146165
mMesh(nullptr),
@@ -150,7 +169,7 @@ MeshShape::MeshShape(
150169
mAlphaMode(BLEND),
151170
mColorIndex(0)
152171
{
153-
setMesh(mesh, path, std::move(resourceRetriever));
172+
setMesh(std::move(mesh), path, std::move(resourceRetriever));
154173
setScale(scale);
155174
}
156175

@@ -163,22 +182,7 @@ MeshShape::~MeshShape()
163182
//==============================================================================
164183
void MeshShape::releaseMesh()
165184
{
166-
if (!mMesh)
167-
return;
168-
169-
switch (mMeshOwnership) {
170-
case MeshOwnership::Imported:
171-
aiReleaseImport(const_cast<aiScene*>(mMesh));
172-
break;
173-
case MeshOwnership::Copied:
174-
aiFreeScene(const_cast<aiScene*>(mMesh));
175-
break;
176-
case MeshOwnership::None:
177-
default:
178-
break;
179-
}
180-
181-
mMesh = nullptr;
185+
mMesh.reset();
182186
mMeshOwnership = MeshOwnership::None;
183187
}
184188

@@ -198,7 +202,7 @@ const std::string& MeshShape::getStaticType()
198202
//==============================================================================
199203
const aiScene* MeshShape::getMesh() const
200204
{
201-
return mMesh;
205+
return mMesh.get();
202206
}
203207

204208
//==============================================================================
@@ -237,7 +241,11 @@ void MeshShape::setMesh(
237241
const std::string& path,
238242
common::ResourceRetrieverPtr resourceRetriever)
239243
{
240-
setMesh(mesh, common::Uri(path), std::move(resourceRetriever));
244+
setMesh(
245+
mesh,
246+
MeshOwnership::Imported,
247+
common::Uri(path),
248+
std::move(resourceRetriever));
241249
}
242250

243251
//==============================================================================
@@ -246,15 +254,95 @@ void MeshShape::setMesh(
246254
const common::Uri& uri,
247255
common::ResourceRetrieverPtr resourceRetriever)
248256
{
249-
if (mesh == mMesh) {
257+
setMesh(mesh, MeshOwnership::Imported, uri, std::move(resourceRetriever));
258+
}
259+
260+
//==============================================================================
261+
namespace {
262+
263+
std::shared_ptr<const aiScene> makeMeshHandle(
264+
const aiScene* mesh, MeshShape::MeshOwnership ownership)
265+
{
266+
if (!mesh)
267+
return nullptr;
268+
269+
switch (ownership) {
270+
case MeshShape::MeshOwnership::Imported:
271+
return std::shared_ptr<const aiScene>(mesh, [](const aiScene* scene) { //
272+
aiReleaseImport(const_cast<aiScene*>(scene));
273+
});
274+
case MeshShape::MeshOwnership::Copied:
275+
return std::shared_ptr<const aiScene>(mesh, [](const aiScene* scene) {
276+
aiFreeScene(const_cast<aiScene*>(scene));
277+
});
278+
case MeshShape::MeshOwnership::Manual:
279+
return std::shared_ptr<const aiScene>(mesh, [](const aiScene* scene) {
280+
delete const_cast<aiScene*>(scene);
281+
});
282+
case MeshShape::MeshOwnership::Custom:
283+
case MeshShape::MeshOwnership::None:
284+
default:
285+
return std::shared_ptr<const aiScene>(
286+
mesh, [](const aiScene*) { /* no-op */ });
287+
}
288+
}
289+
290+
} // namespace
291+
292+
//==============================================================================
293+
void MeshShape::setMesh(
294+
const aiScene* mesh,
295+
MeshOwnership ownership,
296+
const common::Uri& uri,
297+
common::ResourceRetrieverPtr resourceRetriever)
298+
{
299+
if (mesh == mMesh.get() && ownership == mMeshOwnership) {
250300
// Nothing to do.
251301
return;
252302
}
253303

254304
releaseMesh();
255305

256-
mMesh = mesh;
257-
mMeshOwnership = mesh ? MeshOwnership::Imported : MeshOwnership::None;
306+
mMesh = makeMeshHandle(mesh, ownership);
307+
mMeshOwnership = mesh ? ownership : MeshOwnership::None;
308+
309+
if (!mMesh) {
310+
mMeshUri.clear();
311+
mMeshPath.clear();
312+
mResourceRetriever = nullptr;
313+
return;
314+
}
315+
316+
mMeshUri = uri;
317+
318+
if (uri.mScheme.get_value_or("file") == "file" && uri.mPath) {
319+
mMeshPath = uri.getFilesystemPath();
320+
} else if (resourceRetriever) {
321+
DART_SUPPRESS_DEPRECATED_BEGIN
322+
mMeshPath = resourceRetriever->getFilePath(uri);
323+
DART_SUPPRESS_DEPRECATED_END
324+
} else {
325+
mMeshPath.clear();
326+
}
327+
328+
mResourceRetriever = std::move(resourceRetriever);
329+
330+
incrementVersion();
331+
}
332+
333+
//==============================================================================
334+
void MeshShape::setMesh(
335+
std::shared_ptr<const aiScene> mesh,
336+
const common::Uri& uri,
337+
common::ResourceRetrieverPtr resourceRetriever)
338+
{
339+
if (mesh == mMesh && mMeshOwnership == MeshOwnership::Custom) {
340+
return;
341+
}
342+
343+
releaseMesh();
344+
mMesh = std::move(mesh);
345+
mMeshOwnership = mMesh ? MeshOwnership::Custom : MeshOwnership::None;
258346

259347
if (!mMesh) {
260348
mMeshUri.clear();
@@ -363,8 +451,7 @@ ShapePtr MeshShape::clone() const
363451
aiScene* new_scene = cloneMesh();
364452

365453
auto new_shape = std::make_shared<MeshShape>(
366-
mScale, new_scene, mMeshUri, mResourceRetriever);
367-
new_shape->mMeshOwnership = MeshOwnership::Copied;
454+
mScale, new_scene, mMeshUri, mResourceRetriever, MeshOwnership::Copied);
368455
new_shape->mMeshPath = mMeshPath;
369456
new_shape->mDisplayList = mDisplayList;
370457
new_shape->mColorMode = mColorMode;
@@ -418,7 +505,7 @@ aiScene* MeshShape::cloneMesh() const
418505
return nullptr;
419506

420507
aiScene* new_scene = nullptr;
421-
aiCopyScene(mMesh, &new_scene);
508+
aiCopyScene(mMesh.get(), &new_scene);
422509
return new_scene;
423510
}
424511

dart/dynamics/MeshShape.hpp

Lines changed: 33 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939

4040
#include <assimp/scene.h>
4141

42+
#include <memory>
4243
#include <string>
4344

4445
namespace dart {
@@ -47,6 +48,15 @@ namespace dynamics {
4748
class DART_API MeshShape : public Shape
4849
{
4950
public:
51+
enum class MeshOwnership
52+
{
53+
None,
54+
Imported, // from aiImportFile* family; free with aiReleaseImport
55+
Copied, // from aiCopyScene; free with aiFreeScene
56+
Manual, // from manual new aiScene; free with delete
57+
Custom // managed externally via shared_ptr deleter
58+
};
59+
5060
enum ColorMode
5161
{
5262
MATERIAL_COLOR = 0, ///< Use the colors specified by the Mesh's material
@@ -82,6 +92,15 @@ class DART_API MeshShape : public Shape
8292
const Eigen::Vector3d& scale,
8393
const aiScene* mesh,
8494
const common::Uri& uri = "",
95+
common::ResourceRetrieverPtr resourceRetriever = nullptr,
96+
MeshOwnership ownership = MeshOwnership::Imported);
97+
98+
/// Constructor that accepts a shared_ptr so callers can supply a custom
99+
/// deleter for aiScene.
100+
MeshShape(
101+
const Eigen::Vector3d& scale,
102+
std::shared_ptr<const aiScene> mesh,
103+
const common::Uri& uri = "",
85104
common::ResourceRetrieverPtr resourceRetriever = nullptr);
86105

87106
/// Destructor.
@@ -106,11 +125,24 @@ class DART_API MeshShape : public Shape
106125
const std::string& path = "",
107126
common::ResourceRetrieverPtr resourceRetriever = nullptr);
108127

128+
/// Sets the mesh pointer with explicit ownership semantics.
129+
void setMesh(
130+
const aiScene* mesh,
131+
MeshOwnership ownership,
132+
const common::Uri& path,
133+
common::ResourceRetrieverPtr resourceRetriever = nullptr);
134+
109135
void setMesh(
110136
const aiScene* mesh,
111137
const common::Uri& path,
112138
common::ResourceRetrieverPtr resourceRetriever = nullptr);
113139

140+
/// Sets the mesh using a shared_ptr so callers can provide a custom deleter.
141+
void setMesh(
142+
std::shared_ptr<const aiScene> mesh,
143+
const common::Uri& path = "",
144+
common::ResourceRetrieverPtr resourceRetriever = nullptr);
145+
114146
/// Returns URI to the mesh as std::string; an empty string if unavailable.
115147
std::string getMeshUri() const;
116148
// TODO(DART 7): Replace with getMeshUri2().
@@ -192,16 +224,9 @@ class DART_API MeshShape : public Shape
192224

193225
aiScene* cloneMesh() const;
194226

195-
enum class MeshOwnership
196-
{
197-
None,
198-
Imported, // from aiImportFile* family; free with aiReleaseImport
199-
Copied // from aiCopyScene; free with aiFreeScene
200-
};
201-
202227
void releaseMesh();
203228

204-
const aiScene* mMesh;
229+
std::shared_ptr<const aiScene> mMesh;
205230
MeshOwnership mMeshOwnership{MeshOwnership::None};
206231

207232
/// URI the mesh, if available).

dart/dynamics/MetaSkeleton.hpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ class DART_API MetaSkeleton : public common::Subject
130130
/// Deprecated BodyNode list getter kept for downstream consumers (e.g.,
131131
/// gz-physics) until they migrate away from it.
132132
DART_DEPRECATED(6.13)
133-
virtual const std::vector<BodyNode*>& getBodyNodes() = 0;
133+
virtual std::vector<BodyNode*>& getBodyNodes() = 0;
134134

135135
/// Deprecated BodyNode list getter kept for downstream consumers (e.g.,
136136
/// gz-physics) until they migrate away from it.

dart/dynamics/ReferentialSkeleton.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ static std::vector<T2>& convertVector(
139139
}
140140

141141
//==============================================================================
142-
const std::vector<BodyNode*>& ReferentialSkeleton::getBodyNodes()
142+
std::vector<BodyNode*>& ReferentialSkeleton::getBodyNodes()
143143
{
144144
return convertVector<BodyNodePtr, BodyNode*>(mBodyNodes, mRawBodyNodes);
145145
}

dart/dynamics/ReferentialSkeleton.hpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ class ReferentialSkeleton : public MetaSkeleton
109109
const BodyNode* getBodyNode(const std::string& name) const override;
110110

111111
// Documentation inherited
112-
const std::vector<BodyNode*>& getBodyNodes() override;
112+
std::vector<BodyNode*>& getBodyNodes() override;
113113

114114
// Documentation inherited
115115
const std::vector<const BodyNode*>& getBodyNodes() const override;

dart/dynamics/Skeleton.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -954,7 +954,7 @@ static std::vector<const T*>& convertToConstPtrVector(
954954
}
955955

956956
//==============================================================================
957-
const std::vector<BodyNode*>& Skeleton::getBodyNodes()
957+
std::vector<BodyNode*>& Skeleton::getBodyNodes()
958958
{
959959
return mSkelCache.mBodyNodes;
960960
}

dart/dynamics/Skeleton.hpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -359,7 +359,7 @@ class DART_API Skeleton
359359
/// Deprecated list getter retained for backward compatibility until
360360
/// gz-physics migrates.
361361
DART_DEPRECATED(6.13)
362-
const std::vector<BodyNode*>& getBodyNodes() override;
362+
std::vector<BodyNode*>& getBodyNodes() override;
363363

364364
/// Deprecated list getter retained for backward compatibility until
365365
/// gz-physics migrates.

dart/gui/InteractiveFrame.cpp

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -445,7 +445,12 @@ void InteractiveFrame::createStandardVisualizationShapes(
445445
scene->mRootNode = node;
446446

447447
std::shared_ptr<dart::dynamics::MeshShape> shape(
448-
new dart::dynamics::MeshShape(Eigen::Vector3d::Ones(), scene));
448+
new dart::dynamics::MeshShape(
449+
Eigen::Vector3d::Ones(),
450+
scene,
451+
common::Uri(),
452+
nullptr,
453+
dart::dynamics::MeshShape::MeshOwnership::Manual));
449454
shape->setColorMode(dart::dynamics::MeshShape::COLOR_INDEX);
450455

451456
Eigen::Isometry3d tf(Eigen::Isometry3d::Identity());
@@ -529,7 +534,12 @@ void InteractiveFrame::createStandardVisualizationShapes(
529534
scene->mRootNode = node;
530535

531536
std::shared_ptr<dart::dynamics::MeshShape> shape(
532-
new dart::dynamics::MeshShape(Eigen::Vector3d::Ones(), scene));
537+
new dart::dynamics::MeshShape(
538+
Eigen::Vector3d::Ones(),
539+
scene,
540+
common::Uri(),
541+
nullptr,
542+
dart::dynamics::MeshShape::MeshOwnership::Manual));
533543
shape->setColorMode(dart::dynamics::MeshShape::COLOR_INDEX);
534544

535545
Eigen::Isometry3d tf(Eigen::Isometry3d::Identity());

0 commit comments

Comments
 (0)