Replace osgAnimation bone underscore naming at load time, map bone instances, reset root bone transform each frame

CHANGELOG.md

@@ -46,6 +46,7 @@
     Bug #6657: Distant terrain tiles become black when using FWIW mod
     Bug #6661: Saved games that have no preview screenshot cause issues or crashes
     Bug #6716: mwscript comparison operator handling is too restrictive
+    Bug #6723: "Turn to movement direction" makes the player rotate wildly with COLLADA
     Bug #6754: Beast to Non-beast transformation mod is not working on OpenMW
     Bug #6758: Main menu background video can be stopped by opening the options menu
     Bug #6807: Ultimate Galleon is not working properly

apps/openmw/mwrender/animation.cpp

@@ -530,6 +530,7 @@ namespace MWRender
         , mHasMagicEffects(false)
         , mAlpha(1.f)
         , mPlayScriptedOnly(false)
+        , mRequiresBoneMap(false)
     {
         for (size_t i = 0; i < sNumBlendMasks; i++)
             mAnimationTimePtr[i] = std::make_shared<AnimationTime>();
@@ -963,9 +964,18 @@ namespace MWRender
     const Animation::NodeMap& Animation::getNodeMap() const
     {
         if (!mNodeMapCreated && mObjectRoot)
+        {
+            // If the base of this animation is an osgAnimation, we should map the bones not matrix transforms
+            if (mRequiresBoneMap)
+            {
+                SceneUtil::NodeMapVisitorBoneOnly visitor(mNodeMap);
+                mObjectRoot->accept(visitor);
+            }
+            else
             {
                 SceneUtil::NodeMapVisitor visitor(mNodeMap);
                 mObjectRoot->accept(visitor);
+            }
             mNodeMapCreated = true;
         }
         return mNodeMap;
@@ -1447,10 +1457,9 @@ namespace MWRender
             }
         }
 
+        osg::ref_ptr<osg::Node> created = getModelInstance(mResourceSystem, model, baseonly, inject, defaultSkeleton);
         if (!forceskeleton)
         {
-            osg::ref_ptr<osg::Node> created
-                = getModelInstance(mResourceSystem, model, baseonly, inject, defaultSkeleton);
             mInsert->addChild(created);
             mObjectRoot = created->asGroup();
             if (!mObjectRoot)
@@ -1466,8 +1475,6 @@ namespace MWRender
         }
         else
         {
-            osg::ref_ptr<osg::Node> created
-                = getModelInstance(mResourceSystem, model, baseonly, inject, defaultSkeleton);
             osg::ref_ptr<SceneUtil::Skeleton> skel = dynamic_cast<SceneUtil::Skeleton*>(created.get());
             if (!skel)
             {
@@ -1479,6 +1486,10 @@ namespace MWRender
             mInsert->addChild(mObjectRoot);
         }
 
+        // osgAnimation formats with skeletons should have their nodemap be bone instances
+        // FIXME: better way to detect osgAnimation here instead of relying on extension?
+        mRequiresBoneMap = mSkeleton != nullptr && !Misc::StringUtils::ciEndsWith(model, "nif");
+
         if (previousStateset)
             mObjectRoot->setStateSet(previousStateset);
 
@@ -1791,6 +1802,7 @@ namespace MWRender
         osg::ref_ptr<RotateController> controller(new RotateController(mObjectRoot.get()));
         node->addUpdateCallback(controller);
         mActiveControllers.emplace_back(node, controller);
+
         return controller;
     }
 

apps/openmw/mwrender/animation.hpp

@@ -246,6 +246,7 @@ namespace MWRender
         osg::ref_ptr<SceneUtil::LightListCallback> mLightListCallback;
 
         bool mPlayScriptedOnly;
+        bool mRequiresBoneMap;
 
         const NodeMap& getNodeMap() const;
 

components/misc/strings/algorithm.hpp

@@ -15,6 +15,13 @@ namespace Misc::StringUtils
         bool operator()(char x, char y) const { return toLower(x) < toLower(y); }
     };
 
