Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Backport #188: Fix crash when using BVH animations #199

Merged
merged 1 commit into from
Apr 28, 2021
Merged

Backport #188: Fix crash when using BVH animations #199

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions graphics/include/ignition/common/SkeletonAnimation.hh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,15 @@ namespace ignition
/// the animations
public: ~SkeletonAnimation();

// NOTE: this is not needed starting in ign-common4 since ign-common4 uses
// the IGN_UTILS_IMPL_PTR instead of a raw impl pointer. So, this
// method should not be included in forward ports from ign-common3 to
// ign-common4
/// \brief Assignment operator
/// \param[in] _other The new SkeletonAnimation
/// \return A reference to this instance
public: SkeletonAnimation &operator=(const SkeletonAnimation &_other);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this function should not be merged forward to ign-common4 right? Can you add a note as a reminder?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added: af8b84a

Let me know if there's a better way/format of adding a note like this. I don't think it should be a TODO since nothing really needs to be "done", and I also assume it shouldn't be a doxygen comment since this isn't relevant for user documentation (it's more of just an internal developer note).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good to me.


/// \brief Changes the name
/// \param[in] _name the new name
public: void SetName(const std::string& _name);
Expand Down
16 changes: 11 additions & 5 deletions graphics/src/Skeleton.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ class ignition::common::SkeletonPrivate
RawNodeWeights;

/// \brief the root node
public: SkeletonNode *root;
public: SkeletonNode *root{nullptr};

/// \brief The dictionary of nodes, indexed by name
public: SkeletonNodeMap nodes;
Expand Down Expand Up @@ -68,7 +68,6 @@ class ignition::common::SkeletonPrivate
Skeleton::Skeleton()
: data(new SkeletonPrivate)
{
this->data->root = nullptr;
}

//////////////////////////////////////////////////
Expand All @@ -82,10 +81,14 @@ Skeleton::Skeleton(SkeletonNode *_root)
//////////////////////////////////////////////////
Skeleton::~Skeleton()
{
for (auto& kv : this->data->nodes)
for (auto &kv : this->data->nodes)
delete kv.second;
for (auto& a : this->data->anims)
this->data->nodes.clear();

for (auto &a : this->data->anims)
delete a;
this->data->anims.clear();

delete this->data;
this->data = NULL;
chapulina marked this conversation as resolved.
Show resolved Hide resolved
}
Expand Down Expand Up @@ -463,7 +466,10 @@ bool Skeleton::AddBvhAnimation(const std::string &_bvhFile, double _scale)
* math::Matrix4d(skinNode->Transform().Rotation());
}

this->data->anims.push_back(skel->Animation(0u));
// Copy pointer from temp skeleton before it's deleted
auto newAnim = new SkeletonAnimation(skel->Animation(0u)->Name());
*newAnim = *skel->Animation(0u);
this->data->anims.push_back(newAnim);
this->data->mapAnimSkin.push_back(skelMap);
this->data->alignTranslate.push_back(translations);
this->data->alignRotate.push_back(rotations);
Expand Down
13 changes: 13 additions & 0 deletions graphics/src/SkeletonAnimation.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,19 @@ SkeletonAnimation::~SkeletonAnimation()
this->data = NULL;
}

//////////////////////////////////////////////////
SkeletonAnimation &SkeletonAnimation::operator=(const SkeletonAnimation &_other)
{
if (this == &_other)
return *this;

this->data->name = _other.data->name;
this->data->length = _other.data->length;
this->data->animations = _other.data->animations;

return *this;
}

//////////////////////////////////////////////////
void SkeletonAnimation::SetName(const std::string &_name)
{
Expand Down