From 03413a895fbe662e4447caa67fb8dc6b02ed9c45 Mon Sep 17 00:00:00 2001 From: Sam Hellawell Date: Sun, 14 Jul 2024 02:31:17 +0100 Subject: [PATCH 1/3] Fix osgAnimation for multiple creatures (#8045) --- components/resource/keyframemanager.cpp | 9 -- components/resource/scenemanager.cpp | 125 ++++++++++++++++-------- components/sceneutil/osgacontroller.cpp | 4 - 3 files changed, 84 insertions(+), 54 deletions(-) diff --git a/components/resource/keyframemanager.cpp b/components/resource/keyframemanager.cpp index 99ab56d479..41c12837e9 100644 --- a/components/resource/keyframemanager.cpp +++ b/components/resource/keyframemanager.cpp @@ -86,12 +86,6 @@ namespace Resource { if (animation) { - //"Default" is osg dae plugin's default naming scheme for unnamed animations - if (animation->getName() == "Default") - { - animation->setName(std::string("idle")); - } - osg::ref_ptr mergedAnimationTrack = new Resource::Animation; const std::string animationName = animation->getName(); mergedAnimationTrack->setName(animationName); @@ -99,9 +93,6 @@ namespace Resource const osgAnimation::ChannelList& channels = animation->getChannels(); for (const auto& channel : channels) { - // Replace channel target name to match the renamed bones/transforms - channel->setTargetName(Misc::StringUtils::underscoresToSpaces(channel->getTargetName())); - if (name == "Bip01 R Clavicle") { if (!belongsToRightUpperExtremity(channel->getTargetName())) diff --git a/components/resource/scenemanager.cpp b/components/resource/scenemanager.cpp index fbc6acb3cf..1aeb6078a2 100644 --- a/components/resource/scenemanager.cpp +++ b/components/resource/scenemanager.cpp @@ -9,9 +9,9 @@ #include #include +#include #include #include -#include #include #include @@ -63,7 +63,6 @@ namespace { - class InitWorldSpaceParticlesCallback : public SceneUtil::NodeCallback { @@ -272,11 +271,6 @@ namespace Resource void apply(osg::Node& node) override { - // If an osgAnimation bone/transform, ensure underscores in name are replaced with spaces - // this is for compatibility reasons - if (dynamic_cast(&node)) - node.setName(Misc::StringUtils::underscoresToSpaces(node.getName())); - if (osg::StateSet* stateset = node.getStateSet()) { if (stateset->getRenderingHint() == osg::StateSet::TRANSPARENT_BIN) @@ -362,50 +356,99 @@ namespace Resource std::vector> mRigGeometryHolders; }; - void updateVertexInfluenceMap(osgAnimation::RigGeometry& rig) + class ReplaceAnimationUnderscoresVisitor : public osg::NodeVisitor { - osgAnimation::VertexInfluenceMap* vertexInfluenceMap = rig.getInfluenceMap(); - if (!vertexInfluenceMap) - return; - - std::vector renameList; - for (const auto& [boneName, unused] : *vertexInfluenceMap) + public: + ReplaceAnimationUnderscoresVisitor() + : osg::NodeVisitor(osg::NodeVisitor::TRAVERSE_ALL_CHILDREN) { - if (boneName.find('_') != std::string::npos) - renameList.push_back(boneName); } - for (const std::string& oldName : renameList) + void apply(osg::Node& node) override { - const std::string newName = Misc::StringUtils::underscoresToSpaces(oldName); - if (vertexInfluenceMap->find(newName) == vertexInfluenceMap->end()) - (*vertexInfluenceMap)[newName] = std::move((*vertexInfluenceMap)[oldName]); - vertexInfluenceMap->erase(oldName); + // NOTE: MUST update the animation manager names first! + if (auto* animationManager = dynamic_cast(node.getUpdateCallback())) + renameAnimationChannelTargets(*animationManager); + + // Then, any applicable node names + if (auto* rigGeometry = dynamic_cast(&node)) + { + renameNode(*rigGeometry); + updateVertexInfluenceMap(*rigGeometry); + } + else if (auto* matrixTransform = dynamic_cast(&node)) + { + renameNode(*matrixTransform); + renameUpdateCallbacks(*matrixTransform); + } + + traverse(node); } - } - class RenameAnimCallbacksVisitor : public osg::NodeVisitor - { - public: - RenameAnimCallbacksVisitor() - : osg::NodeVisitor(osg::NodeVisitor::TRAVERSE_ALL_CHILDREN) + private: + inline void renameNode(osg::Node& node) { + node.setName(Misc::StringUtils::underscoresToSpaces(node.getName())); } - void apply(osg::MatrixTransform& node) override + void renameUpdateCallbacks(osg::MatrixTransform& node) { - // osgAnimation update callback name must match bone name/channel targets osg::Callback* cb = node.getUpdateCallback(); while (cb) { - auto animCb = dynamic_cast*>(cb); + auto* animCb = dynamic_cast*>(cb); if (animCb) - animCb->setName(Misc::StringUtils::underscoresToSpaces(animCb->getName())); - + { + std::string newAnimCbName = Misc::StringUtils::underscoresToSpaces(animCb->getName()); + animCb->setName(newAnimCbName); + } cb = cb->getNestedCallback(); } + } - traverse(node); + void updateVertexInfluenceMap(osgAnimation::RigGeometry& rig) + { + osgAnimation::VertexInfluenceMap* vertexInfluenceMap = rig.getInfluenceMap(); + if (!vertexInfluenceMap) + return; + + std::vector> renameList; + for (const auto& influence : *vertexInfluenceMap) + { + const std::string& oldBoneName = influence.first; + std::string newBoneName = Misc::StringUtils::underscoresToSpaces(oldBoneName); + if (newBoneName != oldBoneName) + renameList.emplace_back(oldBoneName, newBoneName); + } + + for (const auto& rename : renameList) + { + const std::string& oldName = rename.first; + const std::string& newName = rename.second; + if (vertexInfluenceMap->find(newName) == vertexInfluenceMap->end()) + (*vertexInfluenceMap)[newName] = std::move((*vertexInfluenceMap)[oldName]); + vertexInfluenceMap->erase(oldName); + } + } + + void renameAnimationChannelTargets(osgAnimation::BasicAnimationManager& animManager) + { + for (const auto& animation : animManager.getAnimationList()) + { + if (animation) + { + // "Default" is osg dae plugin's default naming scheme for unnamed animations + if (animation->getName() == "Default") + animation->setName(std::string("idle")); + + auto& channels = animation->getChannels(); + for (auto& channel : channels) + { + std::string newTargetName = Misc::StringUtils::underscoresToSpaces(channel->getTargetName()); + channel->setTargetName(newTargetName); + } + } + } } }; @@ -655,15 +698,20 @@ namespace Resource // Recognize and convert osgAnimation::RigGeometry to OpenMW-optimized type SceneUtil::FindByClassVisitor rigFinder("RigGeometry"); node->accept(rigFinder); + + // If a collada file with rigs, we should replace underscores with spaces + if (isColladaFile && rigFinder.mFoundNodes.size() > 0) + { + ReplaceAnimationUnderscoresVisitor renamingVisitor; + node->accept(renamingVisitor); + } + for (osg::Node* foundRigNode : rigFinder.mFoundNodes) { if (foundRigNode->libraryName() == std::string_view("osgAnimation")) { osgAnimation::RigGeometry* foundRigGeometry = static_cast(foundRigNode); - if (isColladaFile) - Resource::updateVertexInfluenceMap(*foundRigGeometry); - osg::ref_ptr newRig = new SceneUtil::RigGeometryHolder(*foundRigGeometry, osg::CopyOp::DEEP_COPY_ALL); @@ -685,11 +733,6 @@ namespace Resource if (colladaDescriptionVisitor.mSkeleton) { - // Collada bones may have underscores in place of spaces due to a collada limitation - // we should rename the bones and update callbacks here at load time - Resource::RenameAnimCallbacksVisitor renameBoneVisitor; - node->accept(renameBoneVisitor); - if (osg::Group* group = dynamic_cast(node)) { group->removeChildren(0, group->getNumChildren()); diff --git a/components/sceneutil/osgacontroller.cpp b/components/sceneutil/osgacontroller.cpp index d3268fef36..8955cd2c9f 100644 --- a/components/sceneutil/osgacontroller.cpp +++ b/components/sceneutil/osgacontroller.cpp @@ -26,10 +26,6 @@ namespace SceneUtil void LinkVisitor::link(osgAnimation::UpdateMatrixTransform* umt) { - // If osgAnimation had underscores, we should update the umt name also - // otherwise the animation channel and updates wont be applied - umt->setName(Misc::StringUtils::underscoresToSpaces(umt->getName())); - const osgAnimation::ChannelList& channels = mAnimation->getChannels(); for (const auto& channel : channels) { From 56a40577edc27fc7ad8b1d8ec24ada50c3afa063 Mon Sep 17 00:00:00 2001 From: Sam Hellawell Date: Sun, 14 Jul 2024 02:38:57 +0100 Subject: [PATCH 2/3] Revert move of default anim rename --- components/resource/keyframemanager.cpp | 6 ++++++ components/resource/scenemanager.cpp | 4 ---- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/components/resource/keyframemanager.cpp b/components/resource/keyframemanager.cpp index 41c12837e9..c4d3da2938 100644 --- a/components/resource/keyframemanager.cpp +++ b/components/resource/keyframemanager.cpp @@ -86,6 +86,12 @@ namespace Resource { if (animation) { + //"Default" is osg dae plugin's default naming scheme for unnamed animations + if (animation->getName() == "Default") + { + animation->setName(std::string("idle")); + } + osg::ref_ptr mergedAnimationTrack = new Resource::Animation; const std::string animationName = animation->getName(); mergedAnimationTrack->setName(animationName); diff --git a/components/resource/scenemanager.cpp b/components/resource/scenemanager.cpp index 1aeb6078a2..66012e05d4 100644 --- a/components/resource/scenemanager.cpp +++ b/components/resource/scenemanager.cpp @@ -437,10 +437,6 @@ namespace Resource { if (animation) { - // "Default" is osg dae plugin's default naming scheme for unnamed animations - if (animation->getName() == "Default") - animation->setName(std::string("idle")); - auto& channels = animation->getChannels(); for (auto& channel : channels) { From db30d9a37a21125dc66c9b7f18ee5a6d5a0c4b1c Mon Sep 17 00:00:00 2001 From: Sam Hellawell Date: Sun, 14 Jul 2024 20:38:24 +0100 Subject: [PATCH 3/3] Code cleanup, fix missing const --- components/resource/scenemanager.cpp | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/components/resource/scenemanager.cpp b/components/resource/scenemanager.cpp index 66012e05d4..d1f48e533f 100644 --- a/components/resource/scenemanager.cpp +++ b/components/resource/scenemanager.cpp @@ -413,18 +413,15 @@ namespace Resource return; std::vector> renameList; - for (const auto& influence : *vertexInfluenceMap) + for (const auto& [oldBoneName, _] : *vertexInfluenceMap) { - const std::string& oldBoneName = influence.first; - std::string newBoneName = Misc::StringUtils::underscoresToSpaces(oldBoneName); + const std::string newBoneName = Misc::StringUtils::underscoresToSpaces(oldBoneName); if (newBoneName != oldBoneName) renameList.emplace_back(oldBoneName, newBoneName); } - for (const auto& rename : renameList) + for (const auto& [oldName, newName] : renameList) { - const std::string& oldName = rename.first; - const std::string& newName = rename.second; if (vertexInfluenceMap->find(newName) == vertexInfluenceMap->end()) (*vertexInfluenceMap)[newName] = std::move((*vertexInfluenceMap)[oldName]); vertexInfluenceMap->erase(oldName); @@ -433,16 +430,13 @@ namespace Resource void renameAnimationChannelTargets(osgAnimation::BasicAnimationManager& animManager) { - for (const auto& animation : animManager.getAnimationList()) + for (const osgAnimation::Animation* animation : animManager.getAnimationList()) { if (animation) { - auto& channels = animation->getChannels(); - for (auto& channel : channels) - { - std::string newTargetName = Misc::StringUtils::underscoresToSpaces(channel->getTargetName()); - channel->setTargetName(newTargetName); - } + const osgAnimation::ChannelList& channels = animation->getChannels(); + for (osgAnimation::Channel* channel : channels) + channel->setTargetName(Misc::StringUtils::underscoresToSpaces(channel->getTargetName())); } } } @@ -696,7 +690,7 @@ namespace Resource node->accept(rigFinder); // If a collada file with rigs, we should replace underscores with spaces - if (isColladaFile && rigFinder.mFoundNodes.size() > 0) + if (isColladaFile && !rigFinder.mFoundNodes.empty()) { ReplaceAnimationUnderscoresVisitor renamingVisitor; node->accept(renamingVisitor);