+    inline std::string underscoresToSpaces(const std::string_view& oldName)
+    {
+        std::string newName(oldName);
+        std::replace(newName.begin(), newName.end(), '_', ' ');
+        return newName;
+    }
+
     inline bool ciLess(std::string_view x, std::string_view y)
     {
         return std::lexicographical_compare(x.begin(), x.end(), y.begin(), y.end(), CiCharLess());

components/resource/keyframemanager.cpp

@@ -37,11 +37,11 @@ namespace Resource
 
     bool RetrieveAnimationsVisitor::belongsToLeftUpperExtremity(const std::string& name)
     {
-        static const std::array boneNames = { "bip01_l_clavicle", "left_clavicle", "bip01_l_upperarm", "left_upper_arm",
+        static const std::array boneNames = { "bip01 l clavicle", "left clavicle", "bip01 l upperarm", "left upper arm",
-            "bip01_l_forearm", "bip01_l_hand", "left_hand", "left_wrist", "shield_bone", "bip01_l_pinky1",
+            "bip01 l forearm", "bip01 l hand", "left hand", "left wrist", "shield bone", "bip01 l pinky1",
-            "bip01_l_pinky2", "bip01_l_pinky3", "bip01_l_ring1", "bip01_l_ring2", "bip01_l_ring3", "bip01_l_middle1",
+            "bip01 l pinky2", "bip01 l pinky3", "bip01 l ring1", "bip01 l ring2", "bip01 l ring3", "bip01 l middle1",
-            "bip01_l_middle2", "bip01_l_middle3", "bip01_l_pointer1", "bip01_l_pointer2", "bip01_l_pointer3",
+            "bip01 l middle2", "bip01 l middle3", "bip01 l pointer1", "bip01 l pointer2", "bip01 l pointer3",
-            "bip01_l_thumb1", "bip01_l_thumb2", "bip01_l_thumb3", "left_forearm" };
+            "bip01 l thumb1", "bip01 l thumb2", "bip01 l thumb3", "left forearm" };
 
         if (std::find(boneNames.begin(), boneNames.end(), name) != boneNames.end())
             return true;
@@ -51,11 +51,11 @@ namespace Resource
 
     bool RetrieveAnimationsVisitor::belongsToRightUpperExtremity(const std::string& name)
     {
-        static const std::array boneNames = { "bip01_r_clavicle", "right_clavicle", "bip01_r_upperarm",
+        static const std::array boneNames = { "bip01 r clavicle", "right clavicle", "bip01 r upperarm",
-            "right_upper_arm", "bip01_r_forearm", "bip01_r_hand", "right_hand", "right_wrist", "bip01_r_thumb1",
+            "right upper arm", "bip01 r forearm", "bip01 r hand", "right hand", "right wrist", "bip01 r thumb1",
-            "bip01_r_thumb2", "bip01_r_thumb3", "weapon_bone", "bip01_r_pinky1", "bip01_r_pinky2", "bip01_r_pinky3",
+            "bip01 r thumb2", "bip01 r thumb3", "weapon bone", "bip01 r pinky1", "bip01 r pinky2", "bip01 r pinky3",
-            "bip01_r_ring1", "bip01_r_ring2", "bip01_r_ring3", "bip01_r_middle1", "bip01_r_middle2", "bip01_r_middle3",
+            "bip01 r ring1", "bip01 r ring2", "bip01 r ring3", "bip01 r middle1", "bip01 r middle2", "bip01 r middle3",
-            "bip01_r_pointer1", "bip01_r_pointer2", "bip01_r_pointer3", "right_forearm" };
+            "bip01 r pointer1", "bip01 r pointer2", "bip01 r pointer3", "right forearm" };
 
         if (std::find(boneNames.begin(), boneNames.end(), name) != boneNames.end())
             return true;
@@ -66,7 +66,7 @@ namespace Resource
     bool RetrieveAnimationsVisitor::belongsToTorso(const std::string& name)
     {
         static const std::array boneNames
-            = { "bip01_spine1", "bip01_spine2", "bip01_neck", "bip01_head", "head", "neck", "chest", "groin" };
+            = { "bip01 spine1", "bip01 spine2", "bip01 neck", "bip01 head", "head", "neck", "chest", "groin" };
 
         if (std::find(boneNames.begin(), boneNames.end(), name) != boneNames.end())
             return true;
@@ -88,9 +88,7 @@ namespace Resource
             {
                 //"Default" is osg dae plugin's default naming scheme for unnamed animations
                 if (animation->getName() == "Default")
-                {
                     animation->setName(std::string("idle"));
-                }
 
                 osg::ref_ptr<Resource::Animation> mergedAnimationTrack = new Resource::Animation;
                 const std::string animationName = animation->getName();
@@ -99,6 +97,9 @@ namespace Resource
                 const osgAnimation::ChannelList& channels = animation->getChannels();
                 for (const auto& channel : channels)
                 {
+                    // Repalce 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()))

components/resource/scenemanager.cpp

@@ -9,7 +9,10 @@
 #include <osg/Node>
 #include <osg/UserDataContainer>
 
+#include <osgAnimation/Bone>
 #include <osgAnimation/RigGeometry>
+#include <osgAnimation/Skeleton>
+#include <osgAnimation/UpdateBone>
 
 #include <osgParticle/ParticleSystem>
 
@@ -353,6 +356,66 @@ namespace Resource
         std::vector<osg::ref_ptr<SceneUtil::RigGeometryHolder>> mRigGeometryHolders;
     };
 
+    void updateVertexInfluenceMap(osgAnimation::RigGeometry& rig)
+    {
+        osgAnimation::VertexInfluenceMap* vertexInfluenceMap = rig.getInfluenceMap();
+        if (!vertexInfluenceMap)
+            return;
+
+        std::vector<std::pair<std::string, std::string>> renameList;
+
+        // Collecting updates
+        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);
+        }
+
+        // Applying updates (cant update map while iterating it!)
+        for (const auto& rename : renameList)
+        {
+            const std::string& oldName = rename.first;
+            const std::string& newName = rename.second;
+
+            // Check if new name already exists to avoid overwriting
+            if (vertexInfluenceMap->find(newName) == vertexInfluenceMap->end())
+                (*vertexInfluenceMap)[newName] = std::move((*vertexInfluenceMap)[oldName]);
+
+            vertexInfluenceMap->erase(oldName);
+        }
+    }
+
+    class RenameBonesVisitor : public osg::NodeVisitor
+    {
+    public:
+        RenameBonesVisitor()
+            : osg::NodeVisitor(osg::NodeVisitor::TRAVERSE_ALL_CHILDREN)
+        {
+        }
+
+        void apply(osg::MatrixTransform& node) override
+        {
+            node.setName(Misc::StringUtils::underscoresToSpaces(node.getName()));
+
+            // osgAnimation update callback name must match bone name/channel targets
+            osg::Callback* cb = node.getUpdateCallback();
+            while (cb)
+            {
+                osgAnimation::AnimationUpdateCallback<osg::NodeCallback>* animCb
+                    = dynamic_cast<osgAnimation::AnimationUpdateCallback<osg::NodeCallback>*>(cb);
+
+                if (animCb)
+                    animCb->setName(Misc::StringUtils::underscoresToSpaces(animCb->getName()));
+
+                cb = cb->getNestedCallback();
+            }
+
+            traverse(node);
+        }
+    };
+
     SceneManager::SceneManager(const VFS::Manager* vfs, Resource::ImageManager* imageManager,
         Resource::NifFileManager* nifFileManager, double expiryDelay)
         : ResourceManager(vfs, expiryDelay)
@@ -556,6 +619,7 @@ namespace Resource
             VFS::Path::NormalizedView normalizedFilename, std::istream& model, Resource::ImageManager* imageManager)
         {
             const std::string_view ext = Misc::getFileExtension(normalizedFilename.value());
+            const bool isColladaFile = ext == "dae";
             osgDB::ReaderWriter* reader = osgDB::Registry::instance()->getReaderWriterForExtension(std::string(ext));
             if (!reader)
             {
@@ -571,7 +635,7 @@ namespace Resource
             // findFileCallback would be necessary. but findFileCallback does not support virtual files, so we can't
             // implement it.
             options->setReadFileCallback(new ImageReadCallback(imageManager));
-            if (ext == "dae")
+            if (isColladaFile)
                 options->setOptionString("daeUseSequencedTextureUnits");
 
             const std::array<std::uint64_t, 2> fileHash = Files::getHash(normalizedFilename.value(), model);
@@ -599,9 +663,13 @@ namespace Resource
             node->accept(rigFinder);
             for (osg::Node* foundRigNode : rigFinder.mFoundNodes)
             {
-                if (foundRigNode->libraryName() == std::string("osgAnimation"))
+                if (foundRigNode->libraryName() == std::string_view("osgAnimation"))
                 {
                     osgAnimation::RigGeometry* foundRigGeometry = static_cast<osgAnimation::RigGeometry*>(foundRigNode);
+
+                    if (isColladaFile)
+                        Resource::updateVertexInfluenceMap(*foundRigGeometry);
+
                     osg::ref_ptr<SceneUtil::RigGeometryHolder> newRig
                         = new SceneUtil::RigGeometryHolder(*foundRigGeometry, osg::CopyOp::DEEP_COPY_ALL);
 
@@ -616,13 +684,18 @@ namespace Resource
                 }
             }
 
-            if (ext == "dae")
+            if (isColladaFile)
             {
                 Resource::ColladaDescriptionVisitor colladaDescriptionVisitor;
                 node->accept(colladaDescriptionVisitor);
 
                 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::RenameBonesVisitor renameBoneVisitor;
+                    node->accept(renameBoneVisitor);
+
                     if (osg::Group* group = dynamic_cast<osg::Group*>(node))
                     {
                         group->removeChildren(0, group->getNumChildren());

components/sceneutil/extradata.cpp

@@ -9,6 +9,7 @@
 #include <yaml-cpp/yaml.h>
 
 #include <components/misc/osguservalues.hpp>
+#include <components/misc/strings/algorithm.hpp>
 #include <components/resource/scenemanager.hpp>
 #include <components/sceneutil/depth.hpp>
 #include <components/shader/shadermanager.hpp>
@@ -44,11 +45,15 @@ namespace SceneUtil
 
     void ProcessExtraDataVisitor::apply(osg::Node& node)
     {
+        // If an osgAnimation bone/transform, ensure underscores in name are replaced with spaces
+        // this is for compatibility reasons
+        if (node.libraryName() == std::string_view("osgAnimation") && node.className() == std::string_view("Bone"))
+            node.setName(Misc::StringUtils::underscoresToSpaces(node.getName()));
+
         if (!mSceneMgr->getSoftParticles())
             return;
 
         std::string source;
-
         constexpr float defaultFalloffDepth = 300.f; // arbitrary value that simply looks good with common cases
 
         if (node.getUserValue(Misc::OsgUserValues::sExtraData, source) && !source.empty())

components/sceneutil/osgacontroller.cpp

@@ -1,5 +1,6 @@
 #include <components/sceneutil/osgacontroller.hpp>
 
+#include <osg/MatrixTransform>
 #include <osg/Node>
 #include <osg/NodeVisitor>
 #include <osg/ref_ptr>
@@ -9,6 +10,7 @@
 #include <osgAnimation/Sampler>
 #include <osgAnimation/UpdateMatrixTransform>
 
+#include <components/misc/strings/algorithm.hpp>
 #include <components/misc/strings/lower.hpp>
 #include <components/resource/animation.hpp>
 #include <components/sceneutil/controller.hpp>
@@ -24,6 +26,10 @@ 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)
         {
@@ -128,6 +134,47 @@ namespace SceneUtil
         return osg::Vec3f();
     }
 
+    osg::Matrixf OsgAnimationController::getTransformForNode(float time, const std::string& name) const
+    {
+        std::string animationName;
+        float newTime = time;
+
+        // Find the correct animation based on time
+        for (const EmulatedAnimation& emulatedAnimation : mEmulatedAnimations)
+        {
+            if (time >= emulatedAnimation.mStartTime && time <= emulatedAnimation.mStopTime)
+            {
+                newTime = time - emulatedAnimation.mStartTime;
+                animationName = emulatedAnimation.mName;
+            }
+        }
+
+        // Find the bone's transform track in animation
+        for (const auto& mergedAnimationTrack : mMergedAnimationTracks)
+        {
+            if (mergedAnimationTrack->getName() != animationName)
+                continue;
+
+            const osgAnimation::ChannelList& channels = mergedAnimationTrack->getChannels();
+
+            for (const auto& channel : channels)
+            {
+                if (!Misc::StringUtils::ciEqual(name, channel->getTargetName()) || channel->getName() != "transform")
+                    continue;
+
+                if (osgAnimation::MatrixLinearSampler* templateSampler
+                    = dynamic_cast<osgAnimation::MatrixLinearSampler*>(channel->getSampler()))
+                {
+                    osg::Matrixf matrix;
+                    templateSampler->getValueAt(newTime, matrix);
+                    return matrix;
+                }
+            }
+        }
+
+        return osg::Matrixf::identity();
+    }
+
     void OsgAnimationController::update(float time, const std::string& animationName)
     {
         for (const auto& mergedAnimationTrack : mMergedAnimationTracks)
@@ -162,6 +209,12 @@ namespace SceneUtil
                     update(time - emulatedAnimation.mStartTime, emulatedAnimation.mName);
                 }
             }
+
+            // Reset the transform of this node to whats in the animation
+            // we force this here because downstream some code relies on the bone having a non-modified transform
+            // as this is how the NIF controller behaves. RotationController is a good example of this.
+            // Without this here, it causes osgAnimation skeletons to spin wildly
+            static_cast<osg::MatrixTransform*>(node)->setMatrix(getTransformForNode(time, node->getName()));
         }
 
         traverse(node, nv);

components/sceneutil/osgacontroller.hpp

@@ -59,6 +59,9 @@ namespace SceneUtil
         /// @brief Handles the location of the instance
         osg::Vec3f getTranslation(float time) const override;
 
+        /// @brief Handles finding bone position in the animation
+        osg::Matrixf getTransformForNode(float time, const std::string& name) const;
+
         /// @brief Calls animation track update()
         void update(float time, const std::string& animationName);
 

components/sceneutil/visitor.cpp

@@ -5,6 +5,8 @@
 
 #include <osgParticle/ParticleSystem>
 
+#include <osgAnimation/Bone>
+
 #include <components/debug/debuglog.hpp>
 #include <components/misc/strings/algorithm.hpp>
 
@@ -13,7 +15,6 @@
 
 namespace SceneUtil
 {
-
     bool FindByNameVisitor::checkGroup(osg::Group& group)
     {
         if (Misc::StringUtils::ciEqual(group.getName(), mNameToFind))
@@ -22,35 +23,13 @@ namespace SceneUtil
             return true;
         }
 
-        // FIXME: can the nodes/bones be renamed at loading stage rather than each time?
-        // Convert underscores to whitespaces as a workaround for Collada (OpenMW's animation system uses
-        // whitespace-separated names)
-        std::string nodeName = group.getName();
-        std::replace(nodeName.begin(), nodeName.end(), '_', ' ');
-        if (Misc::StringUtils::ciEqual(nodeName, mNameToFind))
-        {
-            mFoundNode = &group;
-            return true;
-        }
         return false;
     }
 
     void FindByClassVisitor::apply(osg::Node& node)
     {
         if (Misc::StringUtils::ciEqual(node.className(), mNameToFind))
-        {
-            mFoundNodes.push_back(&node);
-        }
-        else
-        {
-            // FIXME: can the nodes/bones be renamed at loading stage rather than each time?
-            // Convert underscores to whitespaces as a workaround for Collada (OpenMW's animation system uses
-            // whitespace-separated names)
-            std::string nodeName = node.className();
-            std::replace(nodeName.begin(), nodeName.end(), '_', ' ');
-            if (Misc::StringUtils::ciEqual(nodeName, mNameToFind))
             mFoundNodes.push_back(&node);
-        }
 
         traverse(node);
     }
@@ -69,23 +48,19 @@ namespace SceneUtil
 
     void FindByNameVisitor::apply(osg::Geometry&) {}
 
-    void NodeMapVisitor::apply(osg::MatrixTransform& trans)
+    void NodeMapVisitorBoneOnly::apply(osg::MatrixTransform& trans)
-    {
-        // Choose first found node in file
-
-        if (trans.libraryName() == std::string_view("osgAnimation"))
     {
-            std::string nodeName = trans.getName();
+        // Choose first found bone in file
+        if (dynamic_cast<osgAnimation::Bone*>(&trans) != nullptr)
+            mMap.emplace(trans.getName(), &trans);
 
-            // FIXME: can the nodes/bones be renamed at loading stage rather than each time?
+        traverse(trans);
-            // Convert underscores to whitespaces as a workaround for Collada (OpenMW's animation system uses
-            // whitespace-separated names)
-            std::replace(nodeName.begin(), nodeName.end(), '_', ' ');
-            mMap.emplace(nodeName, &trans);
     }
-        else
-            mMap.emplace(trans.getName(), &trans);
 
+    void NodeMapVisitor::apply(osg::MatrixTransform& trans)
+    {
+        // Choose first found node in file
+        mMap.emplace(trans.getName(), &trans);
         traverse(trans);
     }
 

components/sceneutil/visitor.hpp

@@ -70,6 +70,26 @@ namespace SceneUtil
         NodeMap& mMap;
     };
 
+    /// Maps names to bone nodes
+    class NodeMapVisitorBoneOnly : public osg::NodeVisitor
+    {
+    public:
+        typedef std::unordered_map<std::string, osg::ref_ptr<osg::MatrixTransform>, Misc::StringUtils::CiHash,
+            Misc::StringUtils::CiEqual>
+            NodeMap;
+
+        NodeMapVisitorBoneOnly(NodeMap& map)
+            : osg::NodeVisitor(osg::NodeVisitor::TRAVERSE_ALL_CHILDREN)
+            , mMap(map)
+        {
+        }
+
+        void apply(osg::MatrixTransform& trans) override;
+
+    private:
+        NodeMap& mMap;
+    };
+
     /// @brief Base class for visitors that remove nodes from a scene graph.
     /// Subclasses need to fill the mToRemove vector.
     /// To use, node->accept(removeVisitor); removeVisitor.remove();